| Message ID | 1251912243-32546-1-git-send-email-hemantp@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Hemant,
I would suggest splitting the "bootloader" partition into two -- (i) ubl
and (ii) uboot. UBL can be 5 blocks (1 + 4 spare blocks) and u-boot
could be the rest of the blocks. One of the advantages of this approach
is that u-boot can be upgraded/reflashed easily from within Linux using
flash_eraseall and nandwrite. (Although, even with the current partition
scheme, u-boot can be written, but requires erasing individual blocks
and could be error prone).
Eg:
/* Size: 1 block for UBL + 4 backup blocks in case of bad block */
{
.name = "ubl",
.offset = MTDPART_OFS_APPEND,
.size = NAND_BLOCK_SIZE * 5,
.mask_flags = MTD_WRITEABLE,
},
{
.name = "uboot",
.offset = MTDPART_OFS_APPEND,
.size = NAND_BLOCK_SIZE * 5,
.mask_flags = 0,
},
regards,
-sid
Hemant Pedanekar wrote:
> This patch adds platform data and partition info for NAND on dm6467 EVM.
>
> Note that the partition layout is dependent on the UBL, U-Boot combination used.
> This patch uses partition organization suitable with latest U-Boot of LSP 1.3.
> For example, U-Boot environment goes in block 0, UBL resides in block form 1 to
> 5 and so on.
>
> Signed-off-by: Hemant Pedanekar <hemantp@ti.com>
> ---
> Depends on patch "[MTD] [NAND] davinci: fix to use mask_ale from pdata"
> submitted earlier. Without this patch "No NAND device found" error will be shown
> on dm6467-evm when booting with NAND enabled in config and NAND won't be
> accessible.
>
> arch/arm/mach-davinci/board-dm646x-evm.c | 75 ++++++++++++++++++++++++++++++
> 1 files changed, 75 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c
> index 434253e..d6b0539 100644
> --- a/arch/arm/mach-davinci/board-dm646x-evm.c
> +++ b/arch/arm/mach-davinci/board-dm646x-evm.c
> @@ -34,6 +34,10 @@
> #include <linux/i2c/pcf857x.h>
> #include <linux/etherdevice.h>
> #include <media/tvp514x.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/mtd/partitions.h>
> +
> #include <asm/setup.h>
> #include <asm/mach-types.h>
> #include <asm/mach/arch.h>
> @@ -45,6 +49,7 @@
> #include <mach/psc.h>
> #include <mach/serial.h>
> #include <mach/i2c.h>
> +#include <mach/nand.h>
> #include <mach/mmc.h>
> #include <mach/emac.h>
>
> @@ -55,6 +60,11 @@
> #define HAS_ATA 0
> #endif
>
> +#define DAVINCI_ASYNC_EMIF_CONTROL_BASE 0x20008000
> +#define DAVINCI_ASYNC_EMIF_DATA_CE0_BASE 0x42000000
> +
> +#define NAND_BLOCK_SIZE SZ_128K
> +
> /* CPLD Register 0 bits to control ATA */
> #define DM646X_EVM_ATA_RST BIT(0)
> #define DM646X_EVM_ATA_PWD BIT(1)
> @@ -91,6 +101,69 @@ static struct davinci_uart_config uart_config __initdata = {
> .enabled_uarts = (1 << 0),
> };
>
> +/* Note: The partitioning is driven by combination of UBL and U-Boot. For
> + * example, in the layout below, U-Boot puts environment in block 0
> + * and UBL can be in blocks 1-5 while U-Boot resides after UBL blocks.
> + */
> +static struct mtd_partition davinci_nand_partitions[] = {
> + {
> + /* U-Boot environment */
> + .name = "params",
> + .offset = 0,
> + .size = 1 * NAND_BLOCK_SIZE,
> + .mask_flags = 0,
> + }, {
> + /* UBL, U-Boot */
> + .name = "bootloader",
> + .offset = MTDPART_OFS_APPEND,
> + .size = 10 * NAND_BLOCK_SIZE,
> + .mask_flags = MTD_WRITEABLE, /* force read-only */
> + }, {
> + .name = "kernel",
> + .offset = MTDPART_OFS_APPEND,
> + .size = SZ_4M,
> + .mask_flags = 0,
> + }, {
> + .name = "filesystem",
> + .offset = MTDPART_OFS_APPEND,
> + .size = MTDPART_SIZ_FULL,
> + .mask_flags = 0,
> + }
> +};
> +
> +static struct davinci_nand_pdata davinci_nand_data = {
> + .mask_cle = 0x80000,
> + .mask_ale = 0x40000,
> + .parts = davinci_nand_partitions,
> + .nr_parts = ARRAY_SIZE(davinci_nand_partitions),
> + .ecc_mode = NAND_ECC_HW,
> + .options = 0,
> +};
> +
> +static struct resource davinci_nand_resources[] = {
> + {
> + .start = DAVINCI_ASYNC_EMIF_DATA_CE0_BASE,
> + .end = DAVINCI_ASYNC_EMIF_DATA_CE0_BASE + SZ_32M - 1,
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = DAVINCI_ASYNC_EMIF_CONTROL_BASE,
> + .end = DAVINCI_ASYNC_EMIF_CONTROL_BASE + SZ_4K - 1,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +
> +static struct platform_device davinci_nand_device = {
> + .name = "davinci_nand",
> + .id = 0,
> +
> + .num_resources = ARRAY_SIZE(davinci_nand_resources),
> + .resource = davinci_nand_resources,
> +
> + .dev = {
> + .platform_data = &davinci_nand_data,
> + },
> +};
> +
> /* CPLD Register 0 Client: used for I/O Control */
> static int cpld_reg0_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> @@ -645,6 +718,8 @@ static __init void evm_init(void)
> dm646x_init_mcasp0(&dm646x_evm_snd_data[0]);
> dm646x_init_mcasp1(&dm646x_evm_snd_data[1]);
>
> + platform_device_register(&davinci_nand_device);
> +
> if (HAS_ATA)
> dm646x_init_ide();
>
>
On Wednesday 02 September 2009, Siddharth Choudhuri wrote: > I would suggest splitting the "bootloader" partition into two -- (i) ubl > and (ii) uboot. UBL can be 5 blocks (1 + 4 spare blocks) and u-boot > could be the rest of the blocks. One of the advantages of this approach > is that u-boot can be upgraded/reflashed easily from within Linux  using > flash_eraseall and nandwrite. (Although, even with the current partition > scheme, u-boot can be written, but requires erasing individual blocks > and could be error prone). Heh. My suggestion is to go the other way: just a single "bootloader" partition of eight blocks or so. Don't expose any of the substructure at all. Updating U-Boot from Linux seems kind of nice, but on the other hand why not just do it from U-Boot? Either way you have to stick an extra header on the binary. And most folk don't use that demo-quality U-Boot tool to access the u-boot environment block. Thing is, if details of that boot layout aren't exposed to Linux, they can be improved without impacting Linux. Like adding backups for UBL or ABL/U-Boot; or for the U-Boot environment, for that matter. - Dave
> Heh. My suggestion is to go the other way: just a single > "bootloader" partition of eight blocks or so. Don't expose > any of the substructure at all. > > Updating U-Boot from Linux seems kind of nice, but on the > other hand why not just do it from U-Boot? Either way you > If you have a device on field for which a u-boot upgrade is required, doing it from Linux is easier. You could have a small app that can talk over the network, fetch an updated u-boot and flash it. > Thing is, if details of that boot layout aren't exposed to > Linux, they can be improved without impacting Linux. Like > adding backups for UBL or ABL/U-Boot; or for the U-Boot > environment, for that matter. > Makes sense. For our use case, having the u-boot partitions exposed to Linux has been advantageous. However, if this is not the common case, then having a single partition labeled has boot serves the purpose. -sid
On Thursday 03 September 2009, Siddharth Choudhuri wrote: > > > Heh. My suggestion is to go the other way: just a single > > "bootloader" partition of eight blocks or so. Don't expose > > any of the substructure at all. > > > > Updating U-Boot from Linux seems kind of nice, but on the > > other hand why not just do it from U-Boot? > > If you have a device on field for which a u-boot upgrade is required, > doing it from Linux is easier. You could have a small app that can talk > over the network, fetch an updated u-boot and flash it. Not with the standard in-kernel partition tables, which make the U-Boot and UBL areas read-only: can't write them from Linux. Plus there's also the annoying problem of needing to make sure the ECC bytes are laid out the way the RBL and/or UBL want them, vs the way Linux reads and writes them. > > Thing is, if details of that boot layout aren't exposed to > > Linux, they can be improved without impacting Linux. Like > > adding backups for UBL or ABL/U-Boot; or for the U-Boot > > environment, for that matter. > > > Makes sense. That "not needing to understand funky ECC/OOB layout rules" falls into the same category. :) > For our use case, having the u-boot partitions exposed to Linux has been > advantageous. However, if this is not the common case, then having a > single partition labeled has boot serves the purpose. > > -sid > >
David Brownell <david-b@pacbell.net> writes: > On Wednesday 02 September 2009, Siddharth Choudhuri wrote: >> I would suggest splitting the "bootloader" partition into two -- (i) ubl >> and (ii) uboot. UBL can be 5 blocks (1 + 4 spare blocks) and u-boot >> could be the rest of the blocks. One of the advantages of this approach >> is that u-boot can be upgraded/reflashed easily from within Linux  using >> flash_eraseall and nandwrite. (Although, even with the current partition >> scheme, u-boot can be written, but requires erasing individual blocks >> and could be error prone). > > Heh. My suggestion is to go the other way: just a single > "bootloader" partition of eight blocks or so. Don't expose > any of the substructure at all. I agree with Dave here. Hemant, can you respin with a single bootloader partition? Kevin
Sure. I will send v4 by Friday. Thanks. - Hemant > -----Original Message----- > From: Kevin Hilman [mailto:khilman@deeprootsystems.com] > Sent: Wednesday, September 16, 2009 8:39 PM > To: Pedanekar, Hemant > Cc: davinci-linux-open-source@linux.davincidsp.com; David Brownell > Subject: Re: [PATCH v3] davinci: dm646x-evm: Add platform data for NAND > > David Brownell <david-b@pacbell.net> writes: > > > On Wednesday 02 September 2009, Siddharth Choudhuri wrote: > >> I would suggest splitting the "bootloader" partition into two -- (i) > ubl > >> and (ii) uboot. UBL can be 5 blocks (1 + 4 spare blocks) and u-boot > >> could be the rest of the blocks. One of the advantages of this approach > >> is that u-boot can be upgraded/reflashed easily from within Linux > Â using > >> flash_eraseall and nandwrite. (Although, even with the current > partition > >> scheme, u-boot can be written, but requires erasing individual blocks > >> and could be error prone). > > > > Heh. My suggestion is to go the other way: just a single > > "bootloader" partition of eight blocks or so. Don't expose > > any of the substructure at all. > > I agree with Dave here. > > Hemant, can you respin with a single bootloader partition? > > Kevin
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c b/arch/arm/mach-davinci/board-dm646x-evm.c index 434253e..d6b0539 100644 --- a/arch/arm/mach-davinci/board-dm646x-evm.c +++ b/arch/arm/mach-davinci/board-dm646x-evm.c @@ -34,6 +34,10 @@ #include <linux/i2c/pcf857x.h> #include <linux/etherdevice.h> #include <media/tvp514x.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/partitions.h> + #include <asm/setup.h> #include <asm/mach-types.h> #include <asm/mach/arch.h> @@ -45,6 +49,7 @@ #include <mach/psc.h> #include <mach/serial.h> #include <mach/i2c.h> +#include <mach/nand.h> #include <mach/mmc.h> #include <mach/emac.h> @@ -55,6 +60,11 @@ #define HAS_ATA 0 #endif +#define DAVINCI_ASYNC_EMIF_CONTROL_BASE 0x20008000 +#define DAVINCI_ASYNC_EMIF_DATA_CE0_BASE 0x42000000 + +#define NAND_BLOCK_SIZE SZ_128K + /* CPLD Register 0 bits to control ATA */ #define DM646X_EVM_ATA_RST BIT(0) #define DM646X_EVM_ATA_PWD BIT(1) @@ -91,6 +101,69 @@ static struct davinci_uart_config uart_config __initdata = { .enabled_uarts = (1 << 0), }; +/* Note: The partitioning is driven by combination of UBL and U-Boot. For + * example, in the layout below, U-Boot puts environment in block 0 + * and UBL can be in blocks 1-5 while U-Boot resides after UBL blocks. + */ +static struct mtd_partition davinci_nand_partitions[] = { + { + /* U-Boot environment */ + .name = "params", + .offset = 0, + .size = 1 * NAND_BLOCK_SIZE, + .mask_flags = 0, + }, { + /* UBL, U-Boot */ + .name = "bootloader", + .offset = MTDPART_OFS_APPEND, + .size = 10 * NAND_BLOCK_SIZE, + .mask_flags = MTD_WRITEABLE, /* force read-only */ + }, { + .name = "kernel", + .offset = MTDPART_OFS_APPEND, + .size = SZ_4M, + .mask_flags = 0, + }, { + .name = "filesystem", + .offset = MTDPART_OFS_APPEND, + .size = MTDPART_SIZ_FULL, + .mask_flags = 0, + } +}; + +static struct davinci_nand_pdata davinci_nand_data = { + .mask_cle = 0x80000, + .mask_ale = 0x40000, + .parts = davinci_nand_partitions, + .nr_parts = ARRAY_SIZE(davinci_nand_partitions), + .ecc_mode = NAND_ECC_HW, + .options = 0, +}; + +static struct resource davinci_nand_resources[] = { + { + .start = DAVINCI_ASYNC_EMIF_DATA_CE0_BASE, + .end = DAVINCI_ASYNC_EMIF_DATA_CE0_BASE + SZ_32M - 1, + .flags = IORESOURCE_MEM, + }, { + .start = DAVINCI_ASYNC_EMIF_CONTROL_BASE, + .end = DAVINCI_ASYNC_EMIF_CONTROL_BASE + SZ_4K - 1, + .flags = IORESOURCE_MEM, + }, +}; + +static struct platform_device davinci_nand_device = { + .name = "davinci_nand", + .id = 0, + + .num_resources = ARRAY_SIZE(davinci_nand_resources), + .resource = davinci_nand_resources, + + .dev = { + .platform_data = &davinci_nand_data, + }, +}; + /* CPLD Register 0 Client: used for I/O Control */ static int cpld_reg0_probe(struct i2c_client *client, const struct i2c_device_id *id) @@ -645,6 +718,8 @@ static __init void evm_init(void) dm646x_init_mcasp0(&dm646x_evm_snd_data[0]); dm646x_init_mcasp1(&dm646x_evm_snd_data[1]); + platform_device_register(&davinci_nand_device); + if (HAS_ATA) dm646x_init_ide();
This patch adds platform data and partition info for NAND on dm6467 EVM. Note that the partition layout is dependent on the UBL, U-Boot combination used. This patch uses partition organization suitable with latest U-Boot of LSP 1.3. For example, U-Boot environment goes in block 0, UBL resides in block form 1 to 5 and so on. Signed-off-by: Hemant Pedanekar <hemantp@ti.com> --- Depends on patch "[MTD] [NAND] davinci: fix to use mask_ale from pdata" submitted earlier. Without this patch "No NAND device found" error will be shown on dm6467-evm when booting with NAND enabled in config and NAND won't be accessible. arch/arm/mach-davinci/board-dm646x-evm.c | 75 ++++++++++++++++++++++++++++++ 1 files changed, 75 insertions(+), 0 deletions(-)