Message ID | 20250206210835.2980500-2-erick.shepherd@ni.com |
---|---|
State | New |
Headers | show |
Series | [RFC,V2,1/2] mmc: Update sdhci tune function to return errors | expand |
On 6/02/25 23:08, Erick Shepherd wrote: > Add a new field to the mmc_card struct to disable tuning for the card. > Currently the new field only gets set when a DDR50 card fails to tune, > which indicates the card does not support tuning. You need to explain why this is needed. Presumably it speeds up runtime-resume in some cases? > > Signed-off-by: Erick Shepherd <erick.shepherd@ni.com> > --- > drivers/mmc/core/core.c | 3 +++ > drivers/mmc/core/sd.c | 1 + > include/linux/mmc/card.h | 1 + > 3 files changed, 5 insertions(+) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 5241528f8b90..ee91d53c45d5 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -934,6 +934,9 @@ int mmc_execute_tuning(struct mmc_card *card) > u32 opcode; > int err; > > + if (card->disable_tuning) > + return 0; > + > if (!host->ops->execute_tuning) > return 0; > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index cc757b850e79..dd65485c61d8 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -676,6 +676,7 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > if (err && card->host->ios.timing == MMC_TIMING_UHS_DDR50) { > pr_warn("%s: ddr50 tuning failed\n", > mmc_hostname(card->host)); > + card->disable_tuning = true; > err = 0; > } > } > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 526fce581657..f9733c7ce430 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -332,6 +332,7 @@ struct mmc_card { > > bool written_flag; /* Indicates eMMC has been written since power on */ > bool reenable_cmdq; /* Re-enable Command Queue */ > + bool disable_tuning; /* Disables tuning for the card */ > > unsigned int erase_size; /* erase size in sectors */ > unsigned int erase_shift; /* if erase unit is power 2 */
Sorry about that, the context for this change is that I have been working with a DDR50 swissbit SD card that does not support tuning. The case I'm seeing is that the first tuning times out and any further tuning attempts cause an async page read I/O error. I used this change to prevent the card from attempting to tune again if it is ever reset in the case where we know tuning isn't supported. Regards Erick
On 18/02/25 20:41, Erick Shepherd wrote: > Sorry about that, the context for this change is that I have been > working with a DDR50 swissbit SD card that does not support tuning. > The case I'm seeing is that the first tuning times out and any > further tuning attempts cause an async page read I/O error. I used > this change to prevent the card from attempting to tune again if it > is ever reset in the case where we know tuning isn't supported. Sorry for the slow reply. I would expect if there was a general problem with DDR50 SD cards, it would have come to light before now. Does the card work with any other host controllers with linux? If it is specific to a particular kind of card, a card quirk could be added, say MMC_QUIRK_BROKEN_DDR50_TUNING
> Sorry for the slow reply. > I would expect if there was a general problem with DDR50 SD cards, > it would have come to light before now. > Does the card work with any other host controllers with linux? > If it is specific to a particular kind of card, a card quirk > could be added, say MMC_QUIRK_BROKEN_DDR50_TUNING No worries. I have not tested this with other host controllers but can try to get something set up. This issue has only appeared on one particular SD card model for us so I would not be surprised if the I/O errors we see on subsequent tune requests are specific to this card. I can put together a solution using the card quirk you suggested if you think that is the best way forward. My fix is currently spread across two commits, one to return the error code thrown by the tune request timing out, which prevents the card from retuning, and this one that prevents the initial card tuning if it has already failed. Should both parts be controlled by the new card quirk? Regards, Erick
On 7/03/25 19:45, Erick Shepherd wrote: >> Sorry for the slow reply. > >> I would expect if there was a general problem with DDR50 SD cards, >> it would have come to light before now. > >> Does the card work with any other host controllers with linux? > >> If it is specific to a particular kind of card, a card quirk >> could be added, say MMC_QUIRK_BROKEN_DDR50_TUNING > > No worries. I have not tested this with other host controllers but > can try to get something set up. This issue has only appeared on > one particular SD card model for us so I would not be surprised if > the I/O errors we see on subsequent tune requests are specific to this > card. I can put together a solution using the card quirk you suggested > if you think that is the best way forward. > > My fix is currently spread across two commits, one to return the error > code thrown by the tune request timing out, which prevents the card from > retuning, and this one that prevents the initial card tuning if it has > already failed. Should both parts be controlled by the new card quirk? Does it tuning at all? Maybe MMC_QUIRK_NO_UHS_DDR50_TUNING is a better name, then at the top of mmc_execute_tuning() if ((card->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING) && host->ios->timing == MMC_TIMING_UHS_DDR50) return 0;
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5241528f8b90..ee91d53c45d5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -934,6 +934,9 @@ int mmc_execute_tuning(struct mmc_card *card) u32 opcode; int err; + if (card->disable_tuning) + return 0; + if (!host->ops->execute_tuning) return 0; diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index cc757b850e79..dd65485c61d8 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -676,6 +676,7 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) if (err && card->host->ios.timing == MMC_TIMING_UHS_DDR50) { pr_warn("%s: ddr50 tuning failed\n", mmc_hostname(card->host)); + card->disable_tuning = true; err = 0; } } diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 526fce581657..f9733c7ce430 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -332,6 +332,7 @@ struct mmc_card { bool written_flag; /* Indicates eMMC has been written since power on */ bool reenable_cmdq; /* Re-enable Command Queue */ + bool disable_tuning; /* Disables tuning for the card */ unsigned int erase_size; /* erase size in sectors */ unsigned int erase_shift; /* if erase unit is power 2 */
Add a new field to the mmc_card struct to disable tuning for the card. Currently the new field only gets set when a DDR50 card fails to tune, which indicates the card does not support tuning. Signed-off-by: Erick Shepherd <erick.shepherd@ni.com> --- drivers/mmc/core/core.c | 3 +++ drivers/mmc/core/sd.c | 1 + include/linux/mmc/card.h | 1 + 3 files changed, 5 insertions(+)