| Message ID | 20201113125354.3507-1-wsa+renesas@sang-engineering.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [RFC] mmc: sdhci: tegra: fix wrong unit with busy_timeout | expand |
On Fri, Nov 13, 2020 at 01:53:30PM +0100, Wolfram Sang wrote: > 'busy_timeout' is in msecs, not in jiffies. Use the correct factor. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > Not tested. Found by code investigation about 'busy_timeout'. A quick > grep showed no other problematic code within the MMC host drivers. > > drivers/mmc/host/sdhci-tegra.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Sowjanya, can you take a look at this? Thierry > diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c > index ed12aacb1c73..ad0dc3adc7d1 100644 > --- a/drivers/mmc/host/sdhci-tegra.c > +++ b/drivers/mmc/host/sdhci-tegra.c > @@ -1272,7 +1272,7 @@ static void tegra_sdhci_set_timeout(struct sdhci_host *host, > * busy wait mode. > */ > val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL); > - if (cmd && cmd->busy_timeout >= 11 * HZ) > + if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC) > val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT; > else > val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT; > -- > 2.28.0 >
On 11/13/20 8:38 AM, Thierry Reding wrote: > On Fri, Nov 13, 2020 at 01:53:30PM +0100, Wolfram Sang wrote: >> 'busy_timeout' is in msecs, not in jiffies. Use the correct factor. >> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> >> --- >> >> Not tested. Found by code investigation about 'busy_timeout'. A quick >> grep showed no other problematic code within the MMC host drivers. >> >> drivers/mmc/host/sdhci-tegra.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > Sowjanya, can you take a look at this? > > Thierry Thanks Wolfram. Right cmd busy_timeout is in msec and we have to enable ERASE_TIMEOUT_LIMIT bit for more than 11s busy operations. So it should be MSEC_PER_SEC. > >> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c >> index ed12aacb1c73..ad0dc3adc7d1 100644 >> --- a/drivers/mmc/host/sdhci-tegra.c >> +++ b/drivers/mmc/host/sdhci-tegra.c >> @@ -1272,7 +1272,7 @@ static void tegra_sdhci_set_timeout(struct sdhci_host *host, >> * busy wait mode. >> */ >> val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL); >> - if (cmd && cmd->busy_timeout >= 11 * HZ) >> + if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC) >> val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT; >> else >> val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT; >> -- >> 2.28.0
On Fri, Nov 13, 2020 at 10:34:27AM -0800, Sowjanya Komatineni wrote: > > On 11/13/20 8:38 AM, Thierry Reding wrote: > > On Fri, Nov 13, 2020 at 01:53:30PM +0100, Wolfram Sang wrote: > > > 'busy_timeout' is in msecs, not in jiffies. Use the correct factor. > > > > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > > --- > > > > > > Not tested. Found by code investigation about 'busy_timeout'. A quick > > > grep showed no other problematic code within the MMC host drivers. > > > > > > drivers/mmc/host/sdhci-tegra.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Sowjanya, can you take a look at this? > > > > Thierry > > Thanks Wolfram. > > Right cmd busy_timeout is in msec and we have to enable ERASE_TIMEOUT_LIMIT > bit for more than 11s busy operations. > > So it should be MSEC_PER_SEC. Great, thanks! Acked-by: Thierry Reding <treding@nvidia.com> And perhaps also: Fixes: 5e958e4aacf4 ("sdhci: tegra: Implement Tegra specific set_timeout callback") Not sure it's worth adding the latter because this has been in Linux since 5.7 and I haven't heard of any issues. Thierry
> Great, thanks! > > Acked-by: Thierry Reding <treding@nvidia.com> > > And perhaps also: > > Fixes: 5e958e4aacf4 ("sdhci: tegra: Implement Tegra specific set_timeout callback") Thanks! I will check a build report I got (privately) and resend this as a proper patch.
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c index ed12aacb1c73..ad0dc3adc7d1 100644 --- a/drivers/mmc/host/sdhci-tegra.c +++ b/drivers/mmc/host/sdhci-tegra.c @@ -1272,7 +1272,7 @@ static void tegra_sdhci_set_timeout(struct sdhci_host *host, * busy wait mode. */ val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL); - if (cmd && cmd->busy_timeout >= 11 * HZ) + if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC) val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT; else val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
'busy_timeout' is in msecs, not in jiffies. Use the correct factor. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- Not tested. Found by code investigation about 'busy_timeout'. A quick grep showed no other problematic code within the MMC host drivers. drivers/mmc/host/sdhci-tegra.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)