[PATCH v3 5/7] mtd: m25p80: add support to parse the SPI flash's partitions

[Grant Likely]

On Thu, Sep 30, 2010 at 5:00 PM, Mingkai Hu <Mingkai.hu at freescale.com> wrote:
> Signed-off-by: Mingkai Hu <Mingkai.hu at freescale.com>
> ---
> v3:
>  - Move the SPI flash partition code to the probe function.
>
>  drivers/mtd/devices/m25p80.c |   39 +++++++++++++++++++++++++++------------
>  1 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 6f512b5..47d53c7 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -772,7 +772,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
>  static int __devinit m25p_probe(struct spi_device *spi)
>  {
>        const struct spi_device_id      *id = spi_get_device_id(spi);
> -       struct flash_platform_data      *data;
> +       struct flash_platform_data      data, *pdata;
>        struct m25p                     *flash;
>        struct flash_info               *info;
>        unsigned                        i;
> @@ -782,13 +782,27 @@ static int __devinit m25p_probe(struct spi_device *spi)
>         * a chip ID, try the JEDEC id commands; they'll work for most
>         * newer chips, even if we don't recognize the particular chip.
>         */
> -       data = spi->dev.platform_data;
> -       if (data && data->type) {
> +       pdata = spi->dev.platform_data;
> +       if (!pdata && spi->dev.of_node) {
> +               int nr_parts;
> +               struct mtd_partition *parts;
> +               struct device_node *np = spi->dev.of_node;
> +
> +               nr_parts = of_mtd_parse_partitions(&spi->dev, np, &parts);
> +               if (nr_parts) {
> +                       pdata = &data;
> +                       memset(pdata, 0, sizeof(*pdata));
> +                       pdata->parts = parts;
> +                       pdata->nr_parts = nr_parts;
> +               }
> +       }

Yes, this is the correct way to go about adding the partitions.
However, this patch can be made simpler by not renaming 'data' to
'pdata' and by moving the above code down to just before the partition
information is actually used.  in the OF case, only the parts and the
nr_parts values written into data, and those values aren't used until
the last part of the probe function.

Regardless, in principle this patch is correct:

Acked-by: Grant Likely <grant.likely at secretlab.ca>

> +
> +       if (pdata && pdata->type) {
>                const struct spi_device_id *plat_id;
>
>                for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) {
>                        plat_id = &m25p_ids[i];
> -                       if (strcmp(data->type, plat_id->name))
> +                       if (strcmp(pdata->type, plat_id->name))
>                                continue;
>                        break;
>                }
> @@ -796,7 +810,8 @@ static int __devinit m25p_probe(struct spi_device *spi)
>                if (i < ARRAY_SIZE(m25p_ids) - 1)
>                        id = plat_id;
>                else
> -                       dev_warn(&spi->dev, "unrecognized id %s\n", data->type);
> +                       dev_warn(&spi->dev, "unrecognized id %s\n",
> +                                       pdata->type);
>        }
>
>        info = (void *)id->driver_data;
> @@ -847,8 +862,8 @@ static int __devinit m25p_probe(struct spi_device *spi)
>                write_sr(flash, 0);
>        }
>
> -       if (data && data->name)
> -               flash->mtd.name = data->name;
> +       if (pdata && pdata->name)
> +               flash->mtd.name = pdata->name;
>        else
>                flash->mtd.name = dev_name(&spi->dev);
>
> @@ -919,9 +934,9 @@ static int __devinit m25p_probe(struct spi_device *spi)
>                                        part_probes, &parts, 0);
>                }
>
> -               if (nr_parts <= 0 && data && data->parts) {
> -                       parts = data->parts;
> -                       nr_parts = data->nr_parts;
> +               if (nr_parts <= 0 && pdata && pdata->parts) {
> +                       parts = pdata->parts;
> +                       nr_parts = pdata->nr_parts;
>                }

As per my comment earlier; since parts and nr_parts isn't needed
before this point, this block could simply be:

if (nr_parts <= 0 && data && data->parts) {
        parts = data->parts;
        nr_parts = data->nr_parts;
}
if (nr_parts <= 0 && spi->dev.of_node)
        nr_parts = of_mtd_parse_partitions(&spi->dev, np, &parts);

And most of the other changes to this file goes away.  Simpler, yes?

g.