All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Iwo Mergler <iwo.mergler@netcommwireless.com>
Cc: Richard Weinberger <richard@nod.at>,
	Brian Norris <computersforpeace@gmail.com>,
	linux-mtd@lists.infradead.org
Subject: Re: [PATCH] mtd: nandbiterrs: Support for NAND biterrors test on platforms without raw write
Date: Tue, 10 May 2016 10:48:51 +0200	[thread overview]
Message-ID: <20160510104851.53dc8211@bbrezillon> (raw)
In-Reply-To: <57300D8A.8040607@netcommwireless.com>

Hi Iwo,

On Mon, 9 May 2016 14:09:46 +1000
Iwo Mergler <iwo.mergler@netcommwireless.com> wrote:

> Hi Boris,
> 
> 
> I have to admit that your NACK surprised me.
> 
> My patch removes an unnecessary use of raw write
> from the test. It was only there because of my
> original implementation, which I now consider
> mistaken.

Sorry, I didn't look at the diff itself, and focused on the commit
message :-/. Indeed, using normal write in the overwrite test should be
harmless, but I still think that all controller should properly
implement raw access functions, otherwise the "incremental errors"
test is irrelevant (you'll overwrite ECC bytes along with in-band
data, and will end up with more bitflips than you expected).

> 
> I fully agree with you that raw write should
> be implemented, despite the impediments.
> Although I have seen at least one NAND controller
> that always computed and wrote ECC, with no way
> for software to circumvent it.

Hm, I was told that so many times and each time I had a closer look it
appeared to be untrue, so I tend to be skeptical on these kind of
statement now. Could you tell me more about this controller?

> 
> Could you please elaborate a little why you
> don't want a test module to work with incomplete
> MTD drivers?

As I said, this test module will only work in overwrite mode when the
controller does not support raw accesses.

> Is that supposed to be motivating
> driver writers for better implementations? ;-)

Yes, partly, and also because it's really helpful when you need to debug
NAND stuff.

Honestly, I'd rather see NAND implementations return -ENOTSUPP when
they do not support raw accesses than pretending they are.

> 
> Would you accept the patch if I remove the comment
> about data reshuffling drivers? It's not required
> for the patch and, as you correctly pointed out,
> now inaccurate.

At least rework it to mention that you're only modifying the overwrite
test, and that writing in normal mode in this case is harmless.

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

  reply	other threads:[~2016-05-10  8:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-06  0:00 [PATCH] mtd: nandbiterrs: Support for NAND biterrors test on platforms without raw write Iwo Mergler
2016-05-08 16:44 ` Boris Brezillon
2016-05-08 16:47   ` Boris Brezillon
2016-05-09  4:09   ` Iwo Mergler
2016-05-10  8:48     ` Boris Brezillon [this message]
2016-05-11  6:54       ` Iwo Mergler
2016-05-11  6:54       ` Iwo Mergler
2016-06-20 11:48         ` Boris Brezillon

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20160510104851.53dc8211@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=iwo.mergler@netcommwireless.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.