| Message ID | 1500324959-7682-1-git-send-email-festevam@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Boris Brezillon |
| Headers | show |
Le Mon, 17 Jul 2017 17:55:59 -0300, Fabio Estevam <festevam@gmail.com> a écrit : > From: Fabio Estevam <fabio.estevam@nxp.com> > > clk_prepare_enable() may fail, so we should better check its return value > and propagate it in the case of error. > > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com> > --- > drivers/mtd/nand/vf610_nfc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c > index 744ab10..9e49672 100644 > --- a/drivers/mtd/nand/vf610_nfc.c > +++ b/drivers/mtd/nand/vf610_nfc.c > @@ -814,12 +814,16 @@ static int vf610_nfc_suspend(struct device *dev) > > static int vf610_nfc_resume(struct device *dev) > { > + int err; > + > struct mtd_info *mtd = dev_get_drvdata(dev); > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > > pinctrl_pm_select_default_state(dev); AFAICT pinctrl_pm_select_default_state() can also fail. What makes it different from clk_prepare_enable() failures? > > - clk_prepare_enable(nfc->clk); > + err = clk_prepare_enable(nfc->clk); > + if (err) > + return err; > I guess the idea was that clk_prepare_enabled already succeeded at probe time and there was not real reason for it to fail at resume time. But extra checks are never bad. BTW, I don't remember what happens if ->resume() returns an error. Does it prevent the whole system from resuming or just make this specific device unusable? > vf610_nfc_preinit_controller(nfc); > vf610_nfc_init_controller(nfc);
Le Mon, 17 Jul 2017 23:08:28 +0200, Boris Brezillon <boris.brezillon@free-electrons.com> a écrit : > Le Mon, 17 Jul 2017 17:55:59 -0300, > Fabio Estevam <festevam@gmail.com> a écrit : > > > From: Fabio Estevam <fabio.estevam@nxp.com> > > > > clk_prepare_enable() may fail, so we should better check its return value > > and propagate it in the case of error. > > > > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com> > > --- > > drivers/mtd/nand/vf610_nfc.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c > > index 744ab10..9e49672 100644 > > --- a/drivers/mtd/nand/vf610_nfc.c > > +++ b/drivers/mtd/nand/vf610_nfc.c > > @@ -814,12 +814,16 @@ static int vf610_nfc_suspend(struct device *dev) > > > > static int vf610_nfc_resume(struct device *dev) > > { > > + int err; > > + > > struct mtd_info *mtd = dev_get_drvdata(dev); > > struct vf610_nfc *nfc = mtd_to_nfc(mtd); > > > > pinctrl_pm_select_default_state(dev); > > AFAICT pinctrl_pm_select_default_state() can also fail. What makes it > different from clk_prepare_enable() failures? > > > > > - clk_prepare_enable(nfc->clk); > > + err = clk_prepare_enable(nfc->clk); > > + if (err) > > + return err; > > > > I guess the idea was that clk_prepare_enabled already succeeded at probe > time and there was not real reason for it to fail at resume time. But > extra checks are never bad. BTW, I don't remember what happens if > ->resume() returns an error. Does it prevent the whole system from > resuming or just make this specific device unusable? Just found the answer [1]: it's harmless. [1]http://elixir.free-electrons.com/linux/latest/source/include/linux/pm.h#L262
Hi Boris, On Mon, Jul 17, 2017 at 6:08 PM, Boris Brezillon <boris.brezillon@free-electrons.com> wrote: > AFAICT pinctrl_pm_select_default_state() can also fail. What makes it > different from clk_prepare_enable() failures? IMHO we could even remove pinctrl_pm_select_default_state() from this driver since pinctrl_pm_select_sleep_state() is never used. This is the only driver that uses pinctrl_pm_select_default_state() inside drivers/mtd/.
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c index 744ab10..9e49672 100644 --- a/drivers/mtd/nand/vf610_nfc.c +++ b/drivers/mtd/nand/vf610_nfc.c @@ -814,12 +814,16 @@ static int vf610_nfc_suspend(struct device *dev) static int vf610_nfc_resume(struct device *dev) { + int err; + struct mtd_info *mtd = dev_get_drvdata(dev); struct vf610_nfc *nfc = mtd_to_nfc(mtd); pinctrl_pm_select_default_state(dev); - clk_prepare_enable(nfc->clk); + err = clk_prepare_enable(nfc->clk); + if (err) + return err; vf610_nfc_preinit_controller(nfc); vf610_nfc_init_controller(nfc);