All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: "Rafał Miłecki" <zajec5@gmail.com>,
	"Kamal Dasu" <kdasu.kdev@gmail.com>,
	linux-mtd@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com,
	"Hauke Mehrtens" <hauke@hauke-m.de>,
	"Boris Brezillon" <boris.brezillon@free-electrons.com>
Subject: Re: [PATCH] mtd: brcmnand: set initial ECC params based on info from HW
Date: Mon, 1 Feb 2016 11:02:38 -0800	[thread overview]
Message-ID: <20160201190238.GJ19540@google.com> (raw)
In-Reply-To: <56AF9F58.4050103@gmail.com>

On Mon, Feb 01, 2016 at 10:09:28AM -0800, Florian Fainelli wrote:
> On 01/02/16 09:53, Brian Norris wrote:
> > Anyway, if we are really making the DT properties optional, we'd need to
> > modify Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt to
> > reflect this.
> 
> The Marvell PXA3XX NAND controller has a "marvell,nand-keep-config"
> which seems to be in par with what Rafal is after here, maybe it would
> be worth standardizing/mimicing that kind of property for the brcmnand
> driver?

Maybe. Honestly, it's not that clear to me what the pxa3xx-nand binding
means. But if we make that clear, that could be a possibility.

(Side note: this isn't too far from what we did the Broadcom STB BSP
previously, except that we wouldn't use device tree. We attempted a
heuristic to detect whether the bootloader had initialized the flash
controller properly... I didn't want to maintain that crazy logic
upstream though. A DT flag to enable that behavior might be OK.)

> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> ---
> >> It took me hours to figure out how to setup NAND on my D-Link DIR-885L.
> >> This should be very helpful for ppl adding new devices support.
> > 
> > What if we just print out the hardware defaults when we bail out due to
> > missing ECC DT properties? Then developers can choose to set up their DT
> > with these properties, if those are actually proven correct. Would that
> > save you the hours of setup you mention?
> > 
> > Another option: maybe a commandline boot flag?
> 
> One could argue that the bootloader is patf of the platform, and so, if
> the bootloader is known to provide good defaults, then a DT property
> could be appropriate. The commandline boot flag just sounds a little
> impractical, in case you have more than one NAND controller, but how
> common is that?

Not sure if you mean multiple *controller* or multiple *flash*. The
former is much less common than the latter, but neither is very common
AFAIK.

Brian

  reply	other threads:[~2016-02-01 19:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-27  7:58 [PATCH] mtd: brcmnand: set initial ECC params based on info from HW Rafał Miłecki
2016-02-01 17:53 ` Brian Norris
2016-02-01 18:09   ` Florian Fainelli
2016-02-01 19:02     ` Brian Norris [this message]
2016-02-01 20:33       ` Florian Fainelli
2016-02-01 20:49         ` Brian Norris

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=20160201190238.GJ19540@google.com \
    --to=computersforpeace@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=boris.brezillon@free-electrons.com \
    --cc=f.fainelli@gmail.com \
    --cc=hauke@hauke-m.de \
    --cc=kdasu.kdev@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=zajec5@gmail.com \
    /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.