[OpenWrt-Devel] [PATCH] kernel: ar8xxx: get_arl_table now shows all ports of an entry

[John Crispin]

looks correct but few nitpicks inline

On 22/11/2018 15:46, Günther Kelleter wrote:
> Multicast ARL entries can have multiple destination ports.
this statement is correct but does not describe what the patch does :-)
>
> Signed-off-by: Günther Kelleter <guenther.kelleter at devolo.de>
> ---
>   target/linux/generic/files/drivers/net/phy/ar8216.c | 12 +++---------
>   target/linux/generic/files/drivers/net/phy/ar8216.h |  3 ++-
>   target/linux/generic/files/drivers/net/phy/ar8327.c | 10 ++--------
>   target/linux/generic/files/drivers/net/phy/ar8327.h |  1 +
>   4 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.c b/target/linux/generic/files/drivers/net/phy/ar8216.c
> index 7512ee1b43..9a8e83a2d6 100644
> --- a/target/linux/generic/files/drivers/net/phy/ar8216.c
> +++ b/target/linux/generic/files/drivers/net/phy/ar8216.c
> @@ -749,7 +749,6 @@ static void ar8216_get_arl_entry(struct ar8xxx_priv *priv,
>   	u16 r2, page;
>   	u16 r1_func0, r1_func1, r1_func2;
>   	u32 t, val0, val1, val2;
> -	int i;
>   
>   	split_addr(AR8216_REG_ATU_FUNC0, &r1_func0, &r2, &page);
>   	r2 |= 0x10;
> @@ -785,12 +784,7 @@ static void ar8216_get_arl_entry(struct ar8xxx_priv *priv,
>   		if (!*status)
>   			break;
>   
> -		i = 0;
> -		t = AR8216_ATU_PORT0;
> -		while (!(val2 & t) && ++i < priv->dev.ports)
> -			t <<= 1;
> -
> -		a->port = i;
> +		a->portmap = (val2 & AR8216_ATU_PORTS) >> AR8216_ATU_PORTS_S;
>   		a->mac[0] = (val0 & AR8216_ATU_ADDR5) >> AR8216_ATU_ADDR5_S;
>   		a->mac[1] = (val0 & AR8216_ATU_ADDR4) >> AR8216_ATU_ADDR4_S;
>   		a->mac[2] = (val1 & AR8216_ATU_ADDR3) >> AR8216_ATU_ADDR3_S;
> @@ -1516,7 +1510,7 @@ ar8xxx_sw_get_arl_table(struct switch_dev *dev,
>   		 */
>   		for (j = 0; j < i; ++j) {
>   			a1 = &priv->arl_table[j];
> -			if (a->port == a1->port && !memcmp(a->mac, a1->mac, sizeof(a->mac)))
> +			if (!memcmp(a->mac, a1->mac, sizeof(a->mac)) && !(a->portmap &= ~a1->portmap))

this line is ugly to read and has a mask/assign hidden in the later 
clause making it really bad. please improve the readability.

     John



>   				goto duplicate;
>   		}
>   	}
> @@ -1534,7 +1528,7 @@ ar8xxx_sw_get_arl_table(struct switch_dev *dev,
>   	for (j = 0; j < priv->dev.ports; ++j) {
>   		for (k = 0; k < i; ++k) {
>   			a = &priv->arl_table[k];
> -			if (a->port != j)
> +			if (!(a->portmap & BIT(j)))
>   				continue;
>   			len += snprintf(buf + len, sizeof(priv->arl_buf) - len,
>   					"Port %d: MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
> diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.h b/target/linux/generic/files/drivers/net/phy/ar8216.h
> index ba0e0ddccd..509818c50d 100644
> --- a/target/linux/generic/files/drivers/net/phy/ar8216.h
> +++ b/target/linux/generic/files/drivers/net/phy/ar8216.h
> @@ -112,6 +112,7 @@
>   
>   #define AR8216_REG_ATU_FUNC2		0x0058
>   #define   AR8216_ATU_PORTS		BITS(0, 6)
> +#define   AR8216_ATU_PORTS_S		0
>   #define   AR8216_ATU_PORT0		BIT(0)
>   #define   AR8216_ATU_PORT1		BIT(1)
>   #define   AR8216_ATU_PORT2		BIT(2)
> @@ -367,7 +368,7 @@ enum arl_op {
>   };
>   
>   struct arl_entry {
> -	u8 port;
> +	u16 portmap;
>   	u8 mac[6];
>   };
>   
> diff --git a/target/linux/generic/files/drivers/net/phy/ar8327.c b/target/linux/generic/files/drivers/net/phy/ar8327.c
> index 74f0a08d76..803fb3d49f 100644
> --- a/target/linux/generic/files/drivers/net/phy/ar8327.c
> +++ b/target/linux/generic/files/drivers/net/phy/ar8327.c
> @@ -1057,8 +1057,7 @@ static void ar8327_get_arl_entry(struct ar8xxx_priv *priv,
>   	struct mii_bus *bus = priv->mii_bus;
>   	u16 r2, page;
>   	u16 r1_data0, r1_data1, r1_data2, r1_func;
> -	u32 t, val0, val1, val2;
> -	int i;
> +	u32 val0, val1, val2;
>   
>   	split_addr(AR8327_REG_ATU_DATA0, &r1_data0, &r2, &page);
>   	r2 |= 0x10;
> @@ -1095,12 +1094,7 @@ static void ar8327_get_arl_entry(struct ar8xxx_priv *priv,
>   		if (!*status)
>   			break;
>   
> -		i = 0;
> -		t = AR8327_ATU_PORT0;
> -		while (!(val1 & t) && ++i < AR8327_NUM_PORTS)
> -			t <<= 1;
> -
> -		a->port = i;
> +		a->portmap = (val1 & AR8327_ATU_PORTS) >> AR8327_ATU_PORTS_S;
>   		a->mac[0] = (val0 & AR8327_ATU_ADDR0) >> AR8327_ATU_ADDR0_S;
>   		a->mac[1] = (val0 & AR8327_ATU_ADDR1) >> AR8327_ATU_ADDR1_S;
>   		a->mac[2] = (val0 & AR8327_ATU_ADDR2) >> AR8327_ATU_ADDR2_S;
> diff --git a/target/linux/generic/files/drivers/net/phy/ar8327.h b/target/linux/generic/files/drivers/net/phy/ar8327.h
> index d53ef885b1..38e33ea57e 100644
> --- a/target/linux/generic/files/drivers/net/phy/ar8327.h
> +++ b/target/linux/generic/files/drivers/net/phy/ar8327.h
> @@ -199,6 +199,7 @@
>   #define   AR8327_ATU_ADDR5			BITS(8, 8)
>   #define   AR8327_ATU_ADDR5_S			8
>   #define   AR8327_ATU_PORTS			BITS(16, 7)
> +#define   AR8327_ATU_PORTS_S			16
>   #define   AR8327_ATU_PORT0			BIT(16)
>   #define   AR8327_ATU_PORT1			BIT(17)
>   #define   AR8327_ATU_PORT2			BIT(18)

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel