| Message ID | 1505000687.23171.20.camel@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10 September 2017 at 01:44, Christoph Fritz <chf.fritz@googlemail.com> wrote: > To get an ATP card work reliable on a high speed bus, CMD23 needs > to be disabled. > > Without this patch: > > $ dd if=/dev/urandom of=/mnt/test bs=1M count=10 > > <mmc0: starting CMD23 arg 00000400 flags 00000015> > mmc0: starting CMD25 arg 00a71f00 flags 000000b5 > mmc0: blksz 512 blocks 1024 flags 00000100 tsac 3000 ms nsac 0 > mmc0: CMD12 arg 00000000 flags 0000049d > sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00000001 > mmc0: Timeout waiting for hardware interrupt. Seems like you are using some sdhci variant. Would it be possible for you to test this card on another platform? That to make sure it's not a driver thing. > > Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com> > --- > Changes since v1: > - s/CMD32/CMD23 > > drivers/mmc/core/card.h | 1 + > drivers/mmc/core/quirks.h | 6 ++++++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h > index f06cd91..af9c48c 100644 > --- a/drivers/mmc/core/card.h > +++ b/drivers/mmc/core/card.h > @@ -75,6 +75,7 @@ struct mmc_fixup { > #define EXT_CSD_REV_ANY (-1u) > > #define CID_MANFID_SANDISK 0x2 > +#define CID_MANFID_ATP 0x9 > #define CID_MANFID_TOSHIBA 0x11 > #define CID_MANFID_MICRON 0x13 > #define CID_MANFID_SAMSUNG 0x15 > diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h > index fb72593..3205f0e 100644 > --- a/drivers/mmc/core/quirks.h > +++ b/drivers/mmc/core/quirks.h > @@ -52,6 +52,12 @@ static const struct mmc_fixup mmc_blk_fixups[] = { > MMC_QUIRK_BLK_NO_CMD23), > > /* > + * Some SD cards lockup while using CMD23 multiblock transfers. > + */ > + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, > + MMC_QUIRK_BLK_NO_CMD23), Is really all ATP cards having this problem? Perhaps we should consider making this a bit more fine grained? On the other hand, this may be the safest way to do it... > + > + /* > * Some MMC cards need longer data read timeout than indicated in CSD. > */ > MMC_FIXUP(CID_NAME_ANY, CID_MANFID_MICRON, 0x200, add_quirk_mmc, > -- > 2.1.4 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi On 2017/9/22 16:42, Ulf Hansson wrote: > On 10 September 2017 at 01:44, Christoph Fritz <chf.fritz@googlemail.com> wrote: >> To get an ATP card work reliable on a high speed bus, CMD23 needs >> to be disabled. >> >> Without this patch: >> >> $ dd if=/dev/urandom of=/mnt/test bs=1M count=10 >> >> <mmc0: starting CMD23 arg 00000400 flags 00000015> >> mmc0: starting CMD25 arg 00a71f00 flags 000000b5 >> mmc0: blksz 512 blocks 1024 flags 00000100 tsac 3000 ms nsac 0 >> mmc0: CMD12 arg 00000000 flags 0000049d >> sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00000001 >> mmc0: Timeout waiting for hardware interrupt. > > Seems like you are using some sdhci variant. Would it be possible for > you to test this card on another platform? That to make sure it's not > a driver thing. > I'm just curious about if these ATP cards could work for ACMD23? Could you kindly try this patch? |https://patchwork.kernel.org/patch/9887651/ >> >> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com> >> --- >> Changes since v1: >> - s/CMD32/CMD23 >> >> drivers/mmc/core/card.h | 1 + >> drivers/mmc/core/quirks.h | 6 ++++++ >> 2 files changed, 7 insertions(+) >> >> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h >> index f06cd91..af9c48c 100644 >> --- a/drivers/mmc/core/card.h >> +++ b/drivers/mmc/core/card.h >> @@ -75,6 +75,7 @@ struct mmc_fixup { >> #define EXT_CSD_REV_ANY (-1u) >> >> #define CID_MANFID_SANDISK 0x2 >> +#define CID_MANFID_ATP 0x9 >> #define CID_MANFID_TOSHIBA 0x11 >> #define CID_MANFID_MICRON 0x13 >> #define CID_MANFID_SAMSUNG 0x15 >> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h >> index fb72593..3205f0e 100644 >> --- a/drivers/mmc/core/quirks.h >> +++ b/drivers/mmc/core/quirks.h >> @@ -52,6 +52,12 @@ static const struct mmc_fixup mmc_blk_fixups[] = { >> MMC_QUIRK_BLK_NO_CMD23), >> >> /* >> + * Some SD cards lockup while using CMD23 multiblock transfers. >> + */ >> + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, >> + MMC_QUIRK_BLK_NO_CMD23), > > Is really all ATP cards having this problem? Perhaps we should > consider making this a bit more fine grained? > > On the other hand, this may be the safest way to do it... > >> + >> + /* >> * Some MMC cards need longer data read timeout than indicated in CSD. >> */ >> MMC_FIXUP(CID_NAME_ANY, CID_MANFID_MICRON, 0x200, add_quirk_mmc, >> -- >> 2.1.4 >> > > Kind regards > Uffe > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf and Shawn, thanks for your input, I'm replying to both of you by this mail, please see comments below. On Fri, 2017-09-22 at 17:26 +0800, Shawn Lin wrote: > Hi > > On 2017/9/22 16:42, Ulf Hansson wrote: > > On 10 September 2017 at 01:44, Christoph Fritz <chf.fritz@googlemail.com> wrote: > >> To get an ATP card work reliable on a high speed bus, CMD23 needs > >> to be disabled. > >> > >> Without this patch: > >> > >> $ dd if=/dev/urandom of=/mnt/test bs=1M count=10 > >> > >> <mmc0: starting CMD23 arg 00000400 flags 00000015> > >> mmc0: starting CMD25 arg 00a71f00 flags 000000b5 > >> mmc0: blksz 512 blocks 1024 flags 00000100 tsac 3000 ms nsac 0 > >> mmc0: CMD12 arg 00000000 flags 0000049d > >> sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00000001 > >> mmc0: Timeout waiting for hardware interrupt. > > > > Seems like you are using some sdhci variant. Would it be possible for > > you to test this card on another platform? That to make sure it's not > > a driver thing. SoC am335x (omap_hsmmc) works fine because it lacks CMD23 host support. My RK3288 board supports only micro-SD cards. If you want me to, I could get an adapter (micro-SD -> SD) but the shipping takes ~40 days. > > > > I'm just curious about if these ATP cards could work for ACMD23? > > Could you kindly try this patch? > > |https://patchwork.kernel.org/patch/9887651/ By looking at your patch I can see that bool variable 'need_acmd23' is only set when card does not support CMD23. But here this card (falsely) claims to support CMD23, which is the whole point of the NO_CMD23 quirk. So testing your patch would not make any difference, because its functionality would be disabled, or do I miss something? > > > >> > >> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com> > >> --- > >> Changes since v1: > >> - s/CMD32/CMD23 > >> > >> drivers/mmc/core/card.h | 1 + > >> drivers/mmc/core/quirks.h | 6 ++++++ > >> 2 files changed, 7 insertions(+) > >> > >> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h > >> index f06cd91..af9c48c 100644 > >> --- a/drivers/mmc/core/card.h > >> +++ b/drivers/mmc/core/card.h > >> @@ -75,6 +75,7 @@ struct mmc_fixup { > >> #define EXT_CSD_REV_ANY (-1u) > >> > >> #define CID_MANFID_SANDISK 0x2 > >> +#define CID_MANFID_ATP 0x9 > >> #define CID_MANFID_TOSHIBA 0x11 > >> #define CID_MANFID_MICRON 0x13 > >> #define CID_MANFID_SAMSUNG 0x15 > >> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h > >> index fb72593..3205f0e 100644 > >> --- a/drivers/mmc/core/quirks.h > >> +++ b/drivers/mmc/core/quirks.h > >> @@ -52,6 +52,12 @@ static const struct mmc_fixup mmc_blk_fixups[] = { > >> MMC_QUIRK_BLK_NO_CMD23), > >> > >> /* > >> + * Some SD cards lockup while using CMD23 multiblock transfers. > >> + */ > >> + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, > >> + MMC_QUIRK_BLK_NO_CMD23), > > > > Is really all ATP cards having this problem? Perhaps we should > > consider making this a bit more fine grained? > > > > On the other hand, this may be the safest way to do it... I also thought about this and set CID_OEMID_ANY because I wanted to be on the safe side. Maybe ATP can comment on this (in CC)? Thanks -- Christoph -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi， On 2017/9/24 3:51, Christoph Fritz wrote: > Hi Ulf and Shawn, > > thanks for your input, I'm replying to both of you by this mail, please > see comments below. > > On Fri, 2017-09-22 at 17:26 +0800, Shawn Lin wrote: >> Hi >> >> On 2017/9/22 16:42, Ulf Hansson wrote: >>> On 10 September 2017 at 01:44, Christoph Fritz <chf.fritz@googlemail.com> wrote: >>>> To get an ATP card work reliable on a high speed bus, CMD23 needs >>>> to be disabled. >>>> >>>> Without this patch: >>>> >>>> $ dd if=/dev/urandom of=/mnt/test bs=1M count=10 >>>> >>>> <mmc0: starting CMD23 arg 00000400 flags 00000015> >>>> mmc0: starting CMD25 arg 00a71f00 flags 000000b5 >>>> mmc0: blksz 512 blocks 1024 flags 00000100 tsac 3000 ms nsac 0 >>>> mmc0: CMD12 arg 00000000 flags 0000049d >>>> sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00000001 >>>> mmc0: Timeout waiting for hardware interrupt. >>> >>> Seems like you are using some sdhci variant. Would it be possible for >>> you to test this card on another platform? That to make sure it's not >>> a driver thing. > > SoC am335x (omap_hsmmc) works fine because it lacks CMD23 host support. > My RK3288 board supports only micro-SD cards. If you want me to, I could > get an adapter (micro-SD -> SD) but the shipping takes ~40 days. > >>> >> >> I'm just curious about if these ATP cards could work for ACMD23? >> >> Could you kindly try this patch? >> >> |https://patchwork.kernel.org/patch/9887651/ > > By looking at your patch I can see that bool variable 'need_acmd23' is > only set when card does not support CMD23. But here this card (falsely) > claims to support CMD23, which is the whole point of the NO_CMD23 quirk. > > So testing your patch would not make any difference, because its yup, sorry fot that. I mean you could hack it a little to mask CMD23 support and let ACMD23 take effect, and see how it would go? > functionality would be disabled, or do I miss something? > >> >> >>>> >>>> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com> >>>> --- >>>> Changes since v1: >>>> - s/CMD32/CMD23 >>>> >>>> drivers/mmc/core/card.h | 1 + >>>> drivers/mmc/core/quirks.h | 6 ++++++ >>>> 2 files changed, 7 insertions(+) >>>> >>>> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h >>>> index f06cd91..af9c48c 100644 >>>> --- a/drivers/mmc/core/card.h >>>> +++ b/drivers/mmc/core/card.h >>>> @@ -75,6 +75,7 @@ struct mmc_fixup { >>>> #define EXT_CSD_REV_ANY (-1u) >>>> >>>> #define CID_MANFID_SANDISK 0x2 >>>> +#define CID_MANFID_ATP 0x9 >>>> #define CID_MANFID_TOSHIBA 0x11 >>>> #define CID_MANFID_MICRON 0x13 >>>> #define CID_MANFID_SAMSUNG 0x15 >>>> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h >>>> index fb72593..3205f0e 100644 >>>> --- a/drivers/mmc/core/quirks.h >>>> +++ b/drivers/mmc/core/quirks.h >>>> @@ -52,6 +52,12 @@ static const struct mmc_fixup mmc_blk_fixups[] = { >>>> MMC_QUIRK_BLK_NO_CMD23), >>>> >>>> /* >>>> + * Some SD cards lockup while using CMD23 multiblock transfers. >>>> + */ >>>> + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, >>>> + MMC_QUIRK_BLK_NO_CMD23), >>> >>> Is really all ATP cards having this problem? Perhaps we should >>> consider making this a bit more fine grained? >>> >>> On the other hand, this may be the safest way to do it... > > I also thought about this and set CID_OEMID_ANY because I wanted to be > on the safe side. Maybe ATP can comment on this (in CC)? > > Thanks > -- Christoph > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2017-09-25 at 11:33 +0800, Shawn Lin wrote: > >> I'm just curious about if these ATP cards could work for ACMD23? > >> > >> Could you kindly try this patch? > >> > >> |https://patchwork.kernel.org/patch/9887651/ > > > > By looking at your patch I can see that bool variable 'need_acmd23' is > > only set when card does not support CMD23. But here this card (falsely) > > claims to support CMD23, which is the whole point of the NO_CMD23 quirk. > > > > So testing your patch would not make any difference, because its > > yup, sorry fot that. I mean you could hack it a little to mask > CMD23 support and let ACMD23 take effect, and see how it would > go? forcing ACMD23 instead of CMD23 works fine: dd if=/dev/urandom of=/mnt/test bs=1M count=100 conv=fsync 100+0 records in 100+0 records out 104857600 bytes (105 MB) copied, 9.9496 s, 10.5 MB/s It's as fast as using my NO_CMD23 quirk. > >>>> /* > >>>> + * Some SD cards lockup while using CMD23 multiblock transfers. > >>>> + */ > >>>> + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, > >>>> + MMC_QUIRK_BLK_NO_CMD23), > >>> > >>> Is really all ATP cards having this problem? Perhaps we should > >>> consider making this a bit more fine grained? > >>> > >>> On the other hand, this may be the safest way to do it... > > > > I also thought about this and set CID_OEMID_ANY because I wanted to be > > on the safe side. Maybe ATP can comment on this (in CC)? Thanks -- Christoph -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017/9/25 19:23, Christoph Fritz wrote: > On Mon, 2017-09-25 at 11:33 +0800, Shawn Lin wrote: >>>> I'm just curious about if these ATP cards could work for ACMD23? >>>> >>>> Could you kindly try this patch? >>>> >>>> |https://patchwork.kernel.org/patch/9887651/ >>> >>> By looking at your patch I can see that bool variable 'need_acmd23' is >>> only set when card does not support CMD23. But here this card (falsely) >>> claims to support CMD23, which is the whole point of the NO_CMD23 quirk. >>> >>> So testing your patch would not make any difference, because its >> >> yup, sorry fot that. I mean you could hack it a little to mask >> CMD23 support and let ACMD23 take effect, and see how it would >> go? > > forcing ACMD23 instead of CMD23 works fine: > > dd if=/dev/urandom of=/mnt/test bs=1M count=100 conv=fsync > 100+0 records in > 100+0 records out > 104857600 bytes (105 MB) copied, 9.9496 s, 10.5 MB/s > > It's as fast as using my NO_CMD23 quirk. Thanks for the test. Given that ACMD23 is mandatory but very few sd cards claim to support CMD23, maybe force ACMD23 support is better than current CMD23/open-ending combo. Back to $subject patch, It still need to verify this conclusion on other platforms or get acknowledgement from the vendor. > >>>>>> /* >>>>>> + * Some SD cards lockup while using CMD23 multiblock transfers. >>>>>> + */ >>>>>> + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, >>>>>> + MMC_QUIRK_BLK_NO_CMD23), >>>>> >>>>> Is really all ATP cards having this problem? Perhaps we should >>>>> consider making this a bit more fine grained? >>>>> >>>>> On the other hand, this may be the safest way to do it... >>> >>> I also thought about this and set CID_OEMID_ANY because I wanted to be >>> on the safe side. Maybe ATP can comment on this (in CC)? > > Thanks > -- Christoph > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Shawn and Ulf On Tue, 2017-09-26 at 08:56 +0800, Shawn Lin wrote: > Back to $subject patch, It still need to verify this conclusion on other > platforms or get acknowledgement from the vendor. to quote myself: On Sat, 2017-09-23 at 21:51 +0200, Christoph Fritz wrote: > > SoC am335x (omap_hsmmc) works fine because it lacks CMD23 host support. > > My RK3288 board supports only micro-SD cards. If you want me to, I could > > get an adapter (micro-SD -> SD) but the shipping takes ~40 days. Finally I got this microSD to SD adapter to test the ATP card on RK3288 platform too. As expected, card fails without the patch: $ dd if=/dev/urandom of=./test bs=1M count=10 does freeze system without any response So I can verify failing CMD23 behaviour on imx6 and rk3288 running kernel 4.14. Applying the patch does fix the issue on both platforms. Could you queue up this patch now? Thanks -- Christoph -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2017-12-04 at 14:38 +0100, Christoph Fritz wrote: > Finally I got this microSD to SD adapter to test the ATP card on RK3288 > platform too. As expected, card fails without the patch: > > $ dd if=/dev/urandom of=./test bs=1M count=10 > > does freeze system without any response > > So I can verify failing CMD23 behaviour on imx6 and rk3288 running > kernel 4.14. Applying the patch does fix the issue on both platforms. > > Could you queue up this patch now? *ping* Meanwhile I found another card which fails CMD23 by testing it on rk3288 and i.MX6. I'll send a patch for both cards as v3 of this Patch. best regards -- Christoph -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h index f06cd91..af9c48c 100644 --- a/drivers/mmc/core/card.h +++ b/drivers/mmc/core/card.h @@ -75,6 +75,7 @@ struct mmc_fixup { #define EXT_CSD_REV_ANY (-1u) #define CID_MANFID_SANDISK 0x2 +#define CID_MANFID_ATP 0x9 #define CID_MANFID_TOSHIBA 0x11 #define CID_MANFID_MICRON 0x13 #define CID_MANFID_SAMSUNG 0x15 diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index fb72593..3205f0e 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -52,6 +52,12 @@ static const struct mmc_fixup mmc_blk_fixups[] = { MMC_QUIRK_BLK_NO_CMD23), /* + * Some SD cards lockup while using CMD23 multiblock transfers. + */ + MMC_FIXUP("AF SD", CID_MANFID_ATP, CID_OEMID_ANY, add_quirk_sd, + MMC_QUIRK_BLK_NO_CMD23), + + /* * Some MMC cards need longer data read timeout than indicated in CSD. */ MMC_FIXUP(CID_NAME_ANY, CID_MANFID_MICRON, 0x200, add_quirk_mmc,
To get an ATP card work reliable on a high speed bus, CMD23 needs to be disabled. Without this patch: $ dd if=/dev/urandom of=/mnt/test bs=1M count=10 <mmc0: starting CMD23 arg 00000400 flags 00000015> mmc0: starting CMD25 arg 00a71f00 flags 000000b5 mmc0: blksz 512 blocks 1024 flags 00000100 tsac 3000 ms nsac 0 mmc0: CMD12 arg 00000000 flags 0000049d sdhci [sdhci_irq()]: *** mmc0 got interrupt: 0x00000001 mmc0: Timeout waiting for hardware interrupt. Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com> --- Changes since v1: - s/CMD32/CMD23 drivers/mmc/core/card.h | 1 + drivers/mmc/core/quirks.h | 6 ++++++ 2 files changed, 7 insertions(+)