Blackfin: patch add blackfin support in smc91x ethernet controller driver
PATCH 4/5 Blackfin: patch add blackfin support in smc91x ethernet controller driver
Hi folks,
As SMC91X ethernet controller are used in blackfin STAMP 533 development board, this patch add blackfin support to the smc91x linux driver.
It's name is blackfin-driver-net-stamp533.patch.
[PATCH] Blackfin: patch add blackfin support in smc91x ethernet controller driver
---
drivers/net/Kconfig | 2 +- drivers/net/smc91x.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
drivers/net/Kconfig +1 -1
@@ -822,7 +822,7 @@ tristate "SMC 91C9x/91C1xxx support" select CRC32 select MII - depends on NET_ETHERNET && (ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || SOC_AU1X00) + depends on NET_ETHERNET && (ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || SOC_AU1X00 || BFIN) help This is a driver for SMC's 91x series of Ethernet chipsets, including the SMC91C94 and the SMC91C111. Say Y if you want it
drivers/net/smc91x.h +47 -0
53 lines not on this page · raw message
---
Thanks,
-Bryan
Quoted 4 lines · Bryan
drivers/net/smc91x.h +4
-0+/* check if the mac in reg is valid */ +#define SMC_GET_MAC_ADDR (addr) \ + do { \ + unsigned int __v; \
drivers/net/smc91x.h line 104
Quoted 10 lines
drivers/net/smc91x.h +10
-0+ __v =SMC_inw ( ioaddr,ADDR0_REG ); \ + addr[0 ] = __v; addr[1 ] = __v >>8 ; \ + __v =SMC_inw ( ioaddr,ADDR1_REG ); \ + addr[2 ] = __v; addr[3 ] = __v >>8 ; \ + __v =SMC_inw ( ioaddr,ADDR2_REG ); \ + addr[4 ] = __v; addr[5 ] = __v >>8 ; \ +if (*(u32 *)(&addr[0 ]) ==0xFFFFFFFF ) { \ +random_ether_addr (addr); \ + } \ + }while (0 )
Please follow kernel coding style:
__v = SMC_inw(ioaddr, ADDR2_REG);
Can this not be implemented as a C function?
Quoted 19 lines · Andrew Morton
Quoted 4 lines · Bryan
(quoted diff) +4
-0+/* check if the mac in reg is valid */ +#define SMC_GET_MAC_ADDR(addr) \ + do { \ + unsigned int __v; \Review
Quoted 10 lines
(quoted diff) +10
-0+ __v = SMC_inw( ioaddr, ADDR0_REG ); \ + addr[0] = __v; addr[1] = __v >> 8; \ + __v = SMC_inw( ioaddr, ADDR1_REG ); \ + addr[2] = __v; addr[3] = __v >> 8; \ + __v = SMC_inw( ioaddr, ADDR2_REG ); \ + addr[4] = __v; addr[5] = __v >> 8; \ + if (*(u32 *)(&addr[0]) == 0xFFFFFFFF) { \ + random_ether_addr(addr); \ + } \ + } while (0)Please follow kernel coding style:
Quoted 4 lines
__v = SMC_inw(ioaddr, ADDR2_REG);Can this not be implemented as a C function?
Thanks, Fixed in the new one.
---
drivers/net/Kconfig | 2 +- drivers/net/smc91x.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
drivers/net/Kconfig +1 -1
@@ -822,7 +822,7 @@ tristate "SMC 91C9x/91C1xxx support" select CRC32 select MII - depends on NET_ETHERNET && (ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || SOC_AU1X00) + depends on NET_ETHERNET && (ARM || REDWOOD_5 || REDWOOD_6 || M32R || SUPERH || SOC_AU1X00 || BFIN) help This is a driver for SMC's 91x series of Ethernet chipsets, including the SMC91C94 and the SMC91C111. Say Y if you want it
drivers/net/smc91x.h +47 -0
53 lines not on this page · raw message
_
Best Regards,
-Bryan Wu
Quoted 20 lines · Bryan
Quoted 19 lines · Andrew Morton
Quoted 4 lines · Bryan
drivers/net/smc91x.h +4
-0+/* check if the mac in reg is valid */ +#define SMC_GET_MAC_ADDR (addr) \ + do { \ + unsigned int __v; \drivers/net/smc91x.h line 104
Quoted 10 lines
drivers/net/smc91x.h +10
-0+ __v =SMC_inw ( ioaddr,ADDR0_REG ); \ + addr[0 ] = __v; addr[1 ] = __v >>8 ; \ + __v =SMC_inw ( ioaddr,ADDR1_REG ); \ + addr[2 ] = __v; addr[3 ] = __v >>8 ; \ + __v =SMC_inw ( ioaddr,ADDR2_REG ); \ + addr[4 ] = __v; addr[5 ] = __v >>8 ; \ +if (*(u32 *)(&addr[0 ]) ==0xFFFFFFFF ) { \ +random_ether_addr (addr); \ + } \ + }while (0 )Please follow kernel coding style:
Quoted 2 lines
Quoted 2 lines
__v = SMC_inw(ioaddr, ADDR2_REG);
You skipped this:
Quoted 5 lines
Quoted 1 line
Can this not be implemented as a C function?
drivers/net/smc91x.h +3
-0+#define SMC_GET_MAC_ADDR (addr) \ + do { \ + unsigned int __v; \
drivers/net/smc91x.h line 104
Quoted 10 lines
drivers/net/smc91x.h +10
-0+ __v =SMC_inw (ioaddr,ADDR0_REG ); \ + addr[0 ] = __v; addr[1 ] = __v >>8 ; \ + __v =SMC_inw (ioaddr,ADDR1_REG ); \ + addr[2 ] = __v; addr[3 ] = __v >>8 ; \ + __v =SMC_inw (ioaddr,ADDR2_REG ); \ + addr[4 ] = __v; addr[5 ] = __v >>8 ; \ +if (*(u32 *)(&addr[0 ]) ==0xFFFFFFFF ) { \ +random_ether_addr (addr); \ + } \ + }while (0 )
It can be, I think...
Please always prefer coding in C over coding in cpp.
Quoted 45 lines · Andrew Morton
Quoted 20 lines · Bryan
Quoted 19 lines · Andrew Morton
Quoted 4 lines · Bryan
drivers/net/smc91x.h +4
-0+/* check if the mac in reg is valid */ +#define SMC_GET_MAC_ADDR (addr) \ + do { \ + unsigned int __v; \drivers/net/smc91x.h line 104
Quoted 10 lines
drivers/net/smc91x.h +10
-0+ __v =SMC_inw ( ioaddr,ADDR0_REG ); \ + addr[0 ] = __v; addr[1 ] = __v >>8 ; \ + __v =SMC_inw ( ioaddr,ADDR1_REG ); \ + addr[2 ] = __v; addr[3 ] = __v >>8 ; \ + __v =SMC_inw ( ioaddr,ADDR2_REG ); \ + addr[4 ] = __v; addr[5 ] = __v >>8 ; \ +if (*(u32 *)(&addr[0 ]) ==0xFFFFFFFF ) { \ +random_ether_addr (addr); \ + } \ + }while (0 )Please follow kernel coding style:
Quoted 2 lines
Quoted 2 lines
__v = SMC_inw(ioaddr, ADDR2_REG);You skipped this:
Quoted 5 lines
Quoted 1 line
Can this not be implemented as a C function?
drivers/net/smc91x.h +3
-0+#define SMC_GET_MAC_ADDR (addr) \ + do { \ + unsigned int __v; \drivers/net/smc91x.h line 104
Quoted 10 lines
drivers/net/smc91x.h +10
-0+ __v =SMC_inw (ioaddr,ADDR0_REG ); \ + addr[0 ] = __v; addr[1 ] = __v >>8 ; \ + __v =SMC_inw (ioaddr,ADDR1_REG ); \ + addr[2 ] = __v; addr[3 ] = __v >>8 ; \ + __v =SMC_inw (ioaddr,ADDR2_REG ); \ + addr[4 ] = __v; addr[5 ] = __v >>8 ; \ +if (*(u32 *)(&addr[0 ]) ==0xFFFFFFFF ) { \ +random_ether_addr (addr); \ + } \ + }while (0 )It can be, I think...
Quoted 1 line
Please always prefer coding in C over coding in cpp.
Yes, it can be writen as a C function.
We just follow other implementation in the drivers/net/smc91x.h
start at line 1156 of
---
#ifndef SMC_GET_MAC_ADDR
#define SMC_GET_MAC_ADDR(addr) \
do { \
unsigned int __v; \
__v = SMC_inw( ioaddr, ADDR0_REG ); \
addr[0] = __v; addr[1] = __v >> 8; \
__v = SMC_inw( ioaddr, ADDR1_REG ); \
addr[2] = __v; addr[3] = __v >> 8; \
__v = SMC_inw( ioaddr, ADDR2_REG ); \
addr[4] = __v; addr[5] = __v >> 8; \
} while (0)#endif
_
If you think this is necessary, we may rewrite it in C function.
Thanks
-Bryan
Quoted 18 lines · Bryan
Quoted 1 line
Please always prefer coding in C over coding in cpp.
Yes, it can be writen as a C function.
We just follow other implementation in thedrivers/net/smc91x.hstart at line 1156 of
---
#ifndefSMC_GET_MAC_ADDR#define SMC_GET_MAC_ADDR(addr) \ do { \ unsigned int __v; \ __v = SMC_inw( ioaddr, ADDR0_REG ); \ addr[0] = __v; addr[1] = __v >> 8; \ __v = SMC_inw( ioaddr, ADDR1_REG ); \ addr[2] = __v; addr[3] = __v >> 8; \ __v = SMC_inw( ioaddr, ADDR2_REG ); \ addr[4] = __v; addr[5] = __v >> 8; \ } while (0)
Quoted 2 lines
#endif
_
Please don't treat ghastly old driver code as a model for new kernel code.
Quoted 1 line
If you think this is necessary, we may rewrite it in C function.
hm, well, if it's copied and pasted from somewhere else then there is some merit in leaving it as it is.
It would of course be better to pull it out into a shared header file, avoid the duplication.
Quoted 25 lines · Andrew Morton
Quoted 18 lines · Bryan
Quoted 1 line
Please always prefer coding in C over coding in cpp.
Yes, it can be writen as a C function.
We just follow other implementation in thedrivers/net/smc91x.hstart at line 1156 of
---
#ifndefSMC_GET_MAC_ADDR#define SMC_GET_MAC_ADDR(addr) \ do { \ unsigned int __v; \ __v = SMC_inw( ioaddr, ADDR0_REG ); \ addr[0] = __v; addr[1] = __v >> 8; \ __v = SMC_inw( ioaddr, ADDR1_REG ); \ addr[2] = __v; addr[3] = __v >> 8; \ __v = SMC_inw( ioaddr, ADDR2_REG ); \ addr[4] = __v; addr[5] = __v >> 8; \ } while (0)Quoted 2 lines
#endif
_Please don't treat ghastly old driver code as a model for new kernel code.
Quoted 7 lines
Quoted 1 line
If you think this is necessary, we may rewrite it in C function.
hm, well, if it's copied and pasted from somewhere else then there is some merit in leaving it as it is.
It would of course be better to pull it out into a shared header file, avoid the duplication.
Exactly, this is a better solution. Because smc91x driver is widely used in embedded linux world, we just want to add our arch support into this common driver.
Maybe Nico can help us on this.
Thanks
-Bryan Wu
oh, and please consider removing all the CVS control strings from the blackfin code. Because they become meaningless once the code is merged into mainline.