[PATCH net-next] net: ethernet: ti: cpsw: replace unnecessarily macroses on functions
Subsystems:
networking drivers , the rest , ti ethernet switch driver (cpsw)
2937d
10 messages,
4 authors,
2018-07-27 · open the first message on its own page
Replace ugly macroses on functions.
Signed-off-by: Ivan Khoronzhuk <redacted>
---
Based on net-next/master
drivers/net/ethernet/ti/cpsw.c | 63 +++++++++++++++++-----------------
1 file changed, 32 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 1b54c26c2bec..62326a98945b 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c @@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = {
( func )( slave ++ , ## arg ); \
} while ( 0 )
- #define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \ - do { \- if ( ! cpsw -> data . dual_emac ) \- break ; \- if ( CPDMA_RX_SOURCE_PORT ( status ) == 1 ) { \- ndev = cpsw -> slaves [ 0 ]. ndev ; \- skb -> dev = ndev ; \- } else if ( CPDMA_RX_SOURCE_PORT ( status ) == 2 ) { \- ndev = cpsw -> slaves [ 1 ]. ndev ; \- skb -> dev = ndev ; \- } \- } while ( 0 ) - #define cpsw_add_mcast(cpsw, priv, addr) \ - do { \- if ( cpsw -> data . dual_emac ) { \- struct cpsw_slave * slave = cpsw -> slaves + \- priv -> emac_port ; \- int slave_port = cpsw_get_slave_port ( \- slave -> slave_num ); \- cpsw_ale_add_mcast ( cpsw -> ale , addr , \- 1 << slave_port | ALE_PORT_HOST , \- ALE_VLAN , slave -> port_vlan , 0 ); \- } else { \- cpsw_ale_add_mcast ( cpsw -> ale , addr , \- ALE_ALL_PORTS , \- 0 , 0 , 0 ); \- } \- } while ( 0 ) - static inline int cpsw_get_slave_port ( u32 slave_num )
{
return slave_num + 1 ;
}
+ static inline void cpsw_src_port_detect ( struct cpsw_common * cpsw , int status , + struct sk_buff * skb ) + { + if ( ! cpsw -> data . dual_emac ) + return ; + + if ( CPDMA_RX_SOURCE_PORT ( status ) == 1 ) + skb -> dev = cpsw -> slaves [ 0 ]. ndev ; + else if ( CPDMA_RX_SOURCE_PORT ( status ) == 2 ) + skb -> dev = cpsw -> slaves [ 1 ]. ndev ; + } + + static void cpsw_add_mcast ( struct cpsw_priv * priv , u8 * addr ) + { + struct cpsw_common * cpsw = priv -> cpsw ; + + if ( cpsw -> data . dual_emac ) { + struct cpsw_slave * slave = cpsw -> slaves + priv -> emac_port ; + int slave_port = cpsw_get_slave_port ( slave -> slave_num ); + + cpsw_ale_add_mcast ( cpsw -> ale , addr , + 1 << slave_port | ALE_PORT_HOST , + ALE_VLAN , slave -> port_vlan , 0 ); + return ; + } + + cpsw_ale_add_mcast ( cpsw -> ale , addr , ALE_ALL_PORTS , 0 , 0 , 0 ); + } + static void cpsw_set_promiscious ( struct net_device * ndev , bool enable )
{
struct cpsw_common * cpsw = ndev_to_cpsw ( ndev ); @@ -706,7 +706,7 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
/* program multicast address list into ALE register */
netdev_for_each_mc_addr ( ha , ndev ) {
- cpsw_add_mcast ( cpsw , priv , ( u8 * ) ha -> addr ); + cpsw_add_mcast ( priv , ( u8 * ) ha -> addr ); }
}
} @@ -801,7 +801,8 @@ static void cpsw_rx_handler(void *token, int len, int status)
int ret = 0 ;
struct cpsw_common * cpsw = ndev_to_cpsw ( ndev );
- cpsw_dual_emac_src_port_detect ( cpsw , status , ndev , skb ); + cpsw_src_port_detect ( cpsw , status , skb ); + ndev = skb -> dev ;
if ( unlikely ( status < 0 ) || unlikely ( ! netif_running ( ndev ))) {
/* In dual emac mode check for all interfaces */ --
2.17.1
On 07/27/2018 02:13 PM, Ivan Khoronzhuk wrote: Replace ugly macroses on functions.
Signed-off-by: Ivan Khoronzhuk <redacted>
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
--
regards,
-grygorii
On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote: Replace ugly macroses on functions.
Careful, see below.
quoted hunk diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c [] quoted hunk @@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = {
(func)(slave++, ##arg); \
} while (0)
- #define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \- do { \- if (!cpsw->data.dual_emac) \- break; \- if (CPDMA_RX_SOURCE_PORT(status) == 1) { \- ndev = cpsw->slaves[0].ndev; \- skb->dev = ndev; \- } else if (CPDMA_RX_SOURCE_PORT(status) == 2) { \- ndev = cpsw->slaves[1].ndev; \- skb->dev = ndev; \- } \- } while (0)- #define cpsw_add_mcast(cpsw, priv, addr) \- do { \- if (cpsw->data.dual_emac) { \- struct cpsw_slave *slave = cpsw->slaves + \- priv->emac_port; \- int slave_port = cpsw_get_slave_port( \- slave->slave_num); \- cpsw_ale_add_mcast(cpsw->ale, addr, \- 1 << slave_port | ALE_PORT_HOST, \- ALE_VLAN, slave->port_vlan, 0); \- } else { \- cpsw_ale_add_mcast(cpsw->ale, addr, \- ALE_ALL_PORTS, \- 0, 0, 0); \- } \- } while (0)- static inline int cpsw_get_slave_port(u32 slave_num)
{
return slave_num + 1;
}
+ static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,+ struct sk_buff *skb)+ {+ if (!cpsw->data.dual_emac)+ return;+ + if (CPDMA_RX_SOURCE_PORT(status) == 1)+ skb->dev = cpsw->slaves[0].ndev;+ else if (CPDMA_RX_SOURCE_PORT(status) == 2)+ skb->dev = cpsw->slaves[1].ndev;+ }
perhaps better as a switch/case
quoted hunk +
+static void cpsw_add_mcast(struct cpsw_priv *priv, u8 *addr)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+
+ if (cpsw->data.dual_emac) {
+ struct cpsw_slave *slave = cpsw->slaves + priv->emac_port;
+ int slave_port = cpsw_get_slave_port(slave->slave_num);
+
+ cpsw_ale_add_mcast(cpsw->ale, addr,
+ 1 << slave_port | ALE_PORT_HOST,
+ ALE_VLAN, slave->port_vlan, 0);
+ return;
+ }
+
+ cpsw_ale_add_mcast(cpsw->ale, addr, ALE_ALL_PORTS, 0, 0, 0);
+}
+
static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
{
struct cpsw_common *cpsw = ndev_to_cpsw(ndev); @@ -706,7 +706,7 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
/* program multicast address list into ALE register */
netdev_for_each_mc_addr(ha, ndev) {
- cpsw_add_mcast(cpsw, priv, (u8 *)ha->addr);+ cpsw_add_mcast(priv, (u8 *)ha->addr); }
}
} @@ -801,7 +801,8 @@ static void cpsw_rx_handler(void *token, int len, int status)
int ret = 0;
struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
- cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb);+ cpsw_src_port_detect(cpsw, status, skb);+ ndev = skb->dev;
This is not the same code as the new function
is not guaranteed to succeed or set skb->dev.
if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {
/* In dual emac mode check for all interfaces */
On Fri, Jul 27, 2018 at 12:21:07PM -0700, Joe Perches wrote: On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote: quoted Replace ugly macroses on functions.
Careful, see below.
quoted diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c [] quoted @@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = {
(func)(slave++, ##arg); \
} while (0)
- #define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \- do { \- if (!cpsw->data.dual_emac) \- break; \- if (CPDMA_RX_SOURCE_PORT(status) == 1) { \- ndev = cpsw->slaves[0].ndev; \- skb->dev = ndev; \- } else if (CPDMA_RX_SOURCE_PORT(status) == 2) { \- ndev = cpsw->slaves[1].ndev; \- skb->dev = ndev; \- } \- } while (0)- #define cpsw_add_mcast(cpsw, priv, addr) \- do { \- if (cpsw->data.dual_emac) { \- struct cpsw_slave *slave = cpsw->slaves + \- priv->emac_port; \- int slave_port = cpsw_get_slave_port( \- slave->slave_num); \- cpsw_ale_add_mcast(cpsw->ale, addr, \- 1 << slave_port | ALE_PORT_HOST, \- ALE_VLAN, slave->port_vlan, 0); \- } else { \- cpsw_ale_add_mcast(cpsw->ale, addr, \- ALE_ALL_PORTS, \- 0, 0, 0); \- } \- } while (0)- static inline int cpsw_get_slave_port(u32 slave_num)
{
return slave_num + 1;
}
+ static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,+ struct sk_buff *skb)+ {+ if (!cpsw->data.dual_emac)+ return;+ + if (CPDMA_RX_SOURCE_PORT(status) == 1)+ skb->dev = cpsw->slaves[0].ndev;+ else if (CPDMA_RX_SOURCE_PORT(status) == 2)+ skb->dev = cpsw->slaves[1].ndev;+ }
perhaps better as a switch/case not better, it's shorter.
quoted +
+static void cpsw_add_mcast(struct cpsw_priv *priv, u8 *addr)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+
+ if (cpsw->data.dual_emac) {
+ struct cpsw_slave *slave = cpsw->slaves + priv->emac_port;
+ int slave_port = cpsw_get_slave_port(slave->slave_num);
+
+ cpsw_ale_add_mcast(cpsw->ale, addr,
+ 1 << slave_port | ALE_PORT_HOST,
+ ALE_VLAN, slave->port_vlan, 0);
+ return;
+ }
+
+ cpsw_ale_add_mcast(cpsw->ale, addr, ALE_ALL_PORTS, 0, 0, 0);
+}
+
static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
{
struct cpsw_common *cpsw = ndev_to_cpsw(ndev); @@ -706,7 +706,7 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
/* program multicast address list into ALE register */
netdev_for_each_mc_addr(ha, ndev) {
- cpsw_add_mcast(cpsw, priv, (u8 *)ha->addr);+ cpsw_add_mcast(priv, (u8 *)ha->addr); }
}
} @@ -801,7 +801,8 @@ static void cpsw_rx_handler(void *token, int len, int status)
int ret = 0;
struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
- cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb);+ cpsw_src_port_detect(cpsw, status, skb);+ ndev = skb->dev;
This is not the same code as the new function
is not guaranteed to succeed or set skb->dev. Guaranteed by above skb->dev is initialized already.
The function reflects previous macro.
Even more, seems that here is duplication of ndev=skb->dev;
It should be removed at init ), even if it 100% is optimized by complier.
So guaranteed a little more then needed ), will send v2 with removed
ndev = skb->dev at init if no objection.
quoted if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {
/* In dual emac mode check for all interfaces */
--
Regards,
Ivan Khoronzhuk
On Fri, Jul 27, 2018 at 10:36:43PM +0300, Ivan Khoronzhuk wrote: On Fri, Jul 27, 2018 at 12:21:07PM -0700, Joe Perches wrote: quoted On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote: quoted Replace ugly macroses on functions.
Careful, see below.
quoted diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c [] quoted @@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = { (func)(slave++, ##arg); \
} while (0)
-#define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \
- do { \
- if (!cpsw->data.dual_emac) \
- break; \
- if (CPDMA_RX_SOURCE_PORT(status) == 1) { \
- ndev = cpsw->slaves[0].ndev; \
- skb->dev = ndev; \
- } else if (CPDMA_RX_SOURCE_PORT(status) == 2) { \
- ndev = cpsw->slaves[1].ndev; \
- skb->dev = ndev; \
- } \
- } while (0)
-#define cpsw_add_mcast(cpsw, priv, addr) \
- do { \
- if (cpsw->data.dual_emac) { \
- struct cpsw_slave *slave = cpsw->slaves + \
- priv->emac_port; \
- int slave_port = cpsw_get_slave_port( \
- slave->slave_num); \
- cpsw_ale_add_mcast(cpsw->ale, addr, \
- 1 << slave_port | ALE_PORT_HOST, \
- ALE_VLAN, slave->port_vlan, 0); \
- } else { \
- cpsw_ale_add_mcast(cpsw->ale, addr, \
- ALE_ALL_PORTS, \
- 0, 0, 0); \
- } \
- } while (0)
-
static inline int cpsw_get_slave_port(u32 slave_num)
{
return slave_num + 1;
}
+static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,
+ struct sk_buff *skb)
+{
+ if (!cpsw->data.dual_emac)
+ return;
+
+ if (CPDMA_RX_SOURCE_PORT(status) == 1)
+ skb->dev = cpsw->slaves[0].ndev;
+ else if (CPDMA_RX_SOURCE_PORT(status) == 2)
+ skb->dev = cpsw->slaves[1].ndev;
+}
perhaps better as a switch/case not better, it's shorter.
quoted quoted +
+static void cpsw_add_mcast(struct cpsw_priv *priv, u8 *addr)
+{
+ struct cpsw_common *cpsw = priv->cpsw;
+
+ if (cpsw->data.dual_emac) {
+ struct cpsw_slave *slave = cpsw->slaves + priv->emac_port;
+ int slave_port = cpsw_get_slave_port(slave->slave_num);
+
+ cpsw_ale_add_mcast(cpsw->ale, addr,
+ 1 << slave_port | ALE_PORT_HOST,
+ ALE_VLAN, slave->port_vlan, 0);
+ return;
+ }
+
+ cpsw_ale_add_mcast(cpsw->ale, addr, ALE_ALL_PORTS, 0, 0, 0);
+}
+
static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
{
struct cpsw_common *cpsw = ndev_to_cpsw(ndev); @@ -706,7 +706,7 @@ static void cpsw_ndo_set_rx_mode(struct net_device *ndev)
/* program multicast address list into ALE register */
netdev_for_each_mc_addr(ha, ndev) {
- cpsw_add_mcast(cpsw, priv, (u8 *)ha->addr);
+ cpsw_add_mcast(priv, (u8 *)ha->addr);
}
}
} @@ -801,7 +801,8 @@ static void cpsw_rx_handler(void *token, int len, int status) int ret = 0;
struct cpsw_common *cpsw = ndev_to_cpsw(ndev);
- cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb);
+ cpsw_src_port_detect(cpsw, status, skb);
+ ndev = skb->dev;
This is not the same code as the new function
is not guaranteed to succeed or set skb->dev. Guaranteed by above skb->dev is initialized already.
The function reflects previous macro.
Even more, seems that here is duplication of ndev=skb->dev;
It should be removed at init ), even if it 100% is optimized by complier.
So guaranteed a little more then needed ), will send v2 with removed
ndev = skb->dev at init if no objection.
But no, not duplicates.
It's used to get cpsw. So everyting is fine.
Patch should go as is.
quoted quoted if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {
/* In dual emac mode check for all interfaces */
--
Regards,
Ivan Khoronzhuk
--
Regards,
Ivan Khoronzhuk
On Fri, 2018-07-27 at 22:36 +0300, Ivan Khoronzhuk wrote: On Fri, Jul 27, 2018 at 12:21:07PM -0700, Joe Perches wrote: quoted On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote: quoted Replace ugly macroses on functions.
Careful, see below.
quoted diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
[] quoted @@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = {
(func)(slave++, ##arg); \
} while (0)
- #define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \- do { \- if (!cpsw->data.dual_emac) \- break; \- if (CPDMA_RX_SOURCE_PORT(status) == 1) { \- ndev = cpsw->slaves[0].ndev; \- skb->dev = ndev; \- } else if (CPDMA_RX_SOURCE_PORT(status) == 2) { \- ndev = cpsw->slaves[1].ndev; \- skb->dev = ndev; \- } \- } while (0)- #define cpsw_add_mcast(cpsw, priv, addr) \- do { \- if (cpsw->data.dual_emac) { \- struct cpsw_slave *slave = cpsw->slaves + \- priv->emac_port; \- int slave_port = cpsw_get_slave_port( \- slave->slave_num); \- cpsw_ale_add_mcast(cpsw->ale, addr, \- 1 << slave_port | ALE_PORT_HOST, \- ALE_VLAN, slave->port_vlan, 0); \- } else { \- cpsw_ale_add_mcast(cpsw->ale, addr, \- ALE_ALL_PORTS, \- 0, 0, 0); \- } \- } while (0)- static inline int cpsw_get_slave_port(u32 slave_num)
{
return slave_num + 1;
}
+ static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,+ struct sk_buff *skb)+ {+ if (!cpsw->data.dual_emac)+ return;+ + if (CPDMA_RX_SOURCE_PORT(status) == 1)+ skb->dev = cpsw->slaves[0].ndev;+ else if (CPDMA_RX_SOURCE_PORT(status) == 2)+ skb->dev = cpsw->slaves[1].ndev;+ }
perhaps better as a switch/case
not better, it's shorter.
True for the source code but it compiles to more object code.
$ cat foo.c
struct cpsw_common {
struct {
int dual_emac;
} data;
struct {
int ndev;
} slaves[2];
};
struct sk_buff {
int dev;
};
#define CPDMA_RX_SOURCE_PORT(__status__) ((__status__ >> 16) & 0x7)
#if defined SWITCH
void foo(struct cpsw_common *cpsw, int status, struct sk_buff *skb)
{
if (!cpsw->data.dual_emac)
return;
switch (CPDMA_RX_SOURCE_PORT(status)) {
case 1:
skb->dev = cpsw->slaves[0].ndev;
break;
case 2:
skb->dev = cpsw->slaves[1].ndev;
break;
}
}
#else
void foo(struct cpsw_common *cpsw, int status, struct sk_buff *skb)
{
if (!cpsw->data.dual_emac)
return;
if (CPDMA_RX_SOURCE_PORT(status) == 1)
skb->dev = cpsw->slaves[0].ndev;
else if (CPDMA_RX_SOURCE_PORT(status) == 2)
skb->dev = cpsw->slaves[1].ndev;
}
#endif
$ gcc -c -O2 -DSWITCH foo.c
$ size foo.o
text data bss dec hex filename
94 0 0 94 5e foo.o
$ objdump -d foo.o
foo.o: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <foo>:
0: 8b 07 mov (%rdi),%eax
2: 85 c0 test %eax,%eax
4: 74 15 je 1b <foo+0x1b>
6: c1 fe 10 sar $0x10,%esi
9: 83 e6 07 and $0x7,%esi
c: 83 fe 01 cmp $0x1,%esi
f: 74 17 je 28 <foo+0x28>
11: 83 fe 02 cmp $0x2,%esi
14: 75 0a jne 20 <foo+0x20>
16: 8b 47 08 mov 0x8(%rdi),%eax
19: 89 02 mov %eax,(%rdx)
1b: f3 c3 repz retq
1d: 0f 1f 00 nopl (%rax)
20: f3 c3 repz retq
22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
28: 8b 47 04 mov 0x4(%rdi),%eax
2b: 89 02 mov %eax,(%rdx)
2d: c3 retq
$ gcc -c -O2 foo.c
$ size foo.o
text data bss dec hex filename
102 0 0 102 66 foo.o
$ objdump -d foo.o
foo.o: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <foo>:
0: 8b 07 mov (%rdi),%eax
2: 85 c0 test %eax,%eax
4: 74 10 je 16 <foo+0x16>
6: c1 fe 10 sar $0x10,%esi
9: 83 e6 07 and $0x7,%esi
c: 83 fe 01 cmp $0x1,%esi
f: 74 0f je 20 <foo+0x20>
11: 83 fe 02 cmp $0x2,%esi
14: 74 1a je 30 <foo+0x30>
16: f3 c3 repz retq
18: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
1f: 00
20: 8b 47 04 mov 0x4(%rdi),%eax
23: 89 02 mov %eax,(%rdx)
25: c3 retq
26: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
2d: 00 00 00
30: 8b 47 08 mov 0x8(%rdi),%eax
33: 89 02 mov %eax,(%rdx)
35: c3 retq
On Fri, Jul 27, 2018 at 01:04:22PM -0700, Joe Perches wrote: On Fri, 2018-07-27 at 22:36 +0300, Ivan Khoronzhuk wrote: quoted On Fri, Jul 27, 2018 at 12:21:07PM -0700, Joe Perches wrote: quoted On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote: quoted Replace ugly macroses on functions.
Careful, see below.
quoted diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
[] quoted @@ -565,40 +565,40 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = {
(func)(slave++, ##arg); \
} while (0)
- #define cpsw_dual_emac_src_port_detect(cpsw, status, ndev, skb) \- do { \- if (!cpsw->data.dual_emac) \- break; \- if (CPDMA_RX_SOURCE_PORT(status) == 1) { \- ndev = cpsw->slaves[0].ndev; \- skb->dev = ndev; \- } else if (CPDMA_RX_SOURCE_PORT(status) == 2) { \- ndev = cpsw->slaves[1].ndev; \- skb->dev = ndev; \- } \- } while (0)- #define cpsw_add_mcast(cpsw, priv, addr) \- do { \- if (cpsw->data.dual_emac) { \- struct cpsw_slave *slave = cpsw->slaves + \- priv->emac_port; \- int slave_port = cpsw_get_slave_port( \- slave->slave_num); \- cpsw_ale_add_mcast(cpsw->ale, addr, \- 1 << slave_port | ALE_PORT_HOST, \- ALE_VLAN, slave->port_vlan, 0); \- } else { \- cpsw_ale_add_mcast(cpsw->ale, addr, \- ALE_ALL_PORTS, \- 0, 0, 0); \- } \- } while (0)- static inline int cpsw_get_slave_port(u32 slave_num)
{
return slave_num + 1;
}
+ static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,+ struct sk_buff *skb)+ {+ if (!cpsw->data.dual_emac)+ return;+ + if (CPDMA_RX_SOURCE_PORT(status) == 1)+ skb->dev = cpsw->slaves[0].ndev;+ else if (CPDMA_RX_SOURCE_PORT(status) == 2)+ skb->dev = cpsw->slaves[1].ndev;+ }
perhaps better as a switch/case
not better, it's shorter.
True for the source code but it compiles to more object code.
$ cat foo.c
struct cpsw_common {
struct {
int dual_emac;
} data;
struct {
int ndev;
} slaves[2];
};
struct sk_buff {
int dev;
};
#define CPDMA_RX_SOURCE_PORT(__status__) ((__status__ >> 16) & 0x7)
#if defined SWITCH
void foo(struct cpsw_common *cpsw, int status, struct sk_buff *skb)
{
if (!cpsw->data.dual_emac)
return;
switch (CPDMA_RX_SOURCE_PORT(status)) {
case 1:
skb->dev = cpsw->slaves[0].ndev;
break;
case 2:
skb->dev = cpsw->slaves[1].ndev;
break;
}
}
#else
void foo(struct cpsw_common *cpsw, int status, struct sk_buff *skb)
{
if (!cpsw->data.dual_emac)
return;
if (CPDMA_RX_SOURCE_PORT(status) == 1)
skb->dev = cpsw->slaves[0].ndev;
else if (CPDMA_RX_SOURCE_PORT(status) == 2)
skb->dev = cpsw->slaves[1].ndev;
}
#endif
$ gcc -c -O2 -DSWITCH foo.c
$ size foo.o
text data bss dec hex filename
94 0 0 94 5e foo.o
$ objdump -d foo.o
foo.o: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <foo>:
0: 8b 07 mov (%rdi),%eax
2: 85 c0 test %eax,%eax
4: 74 15 je 1b <foo+0x1b>
6: c1 fe 10 sar $0x10,%esi
9: 83 e6 07 and $0x7,%esi
c: 83 fe 01 cmp $0x1,%esi
f: 74 17 je 28 <foo+0x28>
11: 83 fe 02 cmp $0x2,%esi
14: 75 0a jne 20 <foo+0x20>
16: 8b 47 08 mov 0x8(%rdi),%eax
19: 89 02 mov %eax,(%rdx)
1b: f3 c3 repz retq
1d: 0f 1f 00 nopl (%rax)
20: f3 c3 repz retq
22: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
28: 8b 47 04 mov 0x4(%rdi),%eax
2b: 89 02 mov %eax,(%rdx)
2d: c3 retq
$ gcc -c -O2 foo.c
$ size foo.o
text data bss dec hex filename
102 0 0 102 66 foo.o
$ objdump -d foo.o
foo.o: file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <foo>:
0: 8b 07 mov (%rdi),%eax
2: 85 c0 test %eax,%eax
4: 74 10 je 16 <foo+0x16>
6: c1 fe 10 sar $0x10,%esi
9: 83 e6 07 and $0x7,%esi
c: 83 fe 01 cmp $0x1,%esi
f: 74 0f je 20 <foo+0x20>
11: 83 fe 02 cmp $0x2,%esi
14: 74 1a je 30 <foo+0x30>
16: f3 c3 repz retq
18: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1)
1f: 00
20: 8b 47 04 mov 0x4(%rdi),%eax
23: 89 02 mov %eax,(%rdx)
25: c3 retq
26: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
2d: 00 00 00
30: 8b 47 08 mov 0x8(%rdi),%eax
33: 89 02 mov %eax,(%rdx)
35: c3 retq
This driver, mainly used for ARM
For ARM, situation a little bit different:
$ arm-linux-gnueabihf-gcc -c -O2 -DSWITCH foo.c
$ size foo.o
text data bss dec hex filename
32 0 0 32 20 foo.o
$ arm-linux-gnueabihf-objdump -d foo.o
foo.o: file format elf32-littlearm
Disassembly of section .text:
00000000 <foo>:
0: 6803 ldr r3, [r0, #0]
2: b143 cbz r3, 16 <foo+0x16>
4: f3c1 4102 ubfx r1, r1, #16, #3
8: 2901 cmp r1, #1
a: d005 beq.n 18 <foo+0x18>
c: 2902 cmp r1, #2
e: d000 beq.n 12 <foo+0x12>
10: 4770 bx lr
12: 6883 ldr r3, [r0, #8]
14: 6013 str r3, [r2, #0]
16: 4770 bx lr
18: 6843 ldr r3, [r0, #4]
1a: 6013 str r3, [r2, #0]
1c: 4770 bx lr
1e: bf00 nop
$ arm-linux-gnueabihf-gcc -c -O2 foo.c
$ size foo.o
text data bss dec hex filename
28 0 0 28 1c foo.o
$ arm-linux-gnueabihf-objdump -d foo.o
foo.o: file format elf32-littlearm
Disassembly of section .text:
00000000 <foo>:
0: 6803 ldr r3, [r0, #0]
2: b13b cbz r3, 14 <foo+0x14>
4: f3c1 4102 ubfx r1, r1, #16, #3
8: 2901 cmp r1, #1
a: d004 beq.n 16 <foo+0x16>
c: 2902 cmp r1, #2
e: bf04 itt eq
10: 6883 ldreq r3, [r0, #8]
12: 6013 streq r3, [r2, #0]
14: 4770 bx lr
16: 6843 ldr r3, [r0, #4]
18: 6013 str r3, [r2, #0]
1a: 4770 bx lr
So, it's shorter.
--
Regards,
Ivan Khoronzhuk
On Fri, 2018-07-27 at 23:23 +0300, Ivan Khoronzhuk wrote: For ARM, situation a little bit different:
$ arm-linux-gnueabihf-gcc -c -O2 -DSWITCH foo.c
$ size foo.o
text data bss dec hex filename
32 0 0 32 20 foo.o [] $ arm-linux-gnueabihf-gcc -c -O2 foo.c
$ size foo.o
text data bss dec hex filename
28 0 0 28 1c foo.o [] So, it's shorter.
No worries.
I was kinda surprised the object code wasn't identical anyway.
cheers, Joe
+static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,
+ struct sk_buff *skb)
Please don't use inline. Let the compiler decide.
Andrew
On Fri, Jul 27, 2018 at 10:32:03PM +0200, Andrew Lunn wrote: quoted +static inline void cpsw_src_port_detect(struct cpsw_common *cpsw, int status,
+ struct sk_buff *skb)
Please don't use inline. Let the compiler decide.
Andrew
Corrected in v2
--
Regards,
Ivan Khoronzhuk