[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Wolfgang Grandegger]

This patch adds support for NAND_MAX_CHIPS to the MTD NAND layer.
Multi-chip devices are then displayed as shown:

Device 0: 2x NAND 512MiB 3,3V 8-bit, sector size 128 KiB

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 common/cmd_nand.c       |   10 +++++++++-
 drivers/mtd/nand/nand.c |   10 +++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

Index: u-boot/common/cmd_nand.c
===================================================================
--- u-boot.orig/common/cmd_nand.c
+++ u-boot/common/cmd_nand.c
@@ -191,10 +191,18 @@ int do_nand(cmd_tbl_t * cmdtp, int flag,
 
 		putc('\n');
 		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
-			if (nand_info[i].name)
+			if (nand_info[i].name) {
+#if NAND_MAX_CHIPS > 1
+				struct nand_chip *chip = nand_info[i].priv;
+				printf("Device %d: %dx %s, sector size %u KiB\n",
+				       i, chip->numchips, nand_info[i].name,
+				       nand_info[i].erasesize >> 10);
+#else
 				printf("Device %d: %s, sector size %u KiB\n",
 				       i, nand_info[i].name,
 				       nand_info[i].erasesize >> 10);
+#endif
+			}
 		}
 		return 0;
 	}
Index: u-boot/drivers/mtd/nand/nand.c
===================================================================
--- u-boot.orig/drivers/mtd/nand/nand.c
+++ u-boot/drivers/mtd/nand/nand.c
@@ -28,6 +28,10 @@
 #define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE }
 #endif
 
+#ifndef NAND_MAX_CHIPS
+#define NAND_MAX_CHIPS 1
+#endif
+
 int nand_curr_device = -1;
 nand_info_t nand_info[CONFIG_SYS_MAX_NAND_DEVICE];
 
