Avri Altman wrote on Thu, Oct 26, 2023 at 10:16:53AM +0000:
Reviewed-by: Avri Altman avri.altman@wdc.com
Thanks for the review!
Notes:
- mmc_fixup_device() was rewritten in 5.17, so older stable kernels will need a separate patch... I suppose I can send it to stable after this is merged if we go this way
- struct mmc_cid's and mmc_fixup's oemid fields are unsigned shorts, we probably just want to make them unsigned char instead in which case we don't need that check anymore? But it's kind of nice to have a wider type so CID_OEMID_ANY can never be a match.... Which unfortunately my patch makes moot as ((unsigned short)-1) & 0xff will be 0xff which can match anything...
- this could also be worked around in the _FIXUP_EXT macro that builds the fixup structs, but we're getting ugly here... Or we can just go for the big boom and try to fix all MMC_FIXUP() users in tree and call it a day, but that'll also be fun to backport.
To me, your fix is clean, elegant and does the job. I would let the quirk owners to fix that hard-coded bogus oemid - should they choose to. I guess Sandisk would need to do that as well.
Yes, this was exactly my intention - leave the workaround in place for a while while owners fix their quirks then eventually fix types and remove this when it is no longer needed.
Meanwhile, all stable kernels including the newly released 6.6 have many broken quirks and at the very least the MMC I have here would periodically hang when issuing a flush, so as a selfish user I'd appreciate if this (or something equivalent) could be making its way towards Linus' tree. Ulf, would you have a bit of time to move this forward, or should I ask Greg to temporarily revert Avri's "mmc: core: Capture correct oemid-bits for eMMC cards" commit in stable trees until the way forward is decided?
Thanks!