From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Thu, 16 Jul 2015 14:30:29 -0500 Subject: [U-Boot] [PATCH 1/4] spl: nand: simple: replace readb() with chip specific read_buf() In-Reply-To: <20150716161239.0461519b@lilith> References: <1437003228-14746-1-git-send-email-vz@mleia.com> <1437003228-14746-2-git-send-email-vz@mleia.com> <20150716100231.677f8d16@lilith> <55A7960E.6000809@mleia.com> <20150716144332.64c4edf2@lilith> <55A7B62A.8000603@mleia.com> <20150716161239.0461519b@lilith> Message-ID: <1437075029.2993.100.camel@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > 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