@@ -41,11 +45,15 @@ extern int board_nand_init(struct nand_c
 static void nand_init_chip(struct mtd_info *mtd, struct nand_chip *nand,
 			   ulong base_addr)
 {
+	int maxchips = NAND_MAX_CHIPS;
+
+	if (maxchips < 1)
+		maxchips = 1;
 	mtd->priv = nand;
 
 	nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
 	if (board_nand_init(nand) == 0) {
-		if (nand_scan(mtd, 1) == 0) {
+		if (nand_scan(mtd, maxchips) == 0) {
 			if (!mtd->name)
 				mtd->name = (char *)default_nand_name;
 		} else

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Scott Wood]

On Wed, Jan 07, 2009 at 07:53:45PM +0100, Wolfgang Grandegger wrote:
>  		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
> -			if (nand_info[i].name)
> +			if (nand_info[i].name) {
> +#if NAND_MAX_CHIPS > 1
> +				struct nand_chip *chip = nand_info[i].priv;
> +				printf("Device %d: %dx %s, sector size %u KiB\n",
> +				       i, chip->numchips, nand_info[i].name,
> +				       nand_info[i].erasesize >> 10);
> +#else
>  				printf("Device %d: %s, sector size %u KiB\n",
>  				       i, nand_info[i].name,
>  				       nand_info[i].erasesize >> 10);
> +#endif

Do we really need the ifdef?

> Index: u-boot/drivers/mtd/nand/nand.c
> ===================================================================
> --- u-boot.orig/drivers/mtd/nand/nand.c
> +++ u-boot/drivers/mtd/nand/nand.c
> @@ -28,6 +28,10 @@
>  #define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE }
>  #endif
>  
> +#ifndef NAND_MAX_CHIPS
> +#define NAND_MAX_CHIPS 1
> +#endif

This needs to be seen from cmd_nand.c as well.

-Scott

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Wolfgang Grandegger]

Scott Wood wrote:
> On Wed, Jan 07, 2009 at 07:53:45PM +0100, Wolfgang Grandegger wrote:
>>  		for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++) {
>> -			if (nand_info[i].name)
>> +			if (nand_info[i].name) {
>> +#if NAND_MAX_CHIPS > 1
>> +				struct nand_chip *chip = nand_info[i].priv;
>> +				printf("Device %d: %dx %s, sector size %u KiB\n",
>> +				       i, chip->numchips, nand_info[i].name,
>> +				       nand_info[i].erasesize >> 10);
>> +#else
>>  				printf("Device %d: %s, sector size %u KiB\n",
>>  				       i, nand_info[i].name,
>>  				       nand_info[i].erasesize >> 10);
>> +#endif
> 
> Do we really need the ifdef?

Not really, "1x" (one time) would make clear that it's a single-chip device.

>> Index: u-boot/drivers/mtd/nand/nand.c
>> ===================================================================
>> --- u-boot.orig/drivers/mtd/nand/nand.c
>> +++ u-boot/drivers/mtd/nand/nand.c
>> @@ -28,6 +28,10 @@
>>  #define CONFIG_SYS_NAND_BASE_LIST { CONFIG_SYS_NAND_BASE }
>>  #endif
>>  
>> +#ifndef NAND_MAX_CHIPS
>> +#define NAND_MAX_CHIPS 1
>> +#endif
> 
> This needs to be seen from cmd_nand.c as well.

Yep, in the meantime I realized that it's already pre-set in
include/linux/mtd/nand.h:

  /* The maximum number of NAND chips in an array */
  #ifndef NAND_MAX_CHIPS
  #define NAND_MAX_CHIPS          8
  #endif

but most (if not all) boards set it to 1.

But at that occasion, I think the name should be changed to
CONFIG_SYS_NAND_MAX_CHIPS, right?

Wolfgang.

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Wolfgang Denk]

Dear Wolfgang,

In message <49703E5B.1080604@grandegger.com> you wrote:
> 
> Yep, in the meantime I realized that it's already pre-set in
> include/linux/mtd/nand.h:
> 
>   /* The maximum number of NAND chips in an array */
>   #ifndef NAND_MAX_CHIPS
>   #define NAND_MAX_CHIPS          8
>   #endif
> 
> but most (if not all) boards set it to 1.

So we should change the default setting to 1, and change it only where
needed.

> But at that occasion, I think the name should be changed to
> CONFIG_SYS_NAND_MAX_CHIPS, right?

Right.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
We, the unwilling, led by the unknowing, are doing the impossible for
the ungrateful. We have done so much, for so long, with so little, we
are now qualified to do anything with nothing.

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Wolfgang Grandegger]

Wolfgang Denk wrote:
> Dear Wolfgang,
> 
> In message <49703E5B.1080604@grandegger.com> you wrote:
>> Yep, in the meantime I realized that it's already pre-set in
>> include/linux/mtd/nand.h:
>>
>>   /* The maximum number of NAND chips in an array */
>>   #ifndef NAND_MAX_CHIPS
>>   #define NAND_MAX_CHIPS          8
>>   #endif
>>
>> but most (if not all) boards set it to 1.
> 
> So we should change the default setting to 1, and change it only where
> needed.

Actually, none of the boards use multi-chip support (NAND_MAX_CHIPS >
1). The bamboo and the DU440 define

  #define NAND_MAX_CHIPS          CONFIG_SYS_MAX_NAND_DEVICE

but that's bogus and did not work yet anyhow.

>> But at that occasion, I think the name should be changed to
>> CONFIG_SYS_NAND_MAX_CHIPS, right?
> 
> Right.

OK, I will prepare a patch removing all NAND_MAX_CHIPS from the board
config files, rename if to CONFIG_SYS_NAND_MAX_CHIPS and define a
default of 1 for the legacy and the new MTD NAND layer.

Wolfgang.

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Stefan Roese]

Hi Wolfgang G.,

On Friday 16 January 2009, Wolfgang Grandegger wrote:
> > So we should change the default setting to 1, and change it only where
> > needed.
>
> Actually, none of the boards use multi-chip support (NAND_MAX_CHIPS >
> 1). The bamboo and the DU440 define
>
>   #define NAND_MAX_CHIPS          CONFIG_SYS_MAX_NAND_DEVICE
>
> but that's bogus and did not work yet anyhow.

Not sure what you mean by this. Bamboo has 2 NAND chips and U-Boot can access 
both chips.

Best regards,
Stefan

=====================================================================
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] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Scott Wood]

