[U-Boot] [PATCH] mtd: pxa3xx_nand: Correct allocation and init bug

[Scott Wood]

On Fri, 2015-10-23 at 19:56 +0000, Kevin Smith wrote:
> Hi Scott,
> 
> On 10/23/2015 01:20 PM, Scott Wood wrote:
> > 
> > Yuck.  Could you please rework this driver to not play games with pointers
> > and one giant allocation?  Why can't this function allocate each region it
> > needs separately?
> > 
> > -Scott
> > 
> This driver is taken from Linux.  There are a few API modifications to 
> make it work in U-Boot, but the main form and function of the driver is 
> the same.  The single allocation method is used by Linux and is kept 
> here in U-boot.

Sigh... At least do this in alloc_nand_resources() the way Linux does, rather 
than taking it a step further and allocating an array of these blobs.

> As for why Linux does this, it may be for cache coherency, avoiding
> memory fragmentation, speed (fewer calls to malloc), or something else.  
> I agree it is kind of opaque, but is probably done for a good reason.

I'm sure there was a reason but that doesn't mean it was a good reason.  I 
don't understand how cache coherency would be relevant, nor do I agree that 
trying to optimize a boot path to have fewer malloc calls at the expense of 
making the code more complicated is a "good reason".

>   I didn't port the driver, and I don't know if the reason is applicable to
> U-Boot or if a rework is appropriate.  Maybe Stefan can comment?
> 
> Either way, I am not able to rework it right now.  I think my patch 
> fixes a legitimate issue.  (It at least fixes the crashes I was 
> experiencing).  I hope it can be accepted as-is.

Does Linux have this problem?  Assuming no, please fix this by making the 
driver look more like Linux.  At least then it would be the same ugliness.

Can you explain how the change in the calculation of "chip" and the 
allocation size is relevant to the NULL dereference?  Couldn't that be fixed 
by just removing the "info->host[0]->mtd" line?

-Scott