| Message ID | 20100618133212.GA5276@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
>-----Original Message----- >From: uclinux-dist-devel-bounces@blackfin.uclinux.org >[mailto:uclinux-dist-devel-bounces@blackfin.uclinux.org] On >Behalf Of Anton Vorontsov >Sent: Friday, June 18, 2010 9:32 PM >To: Barry Song >Cc: David Brownell; Artem Bityutskiy; >linux-kernel@vger.kernel.org; linuxppc-dev@ozlabs.org; >linux-mtd@lists.infradead.org; >uclinux-dist-devel@blackfin.uclinux.org; 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 >> <avorontsov@ru.mvista.com> 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 <cbouatmailru@gmail.com> >--- > >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 >Uclinux-dist-devel@blackfin.uclinux.org >https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel >
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