* [PATCH V1 0/2] Introduce new quirk to use reserved timeout @ 2020-05-06 13:44 Sarthak Garg 2020-05-06 13:44 ` [PATCH V1 1/2] mmc: sdhci: " Sarthak Garg 2020-05-06 13:44 ` [PATCH V1 2/2] mmc: sdhci-msm: Use maximum possible data timeout value Sarthak Garg 0 siblings, 2 replies; 8+ messages in thread From: Sarthak Garg @ 2020-05-06 13:44 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: vbadigan, stummala, linux-mmc, linux-kernel, linux-arm-msm, Sarthak Garg Introduce a new quirk for letting vendor drivers to use reserved timeout value (0xF) in timeout control register. Sarthak Garg (2): mmc: sdhci: Introduce new quirk to use reserved timeout mmc: sdhci-msm: Use maximum possible data timeout value drivers/mmc/host/sdhci-msm.c | 3 ++- drivers/mmc/host/sdhci.c | 3 ++- drivers/mmc/host/sdhci.h | 5 +++++ 3 files changed, 9 insertions(+), 2 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V1 1/2] mmc: sdhci: Introduce new quirk to use reserved timeout 2020-05-06 13:44 [PATCH V1 0/2] Introduce new quirk to use reserved timeout Sarthak Garg @ 2020-05-06 13:44 ` Sarthak Garg 2020-05-18 9:39 ` Ulf Hansson 2020-05-06 13:44 ` [PATCH V1 2/2] mmc: sdhci-msm: Use maximum possible data timeout value Sarthak Garg 1 sibling, 1 reply; 8+ messages in thread From: Sarthak Garg @ 2020-05-06 13:44 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: vbadigan, stummala, linux-mmc, linux-kernel, linux-arm-msm, Sarthak Garg Introduce a new quirk for letting vendor drivers to use reserved timeout value (0xF) in timeout control register. Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> --- drivers/mmc/host/sdhci.c | 3 ++- drivers/mmc/host/sdhci.h | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 1bb6b67..07528a9 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -967,7 +967,8 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd, } if (count >= 0xF) { - if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT)) + if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) || + !(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) DBG("Too large timeout 0x%x requested for CMD%d!\n", count, cmd->opcode); count = 0xE; diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 8d2a096..02f8779 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -476,6 +476,11 @@ struct sdhci_host { * block count. */ #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT (1<<18) +/* + * Some controllers define the usage of 0xF in data timeout counter + * register (0x2E) which is actually a reserved bit as per specification. + */ +#define SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT (1<<19) int irq; /* Device IRQ */ void __iomem *ioaddr; /* Mapped address */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V1 1/2] mmc: sdhci: Introduce new quirk to use reserved timeout 2020-05-06 13:44 ` [PATCH V1 1/2] mmc: sdhci: " Sarthak Garg @ 2020-05-18 9:39 ` Ulf Hansson 2020-05-18 11:46 ` Adrian Hunter 0 siblings, 1 reply; 8+ messages in thread From: Ulf Hansson @ 2020-05-18 9:39 UTC (permalink / raw) To: Sarthak Garg Cc: Adrian Hunter, Veerabhadrarao Badiganti, Sahitya Tummala, linux-mmc, Linux Kernel Mailing List, linux-arm-msm On Wed, 6 May 2020 at 15:53, Sarthak Garg <sartgarg@codeaurora.org> wrote: > > Introduce a new quirk for letting vendor drivers to use reserved > timeout value (0xF) in timeout control register. > > Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> > Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> > --- > drivers/mmc/host/sdhci.c | 3 ++- > drivers/mmc/host/sdhci.h | 5 +++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 1bb6b67..07528a9 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -967,7 +967,8 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd, > } > > if (count >= 0xF) { > - if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT)) > + if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) || > + !(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) I don't quite get how this can make your variant use 0xF rather than 0xE? To me it looks like an updated conditional check to print a debug message, no? > DBG("Too large timeout 0x%x requested for CMD%d!\n", > count, cmd->opcode); > count = 0xE; > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 8d2a096..02f8779 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -476,6 +476,11 @@ struct sdhci_host { > * block count. > */ > #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT (1<<18) > +/* > + * Some controllers define the usage of 0xF in data timeout counter > + * register (0x2E) which is actually a reserved bit as per specification. > + */ > +#define SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT (1<<19) > > int irq; /* Device IRQ */ > void __iomem *ioaddr; /* Mapped address */ > -- > 2.7.4 > Kind regards Uffe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V1 1/2] mmc: sdhci: Introduce new quirk to use reserved timeout 2020-05-18 9:39 ` Ulf Hansson @ 2020-05-18 11:46 ` Adrian Hunter 2020-05-18 12:50 ` Ulf Hansson 0 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2020-05-18 11:46 UTC (permalink / raw) To: Ulf Hansson, Sarthak Garg Cc: Veerabhadrarao Badiganti, Sahitya Tummala, linux-mmc, Linux Kernel Mailing List, linux-arm-msm On 18/05/20 12:39 pm, Ulf Hansson wrote: > On Wed, 6 May 2020 at 15:53, Sarthak Garg <sartgarg@codeaurora.org> wrote: >> >> Introduce a new quirk for letting vendor drivers to use reserved >> timeout value (0xF) in timeout control register. >> >> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> >> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> >> --- >> drivers/mmc/host/sdhci.c | 3 ++- >> drivers/mmc/host/sdhci.h | 5 +++++ >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 1bb6b67..07528a9 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -967,7 +967,8 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd, >> } >> >> if (count >= 0xF) { >> - if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT)) >> + if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) || >> + !(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) > > I don't quite get how this can make your variant use 0xF rather than 0xE? > > To me it looks like an updated conditional check to print a debug message, no? Probably need to introduce host->max_timeout_count, set it to 0xE in sdhci_alloc_host(), and change sdhci_calc_timeout() to use it in place of all the 0xE and 0xF constants. > >> DBG("Too large timeout 0x%x requested for CMD%d!\n", >> count, cmd->opcode); >> count = 0xE; >> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >> index 8d2a096..02f8779 100644 >> --- a/drivers/mmc/host/sdhci.h >> +++ b/drivers/mmc/host/sdhci.h >> @@ -476,6 +476,11 @@ struct sdhci_host { >> * block count. >> */ >> #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT (1<<18) >> +/* >> + * Some controllers define the usage of 0xF in data timeout counter >> + * register (0x2E) which is actually a reserved bit as per specification. >> + */ >> +#define SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT (1<<19) >> >> int irq; /* Device IRQ */ >> void __iomem *ioaddr; /* Mapped address */ >> -- >> 2.7.4 >> > > Kind regards > Uffe > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V1 1/2] mmc: sdhci: Introduce new quirk to use reserved timeout 2020-05-18 11:46 ` Adrian Hunter @ 2020-05-18 12:50 ` Ulf Hansson 2020-05-19 14:08 ` sartgarg 0 siblings, 1 reply; 8+ messages in thread From: Ulf Hansson @ 2020-05-18 12:50 UTC (permalink / raw) To: Adrian Hunter Cc: Sarthak Garg, Veerabhadrarao Badiganti, Sahitya Tummala, linux-mmc, Linux Kernel Mailing List, linux-arm-msm On Mon, 18 May 2020 at 13:45, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 18/05/20 12:39 pm, Ulf Hansson wrote: > > On Wed, 6 May 2020 at 15:53, Sarthak Garg <sartgarg@codeaurora.org> wrote: > >> > >> Introduce a new quirk for letting vendor drivers to use reserved > >> timeout value (0xF) in timeout control register. > >> > >> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> > >> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> > >> --- > >> drivers/mmc/host/sdhci.c | 3 ++- > >> drivers/mmc/host/sdhci.h | 5 +++++ > >> 2 files changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > >> index 1bb6b67..07528a9 100644 > >> --- a/drivers/mmc/host/sdhci.c > >> +++ b/drivers/mmc/host/sdhci.c > >> @@ -967,7 +967,8 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd, > >> } > >> > >> if (count >= 0xF) { > >> - if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT)) > >> + if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) || > >> + !(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) > > > > I don't quite get how this can make your variant use 0xF rather than 0xE? > > > > To me it looks like an updated conditional check to print a debug message, no? > > Probably need to introduce host->max_timeout_count, set it to 0xE in > sdhci_alloc_host(), and change sdhci_calc_timeout() to use it in place of > all the 0xE and 0xF constants. Yep, that seems like a reasonable approach to me as well. [...] Kind regards Uffe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V1 1/2] mmc: sdhci: Introduce new quirk to use reserved timeout 2020-05-18 12:50 ` Ulf Hansson @ 2020-05-19 14:08 ` sartgarg 2020-05-24 15:57 ` Adrian Hunter 0 siblings, 1 reply; 8+ messages in thread From: sartgarg @ 2020-05-19 14:08 UTC (permalink / raw) To: Ulf Hansson Cc: Adrian Hunter, Veerabhadrarao Badiganti, Sahitya Tummala, linux-mmc, Linux Kernel Mailing List, linux-arm-msm, linux-mmc-owner On 2020-05-18 18:20, Ulf Hansson wrote: > On Mon, 18 May 2020 at 13:45, Adrian Hunter <adrian.hunter@intel.com> > wrote: >> >> On 18/05/20 12:39 pm, Ulf Hansson wrote: >> > On Wed, 6 May 2020 at 15:53, Sarthak Garg <sartgarg@codeaurora.org> wrote: >> >> >> >> Introduce a new quirk for letting vendor drivers to use reserved >> >> timeout value (0xF) in timeout control register. >> >> >> >> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> >> >> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> >> >> --- >> >> drivers/mmc/host/sdhci.c | 3 ++- >> >> drivers/mmc/host/sdhci.h | 5 +++++ >> >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> >> index 1bb6b67..07528a9 100644 >> >> --- a/drivers/mmc/host/sdhci.c >> >> +++ b/drivers/mmc/host/sdhci.c >> >> @@ -967,7 +967,8 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd, >> >> } >> >> >> >> if (count >= 0xF) { >> >> - if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT)) >> >> + if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) || >> >> + !(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) >> > >> > I don't quite get how this can make your variant use 0xF rather than 0xE? >> > >> > To me it looks like an updated conditional check to print a debug message, no? >> >> Probably need to introduce host->max_timeout_count, set it to 0xE in >> sdhci_alloc_host(), and change sdhci_calc_timeout() to use it in place >> of >> all the 0xE and 0xF constants. > > Yep, that seems like a reasonable approach to me as well. > > [...] > > Kind regards > Uffe Resending the mail again as can't see my comment on the https://patchwork.kernel.org/ page. Sorry for the mistake just want to update the logic as below. - count = 0xE; + if(!(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) + count = 0xE; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V1 1/2] mmc: sdhci: Introduce new quirk to use reserved timeout 2020-05-19 14:08 ` sartgarg @ 2020-05-24 15:57 ` Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: Adrian Hunter @ 2020-05-24 15:57 UTC (permalink / raw) To: sartgarg, Ulf Hansson Cc: Veerabhadrarao Badiganti, Sahitya Tummala, linux-mmc, Linux Kernel Mailing List, linux-arm-msm, linux-mmc-owner On 19/05/20 5:08 pm, sartgarg@codeaurora.org wrote: > On 2020-05-18 18:20, Ulf Hansson wrote: >> On Mon, 18 May 2020 at 13:45, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> >>> On 18/05/20 12:39 pm, Ulf Hansson wrote: >>> > On Wed, 6 May 2020 at 15:53, Sarthak Garg <sartgarg@codeaurora.org> wrote: >>> >> >>> >> Introduce a new quirk for letting vendor drivers to use reserved >>> >> timeout value (0xF) in timeout control register. >>> >> >>> >> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> >>> >> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> >>> >> --- >>> >> drivers/mmc/host/sdhci.c | 3 ++- >>> >> drivers/mmc/host/sdhci.h | 5 +++++ >>> >> 2 files changed, 7 insertions(+), 1 deletion(-) >>> >> >>> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> >> index 1bb6b67..07528a9 100644 >>> >> --- a/drivers/mmc/host/sdhci.c >>> >> +++ b/drivers/mmc/host/sdhci.c >>> >> @@ -967,7 +967,8 @@ static u8 sdhci_calc_timeout(struct sdhci_host >>> *host, struct mmc_command *cmd, >>> >> } >>> >> >>> >> if (count >= 0xF) { >>> >> - if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT)) >>> >> + if (!(host->quirks2 & SDHCI_QUIRK2_DISABLE_HW_TIMEOUT) || >>> >> + !(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) >>> > >>> > I don't quite get how this can make your variant use 0xF rather than 0xE? >>> > >>> > To me it looks like an updated conditional check to print a debug >>> message, no? >>> >>> Probably need to introduce host->max_timeout_count, set it to 0xE in >>> sdhci_alloc_host(), and change sdhci_calc_timeout() to use it in place of >>> all the 0xE and 0xF constants. >> >> Yep, that seems like a reasonable approach to me as well. >> >> [...] >> >> Kind regards >> Uffe > > Resending the mail again as can't see my comment on the > https://patchwork.kernel.org/ page. > Sorry for the mistake just want to update the logic as below. > - count = 0xE; > + if(!(host->quirks2 & SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT)) > + count = 0xE; I think it is conceptually simpler to define SDHCI constants as variables and let vendor drivers change them if need be. In other words, what I wrote above. It changes more code, but the overall result is more consistent. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V1 2/2] mmc: sdhci-msm: Use maximum possible data timeout value 2020-05-06 13:44 [PATCH V1 0/2] Introduce new quirk to use reserved timeout Sarthak Garg 2020-05-06 13:44 ` [PATCH V1 1/2] mmc: sdhci: " Sarthak Garg @ 2020-05-06 13:44 ` Sarthak Garg 1 sibling, 0 replies; 8+ messages in thread From: Sarthak Garg @ 2020-05-06 13:44 UTC (permalink / raw) To: adrian.hunter, ulf.hansson Cc: vbadigan, stummala, linux-mmc, linux-kernel, linux-arm-msm, Sarthak Garg, Andy Gross, Bjorn Andersson The MSM SD controller defines the usage of 0xF in data timeout counter register (0x2E) which is actually a reserved bit as per specification. This would result in maximum of 21.26 secs timeout value. Some SDcard taking more time than 2.67secs (timeout value corresponding to 0xE) and with that observed data timeout errors. So increasing the timeout value to max possible timeout. Signed-off-by: Sahitya Tummala <stummala@codeaurora.org> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org> --- drivers/mmc/host/sdhci-msm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index 8a055dd..909855b 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -1888,7 +1888,8 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = { SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12, - .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | + SDHCI_QUIRK2_USE_RESERVED_MAX_TIMEOUT, .ops = &sdhci_msm_ops, }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-24 15:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-06 13:44 [PATCH V1 0/2] Introduce new quirk to use reserved timeout Sarthak Garg 2020-05-06 13:44 ` [PATCH V1 1/2] mmc: sdhci: " Sarthak Garg 2020-05-18 9:39 ` Ulf Hansson 2020-05-18 11:46 ` Adrian Hunter 2020-05-18 12:50 ` Ulf Hansson 2020-05-19 14:08 ` sartgarg 2020-05-24 15:57 ` Adrian Hunter 2020-05-06 13:44 ` [PATCH V1 2/2] mmc: sdhci-msm: Use maximum possible data timeout value Sarthak Garg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).