public inbox for linux-ide@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libata: make ata_host_suspend() *void*
@ 2022-02-02 20:58 Sergey Shtylyov
  2022-02-02 23:55 ` Damien Le Moal
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Sergey Shtylyov @ 2022-02-02 20:58 UTC (permalink / raw)
  To: Damien Le Moal, linux-ide
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-arm-kernel, Hans de Goede, Jens Axboe,
	Viresh Kumar

ata_host_suspend() always returns 0, so the result checks in many drivers
look pointless.  Let's make this function return *void* instead of *int*.

Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

---
This patch is against the 'for-next' branch of Damien Le Moal's 'libata.git'
repo.

 drivers/ata/ahci.c             |    3 ++-
 drivers/ata/ata_piix.c         |    5 +----
 drivers/ata/libahci_platform.c |    3 ++-
 drivers/ata/libata-core.c      |    8 ++------
 drivers/ata/pata_arasan_cf.c   |    3 ++-
 drivers/ata/pata_cs5520.c      |    5 +----
 drivers/ata/pata_imx.c         |   15 ++++++---------
 drivers/ata/pata_macio.c       |    6 +-----
 drivers/ata/pata_mpc52xx.c     |    3 ++-
 drivers/ata/pata_samsung_cf.c  |    3 ++-
 drivers/ata/pata_triflex.c     |    5 +----
 drivers/ata/sata_fsl.c         |    4 +++-
 drivers/ata/sata_highbank.c    |    3 ++-
 drivers/ata/sata_mv.c          |    6 +++---
 drivers/ata/sata_rcar.c        |   18 ++++++++----------
 include/linux/libata.h         |    2 +-
 16 files changed, 39 insertions(+), 53 deletions(-)

Index: libata/drivers/ata/ahci.c
===================================================================
--- libata.orig/drivers/ata/ahci.c
+++ libata/drivers/ata/ahci.c
@@ -891,7 +891,8 @@ static int ahci_pci_device_suspend(struc
 	}
 
 	ahci_pci_disable_interrupts(host);
-	return ata_host_suspend(host, PMSG_SUSPEND);
+	ata_host_suspend(host, PMSG_SUSPEND);
+	return 0;
 }
 
 static int ahci_pci_device_resume(struct device *dev)
Index: libata/drivers/ata/ata_piix.c
===================================================================
--- libata.orig/drivers/ata/ata_piix.c
+++ libata/drivers/ata/ata_piix.c
@@ -993,11 +993,8 @@ static int piix_pci_device_suspend(struc
 {
 	struct ata_host *host = pci_get_drvdata(pdev);
 	unsigned long flags;
-	int rc = 0;
 
-	rc = ata_host_suspend(host, mesg);
-	if (rc)
-		return rc;
+	ata_host_suspend(host, mesg);
 
 	/* Some braindamaged ACPI suspend implementations expect the
 	 * controller to be awake on entry; otherwise, it burns cpu
Index: libata/drivers/ata/libahci_platform.c
===================================================================
--- libata.orig/drivers/ata/libahci_platform.c
+++ libata/drivers/ata/libahci_platform.c
@@ -733,7 +733,8 @@ int ahci_platform_suspend_host(struct de
 	if (hpriv->flags & AHCI_HFLAG_SUSPEND_PHYS)
 		ahci_platform_disable_phys(hpriv);
 
-	return ata_host_suspend(host, PMSG_SUSPEND);
+	ata_host_suspend(host, PMSG_SUSPEND);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(ahci_platform_suspend_host);
 
Index: libata/drivers/ata/libata-core.c
===================================================================
--- libata.orig/drivers/ata/libata-core.c
+++ libata/drivers/ata/libata-core.c
@@ -5170,10 +5170,9 @@ EXPORT_SYMBOL_GPL(ata_sas_port_resume);
  *
  *	Suspend @host.  Actual operation is performed by port suspend.
  */
-int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
+void ata_host_suspend(struct ata_host *host, pm_message_t mesg)
 {
 	host->dev->power.power_state = mesg;
-	return 0;
 }
 EXPORT_SYMBOL_GPL(ata_host_suspend);
 
@@ -6090,11 +6089,8 @@ EXPORT_SYMBOL_GPL(ata_pci_device_do_resu
 int ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
 {
 	struct ata_host *host = pci_get_drvdata(pdev);
-	int rc = 0;
 
-	rc = ata_host_suspend(host, mesg);
-	if (rc)
-		return rc;
+	ata_host_suspend(host, mesg);
 
 	ata_pci_device_do_suspend(pdev, mesg);
 
Index: libata/drivers/ata/pata_arasan_cf.c
===================================================================
--- libata.orig/drivers/ata/pata_arasan_cf.c
+++ libata/drivers/ata/pata_arasan_cf.c
@@ -937,7 +937,8 @@ static int arasan_cf_suspend(struct devi
 		dmaengine_terminate_all(acdev->dma_chan);
 
 	cf_exit(acdev);
-	return ata_host_suspend(host, PMSG_SUSPEND);
+	ata_host_suspend(host, PMSG_SUSPEND);
+	return 0;
 }
 
 static int arasan_cf_resume(struct device *dev)
Index: libata/drivers/ata/pata_cs5520.c
===================================================================
--- libata.orig/drivers/ata/pata_cs5520.c
+++ libata/drivers/ata/pata_cs5520.c
@@ -259,11 +259,8 @@ static int cs5520_reinit_one(struct pci_
 static int cs5520_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
 {
 	struct ata_host *host = pci_get_drvdata(pdev);
-	int rc = 0;
 
-	rc = ata_host_suspend(host, mesg);
-	if (rc)
-		return rc;
+	ata_host_suspend(host, mesg);
 
 	pci_save_state(pdev);
 	return 0;
Index: libata/drivers/ata/pata_imx.c
===================================================================
--- libata.orig/drivers/ata/pata_imx.c
+++ libata/drivers/ata/pata_imx.c
@@ -223,17 +223,14 @@ static int pata_imx_suspend(struct devic
 {
 	struct ata_host *host = dev_get_drvdata(dev);
 	struct pata_imx_priv *priv = host->private_data;
-	int ret;
 
-	ret = ata_host_suspend(host, PMSG_SUSPEND);
-	if (!ret) {
-		__raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN);
-		priv->ata_ctl =
-			__raw_readl(priv->host_regs + PATA_IMX_ATA_CONTROL);
-		clk_disable_unprepare(priv->clk);
-	}
+	ata_host_suspend(host, PMSG_SUSPEND);
 
-	return ret;
+	__raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN);
+	priv->ata_ctl = __raw_readl(priv->host_regs + PATA_IMX_ATA_CONTROL);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
 }
 
 static int pata_imx_resume(struct device *dev)
Index: libata/drivers/ata/pata_macio.c
===================================================================
--- libata.orig/drivers/ata/pata_macio.c
+++ libata/drivers/ata/pata_macio.c
@@ -853,12 +853,8 @@ static int pata_macio_slave_config(struc
 #ifdef CONFIG_PM_SLEEP
 static int pata_macio_do_suspend(struct pata_macio_priv *priv, pm_message_t mesg)
 {
-	int rc;
-
 	/* First, core libata suspend to do most of the work */
-	rc = ata_host_suspend(priv->host, mesg);
-	if (rc)
-		return rc;
+	ata_host_suspend(priv->host, mesg);
 
 	/* Restore to default timings */
 	pata_macio_default_timings(priv);
Index: libata/drivers/ata/pata_mpc52xx.c
===================================================================
--- libata.orig/drivers/ata/pata_mpc52xx.c
+++ libata/drivers/ata/pata_mpc52xx.c
@@ -824,7 +824,8 @@ mpc52xx_ata_suspend(struct platform_devi
 {
 	struct ata_host *host = platform_get_drvdata(op);
 
-	return ata_host_suspend(host, state);
+	ata_host_suspend(host, state);
+	return 0;
 }
 
 static int
Index: libata/drivers/ata/pata_samsung_cf.c
===================================================================
--- libata.orig/drivers/ata/pata_samsung_cf.c
+++ libata/drivers/ata/pata_samsung_cf.c
@@ -608,7 +608,8 @@ static int pata_s3c_suspend(struct devic
 {
 	struct ata_host *host = dev_get_drvdata(dev);
 
-	return ata_host_suspend(host, PMSG_SUSPEND);
+	ata_host_suspend(host, PMSG_SUSPEND);
+	return 0;
 }
 
 static int pata_s3c_resume(struct device *dev)
Index: libata/drivers/ata/pata_triflex.c
===================================================================
--- libata.orig/drivers/ata/pata_triflex.c
+++ libata/drivers/ata/pata_triflex.c
@@ -198,11 +198,8 @@ static const struct pci_device_id trifle
 static int triflex_ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
 {
 	struct ata_host *host = pci_get_drvdata(pdev);
-	int rc = 0;
 
-	rc = ata_host_suspend(host, mesg);
-	if (rc)
-		return rc;
+	ata_host_suspend(host, mesg);
 
 	/*
 	 * We must not disable or powerdown the device.
Index: libata/drivers/ata/sata_fsl.c
===================================================================
--- libata.orig/drivers/ata/sata_fsl.c
+++ libata/drivers/ata/sata_fsl.c
@@ -1546,7 +1546,9 @@ static int sata_fsl_remove(struct platfo
 static int sata_fsl_suspend(struct platform_device *op, pm_message_t state)
 {
 	struct ata_host *host = platform_get_drvdata(op);
-	return ata_host_suspend(host, state);
+
+	ata_host_suspend(host, state);
+	return 0;
 }
 
 static int sata_fsl_resume(struct platform_device *op)
Index: libata/drivers/ata/sata_highbank.c
===================================================================
--- libata.orig/drivers/ata/sata_highbank.c
+++ libata/drivers/ata/sata_highbank.c
@@ -587,7 +587,8 @@ static int ahci_highbank_suspend(struct
 	writel(ctl, mmio + HOST_CTL);
 	readl(mmio + HOST_CTL); /* flush */
 
-	return ata_host_suspend(host, PMSG_SUSPEND);
+	ata_host_suspend(host, PMSG_SUSPEND);
+	return 0;
 }
 
 static int ahci_highbank_resume(struct device *dev)
Index: libata/drivers/ata/sata_mv.c
===================================================================
--- libata.orig/drivers/ata/sata_mv.c
+++ libata/drivers/ata/sata_mv.c
@@ -4235,10 +4235,10 @@ static int mv_platform_remove(struct pla
 static int mv_platform_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct ata_host *host = platform_get_drvdata(pdev);
+
 	if (host)
-		return ata_host_suspend(host, state);
-	else
-		return 0;
+		ata_host_suspend(host, state);
+	return 0;
 }
 
 static int mv_platform_resume(struct platform_device *pdev)
Index: libata/drivers/ata/sata_rcar.c
===================================================================
--- libata.orig/drivers/ata/sata_rcar.c
+++ libata/drivers/ata/sata_rcar.c
@@ -945,19 +945,17 @@ static int sata_rcar_suspend(struct devi
 	struct ata_host *host = dev_get_drvdata(dev);
 	struct sata_rcar_priv *priv = host->private_data;
 	void __iomem *base = priv->base;
-	int ret;
 
-	ret = ata_host_suspend(host, PMSG_SUSPEND);
-	if (!ret) {
-		/* disable interrupts */
-		iowrite32(0, base + ATAPI_INT_ENABLE_REG);
-		/* mask */
-		iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
+	ata_host_suspend(host, PMSG_SUSPEND);
 
-		pm_runtime_put(dev);
-	}
+	/* disable interrupts */
+	iowrite32(0, base + ATAPI_INT_ENABLE_REG);
+	/* mask */
+	iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
 
-	return ret;
+	pm_runtime_put(dev);
+
+	return 0;
 }
 
 static int sata_rcar_resume(struct device *dev)
Index: libata/include/linux/libata.h
===================================================================
--- libata.orig/include/linux/libata.h
+++ libata/include/linux/libata.h
@@ -1080,7 +1080,7 @@ extern int ata_sas_scsi_ioctl(struct ata
 extern bool ata_link_online(struct ata_link *link);
 extern bool ata_link_offline(struct ata_link *link);
 #ifdef CONFIG_PM
-extern int ata_host_suspend(struct ata_host *host, pm_message_t mesg);
+extern void ata_host_suspend(struct ata_host *host, pm_message_t mesg);
 extern void ata_host_resume(struct ata_host *host);
 extern void ata_sas_port_suspend(struct ata_port *ap);
 extern void ata_sas_port_resume(struct ata_port *ap);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libata: make ata_host_suspend() *void*
  2022-02-02 20:58 [PATCH] libata: make ata_host_suspend() *void* Sergey Shtylyov
@ 2022-02-02 23:55 ` Damien Le Moal
  2022-02-03  6:44 ` Uwe Kleine-König
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2022-02-02 23:55 UTC (permalink / raw)
  To: Sergey Shtylyov, linux-ide
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-arm-kernel, Hans de Goede, Jens Axboe,
	Viresh Kumar

On 2/3/22 05:58, Sergey Shtylyov wrote:
> ata_host_suspend() always returns 0, so the result checks in many drivers
> look pointless.  Let's make this function return *void* instead of *int*.
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

Looks good to me. Let's wait a little for comments/reviews then I will
apply.

> 
> ---
> This patch is against the 'for-next' branch of Damien Le Moal's 'libata.git'
> repo.
> 
>  drivers/ata/ahci.c             |    3 ++-
>  drivers/ata/ata_piix.c         |    5 +----
>  drivers/ata/libahci_platform.c |    3 ++-
>  drivers/ata/libata-core.c      |    8 ++------
>  drivers/ata/pata_arasan_cf.c   |    3 ++-
>  drivers/ata/pata_cs5520.c      |    5 +----
>  drivers/ata/pata_imx.c         |   15 ++++++---------
>  drivers/ata/pata_macio.c       |    6 +-----
>  drivers/ata/pata_mpc52xx.c     |    3 ++-
>  drivers/ata/pata_samsung_cf.c  |    3 ++-
>  drivers/ata/pata_triflex.c     |    5 +----
>  drivers/ata/sata_fsl.c         |    4 +++-
>  drivers/ata/sata_highbank.c    |    3 ++-
>  drivers/ata/sata_mv.c          |    6 +++---
>  drivers/ata/sata_rcar.c        |   18 ++++++++----------
>  include/linux/libata.h         |    2 +-
>  16 files changed, 39 insertions(+), 53 deletions(-)
> 
> Index: libata/drivers/ata/ahci.c
> ===================================================================
> --- libata.orig/drivers/ata/ahci.c
> +++ libata/drivers/ata/ahci.c
> @@ -891,7 +891,8 @@ static int ahci_pci_device_suspend(struc
>  	}
>  
>  	ahci_pci_disable_interrupts(host);
> -	return ata_host_suspend(host, PMSG_SUSPEND);
> +	ata_host_suspend(host, PMSG_SUSPEND);
> +	return 0;
>  }
>  
>  static int ahci_pci_device_resume(struct device *dev)
> Index: libata/drivers/ata/ata_piix.c
> ===================================================================
> --- libata.orig/drivers/ata/ata_piix.c
> +++ libata/drivers/ata/ata_piix.c
> @@ -993,11 +993,8 @@ static int piix_pci_device_suspend(struc
>  {
>  	struct ata_host *host = pci_get_drvdata(pdev);
>  	unsigned long flags;
> -	int rc = 0;
>  
> -	rc = ata_host_suspend(host, mesg);
> -	if (rc)
> -		return rc;
> +	ata_host_suspend(host, mesg);
>  
>  	/* Some braindamaged ACPI suspend implementations expect the
>  	 * controller to be awake on entry; otherwise, it burns cpu
> Index: libata/drivers/ata/libahci_platform.c
> ===================================================================
> --- libata.orig/drivers/ata/libahci_platform.c
> +++ libata/drivers/ata/libahci_platform.c
> @@ -733,7 +733,8 @@ int ahci_platform_suspend_host(struct de
>  	if (hpriv->flags & AHCI_HFLAG_SUSPEND_PHYS)
>  		ahci_platform_disable_phys(hpriv);
>  
> -	return ata_host_suspend(host, PMSG_SUSPEND);
> +	ata_host_suspend(host, PMSG_SUSPEND);
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(ahci_platform_suspend_host);
>  
> Index: libata/drivers/ata/libata-core.c
> ===================================================================
> --- libata.orig/drivers/ata/libata-core.c
> +++ libata/drivers/ata/libata-core.c
> @@ -5170,10 +5170,9 @@ EXPORT_SYMBOL_GPL(ata_sas_port_resume);
>   *
>   *	Suspend @host.  Actual operation is performed by port suspend.
>   */
> -int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
> +void ata_host_suspend(struct ata_host *host, pm_message_t mesg)
>  {
>  	host->dev->power.power_state = mesg;
> -	return 0;
>  }
>  EXPORT_SYMBOL_GPL(ata_host_suspend);
>  
> @@ -6090,11 +6089,8 @@ EXPORT_SYMBOL_GPL(ata_pci_device_do_resu
>  int ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
>  {
>  	struct ata_host *host = pci_get_drvdata(pdev);
> -	int rc = 0;
>  
> -	rc = ata_host_suspend(host, mesg);
> -	if (rc)
> -		return rc;
> +	ata_host_suspend(host, mesg);
>  
>  	ata_pci_device_do_suspend(pdev, mesg);
>  
> Index: libata/drivers/ata/pata_arasan_cf.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_arasan_cf.c
> +++ libata/drivers/ata/pata_arasan_cf.c
> @@ -937,7 +937,8 @@ static int arasan_cf_suspend(struct devi
>  		dmaengine_terminate_all(acdev->dma_chan);
>  
>  	cf_exit(acdev);
> -	return ata_host_suspend(host, PMSG_SUSPEND);
> +	ata_host_suspend(host, PMSG_SUSPEND);
> +	return 0;
>  }
>  
>  static int arasan_cf_resume(struct device *dev)
> Index: libata/drivers/ata/pata_cs5520.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_cs5520.c
> +++ libata/drivers/ata/pata_cs5520.c
> @@ -259,11 +259,8 @@ static int cs5520_reinit_one(struct pci_
>  static int cs5520_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
>  {
>  	struct ata_host *host = pci_get_drvdata(pdev);
> -	int rc = 0;
>  
> -	rc = ata_host_suspend(host, mesg);
> -	if (rc)
> -		return rc;
> +	ata_host_suspend(host, mesg);
>  
>  	pci_save_state(pdev);
>  	return 0;
> Index: libata/drivers/ata/pata_imx.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_imx.c
> +++ libata/drivers/ata/pata_imx.c
> @@ -223,17 +223,14 @@ static int pata_imx_suspend(struct devic
>  {
>  	struct ata_host *host = dev_get_drvdata(dev);
>  	struct pata_imx_priv *priv = host->private_data;
> -	int ret;
>  
> -	ret = ata_host_suspend(host, PMSG_SUSPEND);
> -	if (!ret) {
> -		__raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN);
> -		priv->ata_ctl =
> -			__raw_readl(priv->host_regs + PATA_IMX_ATA_CONTROL);
> -		clk_disable_unprepare(priv->clk);
> -	}
> +	ata_host_suspend(host, PMSG_SUSPEND);
>  
> -	return ret;
> +	__raw_writel(0, priv->host_regs + PATA_IMX_ATA_INT_EN);
> +	priv->ata_ctl = __raw_readl(priv->host_regs + PATA_IMX_ATA_CONTROL);
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
>  }
>  
>  static int pata_imx_resume(struct device *dev)
> Index: libata/drivers/ata/pata_macio.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_macio.c
> +++ libata/drivers/ata/pata_macio.c
> @@ -853,12 +853,8 @@ static int pata_macio_slave_config(struc
>  #ifdef CONFIG_PM_SLEEP
>  static int pata_macio_do_suspend(struct pata_macio_priv *priv, pm_message_t mesg)
>  {
> -	int rc;
> -
>  	/* First, core libata suspend to do most of the work */
> -	rc = ata_host_suspend(priv->host, mesg);
> -	if (rc)
> -		return rc;
> +	ata_host_suspend(priv->host, mesg);
>  
>  	/* Restore to default timings */
>  	pata_macio_default_timings(priv);
> Index: libata/drivers/ata/pata_mpc52xx.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_mpc52xx.c
> +++ libata/drivers/ata/pata_mpc52xx.c
> @@ -824,7 +824,8 @@ mpc52xx_ata_suspend(struct platform_devi
>  {
>  	struct ata_host *host = platform_get_drvdata(op);
>  
> -	return ata_host_suspend(host, state);
> +	ata_host_suspend(host, state);
> +	return 0;
>  }
>  
>  static int
> Index: libata/drivers/ata/pata_samsung_cf.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_samsung_cf.c
> +++ libata/drivers/ata/pata_samsung_cf.c
> @@ -608,7 +608,8 @@ static int pata_s3c_suspend(struct devic
>  {
>  	struct ata_host *host = dev_get_drvdata(dev);
>  
> -	return ata_host_suspend(host, PMSG_SUSPEND);
> +	ata_host_suspend(host, PMSG_SUSPEND);
> +	return 0;
>  }
>  
>  static int pata_s3c_resume(struct device *dev)
> Index: libata/drivers/ata/pata_triflex.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_triflex.c
> +++ libata/drivers/ata/pata_triflex.c
> @@ -198,11 +198,8 @@ static const struct pci_device_id trifle
>  static int triflex_ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
>  {
>  	struct ata_host *host = pci_get_drvdata(pdev);
> -	int rc = 0;
>  
> -	rc = ata_host_suspend(host, mesg);
> -	if (rc)
> -		return rc;
> +	ata_host_suspend(host, mesg);
>  
>  	/*
>  	 * We must not disable or powerdown the device.
> Index: libata/drivers/ata/sata_fsl.c
> ===================================================================
> --- libata.orig/drivers/ata/sata_fsl.c
> +++ libata/drivers/ata/sata_fsl.c
> @@ -1546,7 +1546,9 @@ static int sata_fsl_remove(struct platfo
>  static int sata_fsl_suspend(struct platform_device *op, pm_message_t state)
>  {
>  	struct ata_host *host = platform_get_drvdata(op);
> -	return ata_host_suspend(host, state);
> +
> +	ata_host_suspend(host, state);
> +	return 0;
>  }
>  
>  static int sata_fsl_resume(struct platform_device *op)
> Index: libata/drivers/ata/sata_highbank.c
> ===================================================================
> --- libata.orig/drivers/ata/sata_highbank.c
> +++ libata/drivers/ata/sata_highbank.c
> @@ -587,7 +587,8 @@ static int ahci_highbank_suspend(struct
>  	writel(ctl, mmio + HOST_CTL);
>  	readl(mmio + HOST_CTL); /* flush */
>  
> -	return ata_host_suspend(host, PMSG_SUSPEND);
> +	ata_host_suspend(host, PMSG_SUSPEND);
> +	return 0;
>  }
>  
>  static int ahci_highbank_resume(struct device *dev)
> Index: libata/drivers/ata/sata_mv.c
> ===================================================================
> --- libata.orig/drivers/ata/sata_mv.c
> +++ libata/drivers/ata/sata_mv.c
> @@ -4235,10 +4235,10 @@ static int mv_platform_remove(struct pla
>  static int mv_platform_suspend(struct platform_device *pdev, pm_message_t state)
>  {
>  	struct ata_host *host = platform_get_drvdata(pdev);
> +
>  	if (host)
> -		return ata_host_suspend(host, state);
> -	else
> -		return 0;
> +		ata_host_suspend(host, state);
> +	return 0;
>  }
>  
>  static int mv_platform_resume(struct platform_device *pdev)
> Index: libata/drivers/ata/sata_rcar.c
> ===================================================================
> --- libata.orig/drivers/ata/sata_rcar.c
> +++ libata/drivers/ata/sata_rcar.c
> @@ -945,19 +945,17 @@ static int sata_rcar_suspend(struct devi
>  	struct ata_host *host = dev_get_drvdata(dev);
>  	struct sata_rcar_priv *priv = host->private_data;
>  	void __iomem *base = priv->base;
> -	int ret;
>  
> -	ret = ata_host_suspend(host, PMSG_SUSPEND);
> -	if (!ret) {
> -		/* disable interrupts */
> -		iowrite32(0, base + ATAPI_INT_ENABLE_REG);
> -		/* mask */
> -		iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
> +	ata_host_suspend(host, PMSG_SUSPEND);
>  
> -		pm_runtime_put(dev);
> -	}
> +	/* disable interrupts */
> +	iowrite32(0, base + ATAPI_INT_ENABLE_REG);
> +	/* mask */
> +	iowrite32(priv->sataint_mask, base + SATAINTMASK_REG);
>  
> -	return ret;
> +	pm_runtime_put(dev);
> +
> +	return 0;
>  }
>  
>  static int sata_rcar_resume(struct device *dev)
> Index: libata/include/linux/libata.h
> ===================================================================
> --- libata.orig/include/linux/libata.h
> +++ libata/include/linux/libata.h
> @@ -1080,7 +1080,7 @@ extern int ata_sas_scsi_ioctl(struct ata
>  extern bool ata_link_online(struct ata_link *link);
>  extern bool ata_link_offline(struct ata_link *link);
>  #ifdef CONFIG_PM
> -extern int ata_host_suspend(struct ata_host *host, pm_message_t mesg);
> +extern void ata_host_suspend(struct ata_host *host, pm_message_t mesg);
>  extern void ata_host_resume(struct ata_host *host);
>  extern void ata_sas_port_suspend(struct ata_port *ap);
>  extern void ata_sas_port_resume(struct ata_port *ap);


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libata: make ata_host_suspend() *void*
  2022-02-02 20:58 [PATCH] libata: make ata_host_suspend() *void* Sergey Shtylyov
  2022-02-02 23:55 ` Damien Le Moal
