From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Mon, 04 Jan 2016 15:45:08 -0600 Subject: [U-Boot] [PATCHv2 1/3] nand_spl_simple: Add a simple flash read function In-Reply-To: <20160104211750.GA10207@localhost.localdomain> References: <20160104155242.GA4376@localhost.localdomain> <20160104155431.GB4376@localhost.localdomain> <1451931816.17437.31.camel@buserror.net> <20160104183814.GA8638@localhost.localdomain> <20160104211750.GA10207@localhost.localdomain> Message-ID: <1451943908.19133.6.camel@buserror.net> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, 2016-01-04 at 22:17 +0100, Ladislav Michl wrote: > On Mon, Jan 04, 2016 at 07:38:23PM +0100, Ladislav Michl wrote: > > On Mon, Jan 04, 2016 at 12:23:36PM -0600, Scott Wood wrote: > > > On Mon, 2016-01-04 at 16:54 +0100, Ladislav Michl wrote: > > > > From: Thomas Gleixner > > > > > > > > To support UBI in SPL we need a simple flash read function. Add one to > > > > nand_spl_simple and keep it as simple as it goes. > > > > > > > > Signed-off-by: Thomas Gleixner > > > > > > Where is your signoff? > > > > I'm only messenger and have nothing to do with this part of patch. The > > other > > part which I touched has my signoff. > > > > > Did Thomas really write this patch for U-Boot (if so, why isn't he on > > > CC?) or > > > is it taken from some other project? > > > > Ha! It is my mailer which broke Cc. Thomas was on Cc list and still is in > > my > > sent-mail folder. Also apologize to Marek, let's see how Cc will end this > > time. > > And yes, patch was written for U-Boot: > > https://patchwork.ozlabs.org/patch/367305/ > > Well, seems to be mailman configuration: > https://mail.python.org/pipermail/mailman-users/2006-May/051194.html > so those who received post via mailman are seeing striped Cc line. Thanks to > Nathan Lynch for pointing it out. > Full Cc list: > Cc: Scott Wood > Cc: Tom Rini > Cc: Heiko Schocher > Cc: Thomas Gleixner > Cc: Richard Weinberger > Cc: Marek Vasut Oh right, I forgot about that mailman bug. > > > +int nand_spl_read_flash(u32 pnum, u32 offset, u32 len, void *dest) > > > > > > This name is too generic and doesn't suggest what's different compared > > > to > > > nand_spl_load_image (they both read data from flash into a buffer). > > > > > > How about nand_spl_read_block()? > > > > ok. > > Thinking about it more, I'd rather see nand_spl_read_peb(int peb, ... > but that is not consistent with other nand reading functions. "peb" is cryptic, and what sort of non-physical erase block does the core NAND code deal with that warrants distinction? > > > > > +{ > > > > + u32 offs, page, read, toread = len; > > > > + > > > > + /* Calculate the page number */ > > > > + page = offset / CONFIG_SYS_NAND_PAGE_SIZE; > > > > + > > > > + /* Offset to the start of a flash page */ > > > > + offs = offset % CONFIG_SYS_NAND_PAGE_SIZE; > > > > + > > > > + while (toread) { > > > > + /* > > > > + * Non page aligned reads go to the scratch buffer. > > > > + * Page aligned reads go directly to the destination. > > > > + */ > > > > + if (offs || toread < CONFIG_SYS_NAND_PAGE_SIZE) { > > > > + nand_read_page(pnum, page, scratch_buf); > > > > + read = min(len, toread); > > > > > > toread is always <= len, so this is pointless. As this is the only use > > > of len > > > after the toread init, once this is gone you could also eliminate toread > > > and > > > just use len. > > > > ok, will change that. > > What about something like this? Changelog will be appended to v3. > Btw, any comments to other patches in this serie? > > -- >8 -- > > From: Thomas Gleixner > Subject: nand_spl_simple: Add a simple NAND read function > > To support UBI in SPL we need a simple NAND read function. Add one to > nand_spl_simple and keep it as simple as it goes. > > Signed-off-by: Thomas Gleixner > Signed-off-by: Ladislav Michl Acked-by: Scott Wood -Scott