diff mbox series

[v2] mtd: nand: gpmi: Release DMA channels on failure

Message ID 1514485078-19762-1-git-send-email-festevam@gmail.com
State Rejected
Delegated to: Boris Brezillon
Headers show
Series [v2] mtd: nand: gpmi: Release DMA channels on failure | expand

Commit Message

Fabio Estevam Dec. 28, 2017, 6:17 p.m. UTC 
From: Fabio Estevam <fabio.estevam@nxp.com>

release_dma_channels() should be called in the case of error in
gpmi_init() or bch_set_geometry(), because acquire_dma_channels() has
been called previously.

Handle the error cases correctly.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Remove extra blank line

 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Boris Brezillon Jan. 6, 2018, 10:03 a.m. UTC | #1 
On Thu, 28 Dec 2017 16:17:58 -0200
Fabio Estevam <festevam@gmail.com> wrote:

> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> release_dma_channels() should be called in the case of error in
> gpmi_init() or bch_set_geometry(), because acquire_dma_channels() has
> been called previously.

I'm really not sure this is a good idea. According to [1], the error
code returned by PM hooks is not taken into account, and the device
will be considered as functional by the system even if
->suspend() returns an error. This means, next time we enter suspend,
gpmi_pm_suspend() will be called which will lead to an unbalanced
release_dma_channels()/acquire_dma_channels().

> 
> Handle the error cases correctly.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Remove extra blank line
> 
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 2ef8979..db0d924 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -2137,20 +2137,24 @@ static int gpmi_pm_resume(struct device *dev)
>  	ret = gpmi_init(this);
>  	if (ret) {
>  		dev_err(this->dev, "Error setting GPMI : %d\n", ret);
> -		return ret;
> +		goto release_dma_channels;
>  	}
>  
>  	/* re-init the BCH registers */
>  	ret = bch_set_geometry(this);
>  	if (ret) {
>  		dev_err(this->dev, "Error setting BCH : %d\n", ret);
> -		return ret;
> +		goto release_dma_channels;
>  	}
>  
>  	/* re-init others */
>  	gpmi_extra_init(this);
>  
>  	return 0;
> +
> +release_dma_channels:
> +	release_dma_channels(this);
> +	return ret;
>  }
>  #endif /* CONFIG_PM_SLEEP */
>  

[1]https://elixir.free-electrons.com/linux/v4.15-rc6/source/include/linux/pm.h#L262
diff mbox series

Patch

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 2ef8979..db0d924 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -2137,20 +2137,24 @@  static int gpmi_pm_resume(struct device *dev)
 	ret = gpmi_init(this);
 	if (ret) {
 		dev_err(this->dev, "Error setting GPMI : %d\n", ret);
-		return ret;
+		goto release_dma_channels;
 	}
 
 	/* re-init the BCH registers */
 	ret = bch_set_geometry(this);
 	if (ret) {
 		dev_err(this->dev, "Error setting BCH : %d\n", ret);
-		return ret;
+		goto release_dma_channels;
 	}
 
 	/* re-init others */
 	gpmi_extra_init(this);
 
 	return 0;
+
+release_dma_channels:
+	release_dma_channels(this);
+	return ret;
 }
 #endif /* CONFIG_PM_SLEEP */