[PATCH v3] mmc: core: allow detection of locked cards

[PATCH v3] mmc: core: allow detection of locked cards

[linux-mmc]

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.
 		 */
-- 
2.34.1

Re: [PATCH v3] mmc: core: allow detection of locked cards

[Daniel Kucera]

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.

RE: [PATCH v3] mmc: core: allow detection of locked cards

[Avri Altman]

> 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.

Re: [PATCH v3] mmc: core: allow detection of locked cards

[Daniel Kucera]

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.

RE: [PATCH v3] mmc: core: allow detection of locked cards

[Avri Altman]

> 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.

Re: [PATCH v3] mmc: core: allow detection of locked cards

[Daniel Kucera]

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

RE: [PATCH v3] mmc: core: allow detection of locked cards

[Avri Altman]

 
> 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