[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Gachet Daniel]

This short patch correct corrupted data while reading NAND flash modules on the KARO TX25 module.

The current version of the mxc_nand driver uses the symmetric mode to access the NAND flash, but the devices populated on the KARO TX25 only support an asymmetric mode (i.MX25: bit 8  within NAND_FLASH_CONFIG1 register cleared). 

Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>
Cc: Wolfgang Denk <wd@denx.de >

diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
--- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
+++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
@@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
 	uint16_t tmp;
 
 	tmp = readw(&host->regs->nfc_config1);
+#ifndef MACH_TYPE_TX25
 	tmp |= NFC_ONE_CYCLE;
+#endif
 	tmp |= NFC_4_8N_ECC;
 	writew(tmp, &host->regs->nfc_config1);
 	if (host->pagesize_2k)

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Marek Vasut]

Dear Gachet Daniel,

> This short patch correct corrupted data while reading NAND flash modules on
> the KARO TX25 module.
> 
> The current version of the mxc_nand driver uses the symmetric mode to
> access the NAND flash, but the devices populated on the KARO TX25 only
> support an asymmetric mode (i.MX25: bit 8  within NAND_FLASH_CONFIG1
> register cleared).
> 
> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>
> Cc: Wolfgang Denk <wd@denx.de >
> 
> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
>  	uint16_t tmp;
> 
>  	tmp = readw(&host->regs->nfc_config1);
> +#ifndef MACH_TYPE_TX25
>  	tmp |= NFC_ONE_CYCLE;
> +#endif
>  	tmp |= NFC_4_8N_ECC;
>  	writew(tmp, &host->regs->nfc_config1);
>  	if (host->pagesize_2k)
> 

Won't it be better to make this more generic rather then adding more machine-
specific hacks please?

Best regards,
Marek Vasut

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Gachet Daniel]

Dear Marek,

>
>Dear Gachet Daniel,
>
>> This short patch correct corrupted data while reading NAND flash 
>> modules on the KARO TX25 module.
>> 
>> The current version of the mxc_nand driver uses the symmetric mode to 
>> access the NAND flash, but the devices populated on the KARO TX25 only 
>> support an asymmetric mode (i.MX25: bit 8  within NAND_FLASH_CONFIG1 
>> register cleared).
>> 
>> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>>
>> Cc: Wolfgang Denk <wd@denx.de >>
>> 
>> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
>> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
>> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
>> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
>>  	uint16_t tmp;
>> 
>>  	tmp = readw(&host->>regs->>nfc_config1);
>> +#ifndef MACH_TYPE_TX25
>>  	tmp |= NFC_ONE_CYCLE;
>> +#endif
>>  	tmp |= NFC_4_8N_ECC;
>>  	writew(tmp, &host->>regs->>nfc_config1);
>>  	if (host->>pagesize_2k)
>> 
>
>Won't it be better to make this more generic rather then adding more machine- specific hacks please?
>
>Best regards,
>Marek Vasut

It's really a good question. 

This modification should probably work on other i.MX25 boards, because it relaxes the timing while accessing the NAND flash. But unfortunately I haven't other i.MX25 boards on which I can verify this modification.  Do you have other modules to validate this modification?

Cordially,

Daniel

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Marek Vasut]

Dear Gachet Daniel,

> Dear Marek,
> 
> >Dear Gachet Daniel,
> >
> >> This short patch correct corrupted data while reading NAND flash
> >> modules on the KARO TX25 module.
> >> 
> >> The current version of the mxc_nand driver uses the symmetric mode to
> >> access the NAND flash, but the devices populated on the KARO TX25 only
> >> support an asymmetric mode (i.MX25: bit 8  within NAND_FLASH_CONFIG1
> >> register cleared).
> >> 
> >> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>>
> >> Cc: Wolfgang Denk <wd@denx.de >>
> >> 
> >> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> >> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> >> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
> >> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
> >> 
> >>  	uint16_t tmp;
> >>  	
> >>  	tmp = readw(&host->>regs->>nfc_config1);
> >> 
> >> +#ifndef MACH_TYPE_TX25
> >> 
> >>  	tmp |= NFC_ONE_CYCLE;
> >> 
> >> +#endif
> >> 
> >>  	tmp |= NFC_4_8N_ECC;
> >>  	writew(tmp, &host->>regs->>nfc_config1);
> >>  	if (host->>pagesize_2k)
> >
> >Won't it be better to make this more generic rather then adding more
> >machine- specific hacks please?
> >
> >Best regards,
> >Marek Vasut
> 
> It's really a good question.
> 
> This modification should probably work on other i.MX25 boards, because it
> relaxes the timing while accessing the NAND flash. But unfortunately I
> haven't other i.MX25 boards on which I can verify this modification.  Do
> you have other modules to validate this modification?

