| Message ID | 20090731004100.GB8371@oksana.dev.rtsoft.ru |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 30 July 2009, Anton Vorontsov wrote: > This patch converts the m25p80 driver so that now it uses .id_table > for device matching, making it properly detect devices on OpenFirmware > platforms (prior to this patch the driver misdetected non-JEDEC chips, > seeing all chips as "m25p80"). I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. All others got explicit declarations ... so if there's misbehavior for other chips, it's because those declarations were poorly handled. Maybe they were not properly flagged as non-JDEC > Also, now jedec_probe() only does jedec probing, nothing else. If it > is not able to detect a chip, NULL is returned and the driver fall > backs to the information specified by the platform (platform_data, or > exact ID). I'd rather keep the warning, so there's a clue about what's really going on: JEDEC chip found, but its ID is not handled. > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- > drivers/mtd/devices/m25p80.c | 146 +++++++++++++++++++++++------------------- > 1 files changed, 80 insertions(+), 66 deletions(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 10ed195..0d74b38 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > ... deletia ... > @@ -481,74 +480,83 @@ struct flash_info { > #define SECT_4K 0x01 /* OPCODE_BE_4K works uniformly */ > }; > > +#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \ > + ((kernel_ulong_t)&(struct flash_info) { \ > + .jedec_id = (_jedec_id), \ > + .ext_id = (_ext_id), \ > + .sector_size = (_sector_size), \ > + .n_sectors = (_n_sectors), \ > + .flags = (_flags), \ > + }) Anonymous inlined structures ... kind of ugly, but I can understand why you might not want to declare and name a few dozen single-use structures. > > /* NOTE: double check command sets and memory organization when you add > * more flash chips. This current list focusses on newer chips, which > * have been converging on command sets which including JEDEC ID. > */ > -static struct flash_info __devinitdata m25p_data [] = { > - > +static const struct spi_device_id m25p_ids[] = { > /* Atmel -- some are (confusingly) marketed as "DataFlash" */ > - { "at25fs010", 0x1f6601, 0, 32 * 1024, 4, SECT_4K, }, > - { "at25fs040", 0x1f6604, 0, 64 * 1024, 8, SECT_4K, }, > + { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) }, > + { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) }, > > ... deletia ... > > @@ -596,6 +602,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi) > */ > static int __devinit m25p_probe(struct spi_device *spi) > { > + const struct spi_device_id *id; > struct flash_platform_data *data; > struct m25p *flash; > struct flash_info *info; > @@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi) > */ > data = spi->dev.platform_data; > if (data && data->type) { At this point I wonder why you're not changing the probe sequence more. Get "id" and then "id" here. If it's for "m25p80" assume it's an old-style board init and do the current logic. Else just verify "info". There's a new error case of course: new-style but data->type doesn't match id->name. > - for (i = 0, info = m25p_data; > - i < ARRAY_SIZE(m25p_data); > - i++, info++) { > - if (strcmp(data->type, info->name) == 0) > - break; > + for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) { > + id = &m25p_ids[i]; > + info = (void *)m25p_ids[i].driver_data; > + if (strcmp(data->type, id->name)) > + continue; > + break; > } > > /* unrecognized chip? */ > - if (i == ARRAY_SIZE(m25p_data)) { > + if (i == ARRAY_SIZE(m25p_ids) - 1) { Better: "if (info == NULL) ..." You've got all the pointers in hand; don't use indices. > DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n", > dev_name(&spi->dev), data->type); > info = NULL; > > /* recognized; is that chip really what's there? */ > } else if (info->jedec_id) { > - struct flash_info *chip = jedec_probe(spi); > + id = jedec_probe(spi); > > - if (!chip || chip != info) { > + if (id != &m25p_ids[i]) { Again, don't use indices except during the lookup. > dev_warn(&spi->dev, "found %s, expected %s\n", > - chip ? chip->name : "UNKNOWN", > - info->name); > + id ? id->name : "UNKNOWN", > + m25p_ids[i].name); > info = NULL; > } > } > - } else > - info = jedec_probe(spi); > + } else { > + id = jedec_probe(spi); > + if (!id) > + id = spi_get_device_id(spi); > + > + info = (void *)id->driver_data; > + } > > if (!info) > return -ENODEV;
Hello Anton, Is m25p_probe now valid with dev.platform_data == NULL, for of platforms? Then shouldn't you have the following change as well, near the end of the function? - } else if (data->nr_parts) + } else if (data && data->nr_parts) dev_warn(&spi->dev, "ignoring %d default partitions on %s\n", data->nr_parts, data->name); Or am I misunderstanding something?
On Wed, Aug 12, 2009 at 04:45:55PM -0400, Michael Barkowski wrote: > Hello Anton, > > Is m25p_probe now valid with dev.platform_data == NULL, for of platforms? > > Then shouldn't you have the following change as well, near the end of > the function? > > - } else if (data->nr_parts) > + } else if (data && data->nr_parts) > dev_warn(&spi->dev, "ignoring %d default partitions on %s\n", > data->nr_parts, data->name); > > Or am I misunderstanding something? Yeah, you missed this patch: http://patchwork.kernel.org/patch/38179/ Thanks,
Anton Vorontsov wrote: > On Wed, Aug 12, 2009 at 04:45:55PM -0400, Michael Barkowski wrote: >> Hello Anton, >> >> Is m25p_probe now valid with dev.platform_data == NULL, for of platforms? >> >> Then shouldn't you have the following change as well, near the end of >> the function? >> >> - } else if (data->nr_parts) >> + } else if (data && data->nr_parts) >> dev_warn(&spi->dev, "ignoring %d default partitions on %s\n", >> data->nr_parts, data->name); >> >> Or am I misunderstanding something? > > Yeah, you missed this patch: > http://patchwork.kernel.org/patch/38179/ > > > Thanks, > Beautiful - thanks - sorry to interrupt
Hi David, Thanks for the review, and sorry for the delayed response. On Mon, Aug 03, 2009 at 07:54:50PM -0700, David Brownell wrote: > On Thursday 30 July 2009, Anton Vorontsov wrote: > > This patch converts the m25p80 driver so that now it uses .id_table > > for device matching, making it properly detect devices on OpenFirmware > > platforms (prior to this patch the driver misdetected non-JEDEC chips, > > seeing all chips as "m25p80"). > > I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. Currently the driver always tries JEDEC probing, and it wrongly "assumed" that all non-JEDEC chips are m25p80, because an entry for m25p80 chips had "0" in jedec id field, which isn't correct ID, but 0 is returned by most non-JEDEC chips. Having 0 in the m25p80 entry was a hack. > All others got explicit declarations ... so if there's misbehavior for > other chips, it's because those declarations were poorly handled. Maybe > they were not properly flagged as non-JDEC > > Also, now jedec_probe() only does jedec probing, nothing else. If it > > is not able to detect a chip, NULL is returned and the driver fall > > backs to the information specified by the platform (platform_data, or > > exact ID). > > I'd rather keep the warning, so there's a clue about what's really > going on: JEDEC chip found, but its ID is not handled. We can't tell if the chip was actually found. M25Px0 chips can be JEDEC and non-JEDEC, e.g. Nymonyx manufacturing "M25P80" chips in two variants: "The RDID instruction is available only for parts made with Technology T9HX (0.11μm), ..." Most (but not all) non-JEDEC EEPROMs will return "0" for RDID opcode though (in that case warning is misleading). And for the chips that don't return 0, we shouldn't probe them with jedec at all. [...] > > @@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi) > > */ > > data = spi->dev.platform_data; > > if (data && data->type) { > > At this point I wonder why you're not changing the probe sequence > more. Yep, I was going to do it anyway, but for another reason: to support CAT25 chips. > There's a new error case of course: new-style but data->type > doesn't match id->name. [...] > > + if (i == ARRAY_SIZE(m25p_ids) - 1) { > > Better: "if (info == NULL) ..." You've got all the pointers > in hand; don't use indices. [...] > > + if (id != &m25p_ids[i]) { > > Again, don't use indices except during the lookup. Yep, good ideas. Though, the code will vanish anyway. Patches on the way...
On Mon, 3 Aug 2009 19:54:50 -0700 David Brownell <david-b@pacbell.net> wrote: > On Thursday 30 July 2009, Anton Vorontsov wrote: > > This patch converts the m25p80 driver so that now it uses .id_table > > for device matching, making it properly detect devices on OpenFirmware > > platforms (prior to this patch the driver misdetected non-JEDEC chips, > > seeing all chips as "m25p80"). > > I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. > All others got explicit declarations ... so if there's misbehavior for > other chips, it's because those declarations were poorly handled. Maybe > they were not properly flagged as non-JDEC > > > > Also, now jedec_probe() only does jedec probing, nothing else. If it > > is not able to detect a chip, NULL is returned and the driver fall > > backs to the information specified by the platform (platform_data, or > > exact ID). > > I'd rather keep the warning, so there's a clue about what's really > going on: JEDEC chip found, but its ID is not handled. > afaik there was no response to David's review comments, so this patch is in the "stuck" state. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > > --- > > drivers/mtd/devices/m25p80.c | 146 +++++++++++++++++++++++------------------- > > 1 files changed, 80 insertions(+), 66 deletions(-) > > > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > > index 10ed195..0d74b38 100644 > > --- a/drivers/mtd/devices/m25p80.c > > +++ b/drivers/mtd/devices/m25p80.c > > ... deletia ... > > > @@ -481,74 +480,83 @@ struct flash_info { > > #define SECT_4K 0x01 /* OPCODE_BE_4K works uniformly */ > > }; > > > > +#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \ > > + ((kernel_ulong_t)&(struct flash_info) { \ > > + .jedec_id = (_jedec_id), \ > > + .ext_id = (_ext_id), \ > > + .sector_size = (_sector_size), \ > > + .n_sectors = (_n_sectors), \ > > + .flags = (_flags), \ > > + }) > > Anonymous inlined structures ... kind of ugly, but I can > understand why you might not want to declare and name a > few dozen single-use structures. > > > > > > /* NOTE: double check command sets and memory organization when you add > > * more flash chips. This current list focusses on newer chips, which > > * have been converging on command sets which including JEDEC ID. > > */ > > -static struct flash_info __devinitdata m25p_data [] = { > > - > > +static const struct spi_device_id m25p_ids[] = { > > /* Atmel -- some are (confusingly) marketed as "DataFlash" */ > > - { "at25fs010", 0x1f6601, 0, 32 * 1024, 4, SECT_4K, }, > > - { "at25fs040", 0x1f6604, 0, 64 * 1024, 8, SECT_4K, }, > > + { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) }, > > + { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) }, > > > > ... deletia ... > > > > > @@ -596,6 +602,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi) > > */ > > static int __devinit m25p_probe(struct spi_device *spi) > > { > > + const struct spi_device_id *id; > > struct flash_platform_data *data; > > struct m25p *flash; > > struct flash_info *info; > > @@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi) > > */ > > data = spi->dev.platform_data; > > if (data && data->type) { > > At this point I wonder why you're not changing the probe sequence > more. Get "id" and then "id" here. If it's for "m25p80" assume > it's an old-style board init and do the current logic. Else just > verify "info". > > There's a new error case of course: new-style but data->type > doesn't match id->name. > > > > - for (i = 0, info = m25p_data; > > - i < ARRAY_SIZE(m25p_data); > > - i++, info++) { > > - if (strcmp(data->type, info->name) == 0) > > - break; > > + for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) { > > + id = &m25p_ids[i]; > > + info = (void *)m25p_ids[i].driver_data; > > + if (strcmp(data->type, id->name)) > > + continue; > > + break; > > } > > > > /* unrecognized chip? */ > > - if (i == ARRAY_SIZE(m25p_data)) { > > + if (i == ARRAY_SIZE(m25p_ids) - 1) { > > Better: "if (info == NULL) ..." You've got all the pointers > in hand; don't use indices. > > > DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n", > > dev_name(&spi->dev), data->type); > > info = NULL; > > > > /* recognized; is that chip really what's there? */ > > } else if (info->jedec_id) { > > - struct flash_info *chip = jedec_probe(spi); > > + id = jedec_probe(spi); > > > > - if (!chip || chip != info) { > > + if (id != &m25p_ids[i]) { > > Again, don't use indices except during the lookup. > > > dev_warn(&spi->dev, "found %s, expected %s\n", > > - chip ? chip->name : "UNKNOWN", > > - info->name); > > + id ? id->name : "UNKNOWN", > > + m25p_ids[i].name); > > info = NULL; > > } > > } > > - } else > > - info = jedec_probe(spi); > > + } else { > > + id = jedec_probe(spi); > > + if (!id) > > + id = spi_get_device_id(spi); > > + > > + info = (void *)id->driver_data; > > + } > > > > if (!info) > > return -ENODEV; >
On Tue, Sep 22, 2009 at 02:17:05PM -0700, Andrew Morton wrote: > On Mon, 3 Aug 2009 19:54:50 -0700 > David Brownell <david-b@pacbell.net> wrote: > > > On Thursday 30 July 2009, Anton Vorontsov wrote: > > > This patch converts the m25p80 driver so that now it uses .id_table > > > for device matching, making it properly detect devices on OpenFirmware > > > platforms (prior to this patch the driver misdetected non-JEDEC chips, > > > seeing all chips as "m25p80"). > > > > I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. > > All others got explicit declarations ... so if there's misbehavior for > > other chips, it's because those declarations were poorly handled. Maybe > > they were not properly flagged as non-JDEC > > > > > > > Also, now jedec_probe() only does jedec probing, nothing else. If it > > > is not able to detect a chip, NULL is returned and the driver fall > > > backs to the information specified by the platform (platform_data, or > > > exact ID). > > > > I'd rather keep the warning, so there's a clue about what's really > > going on: JEDEC chip found, but its ID is not handled. > > > > afaik there was no response to David's review comments, so this patch > is in the "stuck" state. Hm? Response: http://lkml.org/lkml/2009/8/18/363 And the two patches I sent on top: http://lkml.org/lkml/2009/8/18/364 http://lkml.org/lkml/2009/8/18/366
On Wed, 2009-09-23 at 03:01 +0400, Anton Vorontsov wrote: > > And the two patches I sent on top: > > http://lkml.org/lkml/2009/8/18/364 > http://lkml.org/lkml/2009/8/18/366 Got versions of those which apply to the mtd-2.6.git tree (which I'm about to ask Linus to pull)?
On Tue, 22 Sep 2009 16:43:47 -0700 David Woodhouse <dwmw2@infradead.org> wrote: > On Wed, 2009-09-23 at 03:01 +0400, Anton Vorontsov wrote: > > > > And the two patches I sent on top: > > > > http://lkml.org/lkml/2009/8/18/364 > > http://lkml.org/lkml/2009/8/18/366 > > Got versions of those which apply to the mtd-2.6.git tree (which I'm > about to ask Linus to pull)? > I'll send them in a sec.
On Tue, Sep 22, 2009 at 04:43:47PM -0700, David Woodhouse wrote: > On Wed, 2009-09-23 at 03:01 +0400, Anton Vorontsov wrote: > > > > And the two patches I sent on top: > > > > http://lkml.org/lkml/2009/8/18/364 > > http://lkml.org/lkml/2009/8/18/366 > > Got versions of those which apply to the mtd-2.6.git tree (which I'm > about to ask Linus to pull)? I'd love to, but they depend on a bunch of SPI patches that are still in -mm tree. As soon as SPI core changes hit Linus' tree, I think Andrew will send all m25p80 patches to you anyway. Thanks,
On Wed, 23 Sep 2009 03:55:34 +0400 Anton Vorontsov <cbouatmailru@gmail.com> wrote: > On Tue, Sep 22, 2009 at 04:43:47PM -0700, David Woodhouse wrote: > > On Wed, 2009-09-23 at 03:01 +0400, Anton Vorontsov wrote: > > > > > > And the two patches I sent on top: > > > > > > http://lkml.org/lkml/2009/8/18/364 > > > http://lkml.org/lkml/2009/8/18/366 > > > > Got versions of those which apply to the mtd-2.6.git tree (which I'm > > about to ask Linus to pull)? > > I'd love to, but they depend on a bunch of SPI patches that are still > in -mm tree. oh, is that why I queued them up where I did. Sigh. > As soon as SPI core changes hit Linus' tree, I think > Andrew will send all m25p80 patches to you anyway. Or David can ack them and I'll send 'em up.
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 10ed195..0d74b38 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -21,6 +21,7 @@ #include <linux/interrupt.h> #include <linux/mutex.h> #include <linux/math64.h> +#include <linux/mod_devicetable.h> #include <linux/mtd/mtd.h> #include <linux/mtd/partitions.h> @@ -462,8 +463,6 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len, */ struct flash_info { - char *name; - /* JEDEC id zero means "no ID" (most older chips); otherwise it has * a high byte of zero plus three data bytes: the manufacturer id, * then a two byte device id. @@ -481,74 +480,83 @@ struct flash_info { #define SECT_4K 0x01 /* OPCODE_BE_4K works uniformly */ }; +#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \ + ((kernel_ulong_t)&(struct flash_info) { \ + .jedec_id = (_jedec_id), \ + .ext_id = (_ext_id), \ + .sector_size = (_sector_size), \ + .n_sectors = (_n_sectors), \ + .flags = (_flags), \ + }) /* NOTE: double check command sets and memory organization when you add * more flash chips. This current list focusses on newer chips, which * have been converging on command sets which including JEDEC ID. */ -static struct flash_info __devinitdata m25p_data [] = { - +static const struct spi_device_id m25p_ids[] = { /* Atmel -- some are (confusingly) marketed as "DataFlash" */ - { "at25fs010", 0x1f6601, 0, 32 * 1024, 4, SECT_4K, }, - { "at25fs040", 0x1f6604, 0, 64 * 1024, 8, SECT_4K, }, + { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) }, + { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) }, - { "at25df041a", 0x1f4401, 0, 64 * 1024, 8, SECT_4K, }, - { "at25df641", 0x1f4800, 0, 64 * 1024, 128, SECT_4K, }, + { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, SECT_4K) }, + { "at25df641", INFO(0x1f4800, 0, 64 * 1024, 128, SECT_4K) }, - { "at26f004", 0x1f0400, 0, 64 * 1024, 8, SECT_4K, }, - { "at26df081a", 0x1f4501, 0, 64 * 1024, 16, SECT_4K, }, - { "at26df161a", 0x1f4601, 0, 64 * 1024, 32, SECT_4K, }, - { "at26df321", 0x1f4701, 0, 64 * 1024, 64, SECT_4K, }, + { "at26f004", INFO(0x1f0400, 0, 64 * 1024, 8, SECT_4K) }, + { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K) }, + { "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, SECT_4K) }, + { "at26df321", INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K) }, /* Macronix */ - { "mx25l12805d", 0xc22018, 0, 64 * 1024, 256, }, + { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 0) }, /* Spansion -- single (large) sector size only, at least * for the chips listed here (without boot sectors). */ - { "s25sl004a", 0x010212, 0, 64 * 1024, 8, }, - { "s25sl008a", 0x010213, 0, 64 * 1024, 16, }, - { "s25sl016a", 0x010214, 0, 64 * 1024, 32, }, - { "s25sl032a", 0x010215, 0, 64 * 1024, 64, }, - { "s25sl064a", 0x010216, 0, 64 * 1024, 128, }, - { "s25sl12800", 0x012018, 0x0300, 256 * 1024, 64, }, - { "s25sl12801", 0x012018, 0x0301, 64 * 1024, 256, }, + { "s25sl004a", INFO(0x010212, 0, 64 * 1024, 8, 0) }, + { "s25sl008a", INFO(0x010213, 0, 64 * 1024, 16, 0) }, + { "s25sl016a", INFO(0x010214, 0, 64 * 1024, 32, 0) }, + { "s25sl032a", INFO(0x010215, 0, 64 * 1024, 64, 0) }, + { "s25sl064a", INFO(0x010216, 0, 64 * 1024, 128, 0) }, + { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) }, + { "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) }, /* SST -- large erase sizes are "overlays", "sectors" are 4K */ - { "sst25vf040b", 0xbf258d, 0, 64 * 1024, 8, SECT_4K, }, - { "sst25vf080b", 0xbf258e, 0, 64 * 1024, 16, SECT_4K, }, - { "sst25vf016b", 0xbf2541, 0, 64 * 1024, 32, SECT_4K, }, - { "sst25vf032b", 0xbf254a, 0, 64 * 1024, 64, SECT_4K, }, + { "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024, 8, SECT_4K) }, + { "sst25vf080b", INFO(0xbf258e, 0, 64 * 1024, 16, SECT_4K) }, + { "sst25vf016b", INFO(0xbf2541, 0, 64 * 1024, 32, SECT_4K) }, + { "sst25vf032b", INFO(0xbf254a, 0, 64 * 1024, 64, SECT_4K) }, /* ST Microelectronics -- newer production may have feature updates */ - { "m25p05", 0x202010, 0, 32 * 1024, 2, }, - { "m25p10", 0x202011, 0, 32 * 1024, 4, }, - { "m25p20", 0x202012, 0, 64 * 1024, 4, }, - { "m25p40", 0x202013, 0, 64 * 1024, 8, }, - { "m25p80", 0, 0, 64 * 1024, 16, }, - { "m25p16", 0x202015, 0, 64 * 1024, 32, }, - { "m25p32", 0x202016, 0, 64 * 1024, 64, }, - { "m25p64", 0x202017, 0, 64 * 1024, 128, }, - { "m25p128", 0x202018, 0, 256 * 1024, 64, }, - - { "m45pe10", 0x204011, 0, 64 * 1024, 2, }, - { "m45pe80", 0x204014, 0, 64 * 1024, 16, }, - { "m45pe16", 0x204015, 0, 64 * 1024, 32, }, - - { "m25pe80", 0x208014, 0, 64 * 1024, 16, }, - { "m25pe16", 0x208015, 0, 64 * 1024, 32, SECT_4K, }, + { "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) }, + { "m25p10", INFO(0x202011, 0, 32 * 1024, 4, 0) }, + { "m25p20", INFO(0x202012, 0, 64 * 1024, 4, 0) }, + { "m25p40", INFO(0x202013, 0, 64 * 1024, 8, 0) }, + { "m25p80", INFO(0x202014, 0, 64 * 1024, 16, 0) }, + { "m25p16", INFO(0x202015, 0, 64 * 1024, 32, 0) }, + { "m25p32", INFO(0x202016, 0, 64 * 1024, 64, 0) }, + { "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) }, + { "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) }, + + { "m45pe10", INFO(0x204011, 0, 64 * 1024, 2, 0) }, + { "m45pe80", INFO(0x204014, 0, 64 * 1024, 16, 0) }, + { "m45pe16", INFO(0x204015, 0, 64 * 1024, 32, 0) }, + + { "m25pe80", INFO(0x208014, 0, 64 * 1024, 16, 0) }, + { "m25pe16", INFO(0x208015, 0, 64 * 1024, 32, SECT_4K) }, /* Winbond -- w25x "blocks" are 64K, "sectors" are 4KiB */ - { "w25x10", 0xef3011, 0, 64 * 1024, 2, SECT_4K, }, - { "w25x20", 0xef3012, 0, 64 * 1024, 4, SECT_4K, }, - { "w25x40", 0xef3013, 0, 64 * 1024, 8, SECT_4K, }, - { "w25x80", 0xef3014, 0, 64 * 1024, 16, SECT_4K, }, - { "w25x16", 0xef3015, 0, 64 * 1024, 32, SECT_4K, }, - { "w25x32", 0xef3016, 0, 64 * 1024, 64, SECT_4K, }, - { "w25x64", 0xef3017, 0, 64 * 1024, 128, SECT_4K, }, + { "w25x10", INFO(0xef3011, 0, 64 * 1024, 2, SECT_4K) }, + { "w25x20", INFO(0xef3012, 0, 64 * 1024, 4, SECT_4K) }, + { "w25x40", INFO(0xef3013, 0, 64 * 1024, 8, SECT_4K) }, + { "w25x80", INFO(0xef3014, 0, 64 * 1024, 16, SECT_4K) }, + { "w25x16", INFO(0xef3015, 0, 64 * 1024, 32, SECT_4K) }, + { "w25x32", INFO(0xef3016, 0, 64 * 1024, 64, SECT_4K) }, + { "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, SECT_4K) }, + { }, }; +MODULE_DEVICE_TABLE(spi, m25p_ids); -static struct flash_info *__devinit jedec_probe(struct spi_device *spi) +static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi) { int tmp; u8 code = OPCODE_RDID; @@ -575,16 +583,14 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi) ext_jedec = id[3] << 8 | id[4]; - for (tmp = 0, info = m25p_data; - tmp < ARRAY_SIZE(m25p_data); - tmp++, info++) { + for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) { + info = (void *)m25p_ids[tmp].driver_data; if (info->jedec_id == jedec) { if (info->ext_id != 0 && info->ext_id != ext_jedec) continue; - return info; + return &m25p_ids[tmp]; } } - dev_err(&spi->dev, "unrecognized JEDEC id %06x\n", jedec); return NULL; } @@ -596,6 +602,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi) */ static int __devinit m25p_probe(struct spi_device *spi) { + const struct spi_device_id *id; struct flash_platform_data *data; struct m25p *flash; struct flash_info *info; @@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi) */ data = spi->dev.platform_data; if (data && data->type) { - for (i = 0, info = m25p_data; - i < ARRAY_SIZE(m25p_data); - i++, info++) { - if (strcmp(data->type, info->name) == 0) - break; + for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) { + id = &m25p_ids[i]; + info = (void *)m25p_ids[i].driver_data; + if (strcmp(data->type, id->name)) + continue; + break; } /* unrecognized chip? */ - if (i == ARRAY_SIZE(m25p_data)) { + if (i == ARRAY_SIZE(m25p_ids) - 1) { DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n", dev_name(&spi->dev), data->type); info = NULL; /* recognized; is that chip really what's there? */ } else if (info->jedec_id) { - struct flash_info *chip = jedec_probe(spi); + id = jedec_probe(spi); - if (!chip || chip != info) { + if (id != &m25p_ids[i]) { dev_warn(&spi->dev, "found %s, expected %s\n", - chip ? chip->name : "UNKNOWN", - info->name); + id ? id->name : "UNKNOWN", + m25p_ids[i].name); info = NULL; } } - } else - info = jedec_probe(spi); + } else { + id = jedec_probe(spi); + if (!id) + id = spi_get_device_id(spi); + + info = (void *)id->driver_data; + } if (!info) return -ENODEV; @@ -680,7 +693,7 @@ static int __devinit m25p_probe(struct spi_device *spi) flash->mtd.dev.parent = &spi->dev; - dev_info(&spi->dev, "%s (%lld Kbytes)\n", info->name, + dev_info(&spi->dev, "%s (%lld Kbytes)\n", id->name, (long long)flash->mtd.size >> 10); DEBUG(MTD_DEBUG_LEVEL2, @@ -766,6 +779,7 @@ static struct spi_driver m25p80_driver = { .bus = &spi_bus_type, .owner = THIS_MODULE, }, + .id_table = m25p_ids, .probe = m25p_probe, .remove = __devexit_p(m25p_remove),
This patch converts the m25p80 driver so that now it uses .id_table for device matching, making it properly detect devices on OpenFirmware platforms (prior to this patch the driver misdetected non-JEDEC chips, seeing all chips as "m25p80"). Also, now jedec_probe() only does jedec probing, nothing else. If it is not able to detect a chip, NULL is returned and the driver fall backs to the information specified by the platform (platform_data, or exact ID). Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- drivers/mtd/devices/m25p80.c | 146 +++++++++++++++++++++++------------------- 1 files changed, 80 insertions(+), 66 deletions(-)