* Re: [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout
2020-11-13 12:53 [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout Wolfram Sang
@ 2020-11-13 16:38 ` Thierry Reding
2020-11-13 18:34 ` Sowjanya Komatineni
2020-11-13 19:28 ` kernel test robot
2020-11-14 2:45 ` kernel test robot
2 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2020-11-13 16:38 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Jonathan Hunter,
linux-tegra, Sowjanya Komatineni
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout
2020-11-13 16:38 ` Thierry Reding
@ 2020-11-13 18:34 ` Sowjanya Komatineni
2020-11-13 19:57 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Sowjanya Komatineni @ 2020-11-13 18:34 UTC (permalink / raw)
To: Thierry Reding, Wolfram Sang
Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Jonathan Hunter,
linux-tegra
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout
2020-11-13 18:34 ` Sowjanya Komatineni
@ 2020-11-13 19:57 ` Thierry Reding
2020-11-14 0:15 ` Wolfram Sang
0 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2020-11-13 19:57 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: Wolfram Sang, linux-mmc, Adrian Hunter, Ulf Hansson,
Jonathan Hunter, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout
2020-11-13 12:53 [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout Wolfram Sang
2020-11-13 16:38 ` Thierry Reding
@ 2020-11-13 19:28 ` kernel test robot
2020-11-14 2:45 ` kernel test robot
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2020-11-13 19:28 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3250 bytes --]
Hi Wolfram,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on tegra/for-next]
[also build test ERROR on v5.10-rc3 next-20201113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Wolfram-Sang/mmc-sdhci-tegra-fix-wrong-unit-with-busy_timeout/20201113-205511
base: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
config: arm64-randconfig-r033-20201113 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 9e0c35655b6e8186baef8840b26ba4090503b554)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/d5481f1c9768ea4e5a43617ef5a853dd716d00f1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wolfram-Sang/mmc-sdhci-tegra-fix-wrong-unit-with-busy_timeout/20201113-205511
git checkout d5481f1c9768ea4e5a43617ef5a853dd716d00f1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/mmc/host/sdhci-tegra.c:1275:39: error: use of undeclared identifier 'MSECS_PER_SEC'
if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC)
^
1 error generated.
vim +/MSECS_PER_SEC +1275 drivers/mmc/host/sdhci-tegra.c
1255
1256 static void tegra_sdhci_set_timeout(struct sdhci_host *host,
1257 struct mmc_command *cmd)
1258 {
1259 u32 val;
1260
1261 /*
1262 * HW busy detection timeout is based on programmed data timeout
1263 * counter and maximum supported timeout is 11s which may not be
1264 * enough for long operations like cache flush, sleep awake, erase.
1265 *
1266 * ERASE_TIMEOUT_LIMIT bit of VENDOR_MISC_CTRL register allows
1267 * host controller to wait for busy state until the card is busy
1268 * without HW timeout.
1269 *
1270 * So, use infinite busy wait mode for operations that may take
1271 * more than maximum HW busy timeout of 11s otherwise use finite
1272 * busy wait mode.
1273 */
1274 val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
> 1275 if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC)
1276 val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
1277 else
1278 val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
1279 sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_MISC_CTRL);
1280
1281 __sdhci_set_timeout(host, cmd);
1282 }
1283
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 43168 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout
2020-11-13 12:53 [RFC PATCH] mmc: sdhci: tegra: fix wrong unit with busy_timeout Wolfram Sang
2020-11-13 16:38 ` Thierry Reding
2020-11-13 19:28 ` kernel test robot
@ 2020-11-14 2:45 ` kernel test robot
2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2020-11-14 2:45 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3403 bytes --]
Hi Wolfram,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on tegra/for-next]
[also build test ERROR on v5.10-rc3 next-20201113]
[cannot apply to ulf.hansson-mmc/next mmc/mmc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Wolfram-Sang/mmc-sdhci-tegra-fix-wrong-unit-with-busy_timeout/20201113-205511
base: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
config: h8300-randconfig-c004-20201113 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/d5481f1c9768ea4e5a43617ef5a853dd716d00f1
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wolfram-Sang/mmc-sdhci-tegra-fix-wrong-unit-with-busy_timeout/20201113-205511
git checkout d5481f1c9768ea4e5a43617ef5a853dd716d00f1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=h8300
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci-tegra.c: In function 'tegra_sdhci_set_timeout':
>> drivers/mmc/host/sdhci-tegra.c:1275:39: error: 'MSECS_PER_SEC' undeclared (first use in this function); did you mean 'MSEC_PER_SEC'?
1275 | if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC)
| ^~~~~~~~~~~~~
| MSEC_PER_SEC
drivers/mmc/host/sdhci-tegra.c:1275:39: note: each undeclared identifier is reported only once for each function it appears in
vim +1275 drivers/mmc/host/sdhci-tegra.c
1255
1256 static void tegra_sdhci_set_timeout(struct sdhci_host *host,
1257 struct mmc_command *cmd)
1258 {
1259 u32 val;
1260
1261 /*
1262 * HW busy detection timeout is based on programmed data timeout
1263 * counter and maximum supported timeout is 11s which may not be
1264 * enough for long operations like cache flush, sleep awake, erase.
1265 *
1266 * ERASE_TIMEOUT_LIMIT bit of VENDOR_MISC_CTRL register allows
1267 * host controller to wait for busy state until the card is busy
1268 * without HW timeout.
1269 *
1270 * So, use infinite busy wait mode for operations that may take
1271 * more than maximum HW busy timeout of 11s otherwise use finite
1272 * busy wait mode.
1273 */
1274 val = sdhci_readl(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
> 1275 if (cmd && cmd->busy_timeout >= 11 * MSECS_PER_SEC)
1276 val |= SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
1277 else
1278 val &= ~SDHCI_MISC_CTRL_ERASE_TIMEOUT_LIMIT;
1279 sdhci_writel(host, val, SDHCI_TEGRA_VENDOR_MISC_CTRL);
1280
1281 __sdhci_set_timeout(host, cmd);
1282 }
1283
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32021 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread