Re: [PATCH] mmc: core: Don't program invalid host driver type for fixed eMMC type

[Ian Ray <ian.ray@gehealthcare.com>]

On Wed, Sep 30, 2026 at 10:00:47PM +0300, Adrian Hunter wrote:
> On 29/09/2026 16:06, Ulf Hansson wrote:
> > + Adrian
> >
> > On Sat, Sep 26, 2026 at 2:29 PM Ian Ray <ian.ray@gehealthcare.com> wrote:
> >>
> >> The host controller driver type value used by the MMC core [1] _mostly_
> >> overlaps with the eMMC device I/O driver strength value [2] as shown in
> >> the table below; however, there is _no_ host mapping for eMMC value 4.
> >
> > I think the mapping exists only because that is what the SD spec
> > provided when this was introduced.
> >
> > We can certainly add something that corresponds to value 4 from the
> > eMMC spec as well, to make this complete.
> >
> >>
> >>   host value/type   | eMMC value | nominal impedance | strength
> >>   ------------------+------------+-------------------+---------
> >>   0 / Type B        | 0          | 50 Ohm            | x1
> >>   1 / Type A        | 1          | 33 Ohm            | x1.5
> >>   2 / Type C        | 2          | 66 Ohm            | x0.75
> >>   3 / Type D        | 3          | 100 Ohm           | x0.5
> >>   (none)            | 4          | 40 Ohm            | x1.2
> >>
> >> [1] MMC_SET_DRIVER_TYPE_* in include/linux/mmc/host.h
> >>
> >> [2] 'fixed-emmc-driver-type', see JESD84-B51 Table 206 and
> >>
> >>
> >> Since commit 5a52c5701a67 ("mmc: core: Fix host controller programming
> >> for fixed driver type"), the selected eMMC driver strength is passed
> >> directly to mmc_set_driver_type().  For a fixed eMMC driver type of 4
> >> this reaches sdhci_set_ios(), which only knows types 0..3 and therefore
> >> complains on every ios update:
> >>
> >>   mmc2: invalid driver type, default to driver type B
> >
> > Perhaps the print above from sdhci_set_ios() is simply a bit
> > misleading. For example, it looks like arasan_select_phy_clock()
> > actually uses the value in ios.drv_type as is.
> >
> > To me, it looks like the problem is that mmc_select_drive_strength()
> > doesn't really do its job correctly, as it should help us to figure
> > out what is supported by the card *and* by the host, so we can request
> > a proper driver type when calling the ->set_ios() callback.
> 
> mmc_select_drive_strength() returns the card value directly and
> the host value in *drv_type.
> 
> fixed-emmc-driver-type as described by
> Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
> doesn't mention the host controller, and the original commits
> 6186d06c519e2 "mmc: parse new binding for eMMC fixed driver type"
> be17f1ce8572d "mmc: core: properly init drv_type" seemingly
> deliberately left it alone.
> 
> 5a52c5701a67d "mmc: core: Fix host controller programming for
> fixed driver type" introduces a new policy: fixed-emmc-driver-type
> is a host controller setting also.
> 
> If that was done by mmc_select_drive_strength(), it would look
> something like below ( note it no longer allows fixed_drv_type to override
> ->select_drive_strength() , and fixed-emmc-driver-type will then hit
> any nonremovable SD or SDIO ):
> 
> int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
>                               int card_drv_type, int *drv_type)
> {
>         struct mmc_host *host = card->host;
>         int fixed_drv_type = host->fixed_drv_type;
>         int host_drv_type = SD_DRIVER_TYPE_B;
> 
>         *drv_type = 0;
> 
>         if (!host->ops->select_drive_strength) {
>                 if (fixed_drv_type >= 0 &&
>                     (card_drv_type & mmc_driver_type_mask(fixed_drv_type))) {
>                         *drv_type = fixed_drv_type;
>                         return fixed_drv_type;
>                 }
>                 return 0;
>         }
> 
>         /* Use SD definition of driver strength for hosts */
>         if (host->caps & MMC_CAP_DRIVER_TYPE_A)
>                 host_drv_type |= SD_DRIVER_TYPE_A;
> 
>         if (host->caps & MMC_CAP_DRIVER_TYPE_C)
>                 host_drv_type |= SD_DRIVER_TYPE_C;
> 
>         if (host->caps & MMC_CAP_DRIVER_TYPE_D)
>                 host_drv_type |= SD_DRIVER_TYPE_D;
> 
>         /*
>          * The drive strength that the hardware can support
>          * depends on the board design.  Pass the appropriate
>          * information and let the hardware specific code
>          * return what is possible given the options
>          */
>         return host->ops->select_drive_strength(card, max_dtr,
>                                                 host_drv_type,
>                                                 card_drv_type,
>                                                 drv_type);
> }
> 
> and:
> 
> static void mmc_select_driver_type(struct mmc_card *card)
> {
>         int card_drv_type, drive_strength, drv_type = 0;
> 
>         card_drv_type = card->ext_csd.raw_driver_strength |
>                         mmc_driver_type_mask(0);
> 
>         drive_strength = mmc_select_drive_strength(card,
>                                                    card->ext_csd.hs200_max_dtr,
>                                                    card_drv_type, &drv_type);
> 
>         card->drive_strength = drive_strength;
> 
>         if (drv_type)
>                 mmc_set_driver_type(card->host, drv_type);
> }
> 
> But that doesn't help with drivers where the controller does
> do not support the value passed by fixed-emmc-driver-type even
> though the card does.  mmc_select_drive_strength() has no way
> of knowing that.  The ->select_drive_strength() callback
> allows selecting different values. fixed-emmc-driver-type is
> one value.
> 
> The warning in sdhci.c seems correct.  SDHCI is an SD standard,
> so doesn't define value 4 (and only has 2 bits for the field).
> The selected driver strength is not being programmed - deserves
> a warning.
> 
> Perhaps if the affected driver implements
> ->select_drive_strength():
> 
> static int ???_select_drive_strength(struct mmc_card *card,
>                                      unsigned int max_dtr, int host_drv,
>                                      int card_drv, int *host_driver_strength)
> {
>         struct mmc_host *host = card->host;
>         int fixed_drv_type = host->fixed_drv_type;
> 
>         if (fixed_drv_type >= 0 &&
>             (card_drv & mmc_driver_type_mask(fixed_drv_type))) {
>                 if (fixed_drv_type == 4)
>                         *drv_type = MMC_SET_DRIVER_TYPE_B;
>                 else
>                         *drv_type = fixed_drv_type;
>                 return fixed_drv_type;
>         }
>         return 0;
> }
> 
> But that requires the changes to
> mmc_select_drive_strength() and mmc_select_driver_type()
> above (with its caveats), so that ->select_drive_strength()
> overrides fixed-emmc-driver-type.  That would be fine for
> drivers/mmc/host/sdhci-pci-core.c, and probably
> amd_select_drive_strength(), but it is starting to get
> complicated.
> 
> A simpler alternative is for the affected driver to hook
> ->set_ios() and fix up the value.
> 
> void ???_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> {
>         if (ios->drv_type == 4)
>                 ios->drv_type = MMC_SET_DRIVER_TYPE_B;
> 
>         return sdhci_set_ios(mmc, ios);
> }
> 
> And in probe (assuming struct sdhci_host *host):
> 
>         host->mmc_host_ops.set_ios = ???_set_ios;
> 

