[U-Boot] [PATCH 4/5] add TI da8xx support: new ethenet driver for da830 EMAC

[Tom]

Nick Thompson wrote:
> Ben Warren wrote:
>> Hi Tom,
>>
>> On Sun, Oct 18, 2009 at 10:26 AM, Tom <Tom.Rix at windriver.com
>> <mailto:Tom.Rix at windriver.com>> wrote:
>>
>>     Thompson, Nick (GE EntSol, Intelligent Platforms) wrote:
>>
>>         Add a driver for the DA830 EMAC.
>>
>>         This is very similar to the davinci_emac driver. It has been
>>         restructured
>>         to make it as similar as possible. Potentially the two could be
>>         merged,
>>         but I don't have access to other davinci type platforms to test for
>>         breakage after the inevitable mangling required.
>>
>>         Signed-off-by: Nick Thompson <nick.thompson at gefanuc.com
>>         <mailto:nick.thompson at gefanuc.com>>
>>
>>
>>     Ben,
>>     Can I pass this review off to you ?
>>     Tom
>>
>>
>>  
>> Yeah, I'll review the next spin.  Since it won't require a new driver,
>> it may make more sense to keep the patch parts together.  Either way,
>> I'll ACK/NAK it.
>>
>> regards,
>> Ben 
> 
> Tom, Thank you for the very through review, I will go away and address the issues you raise and get some checking tools in place. Also I've figured out how to get Thunderbird on linux talking to my exchange server - I will test it's mangling abilities before my next patch.
> 
> Ben, You are right of course, I picked up the driver from an old TI u-boot and updated it for CONFIG_NET_MULTI, but shyed away from making functional changes as it seems to work just fine. I will switch to the davinci driver and pull in changes only as required - with inline statics where I can.
> 
> If I understood correctly, assuming the switch to davinci ethernet, you would prefer a single patch e-mail rather than 5? It will still be rather bigger than the 40kB suggested in the linux SubmittingPatches doc.

The way you did it with 5 is preferred.
The 4/5 is a NET patch and that must be reviewed by Ben.
The others are TI/ARM patches.

Tom

> 
> Thanks,
> Nick.
> 
> This next line is just a test please ignore:
> +static unsigned char		emac_rx_buffers[EMAC_MAX_RX_BUFFERS * (EMAC_MAX_ETHERNET_PKT_SIZE + EMAC_PKT_ALIGN)];