From mboxrd@z Thu Jan 1 00:00:00 1970 MIME-Version: 1.0 In-Reply-To: <4E1C6EB7.80506@compulab.co.il> References: <1310096084-14646-1-git-send-email-leiwen@marvell.com> <1310466535-15287-5-git-send-email-leiwen@marvell.com> <4E1C6EB7.80506@compulab.co.il> Date: Tue, 12 Jul 2011 19:35:30 +0200 Message-ID: Subject: Re: [PATCH V6 4/4] MTD: pxa3xx_nand: enhance suspend and resume routine From: Daniel Mack To: Igor Grinberg Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Eric Miao , David Woodhouse , Artem Bityutskiy , Lei Wen , Yu Tang , Haojian Zhuang , linux-mtd@lists.infradead.org, linux-arm-kernel List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tue, Jul 12, 2011 at 5:56 PM, Igor Grinberg wr= ote: > On 07/12/11 15:02, Daniel Mack wrote: > >> On Tue, Jul 12, 2011 at 1:39 PM, Daniel Mack wrote: >>> On Tue, Jul 12, 2011 at 12:28 PM, Lei Wen wrote: >>>> This patch add protection on the suspend&resume path to prevent >>>> some unexpected behavior, like interrupt occur at the very second >>>> of resume back and it don't follow normal command path, which lead >>>> to bug. >>>> >>>> Signed-off-by: Lei Wen >>>> --- >>>> =A0drivers/mtd/nand/pxa3xx_nand.c | =A0 28 +++++++++++++++++++++++++++= + >>>> =A01 files changed, 28 insertions(+), 0 deletions(-) >>>> >>> [...] >>> >>>> @@ -1267,6 +1283,18 @@ static int pxa3xx_nand_resume(struct platform_d= evice *pdev) >>>> =A0 =A0 =A0 =A0info->cs =3D 0xff; >>>> =A0 =A0 =A0 =A0clk_enable(info->clk); >>>> >>>> + =A0 =A0 =A0 /* >>>> + =A0 =A0 =A0 =A0* As the spec, the NDSR would be updated to 0x1800 wh= en >>>> + =A0 =A0 =A0 =A0* do the nand_clk disable/enable. >>>> + =A0 =A0 =A0 =A0* To prevent it damage state machine of the driver, c= lear >>>> + =A0 =A0 =A0 =A0* all status before resume >>>> + =A0 =A0 =A0 =A0*/ >>>> + =A0 =A0 =A0 nand_writel(nand, NDSR, NDSR_MASK); >>> This doesn't build: >>> >>> =A0CC =A0 =A0 =A0drivers/mtd/nand/pxa3xx_nand.o >>> drivers/mtd/nand/pxa3xx_nand.c: In function 'pxa3xx_nand_resume': >>> drivers/mtd/nand/pxa3xx_nand.c:1292: error: 'nand' undeclared (first >>> use in this function) >>> drivers/mtd/nand/pxa3xx_nand.c:1292: error: (Each undeclared >>> identifier is reported only once >>> drivers/mtd/nand/pxa3xx_nand.c:1292: error: for each function it appear= s in.) >>> drivers/mtd/nand/pxa3xx_nand.c:1294: error: 'mtd' undeclared (first >>> use in this function) >>> make[3]: *** [drivers/mtd/nand/pxa3xx_nand.o] Error 1 >>> >>> I guess this was not even compile tested? Anyway, I did a trivial >>> fix-up and will test. >> Also, with this (fixed) patch applied, the system doesn't resume at >> all. No messages, it simply doesn't come back. > > I was skeptic about the clock being disabled in Lei's patch, > as I observed system hangs if that clock was disabled back then in 2.6.31= , > but wanted to give it a try, because things has changed since then. > > Now I see, that Lei already sent v7 without clock toggling... Yes, we debugged this quickly via Jabber, and without the clock disable, things work fine for me again. Daniel From mboxrd@z Thu Jan 1 00:00:00 1970 From: zonque@gmail.com (Daniel Mack) Date: Tue, 12 Jul 2011 19:35:30 +0200 Subject: [PATCH V6 4/4] MTD: pxa3xx_nand: enhance suspend and resume routine In-Reply-To: <4E1C6EB7.80506@compulab.co.il> References: <1310096084-14646-1-git-send-email-leiwen@marvell.com> <1310466535-15287-5-git-send-email-leiwen@marvell.com> <4E1C6EB7.80506@compulab.co.il> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Jul 12, 2011 at 5:56 PM, Igor Grinberg wrote: > On 07/12/11 15:02, Daniel Mack wrote: > >> On Tue, Jul 12, 2011 at 1:39 PM, Daniel Mack wrote: >>> On Tue, Jul 12, 2011 at 12:28 PM, Lei Wen wrote: >>>> This patch add protection on the suspend&resume path to prevent >>>> some unexpected behavior, like interrupt occur at the very second >>>> of resume back and it don't follow normal command path, which lead >>>> to bug. >>>> >>>> Signed-off-by: Lei Wen >>>> --- >>>> ?drivers/mtd/nand/pxa3xx_nand.c | ? 28 ++++++++++++++++++++++++++++ >>>> ?1 files changed, 28 insertions(+), 0 deletions(-) >>>> >>> [...] >>> >>>> @@ -1267,6 +1283,18 @@ static int pxa3xx_nand_resume(struct platform_device *pdev) >>>> ? ? ? ?info->cs = 0xff; >>>> ? ? ? ?clk_enable(info->clk); >>>> >>>> + ? ? ? /* >>>> + ? ? ? ?* As the spec, the NDSR would be updated to 0x1800 when >>>> + ? ? ? ?* do the nand_clk disable/enable. >>>> + ? ? ? ?* To prevent it damage state machine of the driver, clear >>>> + ? ? ? ?* all status before resume >>>> + ? ? ? ?*/ >>>> + ? ? ? nand_writel(nand, NDSR, NDSR_MASK); >>> This doesn't build: >>> >>> ?CC ? ? ?drivers/mtd/nand/pxa3xx_nand.o >>> drivers/mtd/nand/pxa3xx_nand.c: In function 'pxa3xx_nand_resume': >>> drivers/mtd/nand/pxa3xx_nand.c:1292: error: 'nand' undeclared (first >>> use in this function) >>> drivers/mtd/nand/pxa3xx_nand.c:1292: error: (Each undeclared >>> identifier is reported only once >>> drivers/mtd/nand/pxa3xx_nand.c:1292: error: for each function it appears in.) >>> drivers/mtd/nand/pxa3xx_nand.c:1294: error: 'mtd' undeclared (first >>> use in this function) >>> make[3]: *** [drivers/mtd/nand/pxa3xx_nand.o] Error 1 >>> >>> I guess this was not even compile tested? Anyway, I did a trivial >>> fix-up and will test. >> Also, with this (fixed) patch applied, the system doesn't resume at >> all. No messages, it simply doesn't come back. > > I was skeptic about the clock being disabled in Lei's patch, > as I observed system hangs if that clock was disabled back then in 2.6.31, > but wanted to give it a try, because things has changed since then. > > Now I see, that Lei already sent v7 without clock toggling... Yes, we debugged this quickly via Jabber, and without the clock disable, things work fine for me again. Daniel