From patchwork Sat Nov 1 06:33:10 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 6772 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@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 C59E4DDDDB for ; Sat, 1 Nov 2008 17:39:56 +1100 (EST) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.68 #1 (Red Hat Linux)) id 1KwA4a-0004AL-6m; Sat, 01 Nov 2008 06:35:00 +0000 Received: from out01.mta.xmission.com ([166.70.13.231]) by bombadil.infradead.org with esmtp (Exim 4.68 #1 (Red Hat Linux)) id 1KwA4X-0004AB-FP for linux-mtd@lists.infradead.org; Sat, 01 Nov 2008 06:34:58 +0000 Received: from mx04.mta.xmission.com ([166.70.13.214]) by out01.mta.xmission.com with esmtp (Exim 4.62) (envelope-from ) id 1KwA4W-0005sv-WF; Sat, 01 Nov 2008 00:34:57 -0600 Received: from c-24-130-11-59.hsd1.ca.comcast.net ([24.130.11.59] helo=frodo.ebiederm.org) by mx04.mta.xmission.com with esmtpsa (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1KwA4S-0006Yq-1h; Sat, 01 Nov 2008 00:34:53 -0600 Received: from frodo.ebiederm.org (localhost [127.0.0.1]) by frodo.ebiederm.org (8.14.1/8.14.1/Debian-9) with ESMTP id mA16XBBb032354; Fri, 31 Oct 2008 23:33:11 -0700 Received: (from eric@localhost) by frodo.ebiederm.org (8.14.1/8.14.1/Submit) id mA16XAuO032353; Fri, 31 Oct 2008 23:33:10 -0700 X-Authentication-Warning: frodo.ebiederm.org: eric set sender to ebiederm@xmission.com using -f From: ebiederm@xmission.com (Eric W. Biederman) To: David Woodhouse References: <20081029195349.imqvyxcajuoko0wo@imap.linux.ibm.com> <1225463927.16774.106.camel@macbook.infradead.org> Date: Fri, 31 Oct 2008 23:33:10 -0700 In-Reply-To: <1225463927.16774.106.camel@macbook.infradead.org> (David Woodhouse's message of "Fri, 31 Oct 2008 14:38:47 +0000") Message-ID: User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=; ; ; mid=; ; ; hst=mx04.mta.xmission.com; ; ; ip=24.130.11.59; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-SA-Exim-Connect-IP: 24.130.11.59 X-SA-Exim-Rcpt-To: dwmw2@infradead.org, jwboyer@gmail.com, linux-mtd@lists.infradead.org, cschultz@linux.vnet.ibm.com X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-DCC: XMission; sa01 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on sa01.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.6 required=8.0 tests=ALL_TRUSTED,BAYES_00, DCC_CHECK_NEGATIVE,T_TM2_M_HEADER_IN_MSG,XM_SPF_Neutral,XM_URI_RBL autolearn=disabled version=3.2.5 X-Spam-Combo: **;David Woodhouse X-Spam-Relay-Country: X-Spam-Report: * 7.0 XM_URI_RBL URI's domain appears in surbl.xmission.com * [URIs: infradead.org] * -1.8 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 T_TM2_M_HEADER_IN_MSG BODY: T_TM2_M_HEADER_IN_MSG * -2.6 BAYES_00 BODY: Bayesian spam probability is 0 to 1% * [score: 0.0000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 XM_SPF_Neutral SPF-Neutral Subject: Re: ST M29W320D incorrectly configured X-SA-Exim-Version: 4.2.1 (built Thu, 07 Dec 2006 04:40:56 +0000) X-SA-Exim-Scanned: Yes (on mx04.mta.xmission.com) X-Spam-Score: -1.0 (-) X-Spam-Report: SpamAssassin version 3.2.5 on bombadil.infradead.org summary: Content analysis details: (-1.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.0 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [166.70.13.231 listed in list.dnswl.org] Cc: jwboyer@gmail.com, linux-mtd@lists.infradead.org, Corinna Schultz X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.9 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 David Woodhouse writes: > On Wed, 2008-10-29 at 19:53 -0400, Corinna Schultz wrote: >> I'm having a problem getting the unlock addresses correctly configured >> for the ST M29W320D chip. CFI query data is shown below (from >> debugging statements I enabled and/or added to the driver). The chip >> is wired so that the #BYTE pin is low, putting the chip into x8 mode. >> The data bus is physically 8 bits. >> >> I don't understand everything that's going on in the code, but it >> seems to me that the following code (taken from cfi_cmdset_0002.c, >> which sets the unlock addresses needed for writing and erasing) has a >> logic error. Maybe someone can explain it to me? >> >> if ( /* x16 in x8 mode */ >> ((cfi->device_type == CFI_DEVICETYPE_X8) && >> (cfi->cfiq->InterfaceDesc == 2)) >> >> >> The reason I think this is in error is that both the device type and >> the interfaceDesc are defined by the hardware, and not indicative of >> the mode. Besides, isn't this conditional actually testing if the chip >> is an X8 chip and supports x8 and x16 async modes? > > That condition is doubly wrong, I think. Not only does it never trigger, > but it'd do the wrong thing if it _did_ trigger. I think the answer is > to revert this: > http://lists.infradead.org/pipermail/linux-mtd-cvs/2004-September/004096.html > > It would be nice if we could do it that way, but these ST chips don't > seem to work. When they're in 16-bit mode, they really do need a byte > address of 0x555 for the second unlock address, not 0x554 (0x2aa*2) as > every other chip seems to accept. Although it takes _extra_ logic to > check the input on the 'A-1' pin, they seem to have it. > > So I think the answer is to go back to cfi->addr_unlock[12] being _byte_ > addresses within the chip... Forget my last I see now how we can try a 16bit device on an 8 bit bus. I had missed the type <<=1 in gen_probe_new_chip. Ooops. With that said I think we need to fix cfi_send_gen_cmd as this problem applies to all uses of it. Unless I've missed something the patch below completely fixes the problem. From: Eric W. Biederman Subject: [PATCH] mtd: Fix cfi_send_gen_cmd the handling of x16 devices in x8 mode cfi_send_gen_cmd is only passed in the addresses: 0, 0x55, 0x2aa, 0x555 and the addresses addr_unlock1 and addr_unlock2 from jeded_probe. The address 0, 0x55, and 0x555 will not be effected by this patch. For 16bit devices in 8bit compatibility mode we need to use the byte address: 0xaaa and 0x555. Which effectively match the word address 0x555 and 0x2aa. Except the last has it's low byte set. We need to set the low bit to maintain the alternating bit sequence. Likewise the addresses in jedec_probe whose word address ends in the bits 10 also need their low bit set. So generically modify cfi_build_cmd_addr to extend alternating bit sequences in addresses. And every current cfi_send_gen_cmd that assumes cfi_cmd_set_0002 (i.e. uses addresses 0x2aa and 0x555) needs to be updated. Signed-off-by: Eric W. Biederman --- drivers/mtd/chips/cfi_cmdset_0002.c | 13 ------------- include/linux/mtd/cfi.h | 9 ++++++++- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index a972cc6..9e7a236 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -362,19 +362,6 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary) /* Set the default CFI lock/unlock addresses */ cfi->addr_unlock1 = 0x555; cfi->addr_unlock2 = 0x2aa; - /* Modify the unlock address if we are in compatibility mode */ - if ( /* x16 in x8 mode */ - ((cfi->device_type == CFI_DEVICETYPE_X8) && - (cfi->cfiq->InterfaceDesc == - CFI_INTERFACE_X8_BY_X16_ASYNC)) || - /* x32 in x16 mode */ - ((cfi->device_type == CFI_DEVICETYPE_X16) && - (cfi->cfiq->InterfaceDesc == - CFI_INTERFACE_X16_BY_X32_ASYNC))) - { - cfi->addr_unlock1 = 0xaaa; - cfi->addr_unlock2 = 0x555; - } } /* CFI mode */ else if (cfi->cfi_mode == CFI_MODE_JEDEC) { diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h index d6fb115..0b62217 100644 --- a/include/linux/mtd/cfi.h +++ b/include/linux/mtd/cfi.h @@ -283,7 +283,14 @@ struct cfi_private { */ static inline uint32_t cfi_build_cmd_addr(uint32_t cmd_ofs, int interleave, int type) { - return (cmd_ofs * type) * interleave; + uint32_t addr = cmd_ofs * type; + /* Modify the unlock address if we are in compatiblity mode. + * For 16bit devices on 8 bit busses + * and 32bit devices on 16 bit busses + * set the low bit of the alternating bit sequence of the address. + */ + addr |= ((cmd_ofs & 2) * type) >> 2; + return addr * interleave; } /*