diff mbox series

[net-next] net: ethernet: ti: cpsw: replace unnecessarily macroses on functions

Message ID 20180727191318.18698-1-ivan.khoronzhuk@linaro.org
State Superseded
Headers show
Series [net-next] net: ethernet: ti: cpsw: replace unnecessarily macroses on functions | expand

Commit Message

Ivan Khoronzhuk July 27, 2018, 7:13 p.m. UTC 
Replace ugly macroses on functions.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

---

Based on net-next/master

 drivers/net/ethernet/ti/cpsw.c | 63 +++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 31 deletions(-)

-- 
2.17.1

Comments

Grygorii Strashko July 27, 2018, 7:15 p.m. UTC | #1 
On 07/27/2018 02:13 PM, Ivan Khoronzhuk wrote:
> Replace ugly macroses on functions.

> 

> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>


Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>



-- 
regards,
-grygorii
Ivan Khoronzhuk July 27, 2018, 7:36 p.m. UTC | #2 
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:

>> Replace ugly macroses on functions.

>

>Careful, see below.

>

>> diff --git 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;

>> +}

>

>perhaps better as a switch/case

not better, it's shorter.

>

>> +

>> +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.

>

>>  	if (unlikely(status < 0) || unlikely(!netif_running(ndev))) {

>>  		/* In dual emac mode check for all interfaces */


-- 
Regards,
Ivan Khoronzhuk
Joe Perches July 27, 2018, 8:04 p.m. UTC | #3 
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:

> > On Fri, 2018-07-27 at 22:13 +0300, Ivan Khoronzhuk wrote:

> > > Replace ugly macroses on functions.

> > 

> > Careful, see below.

> > 

> > > diff --git 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;

> > > +}

> > 

> > 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
Ivan Khoronzhuk July 27, 2018, 8:23 p.m. UTC | #4 
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:

>> 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:

>> > > Replace ugly macroses on functions.

>> >

>> > Careful, see below.

>> >

>> > > diff --git 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;

>> > > +}

>> >

>> > 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
Ivan Khoronzhuk July 27, 2018, 8:49 p.m. UTC | #5 
On Fri, Jul 27, 2018 at 10:32:03PM +0200, Andrew Lunn wrote:
>> +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
diff mbox series

Patch

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 */