[PATCH 01/29] pxa3xx_nand: condense the flash definition

Eric Miao eric.y.miao at gmail.com
Wed Jul 28 02:17:02 EDT 2010


On Wed, Jul 28, 2010 at 1:55 PM, Haojian Zhuang
<haojian.zhuang at gmail.com> wrote:
> From 6a43fc8bcad1e8f220696ef84be05b6daee18db9 Mon Sep 17 00:00:00 2001
> From: Lei Wen <leiwen at marvell.com>
> Date: Sat, 20 Mar 2010 19:01:23 +0800
> Subject: [PATCH 01/29] pxa3xx_nand: condense the flash definition
>
> Adding a new flash definition would need less code.
> Keep the platform passing flash definition method.
> If one flash is both defined in platform data and builtin table,
> driver would select the one from platform data first.
>
> By this way, platform could select the timing most suit for itself,
> not need to follow the common settings.
>
> Signed-off-by: Lei Wen <leiwen at marvell.com>
> Signed-off-by: Haojian Zhuang <haojian.zhuang at marvell.com>
> ---
>  arch/arm/plat-pxa/include/plat/pxa3xx_nand.h |   17 ++--
>  drivers/mtd/nand/Kconfig                     |    7 -
>  drivers/mtd/nand/pxa3xx_nand.c               |  175 +++-----------------------
>  3 files changed, 27 insertions(+), 172 deletions(-)
>
> diff --git a/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
> b/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
> index 3478eae..0bf1bcf 100644
> --- a/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
> +++ b/arch/arm/plat-pxa/include/plat/pxa3xx_nand.h
> @@ -30,15 +30,14 @@ struct pxa3xx_nand_cmdset {
>  };
>
>  struct pxa3xx_nand_flash {
> -       const struct pxa3xx_nand_timing *timing; /* NAND Flash timing */
> -       const struct pxa3xx_nand_cmdset *cmdset;
> -
> -       uint32_t page_per_block;/* Pages per block (PG_PER_BLK) */
> -       uint32_t page_size;     /* Page size in bytes (PAGE_SZ) */
> -       uint32_t flash_width;   /* Width of Flash memory (DWIDTH_M) */
> -       uint32_t dfc_width;     /* Width of flash controller(DWIDTH_C) */
> -       uint32_t num_blocks;    /* Number of physical blocks in Flash */
> -       uint32_t chip_id;
> +       uint32_t        chip_id;
> +       uint16_t        page_per_block; /* Pages per block (PG_PER_BLK) */
> +       uint16_t        page_size;      /* Page size in bytes (PAGE_SZ) */
> +       uint8_t         flash_width;    /* Width of Flash memory (DWIDTH_M) */
> +       uint8_t         dfc_width;      /* Width of flash controller(DWIDTH_C) */
> +       uint32_t        num_blocks;     /* Number of physical blocks in Flash */

When size doesn't matter, use 'unsigned int' instead. And btw, changing
uint32_t to uint16_t doesn't necessarily save you two bytes as the compiler
may impose ABI alignment requirement here.

As in the above case, except for chip_id, other fields can just be modified
to use 'unsigned int'.

> +       struct pxa3xx_nand_cmdset *cmdset;      /* NAND command set */
> +       struct pxa3xx_nand_timing *timing;      /* NAND Flash timing */
>  };
>
>  struct pxa3xx_nand_platform_data {
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index ffc3720..cd30a5c 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -400,13 +400,6 @@ config MTD_NAND_PXA3xx
>          This enables the driver for the NAND flash device found on
>          PXA3xx processors
>
> -config MTD_NAND_PXA3xx_BUILTIN
> -       bool "Use builtin definitions for some NAND chips (deprecated)"
> -       depends on MTD_NAND_PXA3xx
> -       help
> -         This enables builtin definitions for some NAND chips. This
> -         is deprecated in favor of platform specific data.
> -
>  config MTD_NAND_CM_X270
>        tristate "Support for NAND Flash on CM-X270 modules"
>        depends on MTD_NAND && MACH_ARMCORE
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index e02fa4f..3ffcbcd 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -176,21 +176,7 @@ MODULE_PARM_DESC(use_dma, "enable DMA for data
> transfering to/from NAND HW");
>  */
>  static struct pxa3xx_nand_timing default_timing;
>  static struct pxa3xx_nand_flash default_flash;
> -
> -static struct pxa3xx_nand_cmdset smallpage_cmdset = {
> -       .read1          = 0x0000,
> -       .read2          = 0x0050,
> -       .program        = 0x1080,
> -       .read_status    = 0x0070,
> -       .read_id        = 0x0090,
> -       .erase          = 0xD060,
> -       .reset          = 0x00FF,
> -       .lock           = 0x002A,
> -       .unlock         = 0x2423,
> -       .lock_status    = 0x007A,
> -};
> -
> -static struct pxa3xx_nand_cmdset largepage_cmdset = {
> +static struct pxa3xx_nand_cmdset default_cmdset = {
>        .read1          = 0x3000,
>        .read2          = 0x0050,
>        .program        = 0x1080,
> @@ -203,143 +189,26 @@ static struct pxa3xx_nand_cmdset largepage_cmdset = {
>        .lock_status    = 0x007A,
>  };
>
> -#ifdef CONFIG_MTD_NAND_PXA3xx_BUILTIN
> -static struct pxa3xx_nand_timing samsung512MbX16_timing = {
> -       .tCH    = 10,
> -       .tCS    = 0,
> -       .tWH    = 20,
> -       .tWP    = 40,
> -       .tRH    = 30,
> -       .tRP    = 40,
> -       .tR     = 11123,
> -       .tWHR   = 110,
> -       .tAR    = 10,
> -};
> -
> -static struct pxa3xx_nand_flash samsung512MbX16 = {
> -       .timing         = &samsung512MbX16_timing,
> -       .cmdset         = &smallpage_cmdset,
> -       .page_per_block = 32,
> -       .page_size      = 512,
> -       .flash_width    = 16,
> -       .dfc_width      = 16,
> -       .num_blocks     = 4096,
> -       .chip_id        = 0x46ec,
> -};
> -
> -static struct pxa3xx_nand_flash samsung2GbX8 = {
> -       .timing         = &samsung512MbX16_timing,
> -       .cmdset         = &smallpage_cmdset,
> -       .page_per_block = 64,
> -       .page_size      = 2048,
> -       .flash_width    = 8,
> -       .dfc_width      = 8,
> -       .num_blocks     = 2048,
> -       .chip_id        = 0xdaec,
> -};
> -
> -static struct pxa3xx_nand_flash samsung32GbX8 = {
> -       .timing         = &samsung512MbX16_timing,
> -       .cmdset         = &smallpage_cmdset,
> -       .page_per_block = 128,
> -       .page_size      = 4096,
> -       .flash_width    = 8,
> -       .dfc_width      = 8,
> -       .num_blocks     = 8192,
> -       .chip_id        = 0xd7ec,
> +static struct pxa3xx_nand_timing __devinitdata timing[] = {
> +       { 10,  0, 20, 40, 30, 40, 11123, 110, 10, },
> +       { 10, 25, 15, 25, 15, 30, 25000,  60, 10, },
> +       { 10, 35, 15, 25, 15, 25, 25000,  60, 10, },
>  };

I like this.

>
> -static struct pxa3xx_nand_timing micron_timing = {
> -       .tCH    = 10,
> -       .tCS    = 25,
> -       .tWH    = 15,
> -       .tWP    = 25,
> -       .tRH    = 15,
> -       .tRP    = 30,
> -       .tR     = 25000,
> -       .tWHR   = 60,
> -       .tAR    = 10,
> +#define NAND_SETTING_SAMSUNG     &default_cmdset, &timing[0]
> +#define NAND_SETTING_MICRON      &default_cmdset, &timing[1]
> +#define NAND_SETTING_ST          &default_cmdset, &timing[2]
> +static struct pxa3xx_nand_flash __devinitdata builtin_flash_types[] = {
> +       { 0x46ec,  32,  512, 16, 16, 4096, NAND_SETTING_SAMSUNG, },
> +       { 0xdaec,  64, 2048,  8,  8, 2048, NAND_SETTING_SAMSUNG, },
> +       { 0xd7ec, 128, 4096,  8,  8, 8192, NAND_SETTING_SAMSUNG, },
> +       { 0xa12c,  64, 2048,  8,  8, 1024, NAND_SETTING_MICRON, },
> +       { 0xb12c,  64, 2048, 16, 16, 1024, NAND_SETTING_MICRON, },
> +       { 0xdc2c,  64, 2048,  8,  8, 4096, NAND_SETTING_MICRON, },
> +       { 0xcc2c,  64, 2048, 16, 16, 4096, NAND_SETTING_MICRON, },
> +       { 0xba20,  64, 2048, 16, 16, 2048, NAND_SETTING_ST, },

I see no point to introduce NAND_SETTING_* here. BTW, where's the
command set for small page NAND?

>  };
>
> -static struct pxa3xx_nand_flash micron1GbX8 = {
> -       .timing         = &micron_timing,
> -       .cmdset         = &largepage_cmdset,
> -       .page_per_block = 64,
> -       .page_size      = 2048,
> -       .flash_width    = 8,
> -       .dfc_width      = 8,
> -       .num_blocks     = 1024,
> -       .chip_id        = 0xa12c,
> -};
> -
> -static struct pxa3xx_nand_flash micron1GbX16 = {
> -       .timing         = &micron_timing,
> -       .cmdset         = &largepage_cmdset,
> -       .page_per_block = 64,
> -       .page_size      = 2048,
> -       .flash_width    = 16,
> -       .dfc_width      = 16,
> -       .num_blocks     = 1024,
> -       .chip_id        = 0xb12c,
> -};
> -
> -static struct pxa3xx_nand_flash micron4GbX8 = {
> -       .timing         = &micron_timing,
> -       .cmdset         = &largepage_cmdset,
> -       .page_per_block = 64,
> -       .page_size      = 2048,
> -       .flash_width    = 8,
> -       .dfc_width      = 8,
> -       .num_blocks     = 4096,
> -       .chip_id        = 0xdc2c,
> -};
> -
> -static struct pxa3xx_nand_flash micron4GbX16 = {
> -       .timing         = &micron_timing,
> -       .cmdset         = &largepage_cmdset,
> -       .page_per_block = 64,
> -       .page_size      = 2048,
> -       .flash_width    = 16,
> -       .dfc_width      = 16,
> -       .num_blocks     = 4096,
> -       .chip_id        = 0xcc2c,
> -};
> -
> -static struct pxa3xx_nand_timing stm2GbX16_timing = {
> -       .tCH = 10,
> -       .tCS = 35,
> -       .tWH = 15,
> -       .tWP = 25,
> -       .tRH = 15,
> -       .tRP = 25,
> -       .tR = 25000,
> -       .tWHR = 60,
> -       .tAR = 10,
> -};
> -
> -static struct pxa3xx_nand_flash stm2GbX16 = {
> -       .timing = &stm2GbX16_timing,
> -       .cmdset = &largepage_cmdset,
> -       .page_per_block = 64,
> -       .page_size = 2048,
> -       .flash_width = 16,
> -       .dfc_width = 16,
> -       .num_blocks = 2048,
> -       .chip_id = 0xba20,
> -};
> -
> -static struct pxa3xx_nand_flash *builtin_flash_types[] = {
> -       &samsung512MbX16,
> -       &samsung2GbX8,
> -       &samsung32GbX8,
> -       &micron1GbX8,
> -       &micron1GbX16,
> -       &micron4GbX8,
> -       &micron4GbX16,
> -       &stm2GbX16,
> -};
> -#endif /* CONFIG_MTD_NAND_PXA3xx_BUILTIN */
> -
>  #define NDTR0_tCH(c)   (min((c), 7) << 19)
>  #define NDTR0_tCS(c)   (min((c), 7) << 16)
>  #define NDTR0_tWH(c)   (min((c), 7) << 11)
> @@ -1027,11 +896,6 @@ static int pxa3xx_nand_detect_config(struct
> pxa3xx_nand_info *info)
>        default_flash.flash_width = ndcr & NDCR_DWIDTH_M ? 16 : 8;
>        default_flash.dfc_width = ndcr & NDCR_DWIDTH_C ? 16 : 8;
>
> -       if (default_flash.page_size == 2048)
> -               default_flash.cmdset = &largepage_cmdset;
> -       else
> -               default_flash.cmdset = &smallpage_cmdset;
> -
>        /* set info fields needed to __readid */
>        info->flash_info = &default_flash;
>        info->read_id_bytes = (default_flash.page_size == 2048) ? 4 : 2;
> @@ -1068,6 +932,7 @@ static int pxa3xx_nand_detect_config(struct
> pxa3xx_nand_info *info)
>
>        pxa3xx_nand_detect_timing(info, &default_timing);
>        default_flash.timing = &default_timing;
> +       default_flash.cmdset = &default_cmdset;

So this default_cmdset works both for large page and small page NAND?
I belive it's true when probing, but really not sure when reading pages, as
small page NAND requires two reads for a 512byte page with different
commands?

>
>        return 0;
>  }
> @@ -1096,10 +961,9 @@ static int pxa3xx_nand_detect_flash(struct
> pxa3xx_nand_info *info,
>                        return 0;
>        }
>
> -#ifdef CONFIG_MTD_NAND_PXA3xx_BUILTIN
>        for (i = 0; i < ARRAY_SIZE(builtin_flash_types); i++) {
>
> -               f = builtin_flash_types[i];
> +               f = &builtin_flash_types[i];
>
>                if (pxa3xx_nand_config_flash(info, f))
>                        continue;
> @@ -1110,7 +974,6 @@ static int pxa3xx_nand_detect_flash(struct
> pxa3xx_nand_info *info,
>                if (id == f->chip_id)
>                        return 0;
>        }
> -#endif
>
>        dev_warn(&info->pdev->dev,
>                 "failed to detect configured nand flash; found %04x instead of\n",
> --
> 1.7.0.4
>

Otherwise looks good to me.



More information about the linux-mtd mailing list