On Mon, Jan 19, 2009 at 05:12:32PM +0100, Stefan Roese wrote:
> Hi Wolfgang G.,
> 
> On Friday 16 January 2009, Wolfgang Grandegger wrote:
> > > So we should change the default setting to 1, and change it only where
> > > needed.
> >
> > Actually, none of the boards use multi-chip support (NAND_MAX_CHIPS >
> > 1). The bamboo and the DU440 define
> >
> >   #define NAND_MAX_CHIPS          CONFIG_SYS_MAX_NAND_DEVICE
> >
> > but that's bogus and did not work yet anyhow.
> 
> Not sure what you mean by this. Bamboo has 2 NAND chips and U-Boot can access 
> both chips.

Are they accessed as 2 NAND controllers, or 2 chips on one controller?

We can make it "#define NAND_MAX_CHIPS 2" if that's appropriate, but
defining it to CONFIG_SYS_MAX_NAND_DEVICE doesn't seem to make sense.

-Scott

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Stefan Roese]

On Monday 19 January 2009, Scott Wood wrote:
> > > Actually, none of the boards use multi-chip support (NAND_MAX_CHIPS >
> > > 1). The bamboo and the DU440 define
> > >
> > >   #define NAND_MAX_CHIPS          CONFIG_SYS_MAX_NAND_DEVICE
> > >
> > > but that's bogus and did not work yet anyhow.
> >
> > Not sure what you mean by this. Bamboo has 2 NAND chips and U-Boot can
> > access both chips.
>
> Are they accessed as 2 NAND controllers, or 2 chips on one controller?

2 chips on one controller.

> We can make it "#define NAND_MAX_CHIPS 2" if that's appropriate, but
> defining it to CONFIG_SYS_MAX_NAND_DEVICE doesn't seem to make sense.

OK with me.

Thanks.

Best regards,
Stefan

=====================================================================
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] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Wolfgang Grandegger]

Hi Stefan,

Stefan Roese wrote:
> On Monday 19 January 2009, Scott Wood wrote:
>>>> Actually, none of the boards use multi-chip support (NAND_MAX_CHIPS >
>>>> 1). The bamboo and the DU440 define
>>>>
>>>>   #define NAND_MAX_CHIPS          CONFIG_SYS_MAX_NAND_DEVICE
>>>>
>>>> but that's bogus and did not work yet anyhow.
>>> Not sure what you mean by this. Bamboo has 2 NAND chips and U-Boot can
>>> access both chips.
>> Are they accessed as 2 NAND controllers, or 2 chips on one controller?
> 
> 2 chips on one controller.
> 
>> We can make it "#define NAND_MAX_CHIPS 2" if that's appropriate, but
>> defining it to CONFIG_SYS_MAX_NAND_DEVICE doesn't seem to make sense.
> 
> OK with me.

NAND_MAX_CHIPS should be set to 1!

NAND_MAX_CHIPS defines the number of NAND chips (or dies) *per* NAND
device (or bank) meaning two or more identical NAND chips can be mapped
to one device. So far this was not supported, because nand_scan() was
always called with maxchips=1 in nand_init_chip:

$ cat drivers/mtd/nand/nand.c:
...
static void nand_init_chip(struct mtd_info *mtd, struct nand_chip *nand,
                           ulong base_addr)
{
        mtd->priv = nand;

        nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
        if (board_nand_init(nand) == 0) {
                if (nand_scan(mtd, 1) == 0) {
                    ^^^^^^^^^^^^^^^^^
                        if (!mtd->name)
                                mtd->name = (char *)default_nand_name;
                } else
                        mtd->name = NULL;
        } else {
                mtd->name = NULL;
                mtd->size = 0;
        }
}

Furthermore, the callback chip->select_chip() must be provided by the
NAND driver to select the chips properly. It's seems not to be
implemented for NDFC and therefore you do not have this choice.
Therefore NAND_MAX_CHIPS should be set to 1 for the Bamboo.

Wolfgang.

[U-Boot] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Stefan Roese]

Hi Wolfgang,

