From patchwork Thu Apr 22 08:36:06 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guillaume LECERF X-Patchwork-Id: 50710 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 7BB69B7D18 for ; Thu, 22 Apr 2010 18:42:16 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1O4rxj-0002gy-6U; Thu, 22 Apr 2010 08:40:43 +0000 Received: from smtp2-g21.free.fr ([212.27.42.2]) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1O4rxg-0002fk-FF for linux-mtd@lists.infradead.org; Thu, 22 Apr 2010 08:40:41 +0000 Received: from smtp2-g21.free.fr (localhost [127.0.0.1]) by smtp2-g21.free.fr (Postfix) with ESMTP id AF4144B004B; Thu, 22 Apr 2010 10:40:35 +0200 (CEST) Received: from shiryu.yomgui.biz (lsd.homeftp.net [82.235.75.105]) by smtp2-g21.free.fr (Postfix) with ESMTP id C19134B0173; Thu, 22 Apr 2010 10:40:32 +0200 (CEST) Received: from localhost ([127.0.0.1] helo=shiryu.yomgui.biz) by shiryu.yomgui.biz with esmtp (Exim 4.71) (envelope-from ) id 1O4rtG-0004UH-8l; Thu, 22 Apr 2010 10:36:06 +0200 Subject: [PATCH v3 5/8] mtd: cfi_cmdset_0002: do not fail on no extended query table as they are both optional To: linux-mtd From: Guillaume LECERF Date: Thu, 22 Apr 2010 10:36:06 +0200 Message-ID: <20100422083606.17166.52192.stgit@shiryu.yomgui.biz> In-Reply-To: <20100422083545.17166.17822.stgit@shiryu.yomgui.biz> References: <20100422083545.17166.17822.stgit@shiryu.yomgui.biz> User-Agent: StGit/0.15 MIME-Version: 1.0 X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20100422_044041_010318_0B590F4C X-CRM114-Status: GOOD ( 11.98 ) X-Spam-Score: 1.2 (+) X-Spam-Report: SpamAssassin version 3.3.1 on bombadil.infradead.org summary: Content analysis details: (1.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, low trust [212.27.42.2 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is freemail (glecerf[at]gmail.com) 0.0 DKIM_ADSP_CUSTOM_MED No valid author signature, adsp_override is CUSTOM_MED 1.2 NML_ADSP_CUSTOM_MED ADSP custom_med hit, and not from a mailing list Cc: David Woodhouse , Wolfram Sang , Artem Bityutskiy 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 After looking at AMD's CFI specification [1], both of the extended query tables are optional. Thus, it looks like relying that at least one of those tables exist is a bug in cfi_cmdset_0002. This patch inverts the logic and checks for unlock function pointers before exiting on error. This approach leaves place to add a call to a fixup function to try to handle chips compatible with the early AMD specification from 1995 [2]. [1] http://www.amd.com/us-en/assets/content_type/DownloadableAssets/cfi_r20.pdf [2] http://noel.feld.cvut.cz/hw/amd/20158a.pdf Signed-off-by: Guillaume LECERF --- drivers/mtd/chips/cfi_cmdset_0002.c | 89 ++++++++++++++++++----------------- 1 files changed, 45 insertions(+), 44 deletions(-) diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c index ea2a7f6..de1b4ba 100644 --- a/drivers/mtd/chips/cfi_cmdset_0002.c +++ b/drivers/mtd/chips/cfi_cmdset_0002.c @@ -353,65 +353,66 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary) if (cfi->cfi_mode==CFI_MODE_CFI){ unsigned char bootloc; - /* - * It's a real CFI chip, not one for which the probe - * routine faked a CFI structure. So we read the feature - * table from it. - */ __u16 adr = primary?cfi->cfiq->P_ADR:cfi->cfiq->A_ADR; struct cfi_pri_amdstd *extp; extp = (struct cfi_pri_amdstd*)cfi_read_pri(map, adr, sizeof(*extp), "Amd/Fujitsu"); - if (!extp) { - kfree(mtd); - return NULL; - } - - cfi_fixup_major_minor(cfi, extp); - - if (extp->MajorVersion != '1' || - (extp->MinorVersion < '0' || extp->MinorVersion > '4')) { - printk(KERN_ERR " Unknown Amd/Fujitsu Extended Query " - "version %c.%c.\n", extp->MajorVersion, - extp->MinorVersion); - kfree(extp); - kfree(mtd); - return NULL; - } + if (extp) { + /* + * It's a real CFI chip, not one for which the probe + * routine faked a CFI structure. + */ + cfi_fixup_major_minor(cfi, extp); + + if (extp->MajorVersion != '1' || + (extp->MinorVersion < '0' || extp->MinorVersion > '4')) { + printk(KERN_ERR " Unknown Amd/Fujitsu Extended Query " + "version %c.%c.\n", extp->MajorVersion, + extp->MinorVersion); + kfree(extp); + kfree(mtd); + return NULL; + } - /* Install our own private info structure */ - cfi->cmdset_priv = extp; + /* Install our own private info structure */ + cfi->cmdset_priv = extp; - /* Apply cfi device specific fixups */ - cfi_fixup(mtd, cfi_fixup_table); + /* Apply cfi device specific fixups */ + cfi_fixup(mtd, cfi_fixup_table); #ifdef DEBUG_CFI_FEATURES - /* Tell the user about it in lots of lovely detail */ - cfi_tell_features(extp); + /* Tell the user about it in lots of lovely detail */ + cfi_tell_features(extp); #endif - bootloc = extp->TopBottom; - if ((bootloc != 2) && (bootloc != 3)) { - printk(KERN_WARNING "%s: CFI does not contain boot " - "bank location. Assuming top.\n", map->name); - bootloc = 2; - } + bootloc = extp->TopBottom; + if ((bootloc != 2) && (bootloc != 3)) { + printk(KERN_WARNING "%s: CFI does not contain boot " + "bank location. Assuming top.\n", map->name); + bootloc = 2; + } - if (bootloc == 3 && cfi->cfiq->NumEraseRegions > 1) { - printk(KERN_WARNING "%s: Swapping erase regions for broken CFI table.\n", map->name); + if (bootloc == 3 && cfi->cfiq->NumEraseRegions > 1) { + printk(KERN_WARNING "%s: Swapping erase regions for broken CFI table.\n", map->name); - for (i=0; icfiq->NumEraseRegions / 2; i++) { - int j = (cfi->cfiq->NumEraseRegions-1)-i; - __u32 swap; + for (i=0; icfiq->NumEraseRegions / 2; i++) { + int j = (cfi->cfiq->NumEraseRegions-1)-i; + __u32 swap; - swap = cfi->cfiq->EraseRegionInfo[i]; - cfi->cfiq->EraseRegionInfo[i] = cfi->cfiq->EraseRegionInfo[j]; - cfi->cfiq->EraseRegionInfo[j] = swap; + swap = cfi->cfiq->EraseRegionInfo[i]; + cfi->cfiq->EraseRegionInfo[i] = cfi->cfiq->EraseRegionInfo[j]; + cfi->cfiq->EraseRegionInfo[j] = swap; + } } + /* Set the default CFI lock/unlock addresses */ + cfi->addr_unlock1 = 0x555; + cfi->addr_unlock2 = 0x2aa; + } + + if (!cfi->addr_unlock1 || !cfi->addr_unlock2) { + kfree(mtd); + return NULL; } - /* Set the default CFI lock/unlock addresses */ - cfi->addr_unlock1 = 0x555; - cfi->addr_unlock2 = 0x2aa; } /* CFI mode */ else if (cfi->cfi_mode == CFI_MODE_JEDEC) {