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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to