[PATCH] mmc: core: Remove FW revision from CID check

[PATCH] mmc: core: Remove FW revision from CID check

[Wenchao Chen]

When the card is reset, mmc_card_init() will check if this
card is the previous card by comparing the CID.

If the firmware is upgraded, the product version may change,
so we remove the product version from the CID check.

Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
---
 drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 89cd48fcec79..32a73378d5c3 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -32,6 +32,9 @@
 #define MIN_CACHE_EN_TIMEOUT_MS 1600
 #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
 
+#define MMC_CID_PRV_MASK GENMASK(23, 16)
+#define MMC_CID_CRC_MASK GENMASK(7, 0)
+
 static const unsigned int tran_exp[] = {
 	10000,		100000,		1000000,	10000000,
 	0,		0,		0,		0
@@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
 	return 0;
 }
 
+static int mmc_check_cid(u32 *cid, u32 *raw_cid)
+{
+	/*
+	 * When comparing CID, we need to remove the product
+	 * version (Field PRV, offset 55:48) and CRC. Because
+	 * the product version will change when the firmware
+	 * is upgraded. Also, the new CRC is different.
+	 */
+	return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
+		(cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] & ~MMC_CID_PRV_MASK) ||
+		(cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] & ~MMC_CID_CRC_MASK);
+}
+
 static void mmc_set_erase_size(struct mmc_card *card)
 {
 	if (card->ext_csd.erase_group_def & 1)
@@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 		goto err;
 
 	if (oldcard) {
-		if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
+		if (mmc_check_cid(cid, oldcard->raw_cid)) {
 			pr_debug("%s: Perhaps the card was replaced\n",
 				mmc_hostname(host));
 			err = -ENOENT;
-- 
2.17.1

Re: [PATCH] mmc: core: Remove FW revision from CID check

[Adrian Hunter]

On 18/07/23 04:15, Wenchao Chen wrote:
> When the card is reset, mmc_card_init() will check if this
> card is the previous card by comparing the CID.
> 
> If the firmware is upgraded, the product version may change,
> so we remove the product version from the CID check.

What is the use-case for this?  I would have thought it is safer
not to assume anything about the card after the firmware has been
upgraded.

> 
> Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
> ---
>  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 89cd48fcec79..32a73378d5c3 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -32,6 +32,9 @@
>  #define MIN_CACHE_EN_TIMEOUT_MS 1600
>  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
>  
> +#define MMC_CID_PRV_MASK GENMASK(23, 16)
> +#define MMC_CID_CRC_MASK GENMASK(7, 0)
> +
>  static const unsigned int tran_exp[] = {
>  	10000,		100000,		1000000,	10000000,
>  	0,		0,		0,		0
> @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
>  	return 0;
>  }
>  
> +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
> +{
> +	/*
> +	 * When comparing CID, we need to remove the product
> +	 * version (Field PRV, offset 55:48) and CRC. Because
> +	 * the product version will change when the firmware
> +	 * is upgraded. Also, the new CRC is different.
> +	 */
> +	return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
> +		(cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] & ~MMC_CID_PRV_MASK) ||
> +		(cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] & ~MMC_CID_CRC_MASK);
> +}
> +
>  static void mmc_set_erase_size(struct mmc_card *card)
>  {
>  	if (card->ext_csd.erase_group_def & 1)
> @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>  		goto err;
>  
>  	if (oldcard) {
> -		if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
> +		if (mmc_check_cid(cid, oldcard->raw_cid)) {
>  			pr_debug("%s: Perhaps the card was replaced\n",
>  				mmc_hostname(host));
>  			err = -ENOENT;

RE: [PATCH] mmc: core: Remove FW revision from CID check

[Avri Altman]

> 
> On 18/07/23 04:15, Wenchao Chen wrote:
> > When the card is reset, mmc_card_init() will check if this
> > card is the previous card by comparing the CID.
> >
> > If the firmware is upgraded, the product version may change,
> > so we remove the product version from the CID check.
> 
> What is the use-case for this?  I would have thought it is safer
> not to assume anything about the card after the firmware has been
> upgraded.
Ack on that.

Regardless, the PRV CID-slice is [48:55]  and CRC [1:7]?

Thanks,
Avri

> 
> >
> > Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
> > ---
> >  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index 89cd48fcec79..32a73378d5c3 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -32,6 +32,9 @@
> >  #define MIN_CACHE_EN_TIMEOUT_MS 1600
> >  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
> >
> > +#define MMC_CID_PRV_MASK GENMASK(23, 16)
> > +#define MMC_CID_CRC_MASK GENMASK(7, 0)
> > +
> >  static const unsigned int tran_exp[] = {
> >       10000,          100000,         1000000,        10000000,
> >       0,              0,              0,              0
> > @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
> >       return 0;
> >  }
> >
> > +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
> > +{
> > +     /*
> > +      * When comparing CID, we need to remove the product
> > +      * version (Field PRV, offset 55:48) and CRC. Because
> > +      * the product version will change when the firmware
> > +      * is upgraded. Also, the new CRC is different.
> > +      */
> > +     return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
> > +             (cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] &
> ~MMC_CID_PRV_MASK) ||
> > +             (cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] &
> ~MMC_CID_CRC_MASK);
> > +}
> > +
> >  static void mmc_set_erase_size(struct mmc_card *card)
> >  {
> >       if (card->ext_csd.erase_group_def & 1)
> > @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host,
> u32 ocr,
> >               goto err;
> >
> >       if (oldcard) {
> > -             if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
> > +             if (mmc_check_cid(cid, oldcard->raw_cid)) {
> >                       pr_debug("%s: Perhaps the card was replaced\n",
> >                               mmc_hostname(host));
> >                       err = -ENOENT;

Re: [PATCH] mmc: core: Remove FW revision from CID check

[Wenchao Chen]

On Tue, Jul 18, 2023 at 8:32 PM Avri Altman <Avri.Altman@wdc.com> wrote:
>
> >
> > On 18/07/23 04:15, Wenchao Chen wrote:
> > > When the card is reset, mmc_card_init() will check if this
> > > card is the previous card by comparing the CID.
> > >
> > > If the firmware is upgraded, the product version may change,
> > > so we remove the product version from the CID check.
> >
> > What is the use-case for this?  I would have thought it is safer
> > not to assume anything about the card after the firmware has been
> > upgraded.
> Ack on that.
>
> Regardless, the PRV CID-slice is [48:55]  and CRC [1:7]?
>
> Thanks,
> Avri
>
Yes, JESD84-B51 7.2 CID register
Table 75 -- CID Fields
Manufacturer ID     MID    8   [127:120]
...
Product revision     RPV    8   [55:48]
...
CRC7 checksum    CRC   7    [7:1]
not used, always “1”  -      1    [0:0]

> >
> > >
> > > Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
> > > ---
> > >  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
> > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > > index 89cd48fcec79..32a73378d5c3 100644
> > > --- a/drivers/mmc/core/mmc.c
> > > +++ b/drivers/mmc/core/mmc.c
> > > @@ -32,6 +32,9 @@
> > >  #define MIN_CACHE_EN_TIMEOUT_MS 1600
> > >  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
> > >
> > > +#define MMC_CID_PRV_MASK GENMASK(23, 16)
> > > +#define MMC_CID_CRC_MASK GENMASK(7, 0)
> > > +
> > >  static const unsigned int tran_exp[] = {
> > >       10000,          100000,         1000000,        10000000,
> > >       0,              0,              0,              0
> > > @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
> > >       return 0;
> > >  }
> > >
> > > +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
> > > +{
> > > +     /*
> > > +      * When comparing CID, we need to remove the product
> > > +      * version (Field PRV, offset 55:48) and CRC. Because
> > > +      * the product version will change when the firmware
> > > +      * is upgraded. Also, the new CRC is different.
> > > +      */
> > > +     return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
> > > +             (cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] &
> > ~MMC_CID_PRV_MASK) ||
> > > +             (cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] &
> > ~MMC_CID_CRC_MASK);
> > > +}
> > > +
> > >  static void mmc_set_erase_size(struct mmc_card *card)
> > >  {
> > >       if (card->ext_csd.erase_group_def & 1)
> > > @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host,
> > u32 ocr,
> > >               goto err;
> > >
> > >       if (oldcard) {
> > > -             if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
> > > +             if (mmc_check_cid(cid, oldcard->raw_cid)) {
> > >                       pr_debug("%s: Perhaps the card was replaced\n",
> > >                               mmc_hostname(host));
> > >                       err = -ENOENT;
>

Re: [PATCH] mmc: core: Remove FW revision from CID check

[Wenchao Chen]

On Tue, Jul 18, 2023 at 2:13 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 18/07/23 04:15, Wenchao Chen wrote:
> > When the card is reset, mmc_card_init() will check if this
> > card is the previous card by comparing the CID.
> >
> > If the firmware is upgraded, the product version may change,
> > so we remove the product version from the CID check.
>
> What is the use-case for this?  I would have thought it is safer
> not to assume anything about the card after the firmware has been
> upgraded.
>
Hi adrian
    Understood, but we have case:
    1.Before the firmware upgrade
        [T5745@C0] mmc0 oldcard raw->cid[2]: 32691160, raw->cid[3]: d9241800
        PRV=69
    2.After the firmware upgrade
        [T5745@C0] mmc0 cid[2]: 32011160 cid[3]: d9241800
        PRV=01
    If the PRV is not excluded in the CID check, then the mmc
initialization will fail after the mmc reset.
    In addition, CRC is excluded because some controllers support
SDHCI_QUIRK2_RSP_136_HAS_CRC.

> >
> > Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
> > ---
> >  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index 89cd48fcec79..32a73378d5c3 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -32,6 +32,9 @@
> >  #define MIN_CACHE_EN_TIMEOUT_MS 1600
> >  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
> >
> > +#define MMC_CID_PRV_MASK GENMASK(23, 16)
> > +#define MMC_CID_CRC_MASK GENMASK(7, 0)
> > +
> >  static const unsigned int tran_exp[] = {
> >       10000,          100000,         1000000,        10000000,
> >       0,              0,              0,              0
> > @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
> >       return 0;
> >  }
> >
> > +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
> > +{
> > +     /*
> > +      * When comparing CID, we need to remove the product
> > +      * version (Field PRV, offset 55:48) and CRC. Because
> > +      * the product version will change when the firmware
> > +      * is upgraded. Also, the new CRC is different.
> > +      */
> > +     return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
> > +             (cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] & ~MMC_CID_PRV_MASK) ||
> > +             (cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] & ~MMC_CID_CRC_MASK);
> > +}
> > +
> >  static void mmc_set_erase_size(struct mmc_card *card)
> >  {
> >       if (card->ext_csd.erase_group_def & 1)
> > @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >               goto err;
> >
> >       if (oldcard) {
> > -             if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
> > +             if (mmc_check_cid(cid, oldcard->raw_cid)) {
> >                       pr_debug("%s: Perhaps the card was replaced\n",
> >                               mmc_hostname(host));
> >                       err = -ENOENT;
>

Re: [PATCH] mmc: core: Remove FW revision from CID check

[Adrian Hunter]

On 19/07/23 05:46, Wenchao Chen wrote:
> On Tue, Jul 18, 2023 at 2:13 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 18/07/23 04:15, Wenchao Chen wrote:
>>> When the card is reset, mmc_card_init() will check if this
>>> card is the previous card by comparing the CID.
>>>
>>> If the firmware is upgraded, the product version may change,
>>> so we remove the product version from the CID check.
>>
>> What is the use-case for this?  I would have thought it is safer
>> not to assume anything about the card after the firmware has been
>> upgraded.
>>
> Hi adrian
>     Understood, but we have case:
>     1.Before the firmware upgrade
>         [T5745@C0] mmc0 oldcard raw->cid[2]: 32691160, raw->cid[3]: d9241800
>         PRV=69
>     2.After the firmware upgrade
>         [T5745@C0] mmc0 cid[2]: 32011160 cid[3]: d9241800
>         PRV=01
>     If the PRV is not excluded in the CID check, then the mmc
> initialization will fail after the mmc reset.
>     In addition, CRC is excluded because some controllers support
> SDHCI_QUIRK2_RSP_136_HAS_CRC.

I do not know what others are doing in this regard, nor what
circumstances are leading to the re-initialization.

Presumably a clean re-initialization could be done by
unbinding and rebinding the host controller.

> 
>>>
>>> Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
>>> ---
>>>  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
>>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 89cd48fcec79..32a73378d5c3 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -32,6 +32,9 @@
>>>  #define MIN_CACHE_EN_TIMEOUT_MS 1600
>>>  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
>>>
>>> +#define MMC_CID_PRV_MASK GENMASK(23, 16)
>>> +#define MMC_CID_CRC_MASK GENMASK(7, 0)
>>> +
>>>  static const unsigned int tran_exp[] = {
>>>       10000,          100000,         1000000,        10000000,
>>>       0,              0,              0,              0
>>> @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
>>>       return 0;
>>>  }
>>>
>>> +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
>>> +{
>>> +     /*
>>> +      * When comparing CID, we need to remove the product
>>> +      * version (Field PRV, offset 55:48) and CRC. Because
>>> +      * the product version will change when the firmware
>>> +      * is upgraded. Also, the new CRC is different.
>>> +      */
>>> +     return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
>>> +             (cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] & ~MMC_CID_PRV_MASK) ||
>>> +             (cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] & ~MMC_CID_CRC_MASK);
>>> +}
>>> +
>>>  static void mmc_set_erase_size(struct mmc_card *card)
>>>  {
>>>       if (card->ext_csd.erase_group_def & 1)
>>> @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>>               goto err;
>>>
>>>       if (oldcard) {
>>> -             if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
>>> +             if (mmc_check_cid(cid, oldcard->raw_cid)) {
>>>                       pr_debug("%s: Perhaps the card was replaced\n",
>>>                               mmc_hostname(host));
>>>                       err = -ENOENT;
>>

Re: [PATCH] mmc: core: Remove FW revision from CID check

[Wenchao Chen]

On Thu, Jul 20, 2023 at 2:45 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 19/07/23 05:46, Wenchao Chen wrote:
> > On Tue, Jul 18, 2023 at 2:13 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>
> >> On 18/07/23 04:15, Wenchao Chen wrote:
> >>> When the card is reset, mmc_card_init() will check if this
> >>> card is the previous card by comparing the CID.
> >>>
> >>> If the firmware is upgraded, the product version may change,
> >>> so we remove the product version from the CID check.
> >>
> >> What is the use-case for this?  I would have thought it is safer
> >> not to assume anything about the card after the firmware has been
> >> upgraded.
> >>
> > Hi adrian
> >     Understood, but we have case:
> >     1.Before the firmware upgrade
> >         [T5745@C0] mmc0 oldcard raw->cid[2]: 32691160, raw->cid[3]: d9241800
> >         PRV=69
> >     2.After the firmware upgrade
> >         [T5745@C0] mmc0 cid[2]: 32011160 cid[3]: d9241800
> >         PRV=01
> >     If the PRV is not excluded in the CID check, then the mmc
> > initialization will fail after the mmc reset.
> >     In addition, CRC is excluded because some controllers support
> > SDHCI_QUIRK2_RSP_136_HAS_CRC.
>
> I do not know what others are doing in this regard, nor what
> circumstances are leading to the re-initialization.
>
There is a way: reboot the machine, but we don't want to do that.

When the firmware is upgraded, we need to complete the firmware
update by reset card, and the card will be initialized by mmc_init_card
after mmc reset.

> Presumably a clean re-initialization could be done by
> unbinding and rebinding the host controller.
>
Could you tell me how to do that?
Thanks.

> >
> >>>
> >>> Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
> >>> ---
> >>>  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
> >>>  1 file changed, 17 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> >>> index 89cd48fcec79..32a73378d5c3 100644
> >>> --- a/drivers/mmc/core/mmc.c
> >>> +++ b/drivers/mmc/core/mmc.c
> >>> @@ -32,6 +32,9 @@
> >>>  #define MIN_CACHE_EN_TIMEOUT_MS 1600
> >>>  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
> >>>
> >>> +#define MMC_CID_PRV_MASK GENMASK(23, 16)
> >>> +#define MMC_CID_CRC_MASK GENMASK(7, 0)
> >>> +
> >>>  static const unsigned int tran_exp[] = {
> >>>       10000,          100000,         1000000,        10000000,
> >>>       0,              0,              0,              0
> >>> @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
> >>>       return 0;
> >>>  }
> >>>
> >>> +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
> >>> +{
> >>> +     /*
> >>> +      * When comparing CID, we need to remove the product
> >>> +      * version (Field PRV, offset 55:48) and CRC. Because
> >>> +      * the product version will change when the firmware
> >>> +      * is upgraded. Also, the new CRC is different.
> >>> +      */
> >>> +     return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
> >>> +             (cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] & ~MMC_CID_PRV_MASK) ||
> >>> +             (cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] & ~MMC_CID_CRC_MASK);
> >>> +}
> >>> +
> >>>  static void mmc_set_erase_size(struct mmc_card *card)
> >>>  {
> >>>       if (card->ext_csd.erase_group_def & 1)
> >>> @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> >>>               goto err;
> >>>
> >>>       if (oldcard) {
> >>> -             if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
> >>> +             if (mmc_check_cid(cid, oldcard->raw_cid)) {
> >>>                       pr_debug("%s: Perhaps the card was replaced\n",
> >>>                               mmc_hostname(host));
> >>>                       err = -ENOENT;
> >>
>

Re: [PATCH] mmc: core: Remove FW revision from CID check

[Adrian Hunter]

On 20/07/23 11:38, Wenchao Chen wrote:
> On Thu, Jul 20, 2023 at 2:45 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 19/07/23 05:46, Wenchao Chen wrote:
>>> On Tue, Jul 18, 2023 at 2:13 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>
>>>> On 18/07/23 04:15, Wenchao Chen wrote:
>>>>> When the card is reset, mmc_card_init() will check if this
>>>>> card is the previous card by comparing the CID.
>>>>>
>>>>> If the firmware is upgraded, the product version may change,
>>>>> so we remove the product version from the CID check.
>>>>
>>>> What is the use-case for this?  I would have thought it is safer
>>>> not to assume anything about the card after the firmware has been
>>>> upgraded.
>>>>
>>> Hi adrian
>>>     Understood, but we have case:
>>>     1.Before the firmware upgrade
>>>         [T5745@C0] mmc0 oldcard raw->cid[2]: 32691160, raw->cid[3]: d9241800
>>>         PRV=69
>>>     2.After the firmware upgrade
>>>         [T5745@C0] mmc0 cid[2]: 32011160 cid[3]: d9241800
>>>         PRV=01
>>>     If the PRV is not excluded in the CID check, then the mmc
>>> initialization will fail after the mmc reset.
>>>     In addition, CRC is excluded because some controllers support
>>> SDHCI_QUIRK2_RSP_136_HAS_CRC.
>>
>> I do not know what others are doing in this regard, nor what
>> circumstances are leading to the re-initialization.
>>
> There is a way: reboot the machine, but we don't want to do that.
> 
> When the firmware is upgraded, we need to complete the firmware
> update by reset card, and the card will be initialized by mmc_init_card
> after mmc reset.
> 
>> Presumably a clean re-initialization could be done by
>> unbinding and rebinding the host controller.
>>
> Could you tell me how to do that?
> Thanks.

It depends on the name of the device and where the host
controller driver is in sysfs, but here is an example for
sdhci-pci with eMMC:

# ls /sys/bus/pci/drivers/sdhci-pci/
0000:00:1a.0/  new_id         uevent
bind           remove_id      unbind
# echo "0000:00:1a.0" > /sys/bus/pci/drivers/sdhci-pci/unbind 
[  484.853761] mmc0: card 0001 removed

# echo "0000:00:1a.0" > /sys/bus/pci/drivers/sdhci-pci/bind 
[  490.621524] sdhci-pci 0000:00:1a.0: SDHCI controller found [8086:4b47] (rev 11)
[  490.638520] mmc0: CQHCI version 5.10
[  490.643630] mmc0: SDHCI controller on PCI [0000:00:1a.0] using ADMA 64-bit
[  490.651837] sdhci-pci 0000:00:1a.1: SDHCI controller found [8086:4b48] (rev 11)
[  490.780139] mmc0: Command Queue Engine enabled
[  490.785132] mmc0: new HS400 MMC card at address 0001
[  490.791171] mmcblk0: mmc0:0001 S0J57X 29.6 GiB 
[  490.796320] mmcblk0boot0: mmc0:0001 S0J57X partition 1 31.5 MiB
[  490.803121] mmcblk0boot1: mmc0:0001 S0J57X partition 2 31.5 MiB
[  490.809918] mmcblk0rpmb: mmc0:0001 S0J57X partition 3 4.00 MiB, chardev (240:0)
[  490.821390] sdhci-pci 0000:00:1a.1: SDHCI controller found [8086:4b48] (rev 11)

> 
>>>
>>>>>
>>>>> Signed-off-by: Wenchao Chen <wenchao.chen@unisoc.com>
>>>>> ---
>>>>>  drivers/mmc/core/mmc.c | 18 +++++++++++++++++-
>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index 89cd48fcec79..32a73378d5c3 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -32,6 +32,9 @@
>>>>>  #define MIN_CACHE_EN_TIMEOUT_MS 1600
>>>>>  #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
>>>>>
>>>>> +#define MMC_CID_PRV_MASK GENMASK(23, 16)
>>>>> +#define MMC_CID_CRC_MASK GENMASK(7, 0)
>>>>> +
>>>>>  static const unsigned int tran_exp[] = {
>>>>>       10000,          100000,         1000000,        10000000,
>>>>>       0,              0,              0,              0
>>>>> @@ -126,6 +129,19 @@ static int mmc_decode_cid(struct mmc_card *card)
>>>>>       return 0;
>>>>>  }
>>>>>
>>>>> +static int mmc_check_cid(u32 *cid, u32 *raw_cid)
>>>>> +{
>>>>> +     /*
>>>>> +      * When comparing CID, we need to remove the product
>>>>> +      * version (Field PRV, offset 55:48) and CRC. Because
>>>>> +      * the product version will change when the firmware
>>>>> +      * is upgraded. Also, the new CRC is different.
>>>>> +      */
>>>>> +     return cid[0] != raw_cid[0] || cid[1] != raw_cid[1] ||
>>>>> +             (cid[2] & ~MMC_CID_PRV_MASK) != (raw_cid[2] & ~MMC_CID_PRV_MASK) ||
>>>>> +             (cid[3] & ~MMC_CID_CRC_MASK) != (raw_cid[3] & ~MMC_CID_CRC_MASK);
>>>>> +}
>>>>> +
>>>>>  static void mmc_set_erase_size(struct mmc_card *card)
>>>>>  {
>>>>>       if (card->ext_csd.erase_group_def & 1)
>>>>> @@ -1640,7 +1656,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>>>>               goto err;
>>>>>
>>>>>       if (oldcard) {
>>>>> -             if (memcmp(cid, oldcard->raw_cid, sizeof(cid)) != 0) {
>>>>> +             if (mmc_check_cid(cid, oldcard->raw_cid)) {
>>>>>                       pr_debug("%s: Perhaps the card was replaced\n",
>>>>>                               mmc_hostname(host));
>>>>>                       err = -ENOENT;
>>>>
>>