@ 2022-02-03  6:44 ` Uwe Kleine-König
  2022-02-03  7:26 ` Hannes Reinecke
  2022-02-04  7:50 ` Damien Le Moal
  3 siblings, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2022-02-03  6:44 UTC (permalink / raw)
  To: Sergey Shtylyov
  Cc: Damien Le Moal, linux-ide, Jens Axboe, Shawn Guo, Sascha Hauer,
	Hans de Goede, NXP Linux Team, Pengutronix Kernel Team,
	Viresh Kumar, Fabio Estevam, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

On Wed, Feb 02, 2022 at 11:58:21PM +0300, Sergey Shtylyov wrote:
> ata_host_suspend() always returns 0, so the result checks in many drivers
> look pointless.  Let's make this function return *void* instead of *int*.
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libata: make ata_host_suspend() *void*
  2022-02-02 20:58 [PATCH] libata: make ata_host_suspend() *void* Sergey Shtylyov
  2022-02-02 23:55 ` Damien Le Moal
  2022-02-03  6:44 ` Uwe Kleine-König
@ 2022-02-03  7:26 ` Hannes Reinecke
  2022-02-04  7:50 ` Damien Le Moal
  3 siblings, 0 replies; 5+ messages in thread
From: Hannes Reinecke @ 2022-02-03  7:26 UTC (permalink / raw)
  To: Sergey Shtylyov, Damien Le Moal, linux-ide
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-arm-kernel, Hans de Goede, Jens Axboe,
	Viresh Kumar

On 2/2/22 21:58, Sergey Shtylyov wrote:
> ata_host_suspend() always returns 0, so the result checks in many drivers
> look pointless.  Let's make this function return *void* instead of *int*.
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> ---
> This patch is against the 'for-next' branch of Damien Le Moal's 'libata.git'
> repo.
> 
>   drivers/ata/ahci.c             |    3 ++-
>   drivers/ata/ata_piix.c         |    5 +----
>   drivers/ata/libahci_platform.c |    3 ++-
>   drivers/ata/libata-core.c      |    8 ++------
>   drivers/ata/pata_arasan_cf.c   |    3 ++-
>   drivers/ata/pata_cs5520.c      |    5 +----
>   drivers/ata/pata_imx.c         |   15 ++++++---------
>   drivers/ata/pata_macio.c       |    6 +-----
>   drivers/ata/pata_mpc52xx.c     |    3 ++-
>   drivers/ata/pata_samsung_cf.c  |    3 ++-
>   drivers/ata/pata_triflex.c     |    5 +----
>   drivers/ata/sata_fsl.c         |    4 +++-
>   drivers/ata/sata_highbank.c    |    3 ++-
>   drivers/ata/sata_mv.c          |    6 +++---
>   drivers/ata/sata_rcar.c        |   18 ++++++++----------
>   include/linux/libata.h         |    2 +-
>   16 files changed, 39 insertions(+), 53 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] libata: make ata_host_suspend() *void*
  2022-02-02 20:58 [PATCH] libata: make ata_host_suspend() *void* Sergey Shtylyov
                   ` (2 preceding siblings ...)
  2022-02-03  7:26 ` Hannes Reinecke
@ 2022-02-04  7:50 ` Damien Le Moal
  3 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2022-02-04  7:50 UTC (permalink / raw)
  To: Sergey Shtylyov, linux-ide
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-arm-kernel, Hans de Goede, Jens Axboe,
	Viresh Kumar

On 2/3/22 05:58, Sergey Shtylyov wrote:
> ata_host_suspend() always returns 0, so the result checks in many drivers
> look pointless.  Let's make this function return *void* instead of *int*.
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>

Applied to for-5.18. Thanks !

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-04  7:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-02 20:58 [PATCH] libata: make ata_host_suspend() *void* Sergey Shtylyov
2022-02-02 23:55 ` Damien Le Moal
2022-02-03  6:44 ` Uwe Kleine-König
2022-02-03  7:26 ` Hannes Reinecke
2022-02-04  7:50 ` Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox