diff mbox series

mmc : fix for check cqe halt.

Message ID 20240823071025.15410-1-sh8267.baek@samsung.com (mailing list archive)
State New
Headers show
Series mmc : fix for check cqe halt. | expand

Commit Message

Seunghwan Baek Aug. 23, 2024, 7:10 a.m. UTC
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(-)

Comments

Dragan Simic Aug. 23, 2024, 7:22 a.m. UTC | #1
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));
>  		}
Dragan Simic Aug. 23, 2024, 7:23 a.m. UTC | #2
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));
>>  		}
Ulf Hansson Aug. 23, 2024, 11 a.m. UTC | #3
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
>
Seunghwan Baek Aug. 26, 2024, 5:39 a.m. UTC | #4
> >
> > 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
> >
kernel test robot Aug. 26, 2024, 11:35 a.m. UTC | #5
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
kernel test robot Aug. 26, 2024, 11:46 a.m. UTC | #6
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 mbox series

Patch

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));
 		}