> -----Original Message----- > From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant > Likely > Sent: Friday, October 01, 2010 5:35 AM > To: Hu Mingkai-B21284 > Cc: linuxppc-...@ozlabs.org; spi-devel-gene...@lists.sourceforge.net; linux- > m...@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...@freescale.com> wrote: > > Signed-off-by: Mingkai Hu <mingkai...@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.lik...@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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
