linux-kernel - Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell Berlin nand controller

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150305130843.GC5265@kwain>
Date:	Thu, 5 Mar 2015 14:08:43 +0100
From:	Antoine Tenart <antoine.tenart@...e-electrons.com>
To:	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Cc:	Antoine Tenart <antoine.tenart@...e-electrons.com>,
	sebastian.hesselbarth@...il.com,
	ezequiel.garcia@...e-electrons.com, dwmw2@...radead.org,
	computersforpeace@...il.com, boris.brezillon@...e-electrons.com,
	zmxu@...vell.com, jszhang@...vell.com,
	linux-arm-kernel@...ts.infradead.org,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 5/9] mtd: pxa3xx_nand: add support for the Marvell
 Berlin nand controller

Thomas,

On Thu, Mar 05, 2015 at 02:00:00PM +0100, Thomas Petazzoni wrote:
> On Thu,  5 Mar 2015 12:31:21 +0100, Antoine Tenart wrote:
> 
> >  struct pxa3xx_nand_host {
> > @@ -253,6 +258,12 @@ static struct pxa3xx_nand_flash builtin_flash_types[] = {
> >  { "512MiB 8-bit",  0xdc2c,  64, 2048,  8,  8, 4096 },
> >  { "512MiB 16-bit", 0xcc2c,  64, 2048, 16, 16, 4096 },
> >  { "256MiB 16-bit", 0xba20,  64, 2048, 16, 16, 2048 },
> > +{ }
> > +};
> > +
> > +static struct pxa3xx_nand_flash berlin_builtin_flash_types[] = {
> > +{ "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
> > +{ },
> 
> This looks fishy. You know have two different definitions for the exact
> same chip_id. In the builtin_flash_types[] array:
> 
> { "4GiB 8-bit",    0xd7ec, 128, 4096,  8,  8, 8192 },
> 
> and in your new berlin_builtin_flash_types[] array:
> 
> { "4GiB 8-bit",    0xd7ec, 128, 8192,  8,  8, 4096 },
> 
> So you have twice a big pages, and twice as less blocks. Are you sure
> about your definition of the 0xd7ec NAND chip_id ?
> 
> Why cannot you use the same data for both the Berlin platform and the
> platforms already supported by the driver? Are you sure your NAND isn't
> using 4k pages ? Or maybe the 0xd7ec entry in builtin_flash_types[] is
> incorrect?
> 
> Or maybe like
> http://lists.infradead.org/pipermail/linux-mtd/2010-June/031159.html,
> the NAND chip_id is the same, but the NAND ext id is different.
> 
> Is there no common NAND mechanism to handle this, rather than having
> this specifically in the driver?

I totally agree, this is one more thing wrong with this driver. Using a
ext id would solve the issue here.

If a NULL table is given to the nand_get_flash_type() function, then the
nand_flash_ids is used. This should be handled this way.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/