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

[Adrian Hunter <adrian.hunter@intel.com>]

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;


> 
>>
>> 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
>>