Re: [PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb handling

[Egil Hjelmeland]

On 26. juli 2017 19:41, Andrew Lunn wrote:

Hi Egil


+/* This function will wait a while until mask & reg == value */
+/* Otherwise, return timeout */
+static int lan9303_csr_reg_wait(struct lan9303 *chip, int regno,
+   int mask, char value)
+{
+   int i;
+
+   for (i = 0; i < 0x1000; i++) {
+   u32 reg;
+
+   lan9303_read_switch_reg(chip, regno, );
+   if ((reg & mask) == value)
+   return 0;
+   }
+   return -ETIMEDOUT;


Busy looping is probably not a good idea. Can you add a usleep()?



Yes


+}
+
+static int _lan9303_alr_make_entry_raw(struct lan9303 *chip, u32 dat0, u32 
dat1)


What does the _ indicate. I could understand having it when you have
lan9303_alr_make_entry_raw() call _lan9303_alr_make_entry_raw() after
taking a lock, but i don't see anything like that here.


Just my sloppy convention for something private, deep down. I can remove
the _.


+{
+   struct lan9303_alr_cache_entry *entr = lan9303_alr_cache_find_mac(
+   chip, mac);


A long line like this should be split into a declaration and an
assignment.



OK




I would probably make this a separate patch.

   Andrew



Got it.

Re: [PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb handling

[Andrew Lunn]

Hi Egil

> +/* This function will wait a while until mask & reg == value */
> +/* Otherwise, return timeout */
> +static int lan9303_csr_reg_wait(struct lan9303 *chip, int regno,
> + int mask, char value)
> +{
> + int i;
> +
> + for (i = 0; i < 0x1000; i++) {
> + u32 reg;
> +
> + lan9303_read_switch_reg(chip, regno, );
> + if ((reg & mask) == value)
> + return 0;
> + }
> + return -ETIMEDOUT;

Busy looping is probably not a good idea. Can you add a usleep()?

> +}
> +
> +static int _lan9303_alr_make_entry_raw(struct lan9303 *chip, u32 dat0, u32 
> dat1)

What does the _ indicate. I could understand having it when you have
lan9303_alr_make_entry_raw() call _lan9303_alr_make_entry_raw() after
taking a lock, but i don't see anything like that here.

> +{
> + lan9303_write_switch_reg(
> + chip, LAN9303_SWE_ALR_WR_DAT_0, dat0);
> + lan9303_write_switch_reg(
> + chip, LAN9303_SWE_ALR_WR_DAT_1, dat1);
> + lan9303_write_switch_reg(
> + chip, LAN9303_SWE_ALR_CMD, ALR_CMD_MAKE_ENTRY);
> + lan9303_csr_reg_wait(
> + chip, LAN9303_SWE_ALR_CMD_STS, ALR_STS_MAKE_PEND, 0);
> + lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
> + return 0;
> +}
> +
> +typedef void alr_loop_cb_t(
> + struct lan9303 *chip, u32 dat0, u32 dat1, int portmap, void *ctx);
> +
> +static void lan9303_alr_loop(struct lan9303 *chip, alr_loop_cb_t *cb, void 
> *ctx)
> +{
> + int i;
> +
> + lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, ALR_CMD_GET_FIRST);
> + lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
> +
> + for (i = 1; i < LAN9303_NUM_ALR_RECORDS; i++) {
> + u32 dat0, dat1;
> + int alrport, portmap;
> +
> + lan9303_read_switch_reg(chip, LAN9303_SWE_ALR_RD_DAT_0, );
> + lan9303_read_switch_reg(chip, LAN9303_SWE_ALR_RD_DAT_1, );
> + if (dat1 & ALR_DAT1_END_OF_TABL)
> + break;
> +
> + alrport = (dat1 & ALR_DAT1_PORT_MASK) >> ALR_DAT1_PORT_BITOFFS;
> + portmap = alrport_2_portmap[alrport];
> +
> + cb(chip, dat0, dat1, portmap, ctx);
> +
> + lan9303_write_switch_reg(
> + chip, LAN9303_SWE_ALR_CMD, ALR_CMD_GET_NEXT);
> + lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
> + }
> +}
> +
> +/* ALR: lan9303_alr_loop callback functions */
> +
> +static void _alr_reg_to_mac(u32 dat0, u32 dat1, u8 mac[6])
> +{
> + mac[0] = (dat0 >>  0) & 0xff;
> + mac[1] = (dat0 >>  8) & 0xff;
> + mac[2] = (dat0 >> 16) & 0xff;
> + mac[3] = (dat0 >> 24) & 0xff;
> + mac[4] = (dat1 >>  0) & 0xff;
> + mac[5] = (dat1 >>  8) & 0xff;
> +}
> +
> +/* Clear learned (non-static) entry on given port */
> +static void alr_loop_cb_del_port_learned(
> + struct lan9303 *chip, u32 dat0, u32 dat1, int portmap, void *ctx)
> +{
> + int *port = ctx;
> +
> + if (((BIT(*port) & portmap) == 0) || (dat1 & ALR_DAT1_STATIC))
> + return;
> +
> + /* learned entries has only one port, we can just delete */
> + dat1 &= ~ALR_DAT1_VALID; /* delete entry */
> + _lan9303_alr_make_entry_raw(chip, dat0, dat1);
> +}
> +
> +struct port_fdb_dump_ctx {
> + int port;
> + struct switchdev_obj_port_fdb *fdb;
> + switchdev_obj_dump_cb_t   *cb;
> +};
> +
> +static void alr_loop_cb_fdb_port_dump(
> + struct lan9303 *chip, u32 dat0, u32 dat1, int portmap, void *ctx)
> +{
> + struct port_fdb_dump_ctx *dump_ctx = ctx;
> + struct switchdev_obj_port_fdb *fdb = dump_ctx->fdb;
> + u8 mac[ETH_ALEN];
> +
> + if ((BIT(dump_ctx->port) & portmap) == 0)
> + return;
> +
> + _alr_reg_to_mac(dat0, dat1, mac);
> + ether_addr_copy(fdb->addr, mac);
> + fdb->vid = 0;
> + fdb->ndm_state = (dat1 & ALR_DAT1_STATIC) ?
> + NUD_NOARP : NUD_REACHABLE;
> + dump_ctx->cb(>obj);
> +}
> +
> +/* ALR: Add/modify/delete ALR entries */
> +
> +/* Set a static ALR entry. Delete entry if port_map is zero */
> +static void _lan9303_alr_set_entry(struct lan9303 *chip, const u8 *mac,
> +u8 port_map, bool stp_override)
> +{
> + u32 dat0, dat1, alr_port;
> +
> + dat1 = ALR_DAT1_STATIC;
> + if (port_map)
> + dat1 |= ALR_DAT1_VALID; /* otherwise no ports: delete entry */
> + if (stp_override)
> + dat1 |= ALR_DAT1_AGE_OVERRID;
> +
> + alr_port = portmap_2_alrport[port_map & 7];
> + dat1 &= ~ALR_DAT1_PORT_MASK;
> + dat1 |= alr_port << ALR_DAT1_PORT_BITOFFS;
> +
> + dat0 = 0;
> + dat0 |= (mac[0] << 0);
> + dat0 |= (mac[1] << 8);
> + dat0 |= (mac[2] << 16);
> + dat0 |= (mac[3] << 24);
> +
> + dat1 |= (mac[4] << 0);
> + dat1 |= (mac[5] << 8);
> +
> + dev_dbg(chip->dev, "%s %pM %d %08x %08x\n",
> + __func__, mac, port_map, dat0, 

[PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb handling

[Egil Hjelmeland]

Added functions for accessing / managing the lan9303 ALR (Address Logic
Resolution).

Implemented DSA methods: set_addr, port_fast_age, port_fdb_prepare,
port_fdb_add, port_fdb_del, port_fdb_dump, port_mdb_prepare,
port_mdb_add and port_mdb_del.

Since the lan9303 do not offer reading specific ALR entry, the driver
caches all static entries - in a flat table.

Signed-off-by: Egil Hjelmeland 
---
 drivers/net/dsa/lan9303-core.c | 369 +
 drivers/net/dsa/lan9303.h  |  11 ++
 2 files changed, 380 insertions(+)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 426a75bd89f4..dc95973d62ed 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "lan9303.h"
 
@@ -121,6 +122,21 @@
 #define LAN9303_MAC_RX_CFG_2 0x0c01
 #define LAN9303_MAC_TX_CFG_2 0x0c40
 #define LAN9303_SWE_ALR_CMD 0x1800
+# define ALR_CMD_MAKE_ENTRYBIT(2)
+# define ALR_CMD_GET_FIRST BIT(1)
+# define ALR_CMD_GET_NEXT  BIT(0)
+#define LAN9303_SWE_ALR_WR_DAT_0 0x1801
+#define LAN9303_SWE_ALR_WR_DAT_1 0x1802
+# define ALR_DAT1_VALIDBIT(26)
+# define ALR_DAT1_END_OF_TABL  BIT(25)
+# define ALR_DAT1_AGE_OVERRID  BIT(25)
+# define ALR_DAT1_STATIC   BIT(24)
+# define ALR_DAT1_PORT_BITOFFS  16
+# define ALR_DAT1_PORT_MASK(7 << ALR_DAT1_PORT_BITOFFS)
+#define LAN9303_SWE_ALR_RD_DAT_0 0x1805
+#define LAN9303_SWE_ALR_RD_DAT_1 0x1806
+#define LAN9303_SWE_ALR_CMD_STS 0x1808
+# define ALR_STS_MAKE_PEND BIT(0)
 #define LAN9303_SWE_VLAN_CMD 0x180b
 # define LAN9303_SWE_VLAN_CMD_RNW BIT(5)
 # define LAN9303_SWE_VLAN_CMD_PVIDNVLAN BIT(4)
@@ -473,6 +489,229 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
return 0;
 }
 
+/* - Address Logic Resolution (ALR)--*/
+
+/* Map ALR-port bits to port bitmap, and back*/
+static const int alrport_2_portmap[] = {1, 2, 4, 0, 3, 5, 6, 7 };
+static const int portmap_2_alrport[] = {3, 0, 1, 4, 2, 5, 6, 7 };
+
+/* ALR: Cache static entries: mac address + port bitmap */
+
+/* Return pointer to first free ALR cache entry, return NULL if none */
+static struct lan9303_alr_cache_entry *lan9303_alr_cache_find_free(
+   struct lan9303 *chip)
+{
+   int i;
+   struct lan9303_alr_cache_entry *entr = chip->alr_cache;
+
+   for (i = 0; i < LAN9303_NUM_ALR_RECORDS; i++, entr++)
+   if (entr->port_map == 0)
+   return entr;
+   return NULL;
+}
+
+/* Return pointer to ALR cache entry matching MAC address */
+static struct lan9303_alr_cache_entry *lan9303_alr_cache_find_mac(
+   struct lan9303 *chip,
+   const u8 *mac_addr)
+{
+   int i;
+   struct lan9303_alr_cache_entry *entr = chip->alr_cache;
+
+   BUILD_BUG_ON_MSG(sizeof(struct lan9303_alr_cache_entry) & 1,
+"ether_addr_equal require u16 alignment");
+
+   for (i = 0; i < LAN9303_NUM_ALR_RECORDS; i++, entr++)
+   if (ether_addr_equal(entr->mac_addr, mac_addr))
+   return entr;
+   return NULL;
+}
+
+/* ALR: Actual register access functions */
+
+/* This function will wait a while until mask & reg == value */
+/* Otherwise, return timeout */
+static int lan9303_csr_reg_wait(struct lan9303 *chip, int regno,
+   int mask, char value)
+{
+   int i;
+
+   for (i = 0; i < 0x1000; i++) {
+   u32 reg;
+
+   lan9303_read_switch_reg(chip, regno, );
+   if ((reg & mask) == value)
+   return 0;
+   }
+   return -ETIMEDOUT;
+}
+
+static int _lan9303_alr_make_entry_raw(struct lan9303 *chip, u32 dat0, u32 
dat1)
+{
+   lan9303_write_switch_reg(
+   chip, LAN9303_SWE_ALR_WR_DAT_0, dat0);
+   lan9303_write_switch_reg(
+   chip, LAN9303_SWE_ALR_WR_DAT_1, dat1);
+   lan9303_write_switch_reg(
+   chip, LAN9303_SWE_ALR_CMD, ALR_CMD_MAKE_ENTRY);
+   lan9303_csr_reg_wait(
+   chip, LAN9303_SWE_ALR_CMD_STS, ALR_STS_MAKE_PEND, 0);
+   lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
+   return 0;
+}
+
+typedef void alr_loop_cb_t(
+   struct lan9303 *chip, u32 dat0, u32 dat1, int portmap, void *ctx);
+
+static void lan9303_alr_loop(struct lan9303 *chip, alr_loop_cb_t *cb, void 
*ctx)
+{
+   int i;
+
+   lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, ALR_CMD_GET_FIRST);
+   lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
+
+   for (i = 1; i < LAN9303_NUM_ALR_RECORDS; i++) {
+   u32 dat0, dat1;
+   int alrport, portmap;
+
+   lan9303_read_switch_reg(chip, LAN9303_SWE_ALR_RD_DAT_0, );
+   lan9303_read_switch_reg(chip, LAN9303_SWE_ALR_RD_DAT_1, );
+   if (dat1 & ALR_DAT1_END_OF_TABL)
+   break;
+
+   alrport