Not yet, but if you have one, you can do it in a generic way and if another one 
pops up, we can simply use it :)

> Cordially,
> 
> Daniel

Best regards,
Marek Vasut

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[stefano babic]

Am 31/03/2012 09:31, schrieb Gachet Daniel:
> This short patch correct corrupted data while reading NAND flash modules on the KARO TX25 module.
> 
> The current version of the mxc_nand driver uses the symmetric mode to access the NAND flash, but the devices populated on the KARO TX25 only support an asymmetric mode (i.MX25: bit 8  within NAND_FLASH_CONFIG1 register cleared). 
> 
> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>
> Cc: Wolfgang Denk <wd@denx.de >
> 
> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
>  	uint16_t tmp;
>  
>  	tmp = readw(&host->regs->nfc_config1);
> +#ifndef MACH_TYPE_TX25
>  	tmp |= NFC_ONE_CYCLE;
> +#endif

NAK. We cleaned up all u-boot occurrencies of machine id, except the
machid passed to the kernel. And when the device tree will be effective,
also the machid will be obsolete.

This change seems not strictly related to the Karo's SOM, but it it a
configuration of the mxc_nand driver, such as CONFIG_NAND_MXC_V1_1. You
can add a CONFIG_NAND_MXC_* value (please add documentation, too) and
use it in the driver code.

Can I also ask you if the same setup is used in the Linux driver (this
SOM is not mainlined, I suppose you have a porting from the
manufacturer) ? As far as I can see, this setup is not used for the
Freescale's mx25pdk (the only MX25 board in mainline kernel).

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Gachet Daniel]

Hi Stefano and Marek,

-----Original Message-----
From: stefano babic [mailto:sbabic at denx.de] 
Sent: dimanche 1 avril 2012 12:33
To: Gachet Daniel
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

>Am 31/03/2012 09:31, schrieb Gachet Daniel:
>> This short patch correct corrupted data while reading NAND flash modules on the KARO TX25 module.
>> 
>> The current version of the mxc_nand driver uses the symmetric mode to access the NAND flash, but the devices 
>>populated on the KARO TX25 only support an asymmetric mode (i.MX25: bit 8  within NAND_FLASH_CONFIG1 register cleared). 
>> 
>> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>
>> Cc: Wolfgang Denk <wd@denx.de >
>> 
>> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
>> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
>> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
>> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
>>  	uint16_t tmp;
>>  
>>  	tmp = readw(&host->regs->nfc_config1);
>> +#ifndef MACH_TYPE_TX25
>>  	tmp |= NFC_ONE_CYCLE;
>> +#endif
>
>NAK. We cleaned up all u-boot occurrencies of machine id, except the machid passed to the kernel. And when the 
>device tree will be effective, also the machid will be obsolete.
>
>This change seems not strictly related to the Karo's SOM, but it it a configuration of the mxc_nand driver, such as 
>CONFIG_NAND_MXC_V1_1. You can add a CONFIG_NAND_MXC_* value (please add documentation, too) and use it in the driver code.
>
>Can I also ask you if the same setup is used in the Linux driver (this SOM is not mainlined, I suppose you have a porting from the
>manufacturer) ? As far as I can see, this setup is not used for the Freescale's mx25pdk (the only MX25 board in mainline kernel).
>
>Best regards,
>Stefano Babic
>
>--
>=====================================================================
>DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de 
>=====================================================================

I found the proposition of Stefano very interesting. My proposition will be to define a specific value for that bit, for instance
CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE. If this value is defined, the NFC_ONE_CYCLE bit will be set, otherwise not.
This value could be defined in the specific board configuration file (./include/configs/...). 

The patch will be the following:
diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
--- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
+++ drivers/mtd/nand/mxc_nand.c	2012-04-01 21:07:13.000000000 +0200
@@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
 	uint16_t tmp;
 
 	tmp = readw(&host->regs->nfc_config1);
