On Sat, Aug 29, 2015 at 11:17:40AM +0200, Nicholas Mc Guire wrote: > On Fri, 28 Aug 2015, Han Xu wrote: > > > From: Huang Shijie <[email protected]> > > > > i.MX6SX supports deep sleep mode(DSM) that may turn off GPMI/BCH power > > during suspend, add gpmi nand suspend/resume function to release DMA > > channel in suspend function and re-init GPMI/BCH controller during > > resume function. > > > > Just a question regarding gpmi_pm_resume() errror reporting > Is there a reason that the retrun value of gpmi_extra_init() is not checked ? > gpmi_extra_init() can retrun -ENOMEM or -EINVAL via return enable_edo_mode(), > while the -EINVAL would produce a dev_err message, -ENOMEM does not so maybe > this also wants to include a if(ret) {...} ? - something like the below > > > Although it is not necessary to restore GPMI/BCH registers value for > > i.MX6QDL, the code doesn't distinguish different platforms to keep the > > code simple. > > > > Signed-off-by: Huang Shijie <[email protected]> > > Signed-off-by: Han Xu <[email protected]> > > --- > > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 45 > > +++++++++++++++++++++++++++++++++- > > 1 file changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > index 1b8f350..dfd0ba1 100644 > > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > > @@ -1,7 +1,7 @@ > > /* > > * Freescale GPMI NAND Flash Driver > > * > > - * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. > > + * Copyright (C) 2010-2015 Freescale Semiconductor, Inc. > > * Copyright (C) 2008 Embedded Alley Solutions, Inc. > > * > > * This program is free software; you can redistribute it and/or modify > > @@ -2036,9 +2036,52 @@ static int gpmi_nand_remove(struct platform_device > > *pdev) > > return 0; > > } > > > > +static int gpmi_pm_suspend(struct device *dev) > > +{ > > + struct gpmi_nand_data *this = dev_get_drvdata(dev); > > + > > + release_dma_channels(this); > > + return 0; > > +} > > + > > +static int gpmi_pm_resume(struct device *dev) > > +{ > > + struct gpmi_nand_data *this = dev_get_drvdata(dev); > > + int ret; > > + > > + ret = acquire_dma_channels(this); > > + if (ret < 0) > > + return ret; > > + > > + /* re-init the GPMI registers */ > > + this->flags &= ~GPMI_TIMING_INIT_OK; > > + ret = gpmi_init(this); > > + if (ret) { > > + dev_err(this->dev, "Error setting GPMI : %d\n", ret); > > + return ret; > > + } > > + > > + /* re-init the BCH registers */ > > + ret = bch_set_geometry(this); > > + if (ret) { > > + dev_err(this->dev, "Error setting BCH : %d\n", ret); > > + return ret; > > + } > > + > > + /* re-init others */ > > + gpmi_extra_init(this); > > report failure of enable_edo_mode() ? > > if (ret) { > dev_err(this->dev, "Error enabling EDO mode : %d\n", ret); > return ret; > } I think it's better that we do not report the failure of the enable_edo_mode() here. If we do so, the gpmi may cannot work when the enable_edo_mode() fails.
But if we do not report it, the gpmi still can work. thanks Huang Shijie -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
