| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AANLkTiksYofs6hhWta6uFX2cla_g8pBu0Mtbtcbv-2tl@mail.gmail.com>
Date: Mon, 21 Jun 2010 11:27:31 +0800
From: Barry Song <21cnbao@...il.com>
To: "Song, Barry" <Barry.Song@...log.com>
Cc: Anton Vorontsov <cbouatmailru@...il.com>,
David Brownell <dbrownell@...rs.sourceforge.net>,
Artem Bityutskiy <dedekind1@...il.com>,
linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org,
linux-mtd@...ts.infradead.org,
uclinux-dist-devel@...ckfin.uclinux.org,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC
code
On Mon, Jun 21, 2010 at 10:42 AM, Song, Barry <Barry.Song@...log.com> wrote:
>
>
>>-----Original Message-----
>>From: uclinux-dist-devel-bounces@...ckfin.uclinux.org
>>[mailto:uclinux-dist-devel-bounces@...ckfin.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@...r.kernel.org; linuxppc-dev@...abs.org;
>>linux-mtd@...ts.infradead.org;
>>uclinux-dist-devel@...ckfin.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@...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?
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 <cbouatmailru@...il.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@...ckfin.uclinux.org
>>https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists