All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: dwmw2@infradead.org, boris.brezillon@free-electrons.com,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: nand: support JEDEC additional redundant parameter pages
Date: Fri, 18 Dec 2015 15:00:38 -0800	[thread overview]
Message-ID: <20151218230038.GU10460@google.com> (raw)
In-Reply-To: <20151211081620.GE28188@kwain>

On Fri, Dec 11, 2015 at 09:16:20AM +0100, Antoine Tenart wrote:
> On Thu, Dec 10, 2015 at 12:20:52PM -0800, Brian Norris wrote:
> > On Fri, Dec 04, 2015 at 11:35:28PM +0100, Antoine Tenart wrote:
> > > The JEDEC standard defines the JEDEC parameter page data structure.
> > > One page plus two redundant pages are always there, in bits 0-1535.
> > > Additionnal redundant parameter pages can be stored at bits 1536+.
> > > Add support to read these pages.
> > > 
> > > The first 3 JEDEC parameter pages are always checked, and if none
> > > is valid we try to read additional redundant pages following the
> > > standard definition: we continue while at least two of the four bytes
> > > of the parameter page signature match (stored in the first dword).
> > > 
> > > There is no limit to the number of additional redundant parameter
> > > page.
> > 
> > Hmm, do we really want this to be unbounded? What if (for example) a
> > driver is buggy and has some kind of wraparound, so that it keeps
> > returning the same parameter page (or a sequence of a few pages)?
> 
> I would say buggy drivers need to be fixed. It's complicated to handle
> all possible bugs a driver may have in the common code.

Well, that's a fair statement to some extent. But generally, it is
*much* preferred to code to real practice, rather than theoretical
standards. And it's also much preferred not to code in potentially
infinite loops. That's why we have timeouts, for instance, on most I/O.

> If you prefer we can put a limit to the tries the code make, but this
> can also impact working drivers by not trying enough. I'm open to
> suggestions.

Yes, please put some kind of limit. Probably a low one (after some
number of damaged parameter pages, what's the likelihood that there will
be any more correct ones?). If you make it a macro with a reasonable
name, then it'll be more obvious what it does, in case someone needs to
increase it.

> > Also, is this actually solving any real problem? Have you seen flash
> > that have more than the first 3 parameter pages? Have you tested
> > this beyond the first 3?
> 
> This does not solve any real world problem I had. I had to look at the
> JEDEC standard and I made this in order to test something. So I thought
> this could be useful to others, as the current code does not fully
> implement the standard.

OK. Well, I guess having the code might be better than nothing. I assume
you at least faked the parameter pages so you could exercise all the
code paths?

Brian

      reply	other threads:[~2015-12-18 23:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-04 22:35 [PATCH] mtd: nand: support JEDEC additional redundant parameter pages Antoine Tenart
2015-12-10 20:20 ` Brian Norris
2015-12-11  8:16   ` Antoine Tenart
2015-12-18 23:00     ` Brian Norris [this message]

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=20151218230038.GU10460@google.com \
    --to=computersforpeace@gmail.com \
    --cc=antoine.tenart@free-electrons.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    /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.