+#ifdef CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE
 	tmp |= NFC_ONE_CYCLE;
+#endif
 	tmp |= NFC_4_8N_ECC;
 	writew(tmp, &host->regs->nfc_config1);
 	if (host->pagesize_2k)

Is that a valuable solution for you?

Cordially,

Daniel 

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Marek Vasut]

Dear Gachet Daniel,

> Hi Stefano and Marek,
> 
> -----Original Message-----
> From: stefano babic [mailto:sbabic at denx.de]
> Sent: dimanche 1 avril 2012 12:33
> To: Gachet Daniel
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO
> TX25 modules
> 
> >Am 31/03/2012 09:31, schrieb Gachet Daniel:
> >> This short patch correct corrupted data while reading NAND flash modules
> >> on the KARO TX25 module.
> >> 
> >> The current version of the mxc_nand driver uses the symmetric mode to
> >> access the NAND flash, but the devices
> >>
> >>populated on the KARO TX25 only support an asymmetric mode (i.MX25: bit 8
> >> within NAND_FLASH_CONFIG1 register cleared).
> >>
> >> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>
> >> Cc: Wolfgang Denk <wd@denx.de >
> >> 
> >> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> >> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> >> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
> >> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
> >> 
> >>  	uint16_t tmp;
> >>  	
> >>  	tmp = readw(&host->regs->nfc_config1);
> >> 
> >> +#ifndef MACH_TYPE_TX25
> >> 
> >>  	tmp |= NFC_ONE_CYCLE;
> >> 
> >> +#endif
> >
> >NAK. We cleaned up all u-boot occurrencies of machine id, except the
> >machid passed to the kernel. And when the device tree will be effective,
> >also the machid will be obsolete.
> >
> >This change seems not strictly related to the Karo's SOM, but it it a
> >configuration of the mxc_nand driver, such as CONFIG_NAND_MXC_V1_1. You
> >can add a CONFIG_NAND_MXC_* value (please add documentation, too) and use
> >it in the driver code.
> >
> >Can I also ask you if the same setup is used in the Linux driver (this SOM
> >is not mainlined, I suppose you have a porting from the manufacturer) ?
> >As far as I can see, this setup is not used for the Freescale's mx25pdk
> >(the only MX25 board in mainline kernel).
> >
> >Best regards,
> >Stefano Babic
> >
> >--
> >=====================================================================
> >DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> >HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> >Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> >=====================================================================
> 
> I found the proposition of Stefano very interesting. My proposition will be
> to define a specific value for that bit, for instance
> CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE. If this value is defined, the
> NFC_ONE_CYCLE bit will be set, otherwise not. This value could be defined
> in the specific board configuration file (./include/configs/...).
> 
> The patch will be the following:
> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> +++ drivers/mtd/nand/mxc_nand.c	2012-04-01 21:07:13.000000000 +0200
> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
>  	uint16_t tmp;
> 
>  	tmp = readw(&host->regs->nfc_config1);
> +#ifdef CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE
>  	tmp |= NFC_ONE_CYCLE;
> +#endif
>  	tmp |= NFC_4_8N_ECC;
>  	writew(tmp, &host->regs->nfc_config1);
>  	if (host->pagesize_2k)
> 
> Is that a valuable solution for you?

Ain't that basically what I said? Though, I might have not been clear enough ;-) 
Yes, that's it. Thanks!

> 
> Cordially,
> 
> Daniel

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[Marek Vasut]

Dear Gachet Daniel,

> Hi Marek,
> 
> Thanks for your answer. Have I to do more?

Did you submit the new patch into the ML? If so, did you CC sbabic at denx.de? If 
so, the patch will be picked by him and applied :)

Thanks for your patch!

