[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 = µn_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 = µn_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 = µn_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 = µn_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,
> - µn1GbX8,
> - µn1GbX16,
> - µn4GbX8,
> - µn4GbX16,
> - &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-arm-kernel
mailing list