[PATCH 3/6] of/spi: add support to parse the SPI flash's partitions

[Hu Mingkai-B21284]

> -----Original Message-----
> From: Grant Likely [mailto:glikely at secretlab.ca] On Behalf Of 
> Grant Likely
> Sent: Monday, July 26, 2010 8:28 AM
> To: Hu Mingkai-B21284
> Cc: linuxppc-dev at ozlabs.org; galak at kernel.crashing.org; Zang 
> Roy-R61911
> Subject: Re: [PATCH 3/6] of/spi: add support to parse the SPI 
> flash's partitions
> 
> On Tue, Jul 20, 2010 at 10:08:22AM +0800, Mingkai Hu wrote:
> > Signed-off-by: Mingkai Hu <Mingkai.hu at freescale.com>
> > ---
> >  drivers/of/of_spi.c       |   11 +++++++++++
> >  drivers/spi/spi_mpc8xxx.c |    1 +
> >  2 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c index 
> > 5fed7e3..284ca0e 100644
> > --- a/drivers/of/of_spi.c
> > +++ b/drivers/of/of_spi.c
> > @@ -10,6 +10,8 @@
> >  #include <linux/device.h>
> >  #include <linux/spi/spi.h>
> >  #include <linux/of_spi.h>
> > +#include <linux/spi/flash.h>
> > +#include <linux/mtd/partitions.h>
> >  
> >  /**
> >   * of_register_spi_devices - Register child devices onto 
> the SPI bus 
> > @@ -26,6 +28,7 @@ void of_register_spi_devices(struct 
> spi_master *master, struct device_node *np)
> >  	const __be32 *prop;
> >  	int rc;
> >  	int len;
> > +	struct flash_platform_data *pdata;
> >  
> >  	for_each_child_of_node(np, nc) {
> >  		/* Alloc an spi_device */
> > @@ -81,6 +84,14 @@ void of_register_spi_devices(struct 
> spi_master *master, struct device_node *np)
> >  		of_node_get(nc);
> >  		spi->dev.of_node = nc;
> >  
> > +		/* Parse the mtd partitions */
> > +		pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> > +		if (!pdata)
> > +			return;
> > +		pdata->nr_parts = of_mtd_parse_partitions(&master->dev,
> > +				nc, &pdata->parts);
> > +		spi->dev.platform_data = pdata;
> > +
> 
> Nack.  Not all spi devices are mtd devices.  In fact, most are not.
> 
> The spi driver itself should call the of_mtd_parse_partitions 
> code to get the partition map.  Do not use pdata in this case.
> 

Yes, we can call of_mtd_parse_partitions to get the partiton map, but
how can we
pass this map to spi device to register to MTD layer?

The spi device is created and registered when call
of_register_spi_device in the
spi controller probe function, then the spi device will traverse the spi
device driver
list to find the proper driver, if matched, then call the spi device
driver probe code
where the mtd partition info is registered to the mtd layer.

so where is the proper place to put the of_mtd_parse_partitions code?


> >  		/* Register the new device */
> >  		request_module(spi->modalias);
> >  		rc = spi_add_device(spi);
> > diff --git a/drivers/spi/spi_mpc8xxx.c b/drivers/spi/spi_mpc8xxx.c 
> > index efed70e..0fadaeb 100644
> > --- a/drivers/spi/spi_mpc8xxx.c
> > +++ b/drivers/spi/spi_mpc8xxx.c
> > @@ -137,6 +137,7 @@ int mpc8xxx_spi_transfer(struct spi_device *spi,
> >  
> >  void mpc8xxx_spi_cleanup(struct spi_device *spi)  {
> > +	kfree(spi->dev.platform_data);
> 
> Irrelevant given the above comment, but how does this even 
> work?  What if a driver was detached and reattached to an 
> spi_device?  The platform_data would be freed.  Not to 
> mention that the pointer isn't cleared, so the driver would 
> have no idea that it has a freed pointer.
> 

It works. If the driver detached, the spi device platform_data will be
released,
when reattached, the of_register_spi_devices will be called again, the
platform_data
will be allocated.

Thanks,
Mingkai