Re: [PATCH] mtd: atmel_nand: fix access to 16 bit NAND devices
From: Eric Bénard
Date: Wed Feb 01 2012 - 06:39:37 EST
Le Mon, 30 Jan 2012 09:57:34 +0100,
Nicolas Ferre <nicolas.ferre@xxxxxxxxx> a écrit :
> 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@xxxxxxxxxxx>
> >> 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)?
I confirm this patch breaks 16 bits NAND flash (tested on a SAM9G45
with linux-3.2.2) and that reverting it solves the problem.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/