[PATCH] scsi: hisi_sas: work around build failure in suspend function

[PATCH] scsi: hisi_sas: work around build failure in suspend function

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The suspend/resume functions in this driver seem to have multiple
problems, the latest one just got introduced by a bugfix:

drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function '_suspend_v3_hw':
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:5142:39: error: 'struct dev_pm_info' has no member named 'usage_count'
 5142 |         if (atomic_read(&device->power.usage_count)) {
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function '_suspend_v3_hw':
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:5142:39: error: 'struct dev_pm_info' has no member named 'usage_count'
 5142 |         if (atomic_read(&device->power.usage_count)) {

As far as I can tell, the 'usage_count' is not meant to be accessed
by device drivers at all, though I don't know what the driver is
supposed to do instead.

Another problem is the use of the deprecated UNIVERSAL_DEV_PM_OPS(),
and marking functions as __maybe_unused to avoid warnings about
unused functions.  This should probably be changed to using
DEFINE_RUNTIME_DEV_PM_OPS().

Both changes require actually understanding what the driver needs to
do, and being able to test this, so instead here is the simplest
patch to make it pass the randconfig builds instead.

Fixes: e368d38cb952 ("scsi: hisi_sas: Exit suspend state when usage count is greater than 0")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Maintainers: If possible, please revisit this to do a proper fix.
If that takes too much time, this patch can be applied as a
workaround in the meantime, and might also help in case the
e368d38cb952 patch gets backported to stable kernels.
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index d160b9b7479b..12d588454f5d 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -5139,11 +5139,13 @@ static int _suspend_v3_hw(struct device *device)
 	flush_workqueue(hisi_hba->wq);
 	interrupt_disable_v3_hw(hisi_hba);
 
+#ifdef CONFIG_PM
 	if (atomic_read(&device->power.usage_count)) {
 		dev_err(dev, "PM suspend: host status cannot be suspended\n");
 		rc = -EBUSY;
 		goto err_out;
 	}
+#endif
 
 	rc = disable_host_v3_hw(hisi_hba);
 	if (rc) {
@@ -5162,7 +5164,9 @@ static int _suspend_v3_hw(struct device *device)
 
 err_out_recover_host:
 	enable_host_v3_hw(hisi_hba);
+#ifdef CONFIG_PM
 err_out:
+#endif
 	interrupt_enable_v3_hw(hisi_hba);
 	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
 	clear_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags);
-- 
2.39.2

Re: [PATCH] scsi: hisi_sas: work around build failure in suspend function

[chenxiang (M)]

Hi Arnd,


在 2023/4/5 16:36, Arnd Bergmann 写道:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The suspend/resume functions in this driver seem to have multiple
> problems, the latest one just got introduced by a bugfix:
>
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function '_suspend_v3_hw':
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:5142:39: error: 'struct dev_pm_info' has no member named 'usage_count'
>   5142 |         if (atomic_read(&device->power.usage_count)) {
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function '_suspend_v3_hw':
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:5142:39: error: 'struct dev_pm_info' has no member named 'usage_count'
>   5142 |         if (atomic_read(&device->power.usage_count)) {
>
> As far as I can tell, the 'usage_count' is not meant to be accessed
> by device drivers at all, though I don't know what the driver is
> supposed to do instead.

Thank you for reporting the issue.
There is a extreme situation that hisi_sas driver tries to resume 
controller when it is in the process of suspend, which will cause a 
deadlock.
So we check usage_count of controller, and if usage_count > 0, failed to 
suspend to avoid the issue. But there is no common function defined
in pm_runtime.h which check the usage_count of device, so use it 
directly (i saw the check also be used in other drivers).
But i didn't realize that member usage_count is defined only under 
CONFIG_PM=y.


>
> Another problem is the use of the deprecated UNIVERSAL_DEV_PM_OPS(),
> and marking functions as __maybe_unused to avoid warnings about
> unused functions.  This should probably be changed to using
> DEFINE_RUNTIME_DEV_PM_OPS().

We use UNIVERSAL_DEV_PM_OPS() just because runtime callbacks 
runtime_{suspend|resume} and system callbacks {suspend|resume} use the same
operations in the driver, otherwise, we need to use 
DEFINE_RUNTIME_DEV_PM_OPS() to define runtime callbacks and use 
DEFINE_SIMPLE_DEV_PM_OPS()
to define system callbacks.

>
> Both changes require actually understanding what the driver needs to
> do, and being able to test this, so instead here is the simplest
> patch to make it pass the randconfig builds instead.
>
> Fixes: e368d38cb952 ("scsi: hisi_sas: Exit suspend state when usage count is greater than 0")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Maintainers: If possible, please revisit this to do a proper fix.
> If that takes too much time, this patch can be applied as a
> workaround in the meantime, and might also help in case the
> e368d38cb952 patch gets backported to stable kernels.

It is ok to apply the patch as a workaround as soon as possible.

Reviewed-by: Xiang Chen <chenxiang66@hisilicon.com>

Thanks!

> ---
>   drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index d160b9b7479b..12d588454f5d 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> @@ -5139,11 +5139,13 @@ static int _suspend_v3_hw(struct device *device)
>   	flush_workqueue(hisi_hba->wq);
>   	interrupt_disable_v3_hw(hisi_hba);
>   
> +#ifdef CONFIG_PM
>   	if (atomic_read(&device->power.usage_count)) {
>   		dev_err(dev, "PM suspend: host status cannot be suspended\n");
>   		rc = -EBUSY;
>   		goto err_out;
>   	}
> +#endif
>   
>   	rc = disable_host_v3_hw(hisi_hba);
>   	if (rc) {
> @@ -5162,7 +5164,9 @@ static int _suspend_v3_hw(struct device *device)
>   
>   err_out_recover_host:
>   	enable_host_v3_hw(hisi_hba);
> +#ifdef CONFIG_PM
>   err_out:
> +#endif
>   	interrupt_enable_v3_hw(hisi_hba);
>   	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
>   	clear_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags);

Re: [PATCH] scsi: hisi_sas: work around build failure in suspend function

[Martin K. Petersen]

Arnd,

> The suspend/resume functions in this driver seem to have multiple
> problems, the latest one just got introduced by a bugfix:

Applied to 6.4/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: hisi_sas: work around build failure in suspend function

[Martin K. Petersen]

On Wed, 05 Apr 2023 10:36:04 +0200, Arnd Bergmann wrote:

> The suspend/resume functions in this driver seem to have multiple
> problems, the latest one just got introduced by a bugfix:
> 
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function '_suspend_v3_hw':
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:5142:39: error: 'struct dev_pm_info' has no member named 'usage_count'
>  5142 |         if (atomic_read(&device->power.usage_count)) {
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function '_suspend_v3_hw':
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:5142:39: error: 'struct dev_pm_info' has no member named 'usage_count'
>  5142 |         if (atomic_read(&device->power.usage_count)) {
> 
> [...]

Applied to 6.4/scsi-queue, thanks!

[1/1] scsi: hisi_sas: work around build failure in suspend function
      https://git.kernel.org/mkp/scsi/c/e01e2290f094

-- 
Martin K. Petersen	Oracle Linux Engineering