[...]
> > +
> > static int sd_uhs2_power_up(struct mmc_host *host)
> > {
> > int err;
> > @@ -42,12 +70,46 @@ static int sd_uhs2_power_off(struct mmc_host *host)
> >
> > host->ios.vdd = 0;
> > host->ios.clock = 0;
> > + /* Must set UHS2 timing to identify UHS2 mode */
>
> That comment is stale, but...
>
> > host->ios.timing = MMC_TIMING_LEGACY;
>
> MMC_TIMING_UHS2_SPEED_A is also checked by mmc_card_uhs2()
> which is used a lot.
>
> It might be that ios.timing should be MMC_TIMING_UHS2_SPEED_A
> until after calling host->ops->uhs2_control() ?
I agree, that was also how the code looked before my previous comment for v22.
Victor, sorry for the confusion, I simply didn't realize that
mmc_card_uhs2() was using MMC_TIMING_UHS2_SPEED_A.
It looks like we should set MMC_TIMING_LEGACY, *after* calling
host->ops->uhs2_control(). Just to make sure we have the correct
initial state after powering-off.
I will fix this when applying, see more comments about that below.
>
> > host->ios.power_mode = MMC_POWER_OFF;
> > + host->uhs2_sd_tran = false;
> >
> > return host->ops->uhs2_control(host, UHS2_SET_IOS);
> > }
[...]
> > +static int sd_uhs2_init_card(struct mmc_host *host, struct mmc_card *oldcard)
> > {
> > struct mmc_card *card;
> > u32 node_id = 0;
> > @@ -131,33 +818,211 @@ static int sd_uhs2_init_card(struct mmc_host *host)
> > if (err)
> > return err;
> >
> > - card = mmc_alloc_card(host, &sd_type);
> > - if (IS_ERR(card))
> > - return PTR_ERR(card);
> > + if (oldcard) {
> > + card = oldcard;
> > + } else {
> > + card = mmc_alloc_card(host, &sd_type);
> > + if (IS_ERR(card))
> > + return PTR_ERR(card);
> > + }
> >
> > card->uhs2_config.node_id = node_id;
> > card->type = MMC_TYPE_SD;
> >
> > err = sd_uhs2_config_read(host, card);
> > if (err)
> > - goto err;
> > + return err;
>
> This leaks card if !oldcard. Still need:
>
> err:
> if (!oldcard)
> mmc_remove_card(card);
>
>
> >
> > err = sd_uhs2_config_write(host, card);
> > if (err)
> > - goto err;
> > + return err;
>
> Ditto
>
> >
> > host->card = card;
> > + /* If change speed to Range B, need to GO_DORMANT_STATE */
> > + if (host->ios.timing == MMC_TIMING_UHS2_SPEED_B ||
> > + host->ios.timing == MMC_TIMING_UHS2_SPEED_B_HD) {
> > + err = sd_uhs2_go_dormant_state(host, node_id);
> > + if (err)
> > + return err;
>
> And here host->card is assigned but we have failed.
> The other card types do not set host->card until
> just before return 0. Then this can be goto err
> instead of return err.
In regards to the error path and how to handle the cleanup of the
mmc_card, indeed things are still not correct.
However, I realize these parts are somewhat a bit tricky to get right.
Therefore, I am simply going to leave this as is when applying and I
am going to send patches that fixes things on top.
In this way we can move things forward and avoid additional new
versions of the complete series to be sent.
> > + }
> > +
> > + host->uhs2_sd_tran = true;
> > +
> > + return 0;
> > +}
> > +
>
[...]
Kind regards
Uffe
