[PATCH v2] mmc: Capture correct oemid

[PATCH v2] mmc: Capture correct oemid

[Avri Altman]

The OEMID is an 8-bit binary number that identifies the Device OEM
and/or the Device contents (when used as a distribution media either on
ROM or FLASH Devices).  It occupies bits [111:104] in the CID register:
see the eMMC spec JESD84-B51 paragraph 7.2.3.

So it is 8 bits, and has been so since ever - this bug is so ancients I
couldn't even find its source.  The furthest I could go is to commit
335eadf2ef6a (sd: initialize SD cards) but its already was wrong.  Could
be because in SD its indeed 16 bits (a 2-characters ASCII string).
Another option as pointed out by Alex (offlist), it seems like this
comes from the legacy MMC specs (v3.31 and before).

It is important to fix it because we are using it as one of our quirk's
token, as well as other tools, e.g. the LVFS
(https://github.com/fwupd/fwupd/).

Signed-off-by: Avri Altman <avri.altman@wdc.com>
Cc: stable@vger.kernel.org
---
Changelog:

v1--v2:
Add Alex's note of the possible origin of this bug.
---
 drivers/mmc/core/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 89cd48fcec79..4a4bab9aa726 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -104,7 +104,7 @@ static int mmc_decode_cid(struct mmc_card *card)
 	case 3: /* MMC v3.1 - v3.3 */
 	case 4: /* MMC v4 */
 		card->cid.manfid	= UNSTUFF_BITS(resp, 120, 8);
-		card->cid.oemid		= UNSTUFF_BITS(resp, 104, 16);
+		card->cid.oemid		= UNSTUFF_BITS(resp, 104, 8);
 		card->cid.prod_name[0]	= UNSTUFF_BITS(resp, 96, 8);
 		card->cid.prod_name[1]	= UNSTUFF_BITS(resp, 88, 8);
 		card->cid.prod_name[2]	= UNSTUFF_BITS(resp, 80, 8);
-- 
2.42.0

Re: [PATCH v2] mmc: Capture correct oemid

[Ulf Hansson]

On Wed, 27 Sept 2023 at 09:16, Avri Altman <avri.altman@wdc.com> wrote:
>
> The OEMID is an 8-bit binary number that identifies the Device OEM
> and/or the Device contents (when used as a distribution media either on
> ROM or FLASH Devices).  It occupies bits [111:104] in the CID register:
> see the eMMC spec JESD84-B51 paragraph 7.2.3.
>
> So it is 8 bits, and has been so since ever - this bug is so ancients I
> couldn't even find its source.  The furthest I could go is to commit
> 335eadf2ef6a (sd: initialize SD cards) but its already was wrong.  Could
> be because in SD its indeed 16 bits (a 2-characters ASCII string).
> Another option as pointed out by Alex (offlist), it seems like this
> comes from the legacy MMC specs (v3.31 and before).
>
> It is important to fix it because we are using it as one of our quirk's
> token, as well as other tools, e.g. the LVFS
> (https://github.com/fwupd/fwupd/).
>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> Cc: stable@vger.kernel.org

Applied for fixes, thanks!

Kind regards
Uffe


> ---
> Changelog:
>
> v1--v2:
> Add Alex's note of the possible origin of this bug.
> ---
>  drivers/mmc/core/mmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 89cd48fcec79..4a4bab9aa726 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -104,7 +104,7 @@ static int mmc_decode_cid(struct mmc_card *card)
>         case 3: /* MMC v3.1 - v3.3 */
>         case 4: /* MMC v4 */
>                 card->cid.manfid        = UNSTUFF_BITS(resp, 120, 8);
> -               card->cid.oemid         = UNSTUFF_BITS(resp, 104, 16);
> +               card->cid.oemid         = UNSTUFF_BITS(resp, 104, 8);
>                 card->cid.prod_name[0]  = UNSTUFF_BITS(resp, 96, 8);
>                 card->cid.prod_name[1]  = UNSTUFF_BITS(resp, 88, 8);
>                 card->cid.prod_name[2]  = UNSTUFF_BITS(resp, 80, 8);
> --
> 2.42.0
>

Re: [PATCH v2] mmc: Capture correct oemid

[Dominique Martinet]

[note this has been applied to all -stable branches as well -- sorry for
noticing this after explicitly testing the 5.10.199-rc1...]

Avri Altman wrote on Wed, Sep 27, 2023 at 10:15:00AM +0300:
> It is important to fix it because we are using it as one of our quirk's
> token, as well as other tools, e.g. the LVFS
> (https://github.com/fwupd/fwupd/)

On the other hand there are many quirks in drivers/mmc/core/quirks.h
that relied on the value being a short -- I noticed because our MMC
started to show some hangs that were worked around in a quirk that is
apparently no longer applied.

Unfortunately almost none of these are using defines so it's stray 0x100
0x5048 0x200 .. in MMC_FIXUP (3rd arg is oemid), so it'll be difficult
to fix -- especially as embedded downstreams often add their own quirks
and you can't fix that for them.

I'd suggest something like this instead:
-------
diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
index 1e14cc69e0ab..892a5bba36ec 100644
--- a/drivers/mmc/core/quirks.h
+++ b/drivers/mmc/core/quirks.h
@@ -189,7 +189,7 @@ static inline void mmc_fixup_device(struct mmc_card *card,
 		if ((f->manfid == CID_MANFID_ANY ||
 		     f->manfid == card->cid.manfid) &&
 		    (f->oemid == CID_OEMID_ANY ||
-		     f->oemid == card->cid.oemid) &&
+		     (f->oemid & 0xff) == (card->cid.oemid & 0xff)) &&
 		    (f->name == CID_NAME_ANY ||
 		     !strncmp(f->name, card->cid.prod_name,
 			      sizeof(card->cid.prod_name))) &&
-------
(whether to mask cid.oemid or not is up for debate, but that leaves less
room for error)

I'm testing this right now for our board, will submit as a proper patch
later today if it works -- but feel free to comment first.
Missing quirks on certain sd/mmc can cause some trouble so might want to
revert this patch on stable kernels unless there's immediate agreement
on this patch

Thanks,
-- 
Dominique Martinet | Asmadeus