| Message ID | 1285833646-12006-6-git-send-email-Mingkai.hu@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kumar Gala |
| Headers | show |
On Thu, Sep 30, 2010 at 5:00 PM, Mingkai Hu <Mingkai.hu@freescale.com> wrote: > Signed-off-by: Mingkai Hu <Mingkai.hu@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@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.
> -----Original Message----- > From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of Grant > Likely > Sent: Friday, October 01, 2010 5:35 AM > To: Hu Mingkai-B21284 > Cc: linuxppc-dev@ozlabs.org; spi-devel-general@lists.sourceforge.net; linux- > mtd@lists.infradead.org; Gala Kumar-B11780; Zang Roy-R61911 > Subject: Re: [PATCH v3 5/7] mtd: m25p80: add support to parse the SPI flash's > partitions > > On Thu, Sep 30, 2010 at 5:00 PM, Mingkai Hu <Mingkai.hu@freescale.com> wrote: > > Signed-off-by: Mingkai Hu <Mingkai.hu@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@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? > Yes, you're right, I'll fix it. Also thanks for your suggestion and ACK. Thanks, Mingkai
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; + } + } + + 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; } if (nr_parts > 0) { @@ -937,9 +952,9 @@ static int __devinit m25p_probe(struct spi_device *spi) flash->partitioned = 1; return add_mtd_partitions(&flash->mtd, parts, nr_parts); } - } else if (data && data->nr_parts) + } else if (pdata && pdata->nr_parts) dev_warn(&spi->dev, "ignoring %d default partitions on %s\n", - data->nr_parts, data->name); + pdata->nr_parts, pdata->name); return add_mtd_device(&flash->mtd) == 1 ? -ENODEV : 0; }
Signed-off-by: Mingkai Hu <Mingkai.hu@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(-)