archive mirror
 help / color / mirror / Atom feed
From: Nicolas Ferre <>
To: "Voss, Nikolaus" <>,
	"''" <>
Cc: "''" <>,
	"''" <>
Subject: Re: [PATCH] mtd: atmel_nand: fix access to 16 bit NAND devices
Date: Mon, 30 Jan 2012 09:57:34 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

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 <>
>> 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

  reply	other threads:[~2012-01-30  8:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-18  9:16 [PATCH] mtd: atmel_nand: fix access to 16 bit NAND devices Nikolaus Voss
2012-01-27 13:29 ` Artem Bityutskiy
2012-01-30  7:57   ` Voss, Nikolaus
2012-01-30  8:57     ` Nicolas Ferre [this message]
2012-02-01 10:43       ` Eric Bénard
2012-02-02 12:06       ` Artem Bityutskiy
2012-02-03  3:35         ` Venu Byravarasu
2012-02-03  5:32           ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).