On Fri, 4 Nov 2022 at 13:16, Christophe JAILLET
<christophe.jaillet@xxxxxxxxxx> wrote:
>
> Le 19/10/2022 à 13:06, Victor Shih a écrit :
> > From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> >
> > Updates in V4:
> > - Re-based, updated a comment and removed white-space.
> > - Moved MMC_VQMMC2_VOLTAGE_180 into a later patch in the series.
> >
> > Update in previous version:
> > The SD UHS-II interface was introduced to the SD spec v4.00 several years
> > ago. The interface is fundamentally different from an electrical and a
> > protocol point of view, comparing to the legacy SD interface.
> >
> > However, the legacy SD protocol is supported through a specific transport
> > layer (SD-TRAN) defined in the UHS-II addendum of the spec. This allows the
> > SD card to be managed in a very similar way as a legacy SD card, hence a
> > lot of code can be re-used to support these new types of cards through the
> > mmc subsystem.
> >
> > Moreover, an SD card that supports the UHS-II interface shall also be
> > backwards compatible with the legacy SD interface, which allows a UHS-II
> > card to be inserted into a legacy slot. As a matter of fact, this is
> > already supported by mmc subsystem as of today.
> >
> > To prepare to add support for UHS-II, this change puts the basic foundation
> > in the mmc core in place, allowing it to be more easily reviewed before
> > subsequent changes implements the actual support.
> >
> > Basically, the approach here adds a new UHS-II bus_ops type and adds a
> > separate initialization path for the UHS-II card. The intent is to avoid us
> > from sprinkling the legacy initialization path, but also to simplify
> > implementation of the UHS-II specific bits.
> >
> > At this point, there is only one new host ops added to manage the various
> > ios settings needed for UHS-II. Additional host ops that are needed, are
> > being added from subsequent changes.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> > ---
>
> []
>
> > +static int sd_uhs2_attach(struct mmc_host *host)
> > +{
> > + int err;
> > +
> > + err = sd_uhs2_power_up(host);
> > + if (err)
> > + goto err;
> > +
> > + err = sd_uhs2_phy_init(host);
> > + if (err)
> > + goto err;
> > +
> > + err = sd_uhs2_init_card(host);
> > + if (err)
> > + goto err;
> > +
> > + mmc_attach_bus(host, &sd_uhs2_ops);
> > +
> > + mmc_release_host(host);
> > +
> > + err = mmc_add_card(host->card);
> > + if (err)
> > + goto remove_card;
> > +
> > + mmc_claim_host(host);
> > + return 0;
> > +
> > +remove_card:
> > + mmc_remove_card(host->card);
>
> Hi,
>
> If we arrive here, mmc_add_card() has failed.
> is it correct to call mmc_remove_card() in such a case?
Yes. There are some additional checks in mmc_add_card() to help to
manage this too.
Although, there are certainly some cleanups that can be made to
simplify the code in the mmc core around this, but that's a different
story.
>
> > + host->card = NULL;
> > + mmc_claim_host(host);
> > + mmc_detach_bus(host);
> > +err:
> > + sd_uhs2_power_off(host);
>
> If sd_uhs2_power_up() fails, we arrive here.
> Is its correct to call sd_uhs2_power_off() in such a case, or should we
> return directly if sd_uhs2_power_up() fails?
That's an option that could make the code a bit clearer. I have no
strong opinion around this.
Although, if changing this, we need to make sure sd_uhs2_power_up()
restores things correctly after a failure, but it doesn't do that in
the current version of the patch.
Kind regards
Uffe
