| Message ID | 1444681992-4208-1-git-send-email-computersforpeace@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2e17497ccfdf0da39595eb98434fdbac93ad3f9b |
| Headers | show |
Robert: Can you test this on a platform with PM support, and make sure the NAND works fine after a resume? On 12 October 2015 at 17:33, Brian Norris <computersforpeace@gmail.com> wrote: > mtd_{suspend,resume}() get called from mtdcore in a class suspend/resume > callback. We don't need to call them again here. In practice, this would > actually work OK, as nand_base actually handles nesting OK -- it just > might print warnings. > > Untested, but there are few (no?) users of PM for this driver AFAIK. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > drivers/mtd/nand/pxa3xx_nand.c | 17 ----------------- > 1 file changed, 17 deletions(-) > > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c > index 232c7074624a..cce5a32eef60 100644 > --- a/drivers/mtd/nand/pxa3xx_nand.c > +++ b/drivers/mtd/nand/pxa3xx_nand.c > @@ -1892,32 +1892,19 @@ static int pxa3xx_nand_probe(struct platform_device *pdev) > static int pxa3xx_nand_suspend(struct platform_device *pdev, pm_message_t state) > { > struct pxa3xx_nand_info *info = platform_get_drvdata(pdev); > - struct pxa3xx_nand_platform_data *pdata; > - struct mtd_info *mtd; > - int cs; > > - pdata = dev_get_platdata(&pdev->dev); > if (info->state) { > dev_err(&pdev->dev, "driver busy, state = %d\n", info->state); > return -EAGAIN; > } > > - for (cs = 0; cs < pdata->num_cs; cs++) { > - mtd = info->host[cs]->mtd; > - mtd_suspend(mtd); > - } > - > return 0; > } > > static int pxa3xx_nand_resume(struct platform_device *pdev) > { > struct pxa3xx_nand_info *info = platform_get_drvdata(pdev); > - struct pxa3xx_nand_platform_data *pdata; > - struct mtd_info *mtd; > - int cs; > > - pdata = dev_get_platdata(&pdev->dev); > /* We don't want to handle interrupt without calling mtd routine */ > disable_int(info, NDCR_INT_MASK); > > @@ -1935,10 +1922,6 @@ static int pxa3xx_nand_resume(struct platform_device *pdev) > * all status before resume > */ > nand_writel(info, NDSR, NDSR_MASK); > - for (cs = 0; cs < pdata->num_cs; cs++) { > - mtd = info->host[cs]->mtd; > - mtd_resume(mtd); > - } > > return 0; > } > -- > 2.6.0.rc2.230.g3dd15c0 > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On 15 Oct 03:33 PM, Ezequiel Garcia wrote: > Robert: > > Can you test this on a platform with PM support, and make sure > the NAND works fine after a resume? > > On 12 October 2015 at 17:33, Brian Norris <computersforpeace@gmail.com> wrote: > > mtd_{suspend,resume}() get called from mtdcore in a class suspend/resume > > callback. We don't need to call them again here. In practice, this would > > actually work OK, as nand_base actually handles nesting OK -- it just > > might print warnings. > > > > Untested, but there are few (no?) users of PM for this driver AFAIK. > > Brian, We discussed this on IRC, but I completely forgot about it :/ Without this patch nand_{suspend, resume} is called twice (which seems harmless), tested on Armada XP-GP using nandtest. Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
On Fri, Oct 16, 2015 at 12:26:54PM -0300, Ezequiel Garcia wrote: > On 15 Oct 03:33 PM, Ezequiel Garcia wrote: > > Robert: > > > > Can you test this on a platform with PM support, and make sure > > the NAND works fine after a resume? > > > > On 12 October 2015 at 17:33, Brian Norris <computersforpeace@gmail.com> wrote: > > > mtd_{suspend,resume}() get called from mtdcore in a class suspend/resume > > > callback. We don't need to call them again here. In practice, this would > > > actually work OK, as nand_base actually handles nesting OK -- it just > > > might print warnings. > > > > > > Untested, but there are few (no?) users of PM for this driver AFAIK. > > > > > Brian, > > We discussed this on IRC, but I completely forgot about it :/ No problem. > Without this patch nand_{suspend, resume} is called twice > (which seems harmless), tested on Armada XP-GP using nandtest. Right, I think it's incidentally harmless. But you do get this warning without the patch set, right? nand_resume called for a chip which is not in suspended state > Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Pushed to l2-mtd.git
On 16 October 2015 at 13:37, Brian Norris <computersforpeace@gmail.com> wrote: > On Fri, Oct 16, 2015 at 12:26:54PM -0300, Ezequiel Garcia wrote: >> On 15 Oct 03:33 PM, Ezequiel Garcia wrote: >> > Robert: >> > >> > Can you test this on a platform with PM support, and make sure >> > the NAND works fine after a resume? >> > >> > On 12 October 2015 at 17:33, Brian Norris <computersforpeace@gmail.com> wrote: >> > > mtd_{suspend,resume}() get called from mtdcore in a class suspend/resume >> > > callback. We don't need to call them again here. In practice, this would >> > > actually work OK, as nand_base actually handles nesting OK -- it just >> > > might print warnings. >> > > >> > > Untested, but there are few (no?) users of PM for this driver AFAIK. >> > > >> >> Brian, >> >> We discussed this on IRC, but I completely forgot about it :/ > > No problem. > >> Without this patch nand_{suspend, resume} is called twice >> (which seems harmless), tested on Armada XP-GP using nandtest. > > Right, I think it's incidentally harmless. But you do get this warning > without the patch set, right? > > nand_resume called for a chip which is not in suspended state > Yup, I get that warning. >> Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >> Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > Pushed to l2-mtd.git
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> writes: > Robert: > > Can you test this on a platform with PM support, and make sure > the NAND works fine after a resume? Euh actually I can't. I'm only sure Suspend to RAM is working for pxa27x, I have not yet this proof for pxa3xx, and my last try wasn't very successful 3 monthes ago ... As this regression might be within my platform's configuration (IPL setup, ...), I don't yet know where the bug is. Cheers. -- Robert
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 232c7074624a..cce5a32eef60 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -1892,32 +1892,19 @@ static int pxa3xx_nand_probe(struct platform_device *pdev) static int pxa3xx_nand_suspend(struct platform_device *pdev, pm_message_t state) { struct pxa3xx_nand_info *info = platform_get_drvdata(pdev); - struct pxa3xx_nand_platform_data *pdata; - struct mtd_info *mtd; - int cs; - pdata = dev_get_platdata(&pdev->dev); if (info->state) { dev_err(&pdev->dev, "driver busy, state = %d\n", info->state); return -EAGAIN; } - for (cs = 0; cs < pdata->num_cs; cs++) { - mtd = info->host[cs]->mtd; - mtd_suspend(mtd); - } - return 0; } static int pxa3xx_nand_resume(struct platform_device *pdev) { struct pxa3xx_nand_info *info = platform_get_drvdata(pdev); - struct pxa3xx_nand_platform_data *pdata; - struct mtd_info *mtd; - int cs; - pdata = dev_get_platdata(&pdev->dev); /* We don't want to handle interrupt without calling mtd routine */ disable_int(info, NDCR_INT_MASK); @@ -1935,10 +1922,6 @@ static int pxa3xx_nand_resume(struct platform_device *pdev) * all status before resume */ nand_writel(info, NDSR, NDSR_MASK); - for (cs = 0; cs < pdata->num_cs; cs++) { - mtd = info->host[cs]->mtd; - mtd_resume(mtd); - } return 0; }
mtd_{suspend,resume}() get called from mtdcore in a class suspend/resume callback. We don't need to call them again here. In practice, this would actually work OK, as nand_base actually handles nesting OK -- it just might print warnings. Untested, but there are few (no?) users of PM for this driver AFAIK. Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- drivers/mtd/nand/pxa3xx_nand.c | 17 ----------------- 1 file changed, 17 deletions(-)