linux-kernel - Re: [PATCH] mtd: atmel_nand: fix access to 16 bit NAND devices

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 30 Jan 2012 09:57:34 +0100
From:	Nicolas Ferre <nicolas.ferre@...el.com>
To:	"Voss, Nikolaus" <N.Voss@...nmann.de>,
	"'dedekind1@...il.com'" <dedekind1@...il.com>
CC:	"'linux-mtd@...ts.infradead.org'" <linux-mtd@...ts.infradead.org>,
	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: atmel_nand: fix access to 16 bit NAND devices

On 01/30/2012 08:57 AM, Voss, Nikolaus :
> Artem Bityutskiy wrote on 2012-01-27:
>> On Wed, 2012-01-18 at 10:16 +0100, Nikolaus Voss wrote:
>>> commit fb5427508abbd635e877fabdf55795488119c2d6 optimizes PIO
>>> NAND accesses by using IO memcpy instead of IO read/write
>>> repeated functions.
>>>
>>> This breaks access to 16 bit NAND devices as memcpy_fromio()/toio()
>>> _always_ use byte accesses (see arch/arm/kernel/io.c), so with
>>> 16 bit NAND, one byte gets lost per NAND access cycle and NAND
>>> address count is wrong.
>>>
>>> Using memcpy() instead of the IO memcpy functions fixes this, but
>>> depends on correct word alignment of the buffer and length has to
>>> be a multiple of four, otherwise it might issue byte accesses and
>>> possibly break 16 bit NAND access (cf arch/arm/lib/copy_template.S).
>>>
>>> Memcpy variants seem to be the wrong approach here, since the
>>> NAND controller doesn't make the NAND appear as truely randomly
>>> accessible memory (as opposed to the DRAM controller which does
>>> exactly that).
>>>
>>> So, my proposal is to use 32 bit IO read/write (and let SMC
>>> map it to 8 bit or 16 bit NAND accesses) and account for
>>> length % 4 > 0 with two additional IO read/writes.
>>>
>>> Signed-off-by: Nikolaus Voss <n.voss@...nmann.de>
>>
>> Why not to revert fb5427508abbd635e877fabdf55795488119c2d6 instead, it
>> is in my opinion a bit more readable?
> 
> No objections. I've tried to save the idea of
> fb5427508abbd635e877fabdf55795488119c2d6 but that length % 4 > 0 stuff
> uglifies it a little bit...

After double checking with designers, I must admit that I misunderstood
the way of optimizing accesses to SMC. 16 bit nand is not so common
those days...

So yes, Nikolaus your correction makes sense.

What I would have liked is to optimize the possibility to trigger
incremental bursts from the processor to the SMC on internal bus. I
suspect that this will need further investigation (moreover, note that
if we use assembly code, we should also think about AVR32).

In conclusion, maybe simply reverting the initial commit will allow us
to rework this part of code from simpler basis.

Artem, do you want me to prepare a patch for reverting initial commit or
you just need my "Acked-by" (feel free to add though)?

Best regards,
-- 
Nicolas Ferre
--
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/