| Message ID | 20230927071500.1791882-1-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] mmc: Capture correct oemid | expand |
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 >
[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,
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);
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(-)