| Message ID | 1281422578-20461-1-git-send-email-sudhakar.raj@ti.com |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 10, 2010 at 02:42, Sudhakar Rajashekhara wrote: > On some platforms, MTD device can hold MAC address, calibration > data, serial numbers etc. pretty standard behavior ... > On TI's DA850/OMAP-L138/AM18xx EVM, MAC address is stored on > the last sector of the SPI flash, which is exported as an MTD > device. again, nothing unique here ... > This patch adds two new members to the 'flash_platform_data' > structure, a function handler which will be called after > add_mtd_device() and an argument to be passed to this function, > for example: offset to read from. ... and your changelog falls apart. the intro gives no basis that i can see as to why you need these hooks. please give examples of what you're actually trying to accomplish and how these hooks help you accomplish said goal and why it is necessary that you do these things from kernel space instead of standard userspace. -mike
Hi, On Tue, Aug 10, 2010 at 12:47:05, Mike Frysinger wrote: > On Tue, Aug 10, 2010 at 02:42, Sudhakar Rajashekhara wrote: > > On some platforms, MTD device can hold MAC address, calibration > > data, serial numbers etc. > > pretty standard behavior ... > > > On TI's DA850/OMAP-L138/AM18xx EVM, MAC address is stored on > > the last sector of the SPI flash, which is exported as an MTD > > device. > > again, nothing unique here ... > > > This patch adds two new members to the 'flash_platform_data' > > structure, a function handler which will be called after > > add_mtd_device() and an argument to be passed to this function, > > for example: offset to read from. > > ... and your changelog falls apart. the intro gives no basis that i > can see as to why you need these hooks. > > please give examples of what you're actually trying to accomplish and > how these hooks help you accomplish said goal and why it is necessary > that you do these things from kernel space instead of standard > userspace. I have submitted an updated version of this patch in which I have addressed your concerns. Regards, Sudhakar
On Tue, Aug 10, 2010 at 18:37:47, David Brownell wrote: > > > On Tue, Aug 10, 2010 at 12:47:05, Mike Frysinger wrote: > > > it is necessary > > > that you do these things from kernel space instead of > > standard > > > userspace. > > Hard to get userspace to do this stuff if > you're doing a network boot, and thus need > to have the MAC address working early.. it's > easy if the kernel can get the MAC address, > else not possible to boot. Right? > You are absolutely right. - Sudhakar
> On Tue, Aug 10, 2010 at 12:47:05, Mike Frysinger wrote: > it is necessary > > that you do these things from kernel space instead of > standard > > userspace. Hard to get userspace to do this stuff if you're doing a network boot, and thus need to have the MAC address working early.. it's easy if the kernel can get the MAC address, else not possible to boot. Right?
Hello. Sudhakar Rajashekhara wrote: > On some platforms, MTD device can hold MAC address, calibration > data, serial numbers etc. > On TI's DA850/OMAP-L138/AM18xx EVM, MAC address is stored on > the last sector of the SPI flash, which is exported as an MTD > device. > This patch adds two new members to the 'flash_platform_data' > structure, a function handler which will be called after > add_mtd_device() and an argument to be passed to this function, > for example: offset to read from. > Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com> > --- > drivers/mtd/devices/m25p80.c | 15 +++++++++++++-- > include/linux/spi/flash.h | 2 ++ > 2 files changed, 15 insertions(+), 2 deletions(-) > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 81e49a9..b832091 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c [...] > @@ -924,13 +925,23 @@ static int __devinit m25p_probe(struct spi_device *spi) > (long long)(parts[i].size >> 10)); > } > flash->partitioned = 1; > - return add_mtd_partitions(&flash->mtd, parts, nr_parts); > + ret = add_mtd_partitions(&flash->mtd, parts, nr_parts); > + if (!ret) > + goto out; > + > + return ret; > } > } else if (data && data->nr_parts) > dev_warn(&spi->dev, "ignoring %d default partitions on %s\n", > data->nr_parts, data->name); > > - return add_mtd_device(&flash->mtd) == 1 ? -ENODEV : 0; > + ret = add_mtd_device(&flash->mtd); > + > +out: > + if (data->setup && !ret) > + (data->setup)(&flash->mtd, (void *)data->context); Parens not necessary around 'data->setup'. > diff --git a/include/linux/spi/flash.h b/include/linux/spi/flash.h > index 3f22932..daa4875 100644 > --- a/include/linux/spi/flash.h > +++ b/include/linux/spi/flash.h > @@ -24,6 +24,8 @@ struct flash_platform_data { > unsigned int nr_parts; > > char *type; > + void (*setup)(struct mtd_info *, void *context); > + void *context; > > /* we'll likely add more ... use JEDEC IDs, etc */ > }; Hm, is m25p80.c the only SPI flash driver using this structure? I guess not -- I'm seeing at least sst25l.c and mtd_dataflash.c... WBR, Sergei
On Tue, Aug 10, 2010 at 09:07, David Brownell wrote: >> On Tue, Aug 10, 2010 at 12:47:05, Mike Frysinger wrote: >> it is necessary >> > that you do these things from kernel space instead of >> standard >> > userspace. > > Hard to get userspace to do this stuff if > you're doing a network boot, and thus need > to have the MAC address working early.. it's > easy if the kernel can get the MAC address, > else not possible to boot. Right? no, that isnt true at all. you can easily have a small initramfs to handle all of your random setup. if that's too much overhead, you can add a late initcall to your boards file that is run after the MTD devices are probed and let that read the required information. all of this is more than documented: http://www.denx.de/wiki/view/DULG/EthernetDoesNotWorkInLinux i still see these callbacks as unnecessary overhead to solve problems that can already be done multiple ways with existing code. -mike
Hi Mike, On Wed, Aug 11, 2010 at 00:32:00, Mike Frysinger wrote: > On Tue, Aug 10, 2010 at 09:07, David Brownell wrote: > >> On Tue, Aug 10, 2010 at 12:47:05, Mike Frysinger wrote: > >> it is necessary > >> > that you do these things from kernel space instead of > >> standard > >> > userspace. > > > > Hard to get userspace to do this stuff if > > you're doing a network boot, and thus need > > to have the MAC address working early.. it's > > easy if the kernel can get the MAC address, > > else not possible to boot. Right? > > no, that isnt true at all. you can easily have a small initramfs to > handle all of your random setup. if that's too much overhead, you can > add a late initcall to your boards file that is run after the MTD > devices are probed and let that read the required information. all of > this is more than documented: > http://www.denx.de/wiki/view/DULG/EthernetDoesNotWorkInLinux > I looked around in the kernel and found mtd_notifier callbacks which get called upon addition of MTD device. Inside this callback, I am checking for the device in which I am interested and doing a mtd->read to get the MAC address. So this patch can be dropped. Thanks, Sudhakar
On Wed, Aug 11, 2010 at 07:56, Sudhakar Rajashekhara wrote: > On Wed, Aug 11, 2010 at 00:32:00, Mike Frysinger wrote: >> On Tue, Aug 10, 2010 at 09:07, David Brownell wrote: >> >> On Tue, Aug 10, 2010 at 12:47:05, Mike Frysinger wrote: >> >> it is necessary >> >> > that you do these things from kernel space instead of >> >> standard >> >> > userspace. >> > >> > Hard to get userspace to do this stuff if >> > you're doing a network boot, and thus need >> > to have the MAC address working early.. it's >> > easy if the kernel can get the MAC address, >> > else not possible to boot. Right? >> >> no, that isnt true at all. you can easily have a small initramfs to >> handle all of your random setup. if that's too much overhead, you can >> add a late initcall to your boards file that is run after the MTD >> devices are probed and let that read the required information. all of >> this is more than documented: >> http://www.denx.de/wiki/view/DULG/EthernetDoesNotWorkInLinux > > I looked around in the kernel and found mtd_notifier callbacks which get > called upon addition of MTD device. Inside this callback, I am checking > for the device in which I am interested and doing a mtd->read to get the > MAC address. So this patch can be dropped. thanks for the follow up explanation -mike
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 81e49a9..b832091 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -765,6 +765,7 @@ static int __devinit m25p_probe(struct spi_device *spi) struct m25p *flash; struct flash_info *info; unsigned i; + int ret; /* Platform data helps sort out which chip type we have, as * well as how this board partitions it. If we don't have @@ -924,13 +925,23 @@ static int __devinit m25p_probe(struct spi_device *spi) (long long)(parts[i].size >> 10)); } flash->partitioned = 1; - return add_mtd_partitions(&flash->mtd, parts, nr_parts); + ret = add_mtd_partitions(&flash->mtd, parts, nr_parts); + if (!ret) + goto out; + + return ret; } } else if (data && data->nr_parts) dev_warn(&spi->dev, "ignoring %d default partitions on %s\n", data->nr_parts, data->name); - return add_mtd_device(&flash->mtd) == 1 ? -ENODEV : 0; + ret = add_mtd_device(&flash->mtd); + +out: + if (data->setup && !ret) + (data->setup)(&flash->mtd, (void *)data->context); + + return (ret == 1) ? -ENODEV : 0; } diff --git a/include/linux/spi/flash.h b/include/linux/spi/flash.h index 3f22932..daa4875 100644 --- a/include/linux/spi/flash.h +++ b/include/linux/spi/flash.h @@ -24,6 +24,8 @@ struct flash_platform_data { unsigned int nr_parts; char *type; + void (*setup)(struct mtd_info *, void *context); + void *context; /* we'll likely add more ... use JEDEC IDs, etc */ };
On some platforms, MTD device can hold MAC address, calibration data, serial numbers etc. On TI's DA850/OMAP-L138/AM18xx EVM, MAC address is stored on the last sector of the SPI flash, which is exported as an MTD device. This patch adds two new members to the 'flash_platform_data' structure, a function handler which will be called after add_mtd_device() and an argument to be passed to this function, for example: offset to read from. Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com> --- drivers/mtd/devices/m25p80.c | 15 +++++++++++++-- include/linux/spi/flash.h | 2 ++ 2 files changed, 15 insertions(+), 2 deletions(-)