| Message ID | 20240823071025.15410-1-sh8267.baek@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmc : fix for check cqe halt. | expand |
Hello Seunghwan, On 2024-08-23 09:10, Seunghwan Baek wrote: > To check if mmc cqe is in halt state, need to check > set/clear of CQHCI_HALT bit. At this time, we need to > check with &, not &&. Therefore, code to check whether > cqe is in halt state is modified to cqhci_halted, > which has already been implemented. > > Signed-off-by: Seunghwan Baek <sh8267.baek@samsung.com> Looking good to me, thanks for the patch. I'd suggest that you resend the patch with a proper "Fixes" tag, together with the "Cc: stable@vger.kernel.org" tag. Maybe also reflow the patch description a bit, to use the 78-column width or so fully. > --- > drivers/mmc/host/cqhci-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/cqhci-core.c > b/drivers/mmc/host/cqhci-core.c > index c14d7251d0bb..3d5bcb92c78e 100644 > --- a/drivers/mmc/host/cqhci-core.c > +++ b/drivers/mmc/host/cqhci-core.c > @@ -282,7 +282,7 @@ static void __cqhci_enable(struct cqhci_host > *cq_host) > > cqhci_writel(cq_host, cqcfg, CQHCI_CFG); > > - if (cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) > + if (cqhci_halted(cq_host)) > cqhci_writel(cq_host, 0, CQHCI_CTL); > > mmc->cqe_on = true; > @@ -617,7 +617,7 @@ static int cqhci_request(struct mmc_host *mmc, > struct mmc_request *mrq) > cqhci_writel(cq_host, 0, CQHCI_CTL); > mmc->cqe_on = true; > pr_debug("%s: cqhci: CQE on\n", mmc_hostname(mmc)); > - if (cqhci_readl(cq_host, CQHCI_CTL) && CQHCI_HALT) { > + if (cqhci_halted(cq_host)) { > pr_err("%s: cqhci: CQE failed to exit halt state\n", > mmc_hostname(mmc)); > }
On 2024-08-23 09:22, Dragan Simic wrote: > Hello Seunghwan, > > On 2024-08-23 09:10, Seunghwan Baek wrote: >> To check if mmc cqe is in halt state, need to check >> set/clear of CQHCI_HALT bit. At this time, we need to >> check with &, not &&. Therefore, code to check whether >> cqe is in halt state is modified to cqhci_halted, >> which has already been implemented. >> >> Signed-off-by: Seunghwan Baek <sh8267.baek@samsung.com> > > Looking good to me, thanks for the patch. I'd suggest that you > resend the patch with a proper "Fixes" tag, together with the > "Cc: stable@vger.kernel.org" tag. > > Maybe also reflow the patch description a bit, to use the 78-column > width or so fully. Oh, and please feel free to include: Reviewed-by: Dragan Simic <dsimic@manjaro.org> >> --- >> drivers/mmc/host/cqhci-core.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/cqhci-core.c >> b/drivers/mmc/host/cqhci-core.c >> index c14d7251d0bb..3d5bcb92c78e 100644 >> --- a/drivers/mmc/host/cqhci-core.c >> +++ b/drivers/mmc/host/cqhci-core.c >> @@ -282,7 +282,7 @@ static void __cqhci_enable(struct cqhci_host >> *cq_host) >> >> cqhci_writel(cq_host, cqcfg, CQHCI_CFG); >> >> - if (cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) >> + if (cqhci_halted(cq_host)) >> cqhci_writel(cq_host, 0, CQHCI_CTL); >> >> mmc->cqe_on = true; >> @@ -617,7 +617,7 @@ static int cqhci_request(struct mmc_host *mmc, >> struct mmc_request *mrq) >> cqhci_writel(cq_host, 0, CQHCI_CTL); >> mmc->cqe_on = true; >> pr_debug("%s: cqhci: CQE on\n", mmc_hostname(mmc)); >> - if (cqhci_readl(cq_host, CQHCI_CTL) && CQHCI_HALT) { >> + if (cqhci_halted(cq_host)) { >> pr_err("%s: cqhci: CQE failed to exit halt state\n", >> mmc_hostname(mmc)); >> }
On Fri, 23 Aug 2024 at 09:10, Seunghwan Baek <sh8267.baek@samsung.com> wrote: > > To check if mmc cqe is in halt state, need to check > set/clear of CQHCI_HALT bit. At this time, we need to > check with &, not &&. Therefore, code to check whether > cqe is in halt state is modified to cqhci_halted, > which has already been implemented. > > Signed-off-by: Seunghwan Baek <sh8267.baek@samsung.com> Hi Seunghwan, Please re-post to include some additional and needed maintainers. ./scripts/get_maintainer.pl drivers/mmc/host/cqhci-core.c should give you the needed information. Kind regards Uffe > --- > drivers/mmc/host/cqhci-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c > index c14d7251d0bb..3d5bcb92c78e 100644 > --- a/drivers/mmc/host/cqhci-core.c > +++ b/drivers/mmc/host/cqhci-core.c > @@ -282,7 +282,7 @@ static void __cqhci_enable(struct cqhci_host *cq_host) > > cqhci_writel(cq_host, cqcfg, CQHCI_CFG); > > - if (cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) > + if (cqhci_halted(cq_host)) > cqhci_writel(cq_host, 0, CQHCI_CTL); > > mmc->cqe_on = true; > @@ -617,7 +617,7 @@ static int cqhci_request(struct mmc_host *mmc, struct mmc_request *mrq) > cqhci_writel(cq_host, 0, CQHCI_CTL); > mmc->cqe_on = true; > pr_debug("%s: cqhci: CQE on\n", mmc_hostname(mmc)); > - if (cqhci_readl(cq_host, CQHCI_CTL) && CQHCI_HALT) { > + if (cqhci_halted(cq_host)) { > pr_err("%s: cqhci: CQE failed to exit halt state\n", > mmc_hostname(mmc)); > } > -- > 2.17.1 >
> > > > To check if mmc cqe is in halt state, need to check set/clear of > > CQHCI_HALT bit. At this time, we need to check with &, not &&. > > Therefore, code to check whether cqe is in halt state is modified to > > cqhci_halted, which has already been implemented. > > > > Signed-off-by: Seunghwan Baek <sh8267.baek@samsung.com> > > Hi Seunghwan, > > Please re-post to include some additional and needed maintainers. > ./scripts/get_maintainer.pl drivers/mmc/host/cqhci-core.c should give you > the needed information. > > Kind regards > Uffe Okay. I will send v2 patch. Thanks. > > > --- > > drivers/mmc/host/cqhci-core.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mmc/host/cqhci-core.c > > b/drivers/mmc/host/cqhci-core.c index c14d7251d0bb..3d5bcb92c78e > > 100644 > > --- a/drivers/mmc/host/cqhci-core.c > > +++ b/drivers/mmc/host/cqhci-core.c > > @@ -282,7 +282,7 @@ static void __cqhci_enable(struct cqhci_host > > *cq_host) > > > > cqhci_writel(cq_host, cqcfg, CQHCI_CFG); > > > > - if (cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) > > + if (cqhci_halted(cq_host)) > > cqhci_writel(cq_host, 0, CQHCI_CTL); > > > > mmc->cqe_on = true; > > @@ -617,7 +617,7 @@ static int cqhci_request(struct mmc_host *mmc, > struct mmc_request *mrq) > > cqhci_writel(cq_host, 0, CQHCI_CTL); > > mmc->cqe_on = true; > > pr_debug("%s: cqhci: CQE on\n", mmc_hostname(mmc)); > > - if (cqhci_readl(cq_host, CQHCI_CTL) && CQHCI_HALT) { > > + if (cqhci_halted(cq_host)) { > > pr_err("%s: cqhci: CQE failed to exit halt state\n", > > mmc_hostname(mmc)); > > } > > -- > > 2.17.1 > >
Hi Seunghwan,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.11-rc5 next-20240823]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Seunghwan-Baek/mmc-fix-for-check-cqe-halt/20240826-130042
base: linus/master
patch link: https://lore.kernel.org/r/20240823071025.15410-1-sh8267.baek%40samsung.com
patch subject: [PATCH] mmc : fix for check cqe halt.
config: arc-randconfig-001-20240826 (https://download.01.org/0day-ci/archive/20240826/202408261932.pcT0dqsD-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240826/202408261932.pcT0dqsD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408261932.pcT0dqsD-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/asm-generic/div64.h:27,
from ./arch/arc/include/generated/asm/div64.h:1,
from include/linux/math.h:6,
from include/linux/delay.h:22,
from drivers/mmc/host/cqhci-core.c:5:
drivers/mmc/host/cqhci-core.c: In function '__cqhci_enable':
>> drivers/mmc/host/cqhci-core.c:285:13: error: implicit declaration of function 'cqhci_halted'; did you mean 'cqhci_writel'? [-Werror=implicit-function-declaration]
285 | if (cqhci_halted(cq_host))
| ^~~~~~~~~~~~
include/linux/compiler.h:57:52: note: in definition of macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^~~~
drivers/mmc/host/cqhci-core.c:285:9: note: in expansion of macro 'if'
285 | if (cqhci_halted(cq_host))
| ^~
drivers/mmc/host/cqhci-core.c: At top level:
>> drivers/mmc/host/cqhci-core.c:956:13: error: conflicting types for 'cqhci_halted'; have 'bool(struct cqhci_host *)' {aka '_Bool(struct cqhci_host *)'}
956 | static bool cqhci_halted(struct cqhci_host *cq_host)
| ^~~~~~~~~~~~
drivers/mmc/host/cqhci-core.c:285:13: note: previous implicit declaration of 'cqhci_halted' with type 'int()'
285 | if (cqhci_halted(cq_host))
| ^~~~~~~~~~~~
include/linux/compiler.h:57:52: note: in definition of macro '__trace_if_var'
57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
| ^~~~
drivers/mmc/host/cqhci-core.c:285:9: note: in expansion of macro 'if'
285 | if (cqhci_halted(cq_host))
| ^~
cc1: some warnings being treated as errors
vim +285 drivers/mmc/host/cqhci-core.c
245
246 static void __cqhci_enable(struct cqhci_host *cq_host)
247 {
248 struct mmc_host *mmc = cq_host->mmc;
249 u32 cqcfg;
250
251 cqcfg = cqhci_readl(cq_host, CQHCI_CFG);
252
253 /* Configuration must not be changed while enabled */
254 if (cqcfg & CQHCI_ENABLE) {
255 cqcfg &= ~CQHCI_ENABLE;
256 cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
257 }
258
259 cqcfg &= ~(CQHCI_DCMD | CQHCI_TASK_DESC_SZ);
260
261 if (mmc->caps2 & MMC_CAP2_CQE_DCMD)
262 cqcfg |= CQHCI_DCMD;
263
264 if (cq_host->caps & CQHCI_TASK_DESC_SZ_128)
265 cqcfg |= CQHCI_TASK_DESC_SZ;
266
267 if (mmc->caps2 & MMC_CAP2_CRYPTO)
268 cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE;
269
270 cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
271
272 cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
273 CQHCI_TDLBA);
274 cqhci_writel(cq_host, upper_32_bits(cq_host->desc_dma_base),
275 CQHCI_TDLBAU);
276
277 cqhci_writel(cq_host, cq_host->rca, CQHCI_SSC2);
278
279 cqhci_set_irqs(cq_host, 0);
280
281 cqcfg |= CQHCI_ENABLE;
282
283 cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
284
> 285 if (cqhci_halted(cq_host))
286 cqhci_writel(cq_host, 0, CQHCI_CTL);
287
288 mmc->cqe_on = true;
289
290 if (cq_host->ops->enable)
291 cq_host->ops->enable(mmc);
292
293 /* Ensure all writes are done before interrupts are enabled */
294 wmb();
295
296 cqhci_set_irqs(cq_host, CQHCI_IS_MASK);
297
298 cq_host->activated = true;
299 }
300
Hi Seunghwan, kernel test robot noticed the following build errors: [auto build test ERROR on linus/master] [also build test ERROR on v6.11-rc5 next-20240826] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Seunghwan-Baek/mmc-fix-for-check-cqe-halt/20240826-130042 base: linus/master patch link: https://lore.kernel.org/r/20240823071025.15410-1-sh8267.baek%40samsung.com patch subject: [PATCH] mmc : fix for check cqe halt. config: i386-buildonly-randconfig-003-20240826 (https://download.01.org/0day-ci/archive/20240826/202408261926.P72BWMr0-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240826/202408261926.P72BWMr0-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408261926.P72BWMr0-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/mmc/host/cqhci-core.c:285:6: error: call to undeclared function 'cqhci_halted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 285 | if (cqhci_halted(cq_host)) | ^ drivers/mmc/host/cqhci-core.c:285:6: note: did you mean 'cqhci_writel'? drivers/mmc/host/cqhci.h:301:20: note: 'cqhci_writel' declared here 301 | static inline void cqhci_writel(struct cqhci_host *host, u32 val, int reg) | ^ drivers/mmc/host/cqhci-core.c:620:7: error: call to undeclared function 'cqhci_halted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] 620 | if (cqhci_halted(cq_host)) { | ^ >> drivers/mmc/host/cqhci-core.c:956:13: error: static declaration of 'cqhci_halted' follows non-static declaration 956 | static bool cqhci_halted(struct cqhci_host *cq_host) | ^ drivers/mmc/host/cqhci-core.c:285:6: note: previous implicit declaration is here 285 | if (cqhci_halted(cq_host)) | ^ 3 errors generated. vim +/cqhci_halted +285 drivers/mmc/host/cqhci-core.c 245 246 static void __cqhci_enable(struct cqhci_host *cq_host) 247 { 248 struct mmc_host *mmc = cq_host->mmc; 249 u32 cqcfg; 250 251 cqcfg = cqhci_readl(cq_host, CQHCI_CFG); 252 253 /* Configuration must not be changed while enabled */ 254 if (cqcfg & CQHCI_ENABLE) { 255 cqcfg &= ~CQHCI_ENABLE; 256 cqhci_writel(cq_host, cqcfg, CQHCI_CFG); 257 } 258 259 cqcfg &= ~(CQHCI_DCMD | CQHCI_TASK_DESC_SZ); 260 261 if (mmc->caps2 & MMC_CAP2_CQE_DCMD) 262 cqcfg |= CQHCI_DCMD; 263 264 if (cq_host->caps & CQHCI_TASK_DESC_SZ_128) 265 cqcfg |= CQHCI_TASK_DESC_SZ; 266 267 if (mmc->caps2 & MMC_CAP2_CRYPTO) 268 cqcfg |= CQHCI_CRYPTO_GENERAL_ENABLE; 269 270 cqhci_writel(cq_host, cqcfg, CQHCI_CFG); 271 272 cqhci_writel(cq_host, lower_32_bits(cq_host->desc_dma_base), 273 CQHCI_TDLBA); 274 cqhci_writel(cq_host, upper_32_bits(cq_host->desc_dma_base), 275 CQHCI_TDLBAU); 276 277 cqhci_writel(cq_host, cq_host->rca, CQHCI_SSC2); 278 279 cqhci_set_irqs(cq_host, 0); 280 281 cqcfg |= CQHCI_ENABLE; 282 283 cqhci_writel(cq_host, cqcfg, CQHCI_CFG); 284 > 285 if (cqhci_halted(cq_host)) 286 cqhci_writel(cq_host, 0, CQHCI_CTL); 287 288 mmc->cqe_on = true; 289 290 if (cq_host->ops->enable) 291 cq_host->ops->enable(mmc); 292 293 /* Ensure all writes are done before interrupts are enabled */ 294 wmb(); 295 296 cqhci_set_irqs(cq_host, CQHCI_IS_MASK); 297 298 cq_host->activated = true; 299 } 300
diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c index c14d7251d0bb..3d5bcb92c78e 100644 --- a/drivers/mmc/host/cqhci-core.c +++ b/drivers/mmc/host/cqhci-core.c @@ -282,7 +282,7 @@ static void __cqhci_enable(struct cqhci_host *cq_host) cqhci_writel(cq_host, cqcfg, CQHCI_CFG); - if (cqhci_readl(cq_host, CQHCI_CTL) & CQHCI_HALT) + if (cqhci_halted(cq_host)) cqhci_writel(cq_host, 0, CQHCI_CTL); mmc->cqe_on = true; @@ -617,7 +617,7 @@ static int cqhci_request(struct mmc_host *mmc, struct mmc_request *mrq) cqhci_writel(cq_host, 0, CQHCI_CTL); mmc->cqe_on = true; pr_debug("%s: cqhci: CQE on\n", mmc_hostname(mmc)); - if (cqhci_readl(cq_host, CQHCI_CTL) && CQHCI_HALT) { + if (cqhci_halted(cq_host)) { pr_err("%s: cqhci: CQE failed to exit halt state\n", mmc_hostname(mmc)); }
To check if mmc cqe is in halt state, need to check set/clear of CQHCI_HALT bit. At this time, we need to check with &, not &&. Therefore, code to check whether cqe is in halt state is modified to cqhci_halted, which has already been implemented. Signed-off-by: Seunghwan Baek <sh8267.baek@samsung.com> --- drivers/mmc/host/cqhci-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)