[PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions

[Grant Likely]

On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu at freescale.com> wrote:
> Signed-off-by: Mingkai Hu <Mingkai.hu at freescale.com>
> ---
>
> v2:
>  - Move the flash partition function from of_spi.c to MTD driver
>
>  drivers/mtd/devices/m25p80.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 81e49a9..5f00075 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -752,6 +752,31 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
>        return NULL;
>  }
>
> +/*
> + * parse_flash_partition - Parse the flash partition on the SPI bus
> + * @spi: Pointer to spi_device device
> + */
> +void parse_flash_partition(struct spi_device *spi)
> +{
> +       struct mtd_partition *parts;
> +       struct flash_platform_data *pdata;
> +       int nr_parts = 0;
> +       struct device_node *np = spi->dev.of_node;
> +
> +       nr_parts = of_mtd_parse_partitions(&spi->dev, np, &parts);
> +       if (!nr_parts)
> +               return;
> +
> +       pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +       if (!pdata)
> +               return;
> +
> +       pdata->parts = parts;
> +       pdata->nr_parts = nr_parts;
> +       spi->dev.platform_data = pdata;

The driver is not allowed to write to the platform_data pointer in the
device structure because it leaves an absolute mess in trying to
figure out who owns the pdata pointer.  Should it be freed when the
driver is unbound?  Is it statically allocated? etc.  There is a safer
way to get the data (see below).

> +
> +       return;
> +}
>
>  /*
>  * board specific setup should have ensured the SPI clock used here
> @@ -771,6 +796,10 @@ 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.
>         */
> +
> +       /* Parse the flash partition */
> +       parse_flash_partition(spi);
> +

Doing this unconditionally is dangerous, and if a platform_data
structure is provided, then it must alwasy take precedence over the
device tree data (because the platform code has gone to extra lengths
to add a pdata structure; that must mean it is important!)  :-)

Instead what can be done is to call of_mtd_parse_partitions() directly
from the probe routine after checking to see if the pdata structure
exists and has partition information.  If it doesn't, then call
of_mtd_parse_partitions() to get values for parts and nr_parts.  Doing
it that way completely eliminates the uncertainty over pdata pointer
ownership, and eliminates having an anonymous pointer buried in the
driver.

Cheers,
g.