| Message ID | 20240523132016.599343-1-linux-mmc@danman.eu |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] mmc: core: allow detection of locked cards | expand |
On 2024-05-23 15:20, linux-mmc@danman.eu wrote: > From: Daniel Kucera <linux-mmc@danman.eu> > > Locked card will not reply to SEND_SCR or SD_STATUS commands > so it was failing to initialize previously. When skipped, > the card will get initialized and CMD42 can be sent using > ioctl to unlock the card or remove password protection. > Until unlocked, all read/write calls will timeout. > > Signed-off-by: Daniel Kucera <linux-mmc@danman.eu> > --- > drivers/mmc/core/sd.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 1c8148cdd..ae821df7d 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host *host, > struct mmc_card *card, > bool reinit) > { > int err; > + u32 card_status; > > - if (!reinit) { > + err = mmc_send_status(card, &card_status); > + if (err){ > + pr_err("%s: unable to get card status\n", mmc_hostname(host)); > + return err; > + } > + > + if (card_status & R1_CARD_IS_LOCKED){ > + pr_warn("%s: card is locked\n", mmc_hostname(host)); > + } > + > + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { > /* > * Fetch SCR from card. > */ Any feedback please? D.
> On 2024-05-23 15:20, linux-mmc@danman.eu wrote: > > From: Daniel Kucera <linux-mmc@danman.eu> > > > > Locked card will not reply to SEND_SCR or SD_STATUS commands so it was > > failing to initialize previously. When skipped, the card will get > > initialized and CMD42 can be sent using ioctl to unlock the card or > > remove password protection. > > Until unlocked, all read/write calls will timeout. > > > > Signed-off-by: Daniel Kucera <linux-mmc@danman.eu> > > --- > > drivers/mmc/core/sd.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index > > 1c8148cdd..ae821df7d 100644 > > --- a/drivers/mmc/core/sd.c > > +++ b/drivers/mmc/core/sd.c > > @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host *host, > > struct mmc_card *card, > > bool reinit) > > { > > int err; > > + u32 card_status; > > > > - if (!reinit) { > > + err = mmc_send_status(card, &card_status); > > + if (err){ > > + pr_err("%s: unable to get card status\n", mmc_hostname(host)); > > + return err; > > + } > > + > > + if (card_status & R1_CARD_IS_LOCKED){ > > + pr_warn("%s: card is locked\n", mmc_hostname(host)); > > + } > > + > > + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { > > /* > > * Fetch SCR from card. > > */ > > Any feedback please? You didn't address my comment from your v1 - Since eMMC & SD shares the very same locking feature (non-COP SD that is) - You should at least explain in your commit log why it isn't an issue for eMMC, If indeed it is not. Thanks, Avri > > D.
Hello Avri, On 2024-06-02 07:26, Avri Altman wrote: >> On 2024-05-23 15:20, linux-mmc@danman.eu wrote: >> > From: Daniel Kucera <linux-mmc@danman.eu> >> > >> > Locked card will not reply to SEND_SCR or SD_STATUS commands so it was >> > failing to initialize previously. When skipped, the card will get >> > initialized and CMD42 can be sent using ioctl to unlock the card or >> > remove password protection. >> > Until unlocked, all read/write calls will timeout. >> > >> > Signed-off-by: Daniel Kucera <linux-mmc@danman.eu> >> > --- >> > drivers/mmc/core/sd.c | 13 ++++++++++++- >> > 1 file changed, 12 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index >> > 1c8148cdd..ae821df7d 100644 >> > --- a/drivers/mmc/core/sd.c >> > +++ b/drivers/mmc/core/sd.c >> > @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host *host, >> > struct mmc_card *card, >> > bool reinit) >> > { >> > int err; >> > + u32 card_status; >> > >> > - if (!reinit) { >> > + err = mmc_send_status(card, &card_status); >> > + if (err){ >> > + pr_err("%s: unable to get card status\n", mmc_hostname(host)); >> > + return err; >> > + } >> > + >> > + if (card_status & R1_CARD_IS_LOCKED){ >> > + pr_warn("%s: card is locked\n", mmc_hostname(host)); >> > + } >> > + >> > + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { >> > /* >> > * Fetch SCR from card. >> > */ >> >> Any feedback please? > You didn't address my comment from your v1 - > Since eMMC & SD shares the very same locking feature (non-COP SD that > is) - > You should at least explain in your commit log why it isn't an issue > for eMMC, > If indeed it is not. I'm sorry, I didn't get what you mean by that. I am touching only the sd.c code, not the mmc.c (where eMMC is initialized, am I correct?). How should I address this? Should I test with eMMC to SD adaptor? I don't have any currently. I am sorry if these are stupid questions, I am a layman. > > Thanks, > Avri > >> >> D. Thank you. Daniel.
> Hello Avri, > > On 2024-06-02 07:26, Avri Altman wrote: > >> On 2024-05-23 15:20, linux-mmc@danman.eu wrote: > >> > From: Daniel Kucera <linux-mmc@danman.eu> > >> > > >> > Locked card will not reply to SEND_SCR or SD_STATUS commands so it > >> > was failing to initialize previously. When skipped, the card will > >> > get initialized and CMD42 can be sent using ioctl to unlock the > >> > card or remove password protection. > >> > Until unlocked, all read/write calls will timeout. > >> > > >> > Signed-off-by: Daniel Kucera <linux-mmc@danman.eu> > >> > --- > >> > drivers/mmc/core/sd.c | 13 ++++++++++++- > >> > 1 file changed, 12 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index > >> > 1c8148cdd..ae821df7d 100644 > >> > --- a/drivers/mmc/core/sd.c > >> > +++ b/drivers/mmc/core/sd.c > >> > @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host *host, > >> > struct mmc_card *card, > >> > bool reinit) > >> > { > >> > int err; > >> > + u32 card_status; > >> > > >> > - if (!reinit) { > >> > + err = mmc_send_status(card, &card_status); > >> > + if (err){ > >> > + pr_err("%s: unable to get card status\n", mmc_hostname(host)); > >> > + return err; > >> > + } > >> > + > >> > + if (card_status & R1_CARD_IS_LOCKED){ > >> > + pr_warn("%s: card is locked\n", mmc_hostname(host)); > >> > + } > >> > + > >> > + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { > >> > /* > >> > * Fetch SCR from card. > >> > */ > >> > >> Any feedback please? > > You didn't address my comment from your v1 - Since eMMC & SD shares > > the very same locking feature (non-COP SD that > > is) - > > You should at least explain in your commit log why it isn't an issue > > for eMMC, If indeed it is not. > > I'm sorry, I didn't get what you mean by that. I am touching only the sd.c code, not > the mmc.c (where eMMC is initialized, am I correct?). > How should I address this? > Should I test with eMMC to SD adaptor? I don't have any currently. Theoretically, looking in the eMMC spec, a locked eMMC device shouldn't have any issue returning from power down. The only flow that is affected is that its not allowed to switch to hs200 in a locked state until unlocked - not to say that it is a problem. If you can't verify that via code review, can you test your mmc-utils code on an eMMC platform? Thanks, Avri > > I am sorry if these are stupid questions, I am a layman. > > > > > Thanks, > > Avri > > > >> > >> D. > > Thank you. > Daniel.
Hi Avri, On 2024-06-02 14:59, Avri Altman wrote: >> Hello Avri, >> >> On 2024-06-02 07:26, Avri Altman wrote: >> >> On 2024-05-23 15:20, linux-mmc@danman.eu wrote: >> >> > From: Daniel Kucera <linux-mmc@danman.eu> >> >> > >> >> > Locked card will not reply to SEND_SCR or SD_STATUS commands so it >> >> > was failing to initialize previously. When skipped, the card will >> >> > get initialized and CMD42 can be sent using ioctl to unlock the >> >> > card or remove password protection. >> >> > Until unlocked, all read/write calls will timeout. >> >> > >> >> > Signed-off-by: Daniel Kucera <linux-mmc@danman.eu> >> >> > --- >> >> > drivers/mmc/core/sd.c | 13 ++++++++++++- >> >> > 1 file changed, 12 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index >> >> > 1c8148cdd..ae821df7d 100644 >> >> > --- a/drivers/mmc/core/sd.c >> >> > +++ b/drivers/mmc/core/sd.c >> >> > @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host *host, >> >> > struct mmc_card *card, >> >> > bool reinit) >> >> > { >> >> > int err; >> >> > + u32 card_status; >> >> > >> >> > - if (!reinit) { >> >> > + err = mmc_send_status(card, &card_status); >> >> > + if (err){ >> >> > + pr_err("%s: unable to get card status\n", mmc_hostname(host)); >> >> > + return err; >> >> > + } >> >> > + >> >> > + if (card_status & R1_CARD_IS_LOCKED){ >> >> > + pr_warn("%s: card is locked\n", mmc_hostname(host)); >> >> > + } >> >> > + >> >> > + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { >> >> > /* >> >> > * Fetch SCR from card. >> >> > */ >> >> >> >> Any feedback please? >> > You didn't address my comment from your v1 - Since eMMC & SD shares >> > the very same locking feature (non-COP SD that >> > is) - >> > You should at least explain in your commit log why it isn't an issue >> > for eMMC, If indeed it is not. >> >> I'm sorry, I didn't get what you mean by that. I am touching only the >> sd.c code, not >> the mmc.c (where eMMC is initialized, am I correct?). >> How should I address this? >> Should I test with eMMC to SD adaptor? I don't have any currently. > Theoretically, looking in the eMMC spec, a locked eMMC device > shouldn't have any issue returning from power down. > The only flow that is affected is that its not allowed to switch to > hs200 in a locked state until unlocked - not to say that it is a > problem. > If you can't verify that via code review, can you test your mmc-utils > code on an eMMC platform? I've just tested with an eMMC to SD adapter in my reader and it is detected correctly: [1463181.072006] mmc1: unexpected status 0x2000900 after switch [1463181.074560] mmc1: unexpected status 0x2000900 after switch [1463181.077038] mmc1: unexpected status 0x2000900 after switch [1463181.079709] mmc1: unexpected status 0x2000900 after switch [1463181.081972] mmc1: unexpected status 0x2000900 after switch [1463181.083412] mmc1: unexpected status 0x2000900 after switch [1463181.084831] mmc1: unexpected status 0x2000900 after switch [1463181.084836] mmc1: new high speed MMC card at address 0001 [1463181.085195] mmcblk1: mmc1:0001 004GA0 2.59 GiB Do I need to do some changes to the patch? > > Thanks, > Avri >> >> I am sorry if these are stupid questions, I am a layman. >> >> > >> > Thanks, >> > Avri >> > >> >> >> >> D. >> >> Thank you. >> Daniel. Thank you, Daniel
> Hi Avri, > > On 2024-06-02 14:59, Avri Altman wrote: > >> Hello Avri, > >> > >> On 2024-06-02 07:26, Avri Altman wrote: > >> >> On 2024-05-23 15:20, linux-mmc@danman.eu wrote: > >> >> > From: Daniel Kucera <linux-mmc@danman.eu> > >> >> > > >> >> > Locked card will not reply to SEND_SCR or SD_STATUS commands so > >> >> > it was failing to initialize previously. When skipped, the card > >> >> > will get initialized and CMD42 can be sent using ioctl to unlock > >> >> > the card or remove password protection. > >> >> > Until unlocked, all read/write calls will timeout. > >> >> > > >> >> > Signed-off-by: Daniel Kucera <linux-mmc@danman.eu> > >> >> > --- > >> >> > drivers/mmc/core/sd.c | 13 ++++++++++++- > >> >> > 1 file changed, 12 insertions(+), 1 deletion(-) > >> >> > > >> >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index > >> >> > 1c8148cdd..ae821df7d 100644 > >> >> > --- a/drivers/mmc/core/sd.c > >> >> > +++ b/drivers/mmc/core/sd.c > >> >> > @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host > >> >> > *host, struct mmc_card *card, > >> >> > bool reinit) > >> >> > { > >> >> > int err; > >> >> > + u32 card_status; > >> >> > > >> >> > - if (!reinit) { > >> >> > + err = mmc_send_status(card, &card_status); > >> >> > + if (err){ > >> >> > + pr_err("%s: unable to get card status\n", mmc_hostname(host)); > >> >> > + return err; > >> >> > + } > >> >> > + > >> >> > + if (card_status & R1_CARD_IS_LOCKED){ > >> >> > + pr_warn("%s: card is locked\n", mmc_hostname(host)); > >> >> > + } > >> >> > + > >> >> > + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { > >> >> > /* > >> >> > * Fetch SCR from card. > >> >> > */ > >> >> > >> >> Any feedback please? > >> > You didn't address my comment from your v1 - Since eMMC & SD shares > >> > the very same locking feature (non-COP SD that > >> > is) - > >> > You should at least explain in your commit log why it isn't an > >> > issue for eMMC, If indeed it is not. > >> > >> I'm sorry, I didn't get what you mean by that. I am touching only the > >> sd.c code, not the mmc.c (where eMMC is initialized, am I correct?). > >> How should I address this? > >> Should I test with eMMC to SD adaptor? I don't have any currently. > > Theoretically, looking in the eMMC spec, a locked eMMC device > > shouldn't have any issue returning from power down. > > The only flow that is affected is that its not allowed to switch to > > hs200 in a locked state until unlocked - not to say that it is a > > problem. > > If you can't verify that via code review, can you test your mmc-utils > > code on an eMMC platform? > > I've just tested with an eMMC to SD adapter in my reader and it is detected > correctly: > > [1463181.072006] mmc1: unexpected status 0x2000900 after switch > [1463181.074560] mmc1: unexpected status 0x2000900 after switch > [1463181.077038] mmc1: unexpected status 0x2000900 after switch > [1463181.079709] mmc1: unexpected status 0x2000900 after switch > [1463181.081972] mmc1: unexpected status 0x2000900 after switch > [1463181.083412] mmc1: unexpected status 0x2000900 after switch > [1463181.084831] mmc1: unexpected status 0x2000900 after switch > [1463181.084836] mmc1: new high speed MMC card at address 0001 > [1463181.085195] mmcblk1: mmc1:0001 004GA0 2.59 GiB > > Do I need to do some changes to the patch? Just add one more line to your commit log saying that no similar attention is needed for eMMC because.... Thanks, Avri > > > > > Thanks, > > Avri > >> > >> I am sorry if these are stupid questions, I am a layman. > >> > >> > > >> > Thanks, > >> > Avri > >> > > >> >> > >> >> D. > >> > >> Thank you. > >> Daniel. > > Thank you, > Daniel
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 1c8148cdd..ae821df7d 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -928,8 +928,19 @@ int mmc_sd_setup_card(struct mmc_host *host, struct mmc_card *card, bool reinit) { int err; + u32 card_status; - if (!reinit) { + err = mmc_send_status(card, &card_status); + if (err){ + pr_err("%s: unable to get card status\n", mmc_hostname(host)); + return err; + } + + if (card_status & R1_CARD_IS_LOCKED){ + pr_warn("%s: card is locked\n", mmc_hostname(host)); + } + + if (!reinit && !(card_status & R1_CARD_IS_LOCKED)) { /* * Fetch SCR from card. */