[U-Boot] [PATCH 1/1][Net] Convert CS8900 Ethernet driver to CONFIG_NET_MULTI API

[Ben Warren]

Hi Wolfgang,

Wolfgang Denk wrote:
> Dear Ben Warren,
>
> In message <1250050332-15531-2-git-send-email-biggerbadderben at gmail.com> you wrote:
>   
>> All in-tree boards that use this controller have CONFIG_NET_MULTI
>> added
>> Also:
>>   - changed CONFIG_DRIVER_CS8900 to CONFIG_CS8900
>>   - changed CS8900_BASE to CONFIG_CS8900_BASE
>>   - changed CS8900_BUS?? to CONFIG_CS8900_BUS??
>>   - cleaned up line lengths
>>   - modified VCMA9 command function that accesses the device
>>   - removed MAC address initialization from lib_arm/board.c
>>
>> Signed-off-by: Ben Warren <biggerbadderben at gmail.com>
>>     
>
> The patch fails on "trab" (which is, as far as this patch is
> concerned, basicly identical to SMDK2400):
>
>   
Thank you very much for the fast, useful feedback!
> U-Boot 2009.08-rc2-00017-g7d4cd15-dirty (Aug 12 2009 - 23:55:37)
>
> I2C:   ready
> DRAM:  32 MB
> Flash:  8 MB
> USB:   scanning bus for devices... 1 USB Device(s) found
> 0 Storage Device(s) found
> Net:   CS8900-0
>
> Enter password - autoboot in 5 sec...
> TRAB # tftp C100000 trab/u-boot.bin-wd
> CS8900 Ethernet chip not found?!
> Using CS8900-0 device
> TFTP from server 192.168.1.1; our IP address is 192.168.3.68
> Filename 'trab/u-boot.bin-wd'.
> Load address: 0xc100000
> Loading: data abort
> pc : [<0df6b310>]          lr : [<0df4cc44>]
> sp : 0df1bd74  ip : 0df1c0e8     fp : 00000045
> r10: 0df7a320  r9 : 00000032     r8 : 0df1bfd8
> r7 : 00000000  r6 : 0df1c0e8     r5 : 0000002a  r4 : 0000005c
> r3 : 000000c0  r2 : ea000012     r1 : 0df7a320  r0 : 0df1c0e8
> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
> resetting ...
>
>
> I see two issues here - the obvious "CS8900 Ethernet chip not
> found?!", and the fact that the (uninitialized?) driver then crashes
> the board.
>
>   
Uh, yeah.  Two serious problems :)
> With DEBUG enabled, I see the same, i. e. no additional information.
>
> With the following patch applied (which you might want to add - but
> please check if the changed return code makes sense; it doesn;t
> change behaviour, it seems) I get this:
>
> diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c
> index 4935290..1f46d9a 100644
> --- a/drivers/net/cs8900.c
> +++ b/drivers/net/cs8900.c
> @@ -154,11 +154,14 @@ void cs8900_halt(struct eth_device *dev)
>  static int cs8900_init(struct eth_device *dev, bd_t * bd)
>  {
>  	uchar *enetaddr = dev->enetaddr;
> +	u16 id;
>  
>  	/* verify chip id */
> -	if (get_reg_init_bus(dev, PP_ChipID) != 0x630e) {
> -		printf ("CS8900 Ethernet chip not found?!\n");
> -		return 0;
> +	id = get_reg_init_bus(dev, PP_ChipID);
> +	if (id != 0x630e) {
> +		printf ("CS8900 Ethernet chip not found: ID=0x%04x instead 0x%04x\n",
> +			id, 0x630e);
> +		return 1;
>  	}
>  
>  	cs8900_reset (dev);
>
>
> TRAB # tftp C100000 trab/u-boot.bin-wd
> CS8900 Ethernet chip not found: ID=0x0012 instead 0x630e
> Using CS8900-0 device
> TFTP from server 192.168.1.1; our IP address is 192.168.3.68
> Filename 'trab/u-boot.bin-wd'.
> Load address: 0xc100000
> Loading: data abort
> pc : [<0df6b320>]          lr : [<0df4cc44>]
> sp : 0df1bd74  ip : 0df1c0e8     fp : 00000045
> r10: 0df7a320  r9 : 00000032     r8 : 0df1bfd8
> r7 : 00000000  r6 : 0df1c0e8     r5 : 0000002a  r4 : 0000005c
> r3 : 000000c0  r2 : ea000012     r1 : 0df7a320  r0 : 0df1c0e8
> Flags: nzCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
>
> So it's reading indeed a bad ID ...
>
>   
It looks like the 'trab' board uses 16-bit accesses 
(CONFIG_CS8900_BUS16).  The function 'get_reg_init_bus()' does some 
funny initialization to get the chip into 16-bit mode.  I wonder if 
maybe that's not working the same as the original.  That could explain 
the crash too.

BTW - it shouldn't matter in this case, but there's a typo in cs8900.c 
that this should fix:

diff --git a/drivers/net/cs8900.c b/drivers/net/cs8900.c
index 4935290..0c2e763 100644
--- a/drivers/net/cs8900.c
+++ b/drivers/net/cs8900.c
@@ -50,7 +50,7 @@
 
 /* packet page register access functions */
 
-#ifdef CS8900_BUS32
+#ifdef CONFIG_CS8900_BUS32
 /* we don't need 16 bit initialisation on 32 bit bus */
 #define get_reg_init_bus(x) get_reg((x))
 #else


Best regards,
> Wolfgang Denk
>
>   
regards,
Ben