diff mbox series

[v5,2/3] mmc: host: sdhci: Implement the request_atomic() API

Message ID 9ed34afa9fb42e0c234065cac5401d7826942b55.1586744073.git.baolin.wang7@gmail.com (mailing list archive)
State New, archived
Headers show
Series Introduce the request_atomic() for the host | expand

Commit Message

Baolin Wang April 13, 2020, 2:46 a.m. UTC
Implement the request_atomic() ops for the sdhci driver to process
one request in the atomic context if the card is nonremovable.

Moreover, we should return BUSY flag if controller has not released
the inhibit bits to allow HSQ trying to send request again in non-atomic
context.

Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
---
 drivers/mmc/host/sdhci.c | 34 ++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.h |  1 +
 2 files changed, 35 insertions(+)

Comments

kernel test robot April 13, 2020, 4:40 a.m. UTC | #1
Hi Baolin,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc1 next-20200412]
[cannot apply to arm-soc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-the-request_atomic-for-the-host/20200413-105346
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8f3d9f354286745c751374f5f1fcafee6b3f3136
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mmc/host/sdhci.c: In function 'sdhci_request_atomic':
>> drivers/mmc/host/sdhci.c:2076:6: error: implicit declaration of function 'sdhci_present_error' [-Werror=implicit-function-declaration]
    2076 |  if (sdhci_present_error(host, mrq->cmd, true)) {
         |      ^~~~~~~~~~~~~~~~~~~
>> drivers/mmc/host/sdhci.c:2081:8: error: implicit declaration of function 'sdhci_manual_cmd23'; did you mean 'sdhci_auto_cmd12'? [-Werror=implicit-function-declaration]
    2081 |  cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
         |        ^~~~~~~~~~~~~~~~~~
         |        sdhci_auto_cmd12
>> drivers/mmc/host/sdhci.c:2090:6: error: invalid use of void expression
    2090 |  if (!sdhci_send_command(host, cmd))
         |      ^
   cc1: some warnings being treated as errors

vim +/sdhci_present_error +2076 drivers/mmc/host/sdhci.c

  2066	
  2067	int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
  2068	{
  2069		struct sdhci_host *host = mmc_priv(mmc);
  2070		struct mmc_command *cmd;
  2071		unsigned long flags;
  2072		int ret = 0;
  2073	
  2074		spin_lock_irqsave(&host->lock, flags);
  2075	
> 2076		if (sdhci_present_error(host, mrq->cmd, true)) {
  2077			sdhci_finish_mrq(host, mrq);
  2078			goto out_finish;
  2079		}
  2080	
> 2081		cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
  2082	
  2083		/*
  2084		 * The HSQ may send a command in interrupt context without polling
  2085		 * the busy signaling, which means we should return BUSY if controller
  2086		 * has not released inhibit bits to allow HSQ trying to send request
  2087		 * again in non-atomic context. So we should not finish this request
  2088		 * here.
  2089		 */
> 2090		if (!sdhci_send_command(host, cmd))
  2091			ret = -EBUSY;
  2092		else
  2093			sdhci_led_activate(host);
  2094	
  2095	out_finish:
  2096		spin_unlock_irqrestore(&host->lock, flags);
  2097		return ret;
  2098	}
  2099	EXPORT_SYMBOL_GPL(sdhci_request_atomic);
  2100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot April 13, 2020, 5:42 a.m. UTC | #2
Hi Baolin,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc1 next-20200412]
[cannot apply to arm-soc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-the-request_atomic-for-the-host/20200413-105346
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8f3d9f354286745c751374f5f1fcafee6b3f3136
config: x86_64-randconfig-s2-20200413 (attached as .config)
compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/build_bug.h:5:0,
                    from include/linux/bitfield.h:10,
                    from drivers/mmc/host/sdhci.c:12:
   drivers/mmc/host/sdhci.c: In function 'sdhci_request_atomic':
   drivers/mmc/host/sdhci.c:2076:6: error: implicit declaration of function 'sdhci_present_error' [-Werror=implicit-function-declaration]
     if (sdhci_present_error(host, mrq->cmd, true)) {
         ^
   include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                       ^
>> drivers/mmc/host/sdhci.c:2076:2: note: in expansion of macro 'if'
     if (sdhci_present_error(host, mrq->cmd, true)) {
     ^
   drivers/mmc/host/sdhci.c:2081:8: error: implicit declaration of function 'sdhci_manual_cmd23' [-Werror=implicit-function-declaration]
     cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
           ^
   drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
     if (!sdhci_send_command(host, cmd))
     ^
   drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
   In file included from include/linux/build_bug.h:5:0,
                    from include/linux/bitfield.h:10,
                    from drivers/mmc/host/sdhci.c:12:
>> include/linux/compiler.h:61:16: error: invalid use of void expression
     static struct ftrace_branch_data  \
                   ^
   include/linux/compiler.h:58:69: note: in expansion of macro '__trace_if_value'
    #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                                                        ^
   include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
    #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                               ^
   drivers/mmc/host/sdhci.c:2090:2: note: in expansion of macro 'if'
     if (!sdhci_send_command(host, cmd))
     ^
   cc1: some warnings being treated as errors

vim +/if +2076 drivers/mmc/host/sdhci.c

  2066	
  2067	int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
  2068	{
  2069		struct sdhci_host *host = mmc_priv(mmc);
  2070		struct mmc_command *cmd;
  2071		unsigned long flags;
  2072		int ret = 0;
  2073	
  2074		spin_lock_irqsave(&host->lock, flags);
  2075	
> 2076		if (sdhci_present_error(host, mrq->cmd, true)) {
  2077			sdhci_finish_mrq(host, mrq);
  2078			goto out_finish;
  2079		}
  2080	
  2081		cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
  2082	
  2083		/*
  2084		 * The HSQ may send a command in interrupt context without polling
  2085		 * the busy signaling, which means we should return BUSY if controller
  2086		 * has not released inhibit bits to allow HSQ trying to send request
  2087		 * again in non-atomic context. So we should not finish this request
  2088		 * here.
  2089		 */
  2090		if (!sdhci_send_command(host, cmd))
  2091			ret = -EBUSY;
  2092		else
  2093			sdhci_led_activate(host);
  2094	
  2095	out_finish:
  2096		spin_unlock_irqrestore(&host->lock, flags);
  2097		return ret;
  2098	}
  2099	EXPORT_SYMBOL_GPL(sdhci_request_atomic);
  2100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Baolin Wang April 13, 2020, 6:52 a.m. UTC | #3
Hi,

On Mon, Apr 13, 2020 at 1:42 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Baolin,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.7-rc1 next-20200412]
> [cannot apply to arm-soc/for-next]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Baolin-Wang/Introduce-the-request_atomic-for-the-host/20200413-105346
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 8f3d9f354286745c751374f5f1fcafee6b3f3136
> config: x86_64-randconfig-s2-20200413 (attached as .config)
> compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kbuild test robot <lkp@intel.com>

This patch set is based on Adrian's patch set. Thanks.
https://www.spinics.net/lists/linux-mmc/msg58529.html

>
> All error/warnings (new ones prefixed by >>):
>
>    In file included from include/linux/build_bug.h:5:0,
>                     from include/linux/bitfield.h:10,
>                     from drivers/mmc/host/sdhci.c:12:
>    drivers/mmc/host/sdhci.c: In function 'sdhci_request_atomic':
>    drivers/mmc/host/sdhci.c:2076:6: error: implicit declaration of function 'sdhci_present_error' [-Werror=implicit-function-declaration]
>      if (sdhci_present_error(host, mrq->cmd, true)) {
>          ^
>    include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
>     #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
>                                                        ^
> >> drivers/mmc/host/sdhci.c:2076:2: note: in expansion of macro 'if'
>      if (sdhci_present_error(host, mrq->cmd, true)) {
>      ^
>    drivers/mmc/host/sdhci.c:2081:8: error: implicit declaration of function 'sdhci_manual_cmd23' [-Werror=implicit-function-declaration]
>      cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
>            ^
>    drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
>      if (!sdhci_send_command(host, cmd))
>      ^
>    drivers/mmc/host/sdhci.c:2090:2: error: invalid use of void expression
>    In file included from include/linux/build_bug.h:5:0,
>                     from include/linux/bitfield.h:10,
>                     from drivers/mmc/host/sdhci.c:12:
> >> include/linux/compiler.h:61:16: error: invalid use of void expression
>      static struct ftrace_branch_data  \
>                    ^
>    include/linux/compiler.h:58:69: note: in expansion of macro '__trace_if_value'
>     #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
>                                                                         ^
>    include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
>     #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
>                                ^
>    drivers/mmc/host/sdhci.c:2090:2: note: in expansion of macro 'if'
>      if (!sdhci_send_command(host, cmd))
>      ^
>    cc1: some warnings being treated as errors
>
> vim +/if +2076 drivers/mmc/host/sdhci.c
>
>   2066
>   2067  int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
>   2068  {
>   2069          struct sdhci_host *host = mmc_priv(mmc);
>   2070          struct mmc_command *cmd;
>   2071          unsigned long flags;
>   2072          int ret = 0;
>   2073
>   2074          spin_lock_irqsave(&host->lock, flags);
>   2075
> > 2076          if (sdhci_present_error(host, mrq->cmd, true)) {
>   2077                  sdhci_finish_mrq(host, mrq);
>   2078                  goto out_finish;
>   2079          }
>   2080
>   2081          cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
>   2082
>   2083          /*
>   2084           * The HSQ may send a command in interrupt context without polling
>   2085           * the busy signaling, which means we should return BUSY if controller
>   2086           * has not released inhibit bits to allow HSQ trying to send request
>   2087           * again in non-atomic context. So we should not finish this request
>   2088           * here.
>   2089           */
>   2090          if (!sdhci_send_command(host, cmd))
>   2091                  ret = -EBUSY;
>   2092          else
>   2093                  sdhci_led_activate(host);
>   2094
>   2095  out_finish:
>   2096          spin_unlock_irqrestore(&host->lock, flags);
>   2097          return ret;
>   2098  }
>   2099  EXPORT_SYMBOL_GPL(sdhci_request_atomic);
>   2100
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Adrian Hunter April 14, 2020, 1:47 p.m. UTC | #4
On 13/04/20 5:46 am, Baolin Wang wrote:
> Implement the request_atomic() ops for the sdhci driver to process
> one request in the atomic context if the card is nonremovable.
> 
> Moreover, we should return BUSY flag if controller has not released
> the inhibit bits to allow HSQ trying to send request again in non-atomic
> context.
> 
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci.c | 34 ++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci.h |  1 +
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 10b9570f48aa..0baef595de26 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2144,6 +2144,40 @@ void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  }
>  EXPORT_SYMBOL_GPL(sdhci_request);
>  
> +int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	struct mmc_command *cmd;
> +	unsigned long flags;
> +	int ret = 0;
> +
> +	spin_lock_irqsave(&host->lock, flags);
> +
> +	if (sdhci_present_error(host, mrq->cmd, true)) {
> +		sdhci_finish_mrq(host, mrq);
> +		goto out_finish;
> +	}
> +
> +	cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
> +
> +	/*
> +	 * The HSQ may send a command in interrupt context without polling
> +	 * the busy signaling, which means we should return BUSY if controller
> +	 * has not released inhibit bits to allow HSQ trying to send request
> +	 * again in non-atomic context. So we should not finish this request
> +	 * here.
> +	 */
> +	if (!sdhci_send_command(host, cmd))
> +		ret = -EBUSY;
> +	else
> +		sdhci_led_activate(host);
> +
> +out_finish:
> +	spin_unlock_irqrestore(&host->lock, flags);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(sdhci_request_atomic);
> +
>  void sdhci_set_bus_width(struct sdhci_host *host, int width)
>  {
>  	u8 ctrl;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index a7e469c00617..4bd70da7aa00 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -776,6 +776,7 @@ void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
>  void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
>  			   unsigned short vdd);
>  void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
> +int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq);
>  void sdhci_set_bus_width(struct sdhci_host *host, int width);
>  void sdhci_reset(struct sdhci_host *host, u8 mask);
>  void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
>
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 10b9570f48aa..0baef595de26 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2144,6 +2144,40 @@  void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 }
 EXPORT_SYMBOL_GPL(sdhci_request);
 
+int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct mmc_command *cmd;
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&host->lock, flags);
+
+	if (sdhci_present_error(host, mrq->cmd, true)) {
+		sdhci_finish_mrq(host, mrq);
+		goto out_finish;
+	}
+
+	cmd = sdhci_manual_cmd23(host, mrq) ? mrq->sbc : mrq->cmd;
+
+	/*
+	 * The HSQ may send a command in interrupt context without polling
+	 * the busy signaling, which means we should return BUSY if controller
+	 * has not released inhibit bits to allow HSQ trying to send request
+	 * again in non-atomic context. So we should not finish this request
+	 * here.
+	 */
+	if (!sdhci_send_command(host, cmd))
+		ret = -EBUSY;
+	else
+		sdhci_led_activate(host);
+
+out_finish:
+	spin_unlock_irqrestore(&host->lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sdhci_request_atomic);
+
 void sdhci_set_bus_width(struct sdhci_host *host, int width)
 {
 	u8 ctrl;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index a7e469c00617..4bd70da7aa00 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -776,6 +776,7 @@  void sdhci_set_power_and_bus_voltage(struct sdhci_host *host,
 void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 			   unsigned short vdd);
 void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq);
+int sdhci_request_atomic(struct mmc_host *mmc, struct mmc_request *mrq);
 void sdhci_set_bus_width(struct sdhci_host *host, int width);
 void sdhci_reset(struct sdhci_host *host, u8 mask);
 void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);