[U-Boot] [PATCH 3/4 V2] OneNAND: Add simple OneNAND SPL

Marek Vasut marek.vasut at gmail.com
Thu Nov 3 18:25:52 CET 2011


> On 11/03/2011 11:56 AM, Marek Vasut wrote:
> >> On 11/02/2011 07:15 PM, Marek Vasut wrote:
> >>>> On 11/01/2011 05:54 PM, Marek Vasut wrote:
> >>>>> +static void spl_onenand_get_geometry(struct spl_onenand_data *data)
> >>>>> +{
> >>> 
> >>> [...]
> >>> 
> >>>>> +	/* The page can be either 2k or 4k, avoid using DIV_ROUND_UP. */
> >>>>> +	if (data.pagesize == 2048) {
> >>>>> +		total_pages = len / 2048;
> >>>>> +		page = offset / 2048;
> >>>>> +		total_pages += !!(len & 2047);
> >>>>> +	} else if (data.pagesize == 4096) {
> >>>>> +		total_pages = len / 4096;
> >>>>> +		page = offset / 4096;
> >>>>> +		total_pages += !!(len & 4095);
> >>>>> +	}
> >>>> 
> >>>> What's wrong with DIV_ROUND_UP?  It should produce smaller code than
> >>>> what you've done here...
> >>> 
> >>> It pulls in aeabi_*div* functions, which won't fit into block 0 of
> >>> Onenand.
> >> 
> >> It shouldn't do that if the divisor is a constant power of 2.  The
> >> compiler will turn it into a shift, just like with the other divides in
> >> the above code fragment.
> >> 
> >> You can't use DIV_ROUND_UP directly on data.pagesize, but you can use it
> >> in each branch of the if statement instead of that awkward and slightly
> >> more expensive !!(len & 4095) construct.
> > 
> > Expensive in what way?
> 
> Compare the resulting asm code.  You're replacing this:
> 
> 	a = (b + 4095) >> 12;
> 
> with this:
> 
> 	a = b >> 12;
> 	if (b & 4095)
> 		a++;
> 
> >  Either way, I don't think this matters that much.
> 
> This is code that has to fit in 1K -- why waste instructions by writing
> code in a way that is *less* readable?

The size is the same (tested). I'll submit a patch with DIV_ROUND_UP, whatever.

> 
> -Scott


More information about the U-Boot mailing list