On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov<[email protected]> wrote:>> Previosly the driver always tried JEDEC probing, assuming that non-JEDEC> chips will return '0'. But truly non-JEDEC chips (like CAT25) won't do> that, their behaviour on RDID command is undefined, so the driver should> not call jedec_probe() for these chips.>> Also, be less strict on error conditions, don't fail to probe if JEDEC> found a chip that is different from what platform code told, instead> just print some warnings and use an information obtained via JEDEC. InThis patch caused a problem:even though the external flash doesn't exist, it will still pass theprobe() and be registerred into kernel and given the partition table.You may refer to this bug report:http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975&start=0
> that case we should not trust partitions any longer, but they might be> still useful (i.e. they could protect some parts of the chip).>> Signed-off-by: Anton Vorontsov <[email protected]>> ---> drivers/mtd/devices/m25p80.c | 69 ++++++++++++++++++++++++-----------------> 1 files changed, 40 insertions(+), 29 deletions(-)>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c> index 0d74b38..b75e319 100644> --- a/drivers/mtd/devices/m25p80.c> +++ b/drivers/mtd/devices/m25p80.c> @@ -581,6 +581,14 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)> jedec = jedec << 8;> jedec |= id[2];>> + /*> + * Some chips (like Numonyx M25P80) have JEDEC and non-JEDEC variants,> + * which depend on technology process. Officially RDID command doesn't> + * exist for non-JEDEC chips, but for compatibility they return ID 0.> + */> + if (jedec == 0)> + return NULL;> +> ext_jedec = id[3] << 8 | id[4];>> for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) {> @@ -602,7 +610,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)> */> static int __devinit m25p_probe(struct spi_device *spi)> {> - const struct spi_device_id *id;> + const struct spi_device_id *id = spi_get_device_id(spi);> struct flash_platform_data *data;> struct m25p *flash;> struct flash_info *info;> @@ -615,41 +623,44 @@ static int __devinit m25p_probe(struct spi_device *spi)> */> data = spi->dev.platform_data;> if (data && data->type) {> + const struct spi_device_id *plat_id;> +> 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))> + plat_id = &m25p_ids[i];> + if (strcmp(data->type, plat_id->name))> continue;> break;> }>> - /* unrecognized chip? */> - 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) {> - id = jedec_probe(spi);> -> - if (id != &m25p_ids[i]) {> - dev_warn(&spi->dev, "found %s, expected %s\n",> - id ? id->name : "UNKNOWN",> - m25p_ids[i].name);> - info = NULL;> - }> - }> - } else {> - id = jedec_probe(spi);> - if (!id)> - id = spi_get_device_id(spi);> -> - info = (void *)id->driver_data;> + if (plat_id)> + id = plat_id;> + else> + dev_warn(&spi->dev, "unrecognized id %s\n", data->type);> }>> - if (!info)> - return -ENODEV;> + info = (void *)id->driver_data;> +> + if (info->jedec_id) {> + const struct spi_device_id *jid;> +> + jid = jedec_probe(spi);> + if (!jid) {> + dev_info(&spi->dev, "non-JEDEC variant of %s\n",> + id->name);> + } else if (jid != id) {> + /*> + * JEDEC knows better, so overwrite platform ID. We> + * can't trust partitions any longer, but we'll let> + * mtd apply them anyway, since some partitions may be> + * marked read-only, and we don't want to lose that> + * information, even if it's not 100% accurate.> + */> + dev_warn(&spi->dev, "found %s, expected %s\n",> + jid->name, id->name);> + id = jid;> + info = (void *)jid->driver_data;> + }> + }>> flash = kzalloc(sizeof *flash, GFP_KERNEL);> if (!flash)> --> 1.6.3.3>> --> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in> the body of a message to [email protected]> More majordomo info at http://vger.kernel.org/majordomo-info.html> Please read the FAQ at http://www.tux.org/lkml/????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Sat, Jun 12, 2010 at 02:27:12PM +0800, Barry Song wrote:
> On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov
> <[email protected]> wrote:
> >
> > Previosly the driver always tried JEDEC probing, assuming that non-JEDEC
> > chips will return '0'. But truly non-JEDEC chips (like CAT25) won't do
> > that, their behaviour on RDID command is undefined, so the driver should
> > not call jedec_probe() for these chips.
> >
> > Also, be less strict on error conditions, don't fail to probe if JEDEC
> > found a chip that is different from what platform code told, instead
> > just print some warnings and use an information obtained via JEDEC. In
> This patch caused a problem:
> even though the external flash doesn't exist, it will still pass the
> probe() and be registerred into kernel and given the partition table.
> You may refer to this bug report:
> http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975&start=0
Thanks for the report.
There's little we can do about it. Platform code asked us
to register the device, and JEDEC probing of M25Pxx chips isn't
reliable (thanks to various vendors that make these JEDEC and
non-JEDEC variants), so the best thing we can do is to register
the chip anyway.
OTOH, if the board pulls MISO line up, then the following patch
should help.
If this won't work, we'll have to add some flag to the platform
data, i.e. to force JEDEC probing, and not trust platform data.
Not-yet-Signed-off-by: Anton Vorontsov <[email protected]>
---
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 81e49a9..a307929 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -16,6 +16,7 @@
*/
#include <linux/init.h>
+#include <linux/errno.h>
#include <linux/module.h>
#include <linux/device.h>
#include <linux/interrupt.h>
@@ -723,7 +724,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
if (tmp < 0) {
DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
dev_name(&spi->dev), tmp);
- return NULL;
+ return ERR_PTR(tmp);
}
jedec = id[0];
jedec = jedec << 8;
@@ -737,7 +738,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
* exist for non-JEDEC chips, but for compatibility they return ID 0.
*/
if (jedec == 0)
- return NULL;
+ return ERR_PTR(-EEXIST);
ext_jedec = id[3] << 8 | id[4];
@@ -749,7 +750,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
return &m25p_ids[tmp];
}
}
- return NULL;
+ return ERR_PTR(-ENODEV);
}
@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct spi_device *spi)
const struct spi_device_id *jid;
jid = jedec_probe(spi);
- if (!jid) {
+ if (IS_ERR(jid) && PTR_ERR(jid) == -EEXIST) {
dev_info(&spi->dev, "non-JEDEC variant of %s\n",
id->name);
+ } else if (IS_ERR(jid)) {
+ return PTR_ERR(jid);
} else if (jid != id) {
/*
* JEDEC knows better, so overwrite platform ID. We
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On
>Behalf Of Anton Vorontsov
>Sent: Friday, June 18, 2010 9:32 PM
>To: Barry Song
>Cc: David Brownell; Artem Bityutskiy;
>[email protected]; [email protected];
>[email protected];
>[email protected]; Andrew Morton
>Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80:
>Reworkprobing/JEDEC code
>
>On Sat, Jun 12, 2010 at 02:27:12PM +0800, Barry Song wrote:
>> On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov
>> <[email protected]> wrote:
>> >
>> > Previosly the driver always tried JEDEC probing, assuming
>that non-JEDEC
>> > chips will return '0'. But truly non-JEDEC chips (like
>CAT25) won't do
>> > that, their behaviour on RDID command is undefined, so the
>driver should
>> > not call jedec_probe() for these chips.
>> >
>> > Also, be less strict on error conditions, don't fail to
>probe if JEDEC
>> > found a chip that is different from what platform code
>told, instead
>> > just print some warnings and use an information obtained
>via JEDEC. In
>> This patch caused a problem:
>> even though the external flash doesn't exist, it will still pass the
>> probe() and be registerred into kernel and given the partition table.
>> You may refer to this bug report:
>>
>http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?ac
>tion=TrackerItemEdit&tracker_item_id=5975&start=0
>
>Thanks for the report.
>
>There's little we can do about it. Platform code asked us
>to register the device, and JEDEC probing of M25Pxx chips isn't
>reliable (thanks to various vendors that make these JEDEC and
>non-JEDEC variants), so the best thing we can do is to register
>the chip anyway.
>
>OTOH, if the board pulls MISO line up, then the following patch
>should help.
Make sense with pullup to keep the value high while external device
doesn't exist.
>
>If this won't work, we'll have to add some flag to the platform
>data, i.e. to force JEDEC probing, and not trust platform data.
How about we add a non_jedec flag in platform_data, if the flag is 1, we
let the detection pass even though the ID is 0? Otherwise, we need a
valid ID?
>
>Not-yet-Signed-off-by: Anton Vorontsov <[email protected]>
>---
>
>diff --git a/drivers/mtd/devices/m25p80.c
>b/drivers/mtd/devices/m25p80.c
>index 81e49a9..a307929 100644
>--- a/drivers/mtd/devices/m25p80.c
>+++ b/drivers/mtd/devices/m25p80.c
>@@ -16,6 +16,7 @@
> */
>
> #include <linux/init.h>
>+#include <linux/errno.h>
> #include <linux/module.h>
> #include <linux/device.h>
> #include <linux/interrupt.h>
>@@ -723,7 +724,7 @@ static const struct spi_device_id
>*__devinit jedec_probe(struct spi_device *spi)
> if (tmp < 0) {
> DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading
>JEDEC ID\n",
> dev_name(&spi->dev), tmp);
>- return NULL;
>+ return ERR_PTR(tmp);
> }
> jedec = id[0];
> jedec = jedec << 8;
>@@ -737,7 +738,7 @@ static const struct spi_device_id
>*__devinit jedec_probe(struct spi_device *spi)
> * exist for non-JEDEC chips, but for compatibility
>they return ID 0.
> */
> if (jedec == 0)
>- return NULL;
>+ return ERR_PTR(-EEXIST);
>
> ext_jedec = id[3] << 8 | id[4];
>
>@@ -749,7 +750,7 @@ static const struct spi_device_id
>*__devinit jedec_probe(struct spi_device *spi)
> return &m25p_ids[tmp];
> }
> }
>- return NULL;
>+ return ERR_PTR(-ENODEV);
> }
>
>
>@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct
>spi_device *spi)
> const struct spi_device_id *jid;
>
> jid = jedec_probe(spi);
>- if (!jid) {
>+ if (IS_ERR(jid) && PTR_ERR(jid) == -EEXIST) {
> dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> id->name);
>+ } else if (IS_ERR(jid)) {
>+ return PTR_ERR(jid);
> } else if (jid != id) {
> /*
> * JEDEC knows better, so overwrite
>platform ID. We
>_______________________________________________
>Uclinux-dist-devel mailing list
>[email protected]
>https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>
On Mon, Jun 21, 2010 at 10:42 AM, Song, Barry <[email protected]> wrote:
>
>
>>-----Original Message-----
>>From: [email protected]
>>[mailto:[email protected]] On
>>Behalf Of Anton Vorontsov
>>Sent: Friday, June 18, 2010 9:32 PM
>>To: Barry Song
>>Cc: David Brownell; Artem Bityutskiy;
>>[email protected]; [email protected];
>>[email protected];
>>[email protected]; Andrew Morton
>>Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80:
>>Reworkprobing/JEDEC code
>>
>>On Sat, Jun 12, 2010 at 02:27:12PM +0800, Barry Song wrote:
>>> On Wed, Aug 19, 2009 at 5:46 AM, Anton Vorontsov
>>> <[email protected]> wrote:
>>> >
>>> > Previosly the driver always tried JEDEC probing, assuming
>>that non-JEDEC
>>> > chips will return '0'. But truly non-JEDEC chips (like
>>CAT25) won't do
>>> > that, their behaviour on RDID command is undefined, so the
>>driver should
>>> > not call jedec_probe() for these chips.
>>> >
>>> > Also, be less strict on error conditions, don't fail to
>>probe if JEDEC
>>> > found a chip that is different from what platform code
>>told, instead
>>> > just print some warnings and use an information obtained
>>via JEDEC. In
>>> This patch caused a problem:
>>> even though the external flash doesn't exist, it will still pass the
>>> probe() and be registerred into kernel and given the partition table.
>>> You may refer to this bug report:
>>>
>>http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?ac
>>tion=TrackerItemEdit&tracker_item_id=5975&start=0
>>
>>Thanks for the report.
>>
>>There's little we can do about it. Platform code asked us
>>to register the device, and JEDEC probing of M25Pxx chips isn't
>>reliable (thanks to various vendors that make these JEDEC and
>>non-JEDEC variants), so the best thing we can do is to register
>>the chip anyway.
>>
>>OTOH, if the board pulls MISO line up, then the following patch
>>should help.
> Make sense with pullup to keep the value high while external device
> doesn't exist.
>>
>>If this won't work, we'll have to add some flag to the platform
>>data, i.e. to force JEDEC probing, and not trust platform data.
>
> How about we add a non_jedec flag in platform_data, if the flag is 1, we
> let the detection pass even though the ID is 0? Otherwise, we need a
> valid ID?
Here i mean:
Index: drivers/mtd/devices/m25p80.c
===================================================================
--- drivers/mtd/devices/m25p80.c (revision 8927)
+++ drivers/mtd/devices/m25p80.c (revision 8929)
@@ -795,8 +795,13 @@
jid = jedec_probe(spi);
if (!jid) {
- dev_info(&spi->dev, "non-JEDEC variant of %s\n",
- id->name);
+ if (!data->non_jedec) {
+ dev_err(&spi->dev, "fail to detect%s\n",
+ id->name);
+ return -ENODEV;
+ } else
+ dev_info(&spi->dev, "non-JEDEC variant of %s\n",
+ id->name);
} else if (jid != id) {
/*
* JEDEC knows better, so overwrite platform ID. We
Index: include/linux/spi/flash.h
===================================================================
--- include/linux/spi/flash.h (revision 8927)
+++ include/linux/spi/flash.h (revision 8929)
@@ -25,6 +25,11 @@
char *type;
+ /*
+ * For non-JEDEC, id will be 0. In this case, we can't be sure
+ * whether the flash exists with runtime probing.
+ */
+ int non_jedec;
/* we'll likely add more ... use JEDEC IDs, etc */
};
>
>>
>>Not-yet-Signed-off-by: Anton Vorontsov <[email protected]>
>>---
>>
>>diff --git a/drivers/mtd/devices/m25p80.c
>>b/drivers/mtd/devices/m25p80.c
>>index 81e49a9..a307929 100644
>>--- a/drivers/mtd/devices/m25p80.c
>>+++ b/drivers/mtd/devices/m25p80.c
>>@@ -16,6 +16,7 @@
>> */
>>
>> #include <linux/init.h>
>>+#include <linux/errno.h>
>> #include <linux/module.h>
>> #include <linux/device.h>
>> #include <linux/interrupt.h>
>>@@ -723,7 +724,7 @@ static const struct spi_device_id
>>*__devinit jedec_probe(struct spi_device *spi)
>> if (tmp < 0) {
>> DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading
>>JEDEC ID\n",
>> dev_name(&spi->dev), tmp);
>>- return NULL;
>>+ return ERR_PTR(tmp);
>> }
>> jedec = id[0];
>> jedec = jedec << 8;
>>@@ -737,7 +738,7 @@ static const struct spi_device_id
>>*__devinit jedec_probe(struct spi_device *spi)
>> * exist for non-JEDEC chips, but for compatibility
>>they return ID 0.
>> */
>> if (jedec == 0)
>>- return NULL;
>>+ return ERR_PTR(-EEXIST);
>>
>> ext_jedec = id[3] << 8 | id[4];
>>
>>@@ -749,7 +750,7 @@ static const struct spi_device_id
>>*__devinit jedec_probe(struct spi_device *spi)
>> return &m25p_ids[tmp];
>> }
>> }
>>- return NULL;
>>+ return ERR_PTR(-ENODEV);
>> }
>>
>>
>>@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct
>>spi_device *spi)
>> const struct spi_device_id *jid;
>>
>> jid = jedec_probe(spi);
>>- if (!jid) {
>>+ if (IS_ERR(jid) && PTR_ERR(jid) == -EEXIST) {
>> dev_info(&spi->dev, "non-JEDEC variant of %s\n",
>> id->name);
>>+ } else if (IS_ERR(jid)) {
>>+ return PTR_ERR(jid);
>> } else if (jid != id) {
>> /*
>> * JEDEC knows better, so overwrite
>>platform ID. We
>>_______________________________________________
>>Uclinux-dist-devel mailing list
>>[email protected]
>>https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>>
>
On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
[...]
> > How about we add a non_jedec flag in platform_data, if the flag is 1, we
> > let the detection pass even though the ID is 0? Otherwise, we need a
> > valid ID?
> Here i mean:
This will break at least OF-enabled platforms (e.g. PowerPC),
they assume that the driver will success for non-JEDEC flashes.
OF platforms don't pass platform data, and even if they did,
device tree doesn't specify if the flash is JEDEC or non-JEDEC.
Which is why I think that, by default, the driver should
successfully register the flash even if JEDEC probe fails. So,
instead of checking for "!non_jedec", I would recommend
"force_jedec" check.
> Index: drivers/mtd/devices/m25p80.c
> ===================================================================
> --- drivers/mtd/devices/m25p80.c (revision 8927)
> +++ drivers/mtd/devices/m25p80.c (revision 8929)
> @@ -795,8 +795,13 @@
>
> jid = jedec_probe(spi);
> if (!jid) {
> - dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> - id->name);
> + if (!data->non_jedec) {
> + dev_err(&spi->dev, "fail to detect%s\n",
> + id->name);
> + return -ENODEV;
> + } else
> + dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> + id->name);
> } else if (jid != id) {
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <[email protected]> wrote:> On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:> [...]>> > How about we add a non_jedec flag in platform_data, if the flag is 1, we>> > let the detection pass even though the ID is 0? Otherwise, we need a>> > valid ID?>> Here i mean:>> This will break at least OF-enabled platforms (e.g. PowerPC),> they assume that the driver will success for non-JEDEC flashes.> OF platforms don't pass platform data, and even if they did,> device tree doesn't specify if the flash is JEDEC or non-JEDEC.>> Which is why I think that, by default, the driver should> successfully register the flash even if JEDEC probe fails. So,> instead of checking for "!non_jedec", I would recommend> "force_jedec" check.
Mike Frysinger suggested to use non_jedec since most devices arestandard jedec devices. Only if non_jedec=1, we let the detection passif ID is 0.
>>> Index: drivers/mtd/devices/m25p80.c>> ===================================================================>> --- drivers/mtd/devices/m25p80.c (revision 8927)>> +++ drivers/mtd/devices/m25p80.c (revision 8929)>> @@ -795,8 +795,13 @@>>>> jid = jedec_probe(spi);>> if (!jid) {>> - dev_info(&spi->dev, "non-JEDEC variant of %s\n",>> - id->name);>> + if (!data->non_jedec) {>> + dev_err(&spi->dev, "fail to detect%s\n",>> + id->name);>> + return -ENODEV;>> + } else>> + dev_info(&spi->dev, "non-JEDEC variant of %s\n",>> + id->name);>> } else if (jid != id) {>> --> Anton Vorontsov> email: [email protected]> irc://irc.freenode.net/bd2>????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Mon, Jun 21, 2010 at 03:22:48PM +0800, Barry Song wrote:
> On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <[email protected]> wrote:
> > On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
> > [...]
> >> > How about we add a non_jedec flag in platform_data, if the flag is 1, we
> >> > let the detection pass even though the ID is 0? Otherwise, we need a
> >> > valid ID?
> >> Here i mean:
> >
> > This will break at least OF-enabled platforms (e.g. PowerPC),
> > they assume that the driver will success for non-JEDEC flashes.
> > OF platforms don't pass platform data, and even if they did,
> > device tree doesn't specify if the flash is JEDEC or non-JEDEC.
> >
> > Which is why I think that, by default, the driver should
> > successfully register the flash even if JEDEC probe fails. So,
> > instead of checking for "!non_jedec", I would recommend
> > "force_jedec" check.
>
> Mike Frysinger suggested to use non_jedec since most devices are
> standard jedec devices.
Well, on OF platforms most devices that I'm aware of are non-JEDEC.
> Only if non_jedec=1, we let the detection pass
> if ID is 0.
Then please #ifdef it with CONFIG_OF.
Thanks,
> >> Index: drivers/mtd/devices/m25p80.c
> >> ===================================================================
> >> --- drivers/mtd/devices/m25p80.c (revision 8927)
> >> +++ drivers/mtd/devices/m25p80.c (revision 8929)
> >> @@ -795,8 +795,13 @@
> >>
> >> jid = jedec_probe(spi);
> >> if (!jid) {
> >> - dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> >> - id->name);
> >> + if (!data->non_jedec) {
> >> + dev_err(&spi->dev, "fail to detect%s\n",
> >> + id->name);
> >> + return -ENODEV;
> >> + } else
> >> + dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> >> + id->name);
> >> } else if (jid != id) {
> >
> > --
> > Anton Vorontsov
> > email: [email protected]
> > irc://irc.freenode.net/bd2
> >
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Mon, Jun 21, 2010 at 3:39 PM, Anton Vorontsov <[email protected]> wrote:
> On Mon, Jun 21, 2010 at 03:22:48PM +0800, Barry Song wrote:
>> On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <[email protected]> wrote:
>> > On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
>> > [...]
>> >> > How about we add a non_jedec flag in platform_data, if the flag is 1, we
>> >> > let the detection pass even though the ID is 0? Otherwise, we need a
>> >> > valid ID?
>> >> Here i mean:
>> >
>> > This will break at least OF-enabled platforms (e.g. PowerPC),
>> > they assume that the driver will success for non-JEDEC flashes.
>> > OF platforms don't pass platform data, and even if they did,
>> > device tree doesn't specify if the flash is JEDEC or non-JEDEC.
>> >
>> > Which is why I think that, by default, the driver should
>> > successfully register the flash even if JEDEC probe fails. So,
>> > instead of checking for "!non_jedec", I would recommend
>> > "force_jedec" check.
>>
>> Mike Frysinger suggested to use non_jedec since most devices are
>> standard jedec devices.
>
> Well, on OF platforms most devices that I'm aware of are non-JEDEC.
>
>> Only if non_jedec=1, we let the detection pass
>> if ID is 0.
>
> Then please #ifdef it with CONFIG_OF.
I think the patch has nothing to do with platform. Here SPI Flash is a
peripherals, doesn't depend on any platform. Adding a CONFIG_OF
doesn't make sense very much.
If you think most devices are non-JEDEC, we can change non_JEDEC to
force_JEDEC as you said. But anyway, is that real that most devices
are non_JEDEC? If not, I think we should change OF platform codes to
fit with this patch.
>
> Thanks,
>
>> >> Index: drivers/mtd/devices/m25p80.c
>> >> ===================================================================
>> >> --- drivers/mtd/devices/m25p80.c (revision 8927)
>> >> +++ drivers/mtd/devices/m25p80.c (revision 8929)
>> >> @@ -795,8 +795,13 @@
>> >>
>> >> jid = jedec_probe(spi);
>> >> if (!jid) {
>> >> - dev_info(&spi->dev, "non-JEDEC variant of %s\n",
>> >> - id->name);
>> >> + if (!data->non_jedec) {
>> >> + dev_err(&spi->dev, "fail to detect%s\n",
>> >> + id->name);
>> >> + return -ENODEV;
>> >> + } else
>> >> + dev_info(&spi->dev, "non-JEDEC variant of %s\n",
>> >> + id->name);
>> >> } else if (jid != id) {
>> >
>> > --
>> > Anton Vorontsov
>> > email: [email protected]
>> > irc://irc.freenode.net/bd2
>> >
>
> --
> Anton Vorontsov
> email: [email protected]
> irc://irc.freenode.net/bd2
>
On Mon, Jun 21, 2010 at 06:31:44PM +0800, Barry Song wrote:
> On Mon, Jun 21, 2010 at 3:39 PM, Anton Vorontsov <[email protected]> wrote:
> > On Mon, Jun 21, 2010 at 03:22:48PM +0800, Barry Song wrote:
> >> On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <[email protected]> wrote:
> >> > On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
> >> > [...]
> >> >> > How about we add a non_jedec flag in platform_data, if the flag is 1, we
> >> >> > let the detection pass even though the ID is 0? Otherwise, we need a
> >> >> > valid ID?
> >> >> Here i mean:
> >> >
> >> > This will break at least OF-enabled platforms (e.g. PowerPC),
> >> > they assume that the driver will success for non-JEDEC flashes.
> >> > OF platforms don't pass platform data, and even if they did,
> >> > device tree doesn't specify if the flash is JEDEC or non-JEDEC.
> >> >
> >> > Which is why I think that, by default, the driver should
> >> > successfully register the flash even if JEDEC probe fails. So,
> >> > instead of checking for "!non_jedec", I would recommend
> >> > "force_jedec" check.
> >>
> >> Mike Frysinger suggested to use non_jedec since most devices are
> >> standard jedec devices.
> >
> > Well, on OF platforms most devices that I'm aware of are non-JEDEC.
> >
> >> Only if non_jedec=1, we let the detection pass
> >> if ID is 0.
> >
> > Then please #ifdef it with CONFIG_OF.
> I think the patch has nothing to do with platform. Here SPI Flash is a
> peripherals, doesn't depend on any platform. Adding a CONFIG_OF
> doesn't make sense very much.
With OF we don't place non-existent devices into the device
tree (or we mark them with status = "not-ok/disabled/absent"
property).
> If you think most devices are non-JEDEC, we can change non_JEDEC to
> force_JEDEC as you said.
> But anyway, is that real that most devices are non_JEDEC?
Why would this matter? We have to support both.
> If not, I think we should change OF platform codes to
> fit with this patch.
You can't easily change OF. It's like "let's change ACPI tables
or BIOS in these PCs". Doable, but involves things like reflashing.
And we usually have to support old BIOSes as well.
OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in
mainline kernel only MPC8569 board has a correct m25p
node, and it is STMicro variant (it is JEDEC capable).
As we don't really have to support out of tree code, I'd
just go with this patch, assuming that we have to change
device tree for boards with non-JEDEC flashes. It's
effectively the same thing as platform data flag, except
that it works automatically for OF platforms.
Signed-off-by: Anton Vorontsov <[email protected]>
---
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 81e49a9..a610ca9 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
{ "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) },
{ "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
+ { "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
+ { "m25p10-nonjedec", INFO(0, 0, 32 * 1024, 4, 0) },
+ { "m25p20-nonjedec", INFO(0, 0, 64 * 1024, 4, 0) },
+ { "m25p40-nonjedec", INFO(0, 0, 64 * 1024, 8, 0) },
+ { "m25p80-nonjedec", INFO(0, 0, 64 * 1024, 16, 0) },
+ { "m25p16-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
+ { "m25p32-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
+ { "m25p64-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
+ { "m25p128-nonjedec", INFO(0, 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) },
@@ -795,8 +805,7 @@ static int __devinit m25p_probe(struct spi_device *spi)
jid = jedec_probe(spi);
if (!jid) {
- dev_info(&spi->dev, "non-JEDEC variant of %s\n",
- id->name);
+ return -ENODEV;
} else if (jid != id) {
/*
* JEDEC knows better, so overwrite platform ID. We
On Mon, Jun 21, 2010 at 07:20, Anton Vorontsov wrote:
> You can't easily change OF. It's like "let's change ACPI tables
> or BIOS in these PCs". Doable, but involves things like reflashing.
> And we usually have to support old BIOSes as well.
>
> OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in
> mainline kernel only MPC8569 board has a correct m25p
> node, and it is STMicro variant (it is JEDEC capable).
>
> As we don't really have to support out of tree code, I'd
> just go with this patch, assuming that we have to change
> device tree for boards with non-JEDEC flashes. It's
> effectively the same thing as platform data flag, except
> that it works automatically for OF platforms.
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 81e49a9..a610ca9 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
> { "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) },
> { "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
>
> + { "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
> + { "m25p10-nonjedec", INFO(0, 0, 32 * 1024, 4, 0) },
> + { "m25p20-nonjedec", INFO(0, 0, 64 * 1024, 4, 0) },
> + { "m25p40-nonjedec", INFO(0, 0, 64 * 1024, 8, 0) },
> + { "m25p80-nonjedec", INFO(0, 0, 64 * 1024, 16, 0) },
> + { "m25p16-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
> + { "m25p32-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
> + { "m25p64-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
> + { "m25p128-nonjedec", INFO(0, 0, 256 * 1024, 64, 0) },
> +
are you picking the m25p because its flash geometry matches whatever
you're using, or because you have some weird variant of the m25p that
has JEDEC commands removed ?
-mike
On Mon, Jun 21, 2010 at 12:34:05PM -0400, Mike Frysinger wrote:
> On Mon, Jun 21, 2010 at 07:20, Anton Vorontsov wrote:
> > You can't easily change OF. It's like "let's change ACPI tables
> > or BIOS in these PCs". Doable, but involves things like reflashing.
> > And we usually have to support old BIOSes as well.
> >
> > OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in
> > mainline kernel only MPC8569 board has a correct m25p
> > node, and it is STMicro variant (it is JEDEC capable).
> >
> > As we don't really have to support out of tree code, I'd
> > just go with this patch, assuming that we have to change
> > device tree for boards with non-JEDEC flashes. It's
> > effectively the same thing as platform data flag, except
> > that it works automatically for OF platforms.
> >
> > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> > index 81e49a9..a610ca9 100644
> > --- a/drivers/mtd/devices/m25p80.c
> > +++ b/drivers/mtd/devices/m25p80.c
> > @@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
> > { "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) },
> > { "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
> >
> > + { "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
> > + { "m25p10-nonjedec", INFO(0, 0, 32 * 1024, 4, 0) },
> > + { "m25p20-nonjedec", INFO(0, 0, 64 * 1024, 4, 0) },
> > + { "m25p40-nonjedec", INFO(0, 0, 64 * 1024, 8, 0) },
> > + { "m25p80-nonjedec", INFO(0, 0, 64 * 1024, 16, 0) },
> > + { "m25p16-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
> > + { "m25p32-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
> > + { "m25p64-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
> > + { "m25p128-nonjedec", INFO(0, 0, 256 * 1024, 64, 0) },
> > +
>
> are you picking the m25p because its flash geometry matches whatever
> you're using, or because you have some weird variant of the m25p that
> has JEDEC commands removed ?
The latter. It's Numonyx M25Pxx flashes, see
http://www.numonyx.com/Documents/Datasheets/M25P80.pdf
The RDID instruction is available only for parts made with 110
nm Technology identified with Process letter '4'.
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Mon, Jun 21, 2010 at 12:47, Anton Vorontsov wrote:
> On Mon, Jun 21, 2010 at 12:34:05PM -0400, Mike Frysinger wrote:
>> On Mon, Jun 21, 2010 at 07:20, Anton Vorontsov wrote:
>> > You can't easily change OF. It's like "let's change ACPI tables
>> > or BIOS in these PCs". Doable, but involves things like reflashing.
>> > And we usually have to support old BIOSes as well.
>> >
>> > OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in
>> > mainline kernel only MPC8569 board has a correct m25p
>> > node, and it is STMicro variant (it is JEDEC capable).
>> >
>> > As we don't really have to support out of tree code, I'd
>> > just go with this patch, assuming that we have to change
>> > device tree for boards with non-JEDEC flashes. It's
>> > effectively the same thing as platform data flag, except
>> > that it works automatically for OF platforms.
>> >
>> > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> > index 81e49a9..a610ca9 100644
>> > --- a/drivers/mtd/devices/m25p80.c
>> > +++ b/drivers/mtd/devices/m25p80.c
>> > @@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
>> > { "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) },
>> > { "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
>> >
>> > + { "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
>> > + { "m25p10-nonjedec", INFO(0, 0, 32 * 1024, 4, 0) },
>> > + { "m25p20-nonjedec", INFO(0, 0, 64 * 1024, 4, 0) },
>> > + { "m25p40-nonjedec", INFO(0, 0, 64 * 1024, 8, 0) },
>> > + { "m25p80-nonjedec", INFO(0, 0, 64 * 1024, 16, 0) },
>> > + { "m25p16-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
>> > + { "m25p32-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
>> > + { "m25p64-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
>> > + { "m25p128-nonjedec", INFO(0, 0, 256 * 1024, 64, 0) },
>> > +
>>
>> are you picking the m25p because its flash geometry matches whatever
>> you're using, or because you have some weird variant of the m25p that
>> has JEDEC commands removed ?
>
> The latter. It's Numonyx M25Pxx flashes, see
> http://www.numonyx.com/Documents/Datasheets/M25P80.pdf
>
> The RDID instruction is available only for parts made with 110
> nm Technology identified with Process letter '4'.
lovely. i guess this patch is the way to go to satisfy everyone's
requirements. i'm also of the mindset that a mtd should not be
created if the SPI flash isnt there simply because the resources say
it might be.
-mike
On Mon, Jun 21, 2010 at 7:20 PM, Anton Vorontsov <[email protected]> wrote:
> On Mon, Jun 21, 2010 at 06:31:44PM +0800, Barry Song wrote:
>> On Mon, Jun 21, 2010 at 3:39 PM, Anton Vorontsov <[email protected]> wrote:
>> > On Mon, Jun 21, 2010 at 03:22:48PM +0800, Barry Song wrote:
>> >> On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <[email protected]> wrote:
>> >> > On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
>> >> > [...]
>> >> >> > How about we add a non_jedec flag in platform_data, if the flag is 1, we
>> >> >> > let the detection pass even though the ID is 0? Otherwise, we need a
>> >> >> > valid ID?
>> >> >> Here i mean:
>> >> >
>> >> > This will break at least OF-enabled platforms (e.g. PowerPC),
>> >> > they assume that the driver will success for non-JEDEC flashes.
>> >> > OF platforms don't pass platform data, and even if they did,
>> >> > device tree doesn't specify if the flash is JEDEC or non-JEDEC.
>> >> >
>> >> > Which is why I think that, by default, the driver should
>> >> > successfully register the flash even if JEDEC probe fails. So,
>> >> > instead of checking for "!non_jedec", I would recommend
>> >> > "force_jedec" check.
>> >>
>> >> Mike Frysinger suggested to use non_jedec since most devices are
>> >> standard jedec devices.
>> >
>> > Well, on OF platforms most devices that I'm aware of are non-JEDEC.
>> >
>> >> Only if non_jedec=1, we let the detection pass
>> >> if ID is 0.
>> >
>> > Then please #ifdef it with CONFIG_OF.
>> I think the patch has nothing to do with platform. Here SPI Flash is a
>> peripherals, doesn't depend on any platform. Adding a CONFIG_OF
>> doesn't make sense very much.
>
> With OF we don't place non-existent devices into the device
> tree (or we mark them with status = "not-ok/disabled/absent"
> property).
>
>> If you think most devices are non-JEDEC, we can change non_JEDEC to
>> force_JEDEC as you said.
>> But anyway, is that real that most devices are non_JEDEC?
>
> Why would this matter? We have to support both.
>
>> If not, I think we should change OF platform codes to
>> fit with this patch.
>
> You can't easily change OF. It's like "let's change ACPI tables
> or BIOS in these PCs". Doable, but involves things like reflashing.
> And we usually have to support old BIOSes as well.
>
> OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in
> mainline kernel only MPC8569 board has a correct m25p
> node, and it is STMicro variant (it is JEDEC capable).
>
> As we don't really have to support out of tree code, I'd
> just go with this patch, assuming that we have to change
> device tree for boards with non-JEDEC flashes. It's
> effectively the same thing as platform data flag, except
> that it works automatically for OF platforms.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 81e49a9..a610ca9 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
> { "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) },
> { "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
>
> + { "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
> + { "m25p10-nonjedec", INFO(0, 0, 32 * 1024, 4, 0) },
> + { "m25p20-nonjedec", INFO(0, 0, 64 * 1024, 4, 0) },
> + { "m25p40-nonjedec", INFO(0, 0, 64 * 1024, 8, 0) },
> + { "m25p80-nonjedec", INFO(0, 0, 64 * 1024, 16, 0) },
> + { "m25p16-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
> + { "m25p32-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
> + { "m25p64-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
> + { "m25p128-nonjedec", INFO(0, 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) },
> @@ -795,8 +805,7 @@ static int __devinit m25p_probe(struct spi_device *spi)
>
> jid = jedec_probe(spi);
> if (!jid) {
> - dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> - id->name);
> + return -ENODEV;
The patch looks good to me. Only problem is NULL is also returned by
spi_write_then_read() fail:
static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
{
int tmp;
u8 code = OPCODE_RDID;
u8 id[5];
u32 jedec;
u16 ext_jedec;
struct flash_info *info;
/* JEDEC also defines an optional "extended device information"
* string for after vendor-specific data, after the three bytes
* we use here. Supporting some chips might require using it.
*/
tmp = spi_write_then_read(spi, &code, 1, id, 5);
if (tmp < 0) {
DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
dev_name(&spi->dev), tmp);
return NULL;
}
...
}
Here much better for -EIO (return tmp)?
> } else if (jid != id) {
> /*
> * JEDEC knows better, so overwrite platform ID. We
>
On Tue, Jun 22, 2010 at 02:37:52PM +0800, Barry Song wrote:
[...]
> > jid = jedec_probe(spi);
> > if (!jid) {
> > - dev_info(&spi->dev, "non-JEDEC variant of %s\n",
> > - id->name);
> > + return -ENODEV;
> The patch looks good to me. Only problem is NULL is also returned by
> spi_write_then_read() fail:
[...]
> Here much better for -EIO (return tmp)?
Agreed. Though, this is not a regression, and I guess desires
its own patch.
Here are two patches, one for 2.6.35 (minimal changes to fix
the JEDEC problem), another for 2.6.36.
spi_write_then_read() may return its own return codes (e.g. -EIO),
so let's propagate the value down to the probe().
Also, remove jedec == 0 check, it isn't needed as nowadays we use
dedicated SPI device IDs for non-JEDEC flashes.
Suggested-by: Barry Song <[email protected]>
Signed-off-by: Anton Vorontsov <[email protected]>
---
This is for 2.6.36.
drivers/mtd/devices/m25p80.c | 18 ++++++------------
1 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index a610ca9..7bf5c45 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -16,6 +16,8 @@
*/
#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/errno.h>
#include <linux/module.h>
#include <linux/device.h>
#include <linux/interrupt.h>
@@ -733,7 +735,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
if (tmp < 0) {
DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
dev_name(&spi->dev), tmp);
- return NULL;
+ return ERR_PTR(tmp);
}
jedec = id[0];
jedec = jedec << 8;
@@ -741,14 +743,6 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
jedec = jedec << 8;
jedec |= id[2];
- /*
- * Some chips (like Numonyx M25P80) have JEDEC and non-JEDEC variants,
- * which depend on technology process. Officially RDID command doesn't
- * exist for non-JEDEC chips, but for compatibility they return ID 0.
- */
- if (jedec == 0)
- return NULL;
-
ext_jedec = id[3] << 8 | id[4];
for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) {
@@ -759,7 +753,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
return &m25p_ids[tmp];
}
}
- return NULL;
+ return ERR_PTR(-ENODEV);
}
@@ -804,8 +798,8 @@ static int __devinit m25p_probe(struct spi_device *spi)
const struct spi_device_id *jid;
jid = jedec_probe(spi);
- if (!jid) {
- return -ENODEV;
+ if (IS_ERR(jid)) {
+ return PTR_ERR(jid);
} else if (jid != id) {
/*
* JEDEC knows better, so overwrite platform ID. We
--
1.7.0.5
Since commit 18c6182bae0acca220ed6611f741034d563cd19f ("Rework
probing/JEDEC code"), m25p80 driver successfully registers chips
even if JEDEC probing fails.
This was needed to support non-JEDEC flashes. Though, it appears
that some platforms (e.g. blackfin bf533 stamp[1]) used the old
behavior to detect if there's any flash connected, so the driver
have to fail on JEDEC probing errors.
This patch restores the old behavior for JEDEC flashes, and adds
"-nonjedec" SPI device IDs for M25Pxx flashes, so that the kernel
still supports non-JEDEC flashes.
[1] http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975
Reported-by: Mingquan Pan
Reported-by: Barry Song <[email protected]>
Signed-off-by: Anton Vorontsov <[email protected]>
---
This is for 2.6.35.
drivers/mtd/devices/m25p80.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 81e49a9..a610ca9 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
{ "m25p64", INFO(0x202017, 0, 64 * 1024, 128, 0) },
{ "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
+ { "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
+ { "m25p10-nonjedec", INFO(0, 0, 32 * 1024, 4, 0) },
+ { "m25p20-nonjedec", INFO(0, 0, 64 * 1024, 4, 0) },
+ { "m25p40-nonjedec", INFO(0, 0, 64 * 1024, 8, 0) },
+ { "m25p80-nonjedec", INFO(0, 0, 64 * 1024, 16, 0) },
+ { "m25p16-nonjedec", INFO(0, 0, 64 * 1024, 32, 0) },
+ { "m25p32-nonjedec", INFO(0, 0, 64 * 1024, 64, 0) },
+ { "m25p64-nonjedec", INFO(0, 0, 64 * 1024, 128, 0) },
+ { "m25p128-nonjedec", INFO(0, 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) },
@@ -795,8 +805,7 @@ static int __devinit m25p_probe(struct spi_device *spi)
jid = jedec_probe(spi);
if (!jid) {
- dev_info(&spi->dev, "non-JEDEC variant of %s\n",
- id->name);
+ return -ENODEV;
} else if (jid != id) {
/*
* JEDEC knows better, so overwrite platform ID. We
--
1.7.0.5
On Tue, Jun 22, 2010 at 12:57, Anton Vorontsov wrote:
> Since commit 18c6182bae0acca220ed6611f741034d563cd19f ("Rework
> probing/JEDEC code"), m25p80 driver successfully registers chips
> even if JEDEC probing fails.
>
> This was needed to support non-JEDEC flashes. Though, it appears
> that some platforms (e.g. blackfin bf533 stamp[1]) used the old
> behavior to detect if there's any flash connected, so the driver
> have to fail on JEDEC probing errors.
>
> This patch restores the old behavior for JEDEC flashes, and adds
> "-nonjedec" SPI device IDs for M25Pxx flashes, so that the kernel
> still supports non-JEDEC flashes.
Acked-by: Mike Frysinger <[email protected]>
> [1] http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975
http://blackfin.uclinux.org/gf/tracker/5975
> Reported-by: Mingquan Pan
<[email protected]>
-mike
On Tue, 2010-06-22 at 20:57 +0400, Anton Vorontsov wrote:
> Since commit 18c6182bae0acca220ed6611f741034d563cd19f ("Rework
> probing/JEDEC code"), m25p80 driver successfully registers chips
> even if JEDEC probing fails.
>
> This was needed to support non-JEDEC flashes. Though, it appears
> that some platforms (e.g. blackfin bf533 stamp[1]) used the old
> behavior to detect if there's any flash connected, so the driver
> have to fail on JEDEC probing errors.
>
> This patch restores the old behavior for JEDEC flashes, and adds
> "-nonjedec" SPI device IDs for M25Pxx flashes, so that the kernel
> still supports non-JEDEC flashes.
>
> [1] http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975
>
> Reported-by: Mingquan Pan
> Reported-by: Barry Song <[email protected]>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
Pushed both patches to my l2-mtd-2.6.git / dunno, added Mike's ack.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)