Re: [spi-devel-general] [PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions
On Tue, Aug 10, 2010 at 1:14 AM, David Brownell wrote: > > > --- On Mon, 8/9/10, Grant Likely wrote: > > >> > + nr_parts = >> of_mtd_parse_partitions(&spi->dev, np, &parts); > > Let's keep OF-specific logic out of drivers like > this one ... intended to work without OF. > > NAK on adding dependencies like OF to drivers > and other infrastructure that starts generic. The OF hooks compile to no-ops when CONFIG_OF is disabled. I've been very careful about that. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [spi-devel-general] [PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions
--- On Mon, 8/9/10, Grant Likely wrote: > > + nr_parts = > of_mtd_parse_partitions(&spi->dev, np, &parts); Let's keep OF-specific logic out of drivers like this one ... intended to work without OF. NAK on adding dependencies like OF to drivers and other infrastructure that starts generic. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions
On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu wrote:
> Signed-off-by: Mingkai Hu
> ---
>
> 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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions
Signed-off-by: Mingkai Hu
---
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;
+
+ 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);
+
data = spi->dev.platform_data;
if (data && data->type) {
const struct spi_device_id *plat_id;
--
1.6.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
