Re: [SPAM] [RFC PATCH v42 90/98] hw/sd/sdcard: Add experimental 'x-aspeed-emmc-kludge' property

[Philippe Mathieu-Daudé]

On 2/7/24 22:05, Philippe Mathieu-Daudé wrote:
> On 2/7/24 18:21, Cédric Le Goater wrote:
> > On 7/2/24 6:15 PM, Philippe Mathieu-Daudé wrote:
> > > On 2/7/24 07:06, Andrew Jeffery wrote:
> > > > On Fri, 2024-06-28 at 11:16 +0200, Cédric Le Goater wrote:
> > > > > On 6/28/24 9:02 AM, Philippe Mathieu-Daudé wrote:
> > > > > > When booting U-boot/Linux on Aspeed boards via eMMC,
> > > > > > some commands don't behave as expected from the spec.
> > > > > >
> > > > > > Add the 'x-aspeed-emmc-kludge' property to allow non
> > > > > > standard uses until we figure out the reasons.
> > > > >
> > > > > I am not aware of any singularity in the eMMC logic provided by 
> > > > > Aspeed.
> > > > > U-Boot and Linux drivers seem very generic. May be others can tell.
> > > >
> > > > I'm not aware of any command kludges. The main problem I had when I
> > > > wrote the Linux driver for the Aspeed controller was the phase tuning,
> > > > but that doesn't sound related.
> > >
> > > Yeah I don't think anything Aspeed nor U-boot related, we
> > > model CSD/CID registers per the SD spec, not MMC. Various
> > > fields are identical, but few differ, this might be the
> > > problem.
> > >
> > > I rather respect the spec by default, so until we figure
> > > the issue, are you OK to use a 'x-emmc-kludge' property
> > > and set it on the Aspeed boards?
> >
> > If these differences are eMMC related, why not simply test :
> >
> >      if (sd_is_emmc(sd)) ...
> >
> > in commands ALL_SEND_CID and APP_CMD ? The extra property looks
> > ambiguous to me.
>
> I'd like to keep the sd_is_emmc() check for code respecting
> the eMMC spec. I believe the commands in sd_proto_emmc[] in
> this series do respect it, modulo some register field
> definitions that are SD specific. So 'x-emmc-kludge' would
> be a property to allow eMMC use -- without delaying it further
> --, by bypassing a *bug* in our current model. I'm willing to
> figure out the problem and fix it, but /after/ the 9.1 release.
> We are too close of the soft freeze and trying to fix that
> before is too much pressure on my right now.

The problem is in the still unreviewed patch #86 of this series
"hw/sd/sdcard: Add emmc_cmd_SEND_OP_COND handler (CMD1)".

SEND_OP_COND should put the card in READY state. We are not
considering the BOOT_PARTITION_ENABLE feature:

  > When BOOT_PARTITION_ENABLE bits are set and master send
  > CMD1 (SEND_OP_COND), slave must enter Card Identification
  > Mode and respond to the command.
  > If the slave does not support boot operation mode, which
  > is compliant with v4.2 or before, or BOOT_PARTITION_ENABLE
  > bit is cleared, slave automatically enter Idle State after
  > power-on.

Then we don't need the change in the next patch (#91) in
ALL_SEND_CID.

And likely neither we need #92 (APP_CMD ) but I still need
to confirm that.