On Monday 19 January 2009, Wolfgang Grandegger wrote:
> >> Are they accessed as 2 NAND controllers, or 2 chips on one controller?
> >
> > 2 chips on one controller.
> >
> >> We can make it "#define NAND_MAX_CHIPS 2" if that's appropriate, but
> >> defining it to CONFIG_SYS_MAX_NAND_DEVICE doesn't seem to make sense.
> >
> > OK with me.
>
> NAND_MAX_CHIPS should be set to 1!
>
> NAND_MAX_CHIPS defines the number of NAND chips (or dies) *per* NAND
> device (or bank) meaning two or more identical NAND chips can be mapped
> to one device. So far this was not supported, because nand_scan() was
> always called with maxchips=1 in nand_init_chip:
>
> $ cat drivers/mtd/nand/nand.c:
> ...
> static void nand_init_chip(struct mtd_info *mtd, struct nand_chip *nand,
>                            ulong base_addr)
> {
>         mtd->priv = nand;
>
>         nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>         if (board_nand_init(nand) == 0) {
>                 if (nand_scan(mtd, 1) == 0) {
>                     ^^^^^^^^^^^^^^^^^
>                         if (!mtd->name)
>                                 mtd->name = (char *)default_nand_name;
>                 } else
>                         mtd->name = NULL;
>         } else {
>                 mtd->name = NULL;
>                 mtd->size = 0;
>         }
> }
>
> Furthermore, the callback chip->select_chip() must be provided by the
> NAND driver to select the chips properly. It's seems not to be
> implemented for NDFC and therefore you do not have this choice.
> Therefore NAND_MAX_CHIPS should be set to 1 for the Bamboo.

The 4xx NDFC driver uses the board_nand_select_device() function to select the 
NAND chip (other platforms do it this way as well). IIRC, this function was 
already introduced with old legacy NAND driver. Could be that it is 
deprecated now and should be replaced with the ->select_device() function.

Best regards,
Stefan

=====================================================================
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] [RFC/PATCH 1/4] Enable multi chip support in the NAND layer

[Wolfgang Grandegger]

Stefan Roese wrote:
> Hi Wolfgang,
> 
> On Monday 19 January 2009, Wolfgang Grandegger wrote:
>>>> Are they accessed as 2 NAND controllers, or 2 chips on one controller?
>>> 2 chips on one controller.
>>>
>>>> We can make it "#define NAND_MAX_CHIPS 2" if that's appropriate, but
>>>> defining it to CONFIG_SYS_MAX_NAND_DEVICE doesn't seem to make sense.
>>> OK with me.
>> NAND_MAX_CHIPS should be set to 1!
>>
>> NAND_MAX_CHIPS defines the number of NAND chips (or dies) *per* NAND
>> device (or bank) meaning two or more identical NAND chips can be mapped
>> to one device. So far this was not supported, because nand_scan() was
>> always called with maxchips=1 in nand_init_chip:
>>
>> $ cat drivers/mtd/nand/nand.c:
>> ...
>> static void nand_init_chip(struct mtd_info *mtd, struct nand_chip *nand,
>>                            ulong base_addr)
>> {
>>         mtd->priv = nand;
>>
>>         nand->IO_ADDR_R = nand->IO_ADDR_W = (void  __iomem *)base_addr;
>>         if (board_nand_init(nand) == 0) {
>>                 if (nand_scan(mtd, 1) == 0) {
>>                     ^^^^^^^^^^^^^^^^^
>>                         if (!mtd->name)
>>                                 mtd->name = (char *)default_nand_name;
>>                 } else
>>                         mtd->name = NULL;
>>         } else {
>>                 mtd->name = NULL;
>>                 mtd->size = 0;
>>         }
>> }
>>
>> Furthermore, the callback chip->select_chip() must be provided by the
>> NAND driver to select the chips properly. It's seems not to be
>> implemented for NDFC and therefore you do not have this choice.
>> Therefore NAND_MAX_CHIPS should be set to 1 for the Bamboo.
> 
> The 4xx NDFC driver uses the board_nand_select_device() function to select the 
> NAND chip (other platforms do it this way as well). IIRC, this function was 
> already introduced with old legacy NAND driver. Could be that it is 
> deprecated now and should be replaced with the ->select_device() function.

NAND device selection is a different issue. The NAND FLASHes are
presented to the user as separate devices. I don't think it's
deprecated, it's just an option, the only one so far.

Wolfgang.