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 |
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 --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 */