On Thu, 2015-07-16 at 16:12 +0200, Albert ARIBAUD wrote:
Hello Vladimir,
On Thu, 16 Jul 2015 16:48:26 +0300, Vladimir Zapolskiy vz@mleia.com wrote:
(cutting short to the essential point remaining)
I personally don't think this is the right way; nand_simple.c should be left unchanged and the board should simply not use it since it does not have a simple NAND controller, and instead it should provide its own nand_spl_load_image().
I don't have a problem with expanding the definition of "simple" to include custom read_byte and such, as long as we make sure that existing targets don't break. For drivers that don't supply these functions, where are the default functions coming from? I don't see nand_spl_simple's nand_init() initializing them.
For me an alternative change to the proposed one is to duplicate nand_spl_simple.c functionality in LPC32xx SLC NAND driver. From maintenance point of view this is not the best thing to do IMHO.
You're right that some of the functionality present in nand_simple.c is duplicated elsewhere; however, that functionality is not the one specific to nand_simple; actually, it is nand_spl_load_image(), the overall load functionality which should be in no driver at all but of which I see ten incarnations in ten drivers.
There should be only one nand_spl_load_image(), in its own file, with placeholders for driver-specific actions such as page read, bad page check, etc. One day, maybe, there will be a patch for that.
You'll never get it down to just one, since there are some targets that rely on the tight integration within a single file to squeak in under a 4 KiB size limit. Maybe some of the implementations could be replaced with an optional generic version, though.
-Scott