From patchwork Mon Jun 21 03:27:31 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Barry Song <21cnbao@gmail.com> X-Patchwork-Id: 56283 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 7EFCB1007D4 for ; Mon, 21 Jun 2010 13:29:32 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1OQXfx-0000bh-5e; Mon, 21 Jun 2010 03:27:57 +0000 Received: from mail-gw0-f49.google.com ([74.125.83.49]) by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1OQXfu-0000ab-MP for linux-mtd@lists.infradead.org; Mon, 21 Jun 2010 03:27:56 +0000 Received: by gwb19 with SMTP id 19so2180877gwb.36 for ; Sun, 20 Jun 2010 20:27:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:mime-version:received:in-reply-to :references:from:date:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=ukD+hxU4g9+xJ90M57z4XWSDPNperkf81lOeuLu8f6I=; b=KIbQVGtSYTuWJ2bJwDZDRfsMTiXoB63SCmgcoB9KE/h/JbOYSTv35qoNxDI6/mxWGK 5Uv553hDRrabxTyOqwmbO+vRLKKuOayQ/uy25AQnLhE/FQlHVwnkTr1NZmpEtqOTeyk3 vmY2fbCvIsafBZavAK1VkIxjCErjELXGpc7iM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=rdL+hKkyWXw1KGayk7xyJjajtMkHaOFCVlLdY6D6AQuVz3GArEj8tdeXHzarUtoH39 B94cH+ZZ+xkbdueKldPPPPpPFbezbnHeJnhLmS8oI4w3riyVUjr47XOp3IDltwNBpgsU byoCxc6EbBOjUtS0o2b9hsRNCGy9sGnJaq2N4= Received: by 10.91.145.13 with SMTP id x13mr2526810agn.16.1277090872174; Sun, 20 Jun 2010 20:27:52 -0700 (PDT) MIME-Version: 1.0 Received: by 10.90.56.20 with HTTP; Sun, 20 Jun 2010 20:27:31 -0700 (PDT) In-Reply-To: <0F1B54C89D5F954D8535DB252AF412FA065551D8@chinexm1.ad.analog.com> References: <20090818214449.GA12848@oksana.dev.rtsoft.ru> <20090818214622.GA22651@oksana.dev.rtsoft.ru> <20100618133212.GA5276@oksana.dev.rtsoft.ru> <0F1B54C89D5F954D8535DB252AF412FA065551D8@chinexm1.ad.analog.com> From: Barry Song <21cnbao@gmail.com> Date: Mon, 21 Jun 2010 11:27:31 +0800 Message-ID: Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code To: "Song, Barry" X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20100620_232754_993649_EDA7C5F1 X-CRM114-Status: GOOD ( 26.66 ) X-Spam-Score: -0.1 (/) X-Spam-Report: SpamAssassin version 3.3.1 on bombadil.infradead.org summary: Content analysis details: (-0.1 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 FREEMAIL_FROM Sender email is freemail (21cnbao[at]gmail.com) -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, low trust [74.125.83.49 listed in list.dnswl.org] 0.0 HS_INDEX_PARAM URI: Link contains a common tracker pattern. -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: David Brownell , Artem Bityutskiy , linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org, linux-mtd@lists.infradead.org, Anton Vorontsov , uclinux-dist-devel@blackfin.uclinux.org, Andrew Morton X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Mon, Jun 21, 2010 at 10:42 AM, Song, Barry wrote: > > >>-----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 >>> 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: > >> >>Not-yet-Signed-off-by: Anton Vorontsov >>--- >> >>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 >>+#include >> #include >> #include >> #include >>@@ -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 >> > 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 */ };