----- Original Message -----

From: "Brian Norris" <redacted>
Sent: Wednesday, January 28, 2015 7:20:42 PM

On Wed, Jan 28, 2015 at 06:37:36PM -0600, Aaron Sierra wrote:

quoted

----- Original Message -----

quoted

From: "Brian Norris" <redacted>

quoted

quoted

I was thinking about this a bit more, and it seems like we could really
just factor this all into the core nand_base code with something like
the following patch. It could possibly use some smarter logic to rule
out certain combinations (but some of those are already caught in
nand_scan_tail() anyway). What do you think?

Brian,
If the NAND device tree property fetching were moved out of fsl_upm,
I think it should not be called within nand_scan(). I think that
it's imperative that each driver be able to access these properties
before handing off to nand_scan(), since there are hardware ECC
modes that only drivers will know how to error check.

That's why nand_scan() is broken into nand_scan_ident() and
nand_scan_tail() functions which can be called individually. This allows
drivers to do the up-front initialization in nand_scan_ident(), do their
own error checking and handling of these parameters, and then call
nand_scan_tail(). See, for example, drivers/mtd/nand/omap2.c.

Thanks for pointing that out; I'll take a look.
 

quoted

Also, catching errors in nand_scan_tail() tends to result in BUG()s.

Well, some of those can be changed. Feel free to propose changes. I'd
prefer to make nand_scan_tail() play nicer than to compensate in
individual drivers.

quoted

That said, this could be useful as a publicly exported function that
individual drivers are responsible for calling (maybe in of_mtd.c).

I don't think of_mtd.c should really contain a lot of mtd_info /
nand_chip knowledge, if we can avoid it.

I really do think that the nand_scan() option is a better idea, if we
can work out the other details (BUG(), error checking, and keeping it
flexible enough). I think it provides the best place to flesh out any
other common DT handling for all NAND drivers.

Related: I believe the question came up recently about how to support a
generic DT binding for using a GPIO as a NAND write-protect line. This
would be another candidate for handling transparently in
nand_scan_ident() and would then immediately apply to all NAND drivers,
not just those that were rewritten to call another specialized init
function.

I really don't want to encourage the anti-pattern of each driver
reimplementing code that might as well be shared, if at all possible.
Adding more decentralized helpers to of_mtd.c does not really help that
cause.

Understood.

[ snipped function prototype discussion ]

quoted

You hinted at implementing stronger error checking. If we went
this route, would it make sense to only error check the software
ECC modes?

Yes. I just elided some of the details for now, since it's not actually
necessary to do some of it (many other drivers can use SW ECC without
the extra error checks).

OK, I'll rework the fsl_upm patch to work with your proposed patch.

-Aaron
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html