Re: [PATCH 1/2] Update ts72xx to use generic platform nand driver

Alexander Clouter
2009-10-07T09:15:10Z

Hi,

In gmane.linux.drivers.mtd H Hartley Sweeten <hartleys@visionengravers.com> wrote:
>
> Update the ts72xx platform's nand driver support.
>
> This changes the ts72xx platform from using a custom nand driver
> (ts7250.c) to the generic platform nand driver (plat_nand.c).
>
> Tested on TS-7250 with 32MB NAND.
>
> [snipped]
>
> +static const char *ts72xx_nand_part_probes[] = { "cmdlinepart", NULL };
> +
> +#define TS72XX_BOOTROM_PART_SIZE (SZ_16K)
> +#define TS72XX_REDBOOT_PART_SIZE (SZ_2M + SZ_1M)
> +
> +static struct mtd_partition ts72xx_part_info32[] = {
> {
> - .virtual = TS72XX_NAND_DATA_VIRT_BASE,
> - .pfn = __phys_to_pfn(TS72XX_NAND1_DATA_PHYS_BASE),
> - .length = TS72XX_NAND_DATA_SIZE,
> - .type = MT_DEVICE,
> + .name = "TS-BOOTROM",
> + .offset = 0,
> + .size = TS72XX_BOOTROM_PART_SIZE,
> }, {
> - .virtual = TS72XX_NAND_CONTROL_VIRT_BASE,
> - .pfn = __phys_to_pfn(TS72XX_NAND1_CONTROL_PHYS_BASE),
> - .length = TS72XX_NAND_CONTROL_SIZE,
> - .type = MT_DEVICE,
> + .name = "Linux",
> + .offset = MTDPART_OFS_APPEND,
> + .size = SZ_32M - TS72XX_REDBOOT_PART_SIZE,
> }, {
> - .virtual = TS72XX_NAND_BUSY_VIRT_BASE,
> - .pfn = __phys_to_pfn(TS72XX_NAND1_BUSY_PHYS_BASE),
> - .length = TS72XX_NAND_BUSY_SIZE,
> - .type = MT_DEVICE,
> - }
> + .name = "RedBoot",
> + .offset = MTDPART_OFS_APPEND,
> + .size = MTDPART_SIZ_FULL,
> + },
> };
>
> -static struct map_desc ts72xx_alternate_nand_io_desc[] __initdata = {
> +static struct mtd_partition ts72xx_part_info128[] = {
> {
> - .virtual = TS72XX_NAND_DATA_VIRT_BASE,
> - .pfn = __phys_to_pfn(TS72XX_NAND2_DATA_PHYS_BASE),
> - .length = TS72XX_NAND_DATA_SIZE,
> - .type = MT_DEVICE,
> + .name = "TS-BOOTROM",
> + .offset = 0,
> + .size = TS72XX_BOOTROM_PART_SIZE,
> }, {
> - .virtual = TS72XX_NAND_CONTROL_VIRT_BASE,
> - .pfn = __phys_to_pfn(TS72XX_NAND2_CONTROL_PHYS_BASE),
> - .length = TS72XX_NAND_CONTROL_SIZE,
> - .type = MT_DEVICE,
> + .name = "Linux",
> + .offset = MTDPART_OFS_APPEND,
> + .size = SZ_128M - TS72XX_REDBOOT_PART_SIZE,
> }, {
> - .virtual = TS72XX_NAND_BUSY_VIRT_BASE,
> - .pfn = __phys_to_pfn(TS72XX_NAND2_BUSY_PHYS_BASE),
> - .length = TS72XX_NAND_BUSY_SIZE,
> - .type = MT_DEVICE,
> - }
> + .name = "RedBoot",
> + .offset = MTDPART_OFS_APPEND,
> + .size = MTDPART_SIZ_FULL,
> + },
> };
>
These struct's are almost identical so could you not...

> -static void __init ts72xx_map_io(void)
> +static void ts72xx_nand_set_parts(uint64_t size,
> + struct platform_nand_chip *chip)
> {
> - ep93xx_map_io();
> - iotable_init(ts72xx_io_desc, ARRAY_SIZE(ts72xx_io_desc));
> -
> - /*
> - * The TS-7200 has NOR flash, the other models have NAND flash.
> - */
> - if (!board_is_ts7200()) {
> - if (is_ts9420_installed()) {
> - iotable_init(ts72xx_alternate_nand_io_desc,
> - ARRAY_SIZE(ts72xx_alternate_nand_io_desc));
> - } else {
> - iotable_init(ts72xx_nand_io_desc,
> - ARRAY_SIZE(ts72xx_nand_io_desc));
> - }
> + switch (size) {
> + case SZ_32M:
> + chip->partitions = ts72xx_part_info32;
> + chip->nr_partitions = ARRAY_SIZE(ts72xx_part_info32);
> + break;
> + case SZ_128M:
> + chip->partitions = ts72xx_part_info128;
> + chip->nr_partitions = ARRAY_SIZE(ts72xx_part_info128);
> + break;
> + default:
> + pr_warning("ts72xx: Unknown nand disk size:%lluMB\n", size >> 20);
> + break;
> }
> }
>
-----------
static struct mtd_partition ts72xx_part_info[] = {
{
/* gut feeling for *Joe Public* is to go MTD_WRITABLE */
.name = "TS-BOOTROM",
.offset = 0,
.size = TS72XX_BOOTROM_PART_SIZE,
}, {
.name = "Linux",
.offset = MTDPART_OFS_APPEND,
/* to be filled in later */
.size = 0,
}, {
/* mask MTD_WRITABLE here too? */
.name = "Redboot",
.offset = MTDPART_OFS_APPEND,
.size = MTDPART_SIZ_FULL,
}
};

static void ts72xx_nand_set_parts(....)
{
....

/* or I guess you could just bailout... */
if (size != SZ_32M && size != SZ_128M)
pr_warning("ts72xx: Unknown nand disk size:%lluMB\n", size >> 20);

chip->partitions = ts72xx_part_info;
chip->nr_partitions = ARRAY_SIZE(ts72xx_part_info);

ts72xx_part_info[1].size = size - TS72XX_REDBOOT_PART_SIZE;
}
---------

Cheers

--
Alexander Clouter
.sigmonster says: Life's too short to dance with ugly women.