Patchwork [2/6] mtd: m25p80: Convert to device table matching

login
register
mail settings
Submitter Anton Vorontsov
Date July 31, 2009, 12:41 a.m.
Message ID <20090731004100.GB8371@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/30411/
State New, archived
Headers show

Comments

Anton Vorontsov - July 31, 2009, 12:41 a.m.
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(-)
David Brownell - Aug. 4, 2009, 2:54 a.m.
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;
Michael Barkowski - Aug. 12, 2009, 8:45 p.m.
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?
Anton Vorontsov - Aug. 12, 2009, 8:58 p.m.
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,
Michael Barkowski - Aug. 12, 2009, 8:58 p.m.
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
Anton Vorontsov - Aug. 18, 2009, 9:44 p.m.
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...
Andrew Morton - Sept. 22, 2009, 9:17 p.m.
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;
>
Anton Vorontsov - Sept. 22, 2009, 11:01 p.m.
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
David Woodhouse - Sept. 22, 2009, 11:43 p.m.
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)?
Andrew Morton - Sept. 22, 2009, 11:52 p.m.
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.
Anton Vorontsov - Sept. 22, 2009, 11:55 p.m.
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,
Andrew Morton - Sept. 23, 2009, 12:02 a.m.
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.

Patch

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),