From patchwork Fri Jun 18 13:32:12 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 56182 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 5F56D1007D7 for ; Fri, 18 Jun 2010 23:34:34 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1OPbgJ-0003Mu-At; Fri, 18 Jun 2010 13:32:27 +0000 Received: from mail-wy0-f177.google.com ([74.125.82.177]) by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1OPbgH-0003CH-3k for linux-mtd@lists.infradead.org; Fri, 18 Jun 2010 13:32:25 +0000 Received: by wyg36 with SMTP id 36so736945wyg.36 for ; Fri, 18 Jun 2010 06:32:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:references:mime-version:content-type:content-disposition :in-reply-to:user-agent; bh=XGGA+2ovcfaSVsU3Vf9lVRZTol5JVqmuHHh0GPHbQuo=; b=MQLLAVesTy6K7SWYqN0oyy9JF0obvw91+T9AU9InVReTsYiZ+/12rlt7GHpOZwl/uO QmW2GXCmuASakhMgeYUXa0YzIaNzZl2r+1heXlWs7FVaFtUQSu9/pVnZ4LOFtLDivHoT kmbY1wiGVvp80VOATRe6JTto6n/MZz2C4bjCk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=EfLmngI9/hm5to2KZWYU7L5OjzDJQhE+vV2mdLR9AyR0pHU3xYO0MzdhZ14gPrg5M5 9NBtOEFDlfq6f4EVDSf7r1Ljc+y/MmoDpIdQwwy6fdX4aqLwyMdNxqxbNJ/mxZyK4ArS bU40t6E3hpRPRQvekUvvoPlVHg+RqSSwMvwxg= Received: by 10.227.156.84 with SMTP id v20mr1016390wbw.191.1276867939545; Fri, 18 Jun 2010 06:32:19 -0700 (PDT) Received: from localhost (mail.dev.rtsoft.ru [213.79.90.226]) by mx.google.com with ESMTPS id p17sm292286wbe.14.2010.06.18.06.32.14 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 18 Jun 2010 06:32:15 -0700 (PDT) Date: Fri, 18 Jun 2010 17:32:12 +0400 From: Anton Vorontsov To: Barry Song <21cnbao@gmail.com> Subject: Re: [PATCH 1/2] mtd: m25p80: Rework probing/JEDEC code Message-ID: <20100618133212.GA5276@oksana.dev.rtsoft.ru> References: <20090818214449.GA12848@oksana.dev.rtsoft.ru> <20090818214622.GA22651@oksana.dev.rtsoft.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20100618_093225_277978_63DEB51B X-CRM114-Status: GOOD ( 27.02 ) 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 (cbouatmailru[at]gmail.com) -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, uclinux-dist-devel@blackfin.uclinux.org, Andrew Morton , David Woodhouse 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 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/?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 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