> 
> Have a good day.
> 
> Cordially,
> 
> Daniel
> 
> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: dimanche 1 avril 2012 23:02
> To: u-boot at lists.denx.de
> Cc: Gachet Daniel
> Subject: Re: [U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO
> TX25 modules
> 
> Dear Gachet Daniel,
> 
> > Hi Stefano and Marek,
> > 
> > -----Original Message-----
> > From: stefano babic [mailto:sbabic at denx.de]
> > Sent: dimanche 1 avril 2012 12:33
> > To: Gachet Daniel
> > Cc: u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [PATCH] Correct corrupted NAND Flash access on
> > KARO
> > TX25 modules
> > 
> > >Am 31/03/2012 09:31, schrieb Gachet Daniel:
> > >> This short patch correct corrupted data while reading NAND flash
> > >> modules on the KARO TX25 module.
> > >> 
> > >> The current version of the mxc_nand driver uses the symmetric mode
> > >> to access the NAND flash, but the devices
> > >>
> > >>populated on the KARO TX25 only support an asymmetric mode (i.MX25:
> > >>bit 8  within NAND_FLASH_CONFIG1 register cleared).
> > >>
> > >> Signed-off-by: Daniel Gachet <daniel.gachet@hefr.ch>
> > >> Cc: Wolfgang Denk <wd@denx.de >
> > >> 
> > >> diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> > >> --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> > >> +++ drivers/mtd/nand/mxc_nand.c	2012-03-30 08:10:13.000000000 +0200
> > >> @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
> > >> 
> > >>  	uint16_t tmp;
> > >>  	
> > >>  	tmp = readw(&host->regs->nfc_config1);
> > >> 
> > >> +#ifndef MACH_TYPE_TX25
> > >> 
> > >>  	tmp |= NFC_ONE_CYCLE;
> > >> 
> > >> +#endif
> > >
> > >NAK. We cleaned up all u-boot occurrencies of machine id, except the
> > >machid passed to the kernel. And when the device tree will be
> > >effective, also the machid will be obsolete.
> > >
> > >This change seems not strictly related to the Karo's SOM, but it it a
> > >configuration of the mxc_nand driver, such as CONFIG_NAND_MXC_V1_1.
> > >You can add a CONFIG_NAND_MXC_* value (please add documentation, too)
> > >and use it in the driver code.
> > >
> > >Can I also ask you if the same setup is used in the Linux driver
> > >(this SOM is not mainlined, I suppose you have a porting from the
> > >manufacturer) ? As far as I can see, this setup is not used for the
> > >Freescale's mx25pdk (the only MX25 board in mainline kernel).
> > >
> > >Best regards,
> > >Stefano Babic
> > >
> > >--
> > >=====================================================================
> > >DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> > >HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > >Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> > >=====================================================================
> > 
> > I found the proposition of Stefano very interesting. My proposition
> > will be to define a specific value for that bit, for instance
> > CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE. If this value is defined, the
> > NFC_ONE_CYCLE bit will be set, otherwise not. This value could be
> > defined in the specific board configuration file (./include/configs/...).
> > 
> > The patch will be the following:
> > diff -upr drivers/mtd/nand/mxc_nand.c drivers/mtd/nand/mxc_nand.c
> > --- drivers/mtd/nand/mxc_nand.c	2011-12-23 20:25:35.000000000 +0100
> > +++ drivers/mtd/nand/mxc_nand.c	2012-04-01 21:07:13.000000000 +0200
> > @@ -1290,7 +1290,9 @@ static void mxc_setup_config1(void)
> > 
> >  	uint16_t tmp;
> >  	
> >  	tmp = readw(&host->regs->nfc_config1);
> > 
> > +#ifdef CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE
> > 
> >  	tmp |= NFC_ONE_CYCLE;
> > 
> > +#endif
> > 
> >  	tmp |= NFC_4_8N_ECC;
> >  	writew(tmp, &host->regs->nfc_config1);
> >  	if (host->pagesize_2k)
> > 
> > Is that a valuable solution for you?
> 
> Ain't that basically what I said? Though, I might have not been clear
> enough ;-) Yes, that's it. Thanks!
> 
> > Cordially,
> > 
> > Daniel

Best regards,
Marek Vasut

[U-Boot] [PATCH] Correct corrupted NAND Flash access on KARO TX25 modules

[stefano babic]

Am 02/04/2012 19:47, schrieb Gachet Daniel:
> Hi Stefano,
> 
> Have I to do more for the patch? Are you ok with the proposal?
> 

That is ok. Please use a shorter name: the function you patch is already
surrounded by #ifdef MXC_NFC_V1_1, so it is not required to repeat it in
the name (we have a lot of long names, but
CONFIG_NAND_MXC_V1_1_NFC_ONE_CYCLE is really quite long). The name
suggest also that you want to set ONE_CYCLE, but really you want the
opposite. Something like CONFIG_NAND_MXC_NO_ONE_CYLE is still long, but
maybe more correct.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================