Thank you for the feedback.

The "invalid driver type" warning that I originally observed originates
in sdhci_set_ios_common() which is shared by all SDHCI hosts (IIUC).

Thus any SDHCI host configured with `fixed-emmc-driver-type = <4>` would
log that warning.

The affected driver for my scenario is sdhci-esdhc-imx.c, and the
proposed change (below) suppresses the warning.

But I am hesitant to open-code the same fixup in every SDHCI driver.
And I am unsure whether this belongs per-driver, or whether the right
fix is in the core / or to the warning itself.

Comments welcome!

drivers/mmc/host/sdhci-esdhc-imx.c

```
static void sdhci_esdhc_imx_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
       /*
        * A drv_type of 4 reaches this callback only via the eMMC
        * 'fixed-emmc-driver-type' path: it is the eMMC I/O driver strength
        * value 4 (40 Ohm, see JESD84-B51 Table 206), which has no equivalent
        * host driver type in the SD standard that SDHCI implements (only
        * types B/A/C/D are defined).  Map it to the default Type B here to
        * avoid an "invalid driver type" warning from sdhci_set_ios().  The
        * card's HS_TIMING field is programmed separately from
        * card->drive_strength and is unaffected.
        */
       if (ios->drv_type == 4)
               ios->drv_type = MMC_SET_DRIVER_TYPE_B;

       sdhci_set_ios(mmc, ios);
}

static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
{
...
               /*
                * Support fix up of eMMC driver types which have no SDHCI
                * host driver type equivalent.
                */
               host->mmc_host_ops.set_ios = sdhci_esdhc_imx_set_ios;

```

> 
> >
> >>
> >> The mmc card itself is still programmed correctly (the HS_TIMING driver
> >> strength field is set from card->drive_strength independently), so this
> >> is a spurious, repeated warning for a valid, documented configuration.
> >>
> >> Change to only program the host when the eMMC driver type value maps to
> >> a valid host driver type.
> >>
> >> Tested on i.MX8MP (usdhc3, eMMC in HS400) with
> >> `fixed-emmc-driver-type = <4>`: the warning is no longer logged and the
> >> HS_TIMING register contains 0x43 as expected.
> >>
> >> Fixes: 5a52c5701a67 ("mmc: core: Fix host controller programming for fixed driver type")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> >
> > I have looped in Adrian, to get his opinion on this.
> >
> > Kind regards
> > Uffe
> >
> >> ---
> >>  drivers/mmc/core/mmc.c | 8 +++++---
> >>  1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >> index 05444ecf3909..ff5d4b805998 100644
> >> --- a/drivers/mmc/core/mmc.c
> >> +++ b/drivers/mmc/core/mmc.c
> >> @@ -1371,9 +1371,11 @@ static void mmc_select_driver_type(struct mmc_card *card)
> >>
> >>         card->drive_strength = drive_strength;
> >>
> >> -       if (fixed_drv_type >= 0 && drive_strength)
> >> -               mmc_set_driver_type(card->host, drive_strength);
> >> -       else if (drv_type)
> >> +       if (fixed_drv_type >= 0 && drive_strength) {
> >> +               /* eMMC driver types >= 4 have no host controller equivalent. */
> >> +               if (drive_strength <= MMC_SET_DRIVER_TYPE_D)
> >> +                       mmc_set_driver_type(card->host, drive_strength);
> >> +       } else if (drv_type)
> >>                 mmc_set_driver_type(card->host, drv_type);
> >>  }
> >>
> >> --
> >> 2.49.0
> >>
>