Re: [PATCH v2 1/2] mmc: core: sdio: retry without UHS if UHS init fails

[Deomid "rojer" Ryabkov]

Hi Ulf,

it's not wrong: after the "Read the common CIS tuples" and the
subsequent vendor check the only two outcomes can be oldcard == NULL
or card == oldcard.
but your way is clearer, i will send v3 with it.
thanks for the review!

On Fri, 30 Oct 2026 at 16:10, Ulf Hansson <ulf.hansson@xxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Oct 27, 2026 at 1:24 PM Deomid rojer Ryabkov <rojer@xxxxxxxx> wrote:
> >
> > If mmc_sdio_init_uhs_card() fails, e.g. because tuning finds no
> > working sample phase, card initialisation fails and the card is not
> > usable at all, even though it would work fine in high-speed mode.
> >
> > This happens for example on the FriendlyElec NanoPi R2S Plus when the
> > SDIO pins of the RTL8822CS Wi-Fi module are left at their default 4 mA
> > drive strength:
> >
> >   dwmmc_rockchip ff5f0000.mmc: All phases bad!
> >   mmc2: tuning execution failed: -5
> >   mmc2: error -5 whilst initialising SDIO card
> >   [... repeated for every init frequency ...]
> >   mmc2: Failed to initialize a non-removable card
> >
> > There is already a fallback for a failure to read the CCCR in UHS mode:
> > reset the card and restart the init sequence without R4_18V_PRESENT.
> > Do the same when UHS init fails, so that the card comes up in
> > high-speed mode instead:
> >
> >   mmc2: UHS init failed (-5), retrying without UHS
> >   mmc2: Skipping voltage switch
> >   mmc2: new high speed SDIO card at address 0001
> >
> > The fallback is taken at most once, as the retry doesn't request UHS.
> >
> > Reviewed-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
> > Signed-off-by: Deomid rojer Ryabkov <rojer@xxxxxxxx>
> > ---
> >  drivers/mmc/core/sdio.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> > index 83085e76486..6787fdf1c53 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -867,8 +867,14 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
> >         /* Only if card supports 1.8v and UHS signaling */
> >         if ((ocr & R4_18V_PRESENT) && card->sw_caps.sd3_bus_mode) {
> >                 err = mmc_sdio_init_uhs_card(card);
> > -               if (err)
> > -                       goto remove;
> > +               if (err) {
> > +                       pr_warn("%s: UHS init failed (%d), retrying without UHS\n",
> > +                               mmc_hostname(host), err);
> > +                       mmc_sdio_pre_init(host, ocr_card,
> > +                                         card != oldcard ? card : NULL);
>
> Is this really correct?
>
> If there is an oldcard we should *not* pass the card (since it would
> be equal to oldcard) to mmc_sdio_pre_init() as it would remove it.
>
> If there is no oldcard (oldcard == NULL), we should pass the card to
> mmc_sdio_pre_init() to make it remove it.
>
> In other words, how about just:
> "oldcard ? NULL : card"
>
> > +                       retries = 0;
> > +                       goto try_again;
> > +               }
> >         } else {
> >                 /*
> >                  * Switch to high-speed (if supported).
> > --
> > 2.53.0
> >
>
> Kind regards
> Uffe



-- 
Deomid "rojer" Ryabkov
rojer@xxxxxxxx