On Mon, Jul 26, 2010 at 1:25 AM, Hu Mingkai-B21284 <b21...@freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: Grant Likely [mailto:glik...@secretlab.ca] On Behalf Of
>> Grant Likely
>> Sent: Monday, July 26, 2010 8:28 AM
>> To: Hu Mingkai-B21284
>> Cc: linuxppc-...@ozlabs.org; ga...@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...@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?
In the device driver probe code.
>> > 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.
Ah right, I wasn't looking in the right place. On that note however,
the cleanup of spi_device allocated by the OF code should also have a
reciprocal helper that frees them too. It shouldn't be done in the
individual drivers.
g.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
