[U-Boot] [PATCH v2] net: add opencore 10/100 ethernet mac driver

[Ben Warren]

On 4/6/2010 8:34 PM, Thomas Chou wrote:
> Hi Ben,
>
> Thanks.
>
> On 04/05/2010 02:31 PM, Ben Warren wrote:
>> Hi Thomas,
>>
>>> + */
>>> +struct ethoc {
>>> +    void *iobase;
>> eth_device struct already has this.  If you also want it in the 
>> private struct, please don't use void *.
> OK. I will use eth_device and remove the private iobase.
>>> +
>>> +    unsigned int num_tx;
>>> +    unsigned int cur_tx;
>>> +    unsigned int dty_tx;
>>> +
>>> +    unsigned int num_rx;
>>> +    unsigned int cur_rx;
>>> +
>>> +    u32 msg_enable;
>> Please don't mix types like this.  Using 'u32' and friends globally 
>> is preferred.
> OK. I will use u32 and friends globally.
>>>
>>> +
>>> +int ethoc_initialize(bd_t *bis, int base_addr)
>> You don't use 'bis', so don't pass it in.  I'd prefer to see you pass 
>> in the base address and an index in case somebody wants more than one 
>> (mainly useful for debugging)
> Do you mean adding dev_num as index?
>
> int ethoc_initialize(u8 dev_num, int base_addr)
>
Yes.  It's useful for doing things like this in the initialize() function:

sprintf(dev->name, "%s-%hu", CS8900_DRIVERNAME, dev_num);

> Best regards,
> Thomas
regards,
Ben