From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <4E157561.6060600@compulab.co.il> Date: Thu, 07 Jul 2011 11:59:13 +0300 From: Igor Grinberg MIME-Version: 1.0 To: Lei Wen Subject: Re: [PATCH 3/4] MTD: pxa3xx_nand: enable multiple chip select support References: <1309319494-17951-1-git-send-email-leiwen@marvell.com> <1309771536-10597-4-git-send-email-leiwen@marvell.com> <4E1411BB.4010000@compulab.co.il> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Eric Miao , David Woodhouse , Artem Bityutskiy , Lei Wen , Yu Tang , Haojian Zhuang , Daniel Mack , 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 07/07/11 09:26, Lei Wen wrote: > Hi Igor && Daniel, > > On Wed, Jul 6, 2011 at 3:41 PM, Igor Grinberg wrote: >> On 07/04/11 12:25, Lei Wen wrote: >> >>> #ifdef CONFIG_PM >>> @@ -1203,8 +1259,12 @@ static int pxa3xx_nand_resume(struct platform_device *pdev) >>> { >>> struct pxa3xx_nand_info *info = platform_get_drvdata(pdev); >>> >>> - nand_writel(info, NDTR0CS0, info->host->ndtr0cs0); >>> - nand_writel(info, NDTR1CS0, info->host->ndtr1cs0); >>> + /* >>> + * Directly set the chip select to a invalid value, >>> + * then the driver would reset the timing according >>> + * to current chip select at the beginning of cmdfunc >>> + */ >>> + info->cs = 0xff; >> Thinking of this for the second (or third) time, >> If you have keep config enabled and have only one nand chip, >> this will brake the keep config... >> >> Daniel, >> >> have you tested the suspend/resume with this patch? >> (and keep_config on?) >> > Do you still have concern with this change? > If not, I would push the next round of patch set including merging > patch 3 and 4. Though I can't test it right now, but yes it looks like it breaks the keep_config after the resume (actually it disables the keep_config silently). I think you need here some kind of check if keep_config is enabled. keep_config is enabled means that only one chip select is used for NAND and you don't need to reset the timings. -- Regards, Igor. From mboxrd@z Thu Jan 1 00:00:00 1970 From: grinberg@compulab.co.il (Igor Grinberg) Date: Thu, 07 Jul 2011 11:59:13 +0300 Subject: [PATCH 3/4] MTD: pxa3xx_nand: enable multiple chip select support In-Reply-To: References: <1309319494-17951-1-git-send-email-leiwen@marvell.com> <1309771536-10597-4-git-send-email-leiwen@marvell.com> <4E1411BB.4010000@compulab.co.il> Message-ID: <4E157561.6060600@compulab.co.il> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 07/07/11 09:26, Lei Wen wrote: > Hi Igor && Daniel, > > On Wed, Jul 6, 2011 at 3:41 PM, Igor Grinberg wrote: >> On 07/04/11 12:25, Lei Wen wrote: >> >>> #ifdef CONFIG_PM >>> @@ -1203,8 +1259,12 @@ static int pxa3xx_nand_resume(struct platform_device *pdev) >>> { >>> struct pxa3xx_nand_info *info = platform_get_drvdata(pdev); >>> >>> - nand_writel(info, NDTR0CS0, info->host->ndtr0cs0); >>> - nand_writel(info, NDTR1CS0, info->host->ndtr1cs0); >>> + /* >>> + * Directly set the chip select to a invalid value, >>> + * then the driver would reset the timing according >>> + * to current chip select at the beginning of cmdfunc >>> + */ >>> + info->cs = 0xff; >> Thinking of this for the second (or third) time, >> If you have keep config enabled and have only one nand chip, >> this will brake the keep config... >> >> Daniel, >> >> have you tested the suspend/resume with this patch? >> (and keep_config on?) >> > Do you still have concern with this change? > If not, I would push the next round of patch set including merging > patch 3 and 4. Though I can't test it right now, but yes it looks like it breaks the keep_config after the resume (actually it disables the keep_config silently). I think you need here some kind of check if keep_config is enabled. keep_config is enabled means that only one chip select is used for NAND and you don't need to reset the timings. -- Regards, Igor.