Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1695935 > unrolled thread

[PATCH net-next v2 00/10] net: dsa: lan9303: unicast offload, fdb,mdb,STP

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next v2 00/10] net: dsa: lan9303: unicast offload, fdb,mdb,STP Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 18:40 +0200
    [PATCH net-next v2 07/10] net: dsa: lan9303: Added basic offloading of unicast traffic Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 18:50 +0200
      Re: [PATCH net-next v2 07/10] net: dsa: lan9303: Added basic  offloading of unicast traffic Andrew Lunn <andrew@lunn.ch> - 2017-07-26 19:30 +0200
        Re: [PATCH net-next v2 07/10] net: dsa: lan9303: Added basic  offloading of unicast traffic Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-27 13:30 +0200
          Re: [PATCH net-next v2 07/10] net: dsa: lan9303: Added basic  offloading of unicast traffic Andrew Lunn <andrew@lunn.ch> - 2017-07-27 15:40 +0200
            Re: [PATCH net-next v2 07/10] net: dsa: lan9303: Added basic  offloading of unicast traffic Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-27 16:10 +0200
    [PATCH net-next v2 03/10] net: dsa: lan9303: Refactor lan9303_enable_packet_processing() Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 18:50 +0200
    [PATCH net-next v2 04/10] net: dsa: lan9303: Added adjust_link() method Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 18:50 +0200
      Re: [PATCH net-next v2 04/10] net: dsa: lan9303: Added adjust_link()  method Andrew Lunn <andrew@lunn.ch> - 2017-07-26 19:10 +0200
        Re: [PATCH net-next v2 04/10] net: dsa: lan9303: Added adjust_link()  method Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-27 12:50 +0200
    [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 18:50 +0200
      Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-07-25 21:20 +0200
        Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-26 14:20 +0200
          Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-07-26 16:40 +0200
            Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-26 17:00 +0200
            Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Andrew Lunn <andrew@lunn.ch> - 2017-07-26 20:00 +0200
              Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO  interface David Miller <davem@davemloft.net> - 2017-07-26 22:10 +0200
                Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-26 22:50 +0200
                  Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Andrew Lunn <andrew@lunn.ch> - 2017-07-26 23:50 +0200
      Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Andrew Lunn <andrew@lunn.ch> - 2017-07-26 19:00 +0200
        Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-28 13:10 +0200
          Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface Andrew Lunn <andrew@lunn.ch> - 2017-07-28 15:40 +0200
    [PATCH net-next v2 09/10] net: dsa: lan9303: Added Documentation/networking/dsa/lan9303.txt Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 19:20 +0200
    [PATCH net-next v2 10/10] net: dsa: lan9303: Only allocate 3 ports Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 19:20 +0200
    [PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb handling Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 19:20 +0200
      Re: [PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb  handling Andrew Lunn <andrew@lunn.ch> - 2017-07-26 19:50 +0200
        Re: [PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb  handling Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-27 13:10 +0200
    [PATCH net-next v2 05/10] net: dsa: added dsa_net_device_to_dsa_port() Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 19:30 +0200
    [PATCH net-next v2 02/10] net: dsa: lan9303: Do not disable/enable switch fabric port 0 at startup Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 20:10 +0200
      Re: [PATCH net-next v2 02/10] net: dsa: lan9303: Do not  disable/enable switch fabric port 0 at startup Andrew Lunn <andrew@lunn.ch> - 2017-07-26 19:00 +0200
        Re: [PATCH net-next v2 02/10] net: dsa: lan9303: Do not  disable/enable switch fabric port 0 at startup Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-27 12:40 +0200
    [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node swe_bcst_throt Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-25 20:20 +0200
      Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node  swe_bcst_throt Andrew Lunn <andrew@lunn.ch> - 2017-07-26 19:20 +0200
        Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node  swe_bcst_throt Egil Hjelmeland <privat@egil-hjelmeland.no> - 2017-07-27 13:00 +0200
          Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node  swe_bcst_throt Andrew Lunn <andrew@lunn.ch> - 2017-07-27 15:30 +0200
            Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node  swe_bcst_throt Jiri Pirko <jiri@resnulli.us> - 2017-07-27 15:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1698698 — Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface

On 26. juli 2017 18:55, Andrew Lunn wrote:
> On Tue, Jul 25, 2017 at 06:15:44PM +0200, Egil Hjelmeland wrote:
> It is better to use mdiobus_read/write or if you are nesting mdio
> busses, mdiobus_read_nested/mdiobus_write_nested. Please test this
> code with lockdep enabled.
> 

I have CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_MUTEXES. Should I enable
more?

Egil

[toc] | [prev] | [next] | [standalone]

#1698792 — Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface

On Fri, Jul 28, 2017 at 01:08:25PM +0200, Egil Hjelmeland wrote:
> On 26. juli 2017 18:55, Andrew Lunn wrote:
> >On Tue, Jul 25, 2017 at 06:15:44PM +0200, Egil Hjelmeland wrote:
> >It is better to use mdiobus_read/write or if you are nesting mdio
> >busses, mdiobus_read_nested/mdiobus_write_nested. Please test this
> >code with lockdep enabled.
> >
> 
> I have CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_MUTEXES. Should I enable
> more?

Hi Egil

Enable CONFIG_LOCKDEP and CONFIG_PROVE_LOCKING.

Any lockdep splat you get while accessing the mdio bus at this point
are probably false positives, since it is a different mutex. Using the
_nested() version should avoid these false positives. But you might
find other places your locking is not right.

	Andrew

[toc] | [prev] | [next] | [standalone]

#1695959 — [PATCH net-next v2 09/10] net: dsa: lan9303: Added Documentation/networking/dsa/lan9303.txt

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 Documentation/networking/dsa/lan9303.txt | 63 ++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/networking/dsa/lan9303.txt

diff --git a/Documentation/networking/dsa/lan9303.txt b/Documentation/networking/dsa/lan9303.txt
new file mode 100644
index 000000000000..ef5b3ca12a29
--- /dev/null
+++ b/Documentation/networking/dsa/lan9303.txt
@@ -0,0 +1,63 @@
+LAN9303 Ethernet switch driver
+==============================
+
+The LAN9303 is a three port 10/100 ethernet switch with integrated phys
+for the two external ethernet ports. The third port is an RMII/MII
+interface to a host master network interface (e.g. fixed link).
+
+
+Driver details
+==============
+
+The driver is implemented as a DSA driver, see
+Documentation/networking/dsa/dsa.txt.
+
+See Documentation/devicetree/bindings/net/dsa/lan9303.txt for device
+tree binding.
+
+The LAN9303 can be managed both via MDIO and I2C, both supported by this
+driver.
+
+At startup the driver configures the device to provide two separate
+network interfaces (which is the default state of a DSA device).
+
+When both user ports are joined to the same bridge, the normal
+HW MAC learning is enabled. This means that unicast traffic is forwarded
+in HW. STP is also supported in this mode.
+
+If one of the user ports leave the bridge,
+the ports goes back to the initial separated operation.
+
+The driver implements the port_fdb_xxx/port_mdb_xxx methods.
+
+
+Sysfs nodes
+===========
+
+When a user port is enabled, the driver creates sysfs directory
+/sys/class/net/xxx/lan9303 with the following files:
+
+ - swe_bcst_throt (RW): Set/get 6.4.7 Broadcast Storm Control
+      Throttle Level for the port. Accesses the corresponding bits of
+      the SWE_BCST_THROT register (13.4.3.23).
+
+
+Driver limitations
+==================
+
+ - No support for VLAN
+
+
+Bridging notes
+==============
+When the user ports are bridged, broadcasts, multicasts and unknown
+frames with unknown destination are flooded by the chip. Therefore SW
+flooding must be disabled by:
+
+   echo 0 > /sys/class/net/p1/brport/broadcast_flood
+   echo 0 > /sys/class/net/p1/brport/multicast_flood
+   echo 0 > /sys/class/net/p1/brport/unicast_flood
+   echo 0 > /sys/class/net/p2/brport/broadcast_flood
+   echo 0 > /sys/class/net/p2/brport/multicast_flood
+   echo 0 > /sys/class/net/p2/brport/unicast_flood
+
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1695960 — [PATCH net-next v2 10/10] net: dsa: lan9303: Only allocate 3 ports

Saving 2628 bytes.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/lan9303-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index dc95973d62ed..ad7a4c72e1fb 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -23,6 +23,8 @@
 
 #include "lan9303.h"
 
+#define LAN9303_NUM_PORTS 3
+
 /* 13.2 System Control and Status Registers
  * Multiply register number by 4 to get address offset.
  */
@@ -1361,7 +1363,7 @@ static struct dsa_switch_ops lan9303_switch_ops = {
 
 static int lan9303_register_switch(struct lan9303 *chip)
 {
-	chip->ds = dsa_switch_alloc(chip->dev, DSA_MAX_PORTS);
+	chip->ds = dsa_switch_alloc(chip->dev, LAN9303_NUM_PORTS);
 	if (!chip->ds)
 		return -ENOMEM;
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

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

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 <privat@egil-hjelmeland.no>
---
 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 <linux/mii.h>
 #include <linux/phy.h>
 #include <linux/if_bridge.h>
+#include <linux/etherdevice.h>
 
 #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_ENTRY    BIT(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_VALID        BIT(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, &reg);
+		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, &dat0);
+		lan9303_read_switch_reg(chip, LAN9303_SWE_ALR_RD_DAT_1, &dat1);
+		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(&fdb->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, dat1);
+	_lan9303_alr_make_entry_raw(chip, dat0, dat1);
+}
+
+/* Add port to static ALR entry, create new static entry if needed */
+static int lan9303_alr_add_port(struct lan9303 *chip, const u8 *mac,
+				int port, bool stp_override)
+{
+	struct lan9303_alr_cache_entry *entr = lan9303_alr_cache_find_mac(
+		chip, mac);
+
+	if (!entr) { /*New entry */
+		entr = lan9303_alr_cache_find_free(chip);
+		if (!entr)
+			return -ENOSPC;
+		ether_addr_copy(entr->mac_addr, mac);
+	}
+	entr->port_map |= BIT(port);
+	entr->stp_override = stp_override;
+	_lan9303_alr_set_entry(chip, mac, entr->port_map, stp_override);
+	return 0;
+}
+
+/* Delete static port from ALR entry, delete entry if last port */
+static int lan9303_alr_del_port(struct lan9303 *chip, const u8 *mac,
+				int port)
+{
+	struct lan9303_alr_cache_entry *entr = lan9303_alr_cache_find_mac(
+		chip, mac);
+
+	if (!entr) { /* no static entry found */
+		/* Should we delete any learned entry?
+		 * _lan9303_alr_set_entry(chip, mac, 0, false);
+		 */
+		return 0;
+	}
+	entr->port_map &= ~BIT(port); /* zero means its free again */
+	if (entr->port_map == 0)
+		eth_zero_addr(&entr->port_map);
+	_lan9303_alr_set_entry(chip, mac, entr->port_map, entr->stp_override);
+	return 0;
+}
+
+/* --------------------- Various chip setup ----------------------*/
 static int lan9303_disable_packet_processing(struct lan9303 *chip,
 					     unsigned int port)
 {
@@ -729,6 +968,14 @@ static int lan9303_setup(struct dsa_switch *ds)
 	return 0;
 }
 
+static int lan9303_set_addr(struct dsa_switch *ds, u8 *addr)
+{
+	struct lan9303 *chip = ds->priv;
+
+	lan9303_alr_add_port(chip, addr, 0, false);
+	return 0;
+}
+
 struct lan9303_mib_desc {
 	unsigned int offset; /* offset of first MAC */
 	const char *name;
@@ -974,9 +1221,123 @@ static void lan9303_port_stp_state_set(struct dsa_switch *ds, int port,
 				      portstate, portmask);
 }
 
+static void lan9303_port_fast_age(struct dsa_switch *ds, int port)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(%d)\n", __func__, port);
+	lan9303_alr_loop(chip, alr_loop_cb_del_port_learned, &port);
+}
+
+static int _lan9303_port_fdb_check(
+	struct lan9303 *chip, const u8 *mac, int vid)
+{
+	if (vid)
+		return -EOPNOTSUPP;
+	if (lan9303_alr_cache_find_mac(chip, mac))
+		return 0;
+	if (!lan9303_alr_cache_find_free(chip))
+		return -ENOSPC;
+	return 0;
+}
+
+static int lan9303_port_fdb_prepare(
+		struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_fdb *fdb,
+		struct switchdev_trans *trans)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(port %d, vid %d, %pM)\n",
+		__func__, port, fdb->vid, fdb->addr);
+	return _lan9303_port_fdb_check(chip, fdb->addr, fdb->vid);
+}
+
+static void lan9303_port_fdb_add(
+		struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_fdb *fdb,
+		struct switchdev_trans *trans)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(port %d, vid %d, %pM)\n",
+		__func__, port, fdb->vid, fdb->addr);
+	lan9303_alr_add_port(chip, fdb->addr, port, false);
+}
+
+static int lan9303_port_fdb_del(
+		struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_fdb *fdb)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(port %d, vid %d, %pM)\n",
+		__func__, port, fdb->vid, fdb->addr);
+	if (fdb->vid)
+		return -EOPNOTSUPP;
+	lan9303_alr_del_port(chip, fdb->addr, port);
+	return 0;
+}
+
+static int lan9303_port_fdb_dump(
+		struct dsa_switch *ds, int port,
+		struct switchdev_obj_port_fdb *fdb,
+		switchdev_obj_dump_cb_t *cb)
+{
+	struct lan9303 *chip = ds->priv;
+	struct port_fdb_dump_ctx dump_ctx = {
+		.port = port,
+		.fdb  = fdb,
+		.cb   = cb,
+	};
+
+	dev_dbg(chip->dev, "%s(%d)\n", __func__, port);
+	lan9303_alr_loop(chip, alr_loop_cb_fdb_port_dump, &dump_ctx);
+	return 0;
+}
+
+static int lan9303_port_mdb_prepare(
+		struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_mdb *mdb,
+		struct switchdev_trans *trans)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(port %d, vid %d, %pM)\n",
+		__func__, port, mdb->vid, mdb->addr);
+	return _lan9303_port_fdb_check(chip, mdb->addr, mdb->vid);
+}
+
+static void lan9303_port_mdb_add(
+		struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_mdb *mdb,
+		struct switchdev_trans *trans)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(port %d, vid %d, %pM)\n",
+		__func__, port, mdb->vid, mdb->addr);
+	lan9303_alr_add_port(chip, mdb->addr, port, false);
+}
+
+static int lan9303_port_mdb_del(
+		struct dsa_switch *ds, int port,
+		const struct switchdev_obj_port_mdb *mdb)
+{
+	struct lan9303 *chip = ds->priv;
+
+	dev_dbg(chip->dev, "%s(port %d, vid %d, %pM)\n",
+		__func__, port, mdb->vid, mdb->addr);
+	if (mdb->vid)
+		return -EOPNOTSUPP;
+	lan9303_alr_del_port(chip, mdb->addr, port);
+	return 0;
+}
+
 static struct dsa_switch_ops lan9303_switch_ops = {
 	.get_tag_protocol = lan9303_get_tag_protocol,
 	.setup = lan9303_setup,
+	.set_addr = lan9303_set_addr,
 	.phy_read = lan9303_phy_read,
 	.phy_write = lan9303_phy_write,
 	.adjust_link = lan9303_adjust_link,
@@ -988,6 +1349,14 @@ static struct dsa_switch_ops lan9303_switch_ops = {
 	.port_bridge_join       = lan9303_port_bridge_join,
 	.port_bridge_leave      = lan9303_port_bridge_leave,
 	.port_stp_state_set     = lan9303_port_stp_state_set,
+	.port_fast_age          = lan9303_port_fast_age,
+	.port_fdb_prepare       = lan9303_port_fdb_prepare,
+	.port_fdb_add           = lan9303_port_fdb_add,
+	.port_fdb_del           = lan9303_port_fdb_del,
+	.port_fdb_dump          = lan9303_port_fdb_dump,
+	.port_mdb_prepare       = lan9303_port_mdb_prepare,
+	.port_mdb_add           = lan9303_port_mdb_add,
+	.port_mdb_del           = lan9303_port_mdb_del,
 };
 
 static int lan9303_register_switch(struct lan9303 *chip)
diff --git a/drivers/net/dsa/lan9303.h b/drivers/net/dsa/lan9303.h
index 2d74d02c9cef..f714addbf1e2 100644
--- a/drivers/net/dsa/lan9303.h
+++ b/drivers/net/dsa/lan9303.h
@@ -11,6 +11,13 @@ struct lan9303_phy_ops {
 			     int regnum, u16 val);
 };
 
+#define LAN9303_NUM_ALR_RECORDS 512
+struct lan9303_alr_cache_entry {
+	u8  mac_addr[ETH_ALEN];
+	u8  port_map;           /* Bitmap of ports. Zero if unused entry */
+	u8  stp_override;       /* non zero if set ALR_DAT1_AGE_OVERRID */
+};
+
 struct lan9303 {
 	struct device *dev;
 	struct regmap *regmap;
@@ -22,6 +29,10 @@ struct lan9303 {
 	struct mutex indirect_mutex; /* protect indexed register access */
 	const struct lan9303_phy_ops *ops;
 	bool is_bridged; /* true if port 1 and 2 is bridged */
+	/* LAN9303 do not offer reading specific ALR entry. Cache all
+	 * static entries in a flat table
+	 **/
+	struct lan9303_alr_cache_entry alr_cache[LAN9303_NUM_ALR_RECORDS];
 };
 
 extern const struct regmap_access_table lan9303_register_set;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

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

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, &reg);
> +		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, &dat0);
> +		lan9303_read_switch_reg(chip, LAN9303_SWE_ALR_RD_DAT_1, &dat1);
> +		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(&fdb->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, dat1);
> +	_lan9303_alr_make_entry_raw(chip, dat0, dat1);
> +}
> +
> +/* Add port to static ALR entry, create new static entry if needed */
> +static int lan9303_alr_add_port(struct lan9303 *chip, const u8 *mac,
> +				int port, bool stp_override)
> +{
> +	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.

> +
> +	if (!entr) { /*New entry */
> +		entr = lan9303_alr_cache_find_free(chip);
> +		if (!entr)
> +			return -ENOSPC;
> +		ether_addr_copy(entr->mac_addr, mac);
> +	}
> +	entr->port_map |= BIT(port);
> +	entr->stp_override = stp_override;
> +	_lan9303_alr_set_entry(chip, mac, entr->port_map, stp_override);
> +	return 0;
> +}
> +
> +/* Delete static port from ALR entry, delete entry if last port */
> +static int lan9303_alr_del_port(struct lan9303 *chip, const u8 *mac,
> +				int port)
> +{
> +	struct lan9303_alr_cache_entry *entr = lan9303_alr_cache_find_mac(
> +		chip, mac);
> +
> +	if (!entr) { /* no static entry found */
> +		/* Should we delete any learned entry?
> +		 * _lan9303_alr_set_entry(chip, mac, 0, false);
> +		 */
> +		return 0;
> +	}
> +	entr->port_map &= ~BIT(port); /* zero means its free again */
> +	if (entr->port_map == 0)
> +		eth_zero_addr(&entr->port_map);
> +	_lan9303_alr_set_entry(chip, mac, entr->port_map, entr->stp_override);
> +	return 0;
> +}
> +
> +/* --------------------- Various chip setup ----------------------*/
>  static int lan9303_disable_packet_processing(struct lan9303 *chip,
>  					     unsigned int port)
>  {
> @@ -729,6 +968,14 @@ static int lan9303_setup(struct dsa_switch *ds)
>  	return 0;
>  }
>  
> +static int lan9303_set_addr(struct dsa_switch *ds, u8 *addr)
> +{
> +	struct lan9303 *chip = ds->priv;
> +
> +	lan9303_alr_add_port(chip, addr, 0, false);
> +	return 0;
> +}
> +

I would probably make this a separate patch.

  Andrew

[toc] | [prev] | [next] | [standalone]

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

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, &reg);
>> +		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.

[toc] | [prev] | [next] | [standalone]

#1695965 — [PATCH net-next v2 05/10] net: dsa: added dsa_net_device_to_dsa_port()

Allowing dsa drivers to attach sysfs nodes.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 include/net/dsa.h |  1 +
 net/dsa/slave.c   | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 88da272d20d0..a71c0a2401ee 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -450,6 +450,7 @@ void unregister_switch_driver(struct dsa_switch_driver *type);
 struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
 
 struct net_device *dsa_dev_to_net_device(struct device *dev);
+struct dsa_port *dsa_net_device_to_dsa_port(struct net_device *dev);
 
 /* Keep inline for faster access in hot path */
 static inline bool netdev_uses_dsa(struct net_device *dev)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9507bd38cf04..40410f1740de 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -209,6 +209,16 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	return -EOPNOTSUPP;
 }
 
+struct dsa_port *dsa_net_device_to_dsa_port(struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+
+	if (!dsa_slave_dev_check(dev))
+		return NULL;
+	return p->dp;
+}
+EXPORT_SYMBOL_GPL(dsa_net_device_to_dsa_port);
+
 static int dsa_slave_port_attr_set(struct net_device *dev,
 				   const struct switchdev_attr *attr,
 				   struct switchdev_trans *trans)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1695995 — [PATCH net-next v2 02/10] net: dsa: lan9303: Do not disable/enable switch fabric port 0 at startup

For some mysterious reason enable switch fabric port 0 TX fails to
work, when the TX has previous been disabled. Resolved by not
disable/enable switch fabric port 0 at startup. Port 1 and 2 are
still disabled in early init.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 drivers/net/dsa/lan9303-core.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index e622db586c3d..c2b53659f58f 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -557,9 +557,6 @@ static int lan9303_disable_processing(struct lan9303 *chip)
 {
 	int ret;
 
-	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
-	if (ret)
-		return ret;
 	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
 	if (ret)
 		return ret;
@@ -633,10 +630,6 @@ static int lan9303_setup(struct dsa_switch *ds)
 	if (ret)
 		dev_err(chip->dev, "failed to separate ports %d\n", ret);
 
-	ret = lan9303_enable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
-	if (ret)
-		dev_err(chip->dev, "failed to re-enable switching %d\n", ret);
-
 	return 0;
 }
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1697364 — Re: [PATCH net-next v2 02/10] net: dsa: lan9303: Do not disable/enable switch fabric port 0 at startup

On Tue, Jul 25, 2017 at 06:15:45PM +0200, Egil Hjelmeland wrote:
> For some mysterious reason enable switch fabric port 0 TX fails to
> work, when the TX has previous been disabled. Resolved by not
> disable/enable switch fabric port 0 at startup. Port 1 and 2 are
> still disabled in early init.
> 
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
> ---
>  drivers/net/dsa/lan9303-core.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index e622db586c3d..c2b53659f58f 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -557,9 +557,6 @@ static int lan9303_disable_processing(struct lan9303 *chip)
>  {
>  	int ret;
>  
> -	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
> -	if (ret)
> -		return ret;
>  	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
>  	if (ret)
>  		return ret;
> @@ -633,10 +630,6 @@ static int lan9303_setup(struct dsa_switch *ds)
>  	if (ret)
>  		dev_err(chip->dev, "failed to separate ports %d\n", ret);
>  
> -	ret = lan9303_enable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
> -	if (ret)
> -		dev_err(chip->dev, "failed to re-enable switching %d\n", ret);
> -

Does this mean you are relying on something else enabling port 0? The
bootloader?

I'm wondering if it is better to keep the enable, but remove the
disable?

	Andrew

[toc] | [prev] | [next] | [standalone]

#1697879 — Re: [PATCH net-next v2 02/10] net: dsa: lan9303: Do not disable/enable switch fabric port 0 at startup

On 26. juli 2017 18:58, Andrew Lunn wrote:
> On Tue, Jul 25, 2017 at 06:15:45PM +0200, Egil Hjelmeland wrote:
>> For some mysterious reason enable switch fabric port 0 TX fails to
>> work, when the TX has previous been disabled. Resolved by not
>> disable/enable switch fabric port 0 at startup. Port 1 and 2 are
>> still disabled in early init.
>>
>> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
>> ---
>>   drivers/net/dsa/lan9303-core.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
>> index e622db586c3d..c2b53659f58f 100644
>> --- a/drivers/net/dsa/lan9303-core.c
>> +++ b/drivers/net/dsa/lan9303-core.c
>> @@ -557,9 +557,6 @@ static int lan9303_disable_processing(struct lan9303 *chip)
>>   {
>>   	int ret;
>>   
>> -	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
>> -	if (ret)
>> -		return ret;
>>   	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
>>   	if (ret)
>>   		return ret;
>> @@ -633,10 +630,6 @@ static int lan9303_setup(struct dsa_switch *ds)
>>   	if (ret)
>>   		dev_err(chip->dev, "failed to separate ports %d\n", ret);
>>   
>> -	ret = lan9303_enable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
>> -	if (ret)
>> -		dev_err(chip->dev, "failed to re-enable switching %d\n", ret);
>> -
> 
> Does this mean you are relying on something else enabling port 0? The
> bootloader?
> 
> I'm wondering if it is better to keep the enable, but remove the
> disable?
> 
> 	Andrew
> 

The (switch engine) ports are enabled by default. The only thing our
bootloader does is to set gpo so the lan9303 is kept in reset until
the linux driver starts. When I test with the next-next kernel I just
specify the reset-gpo in DTS and the driver pulls it out of reset.

Keeping the enable does no harm, as far as I recall, but I can
double check that when I get time. I have no idea why the original
mainline code does not work for me. Maybe it is a timing issue?

[toc] | [prev] | [next] | [standalone]

#1696004 — [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node swe_bcst_throt

Allowing per-port access to Switch Engine Broadcast Throttling Register

Also added lan9303_write_switch_reg_mask()

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 drivers/net/dsa/lan9303-core.c | 83 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index be6d78f45a5f..b70acb73aad6 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -154,6 +154,7 @@
 # define LAN9303_SWE_PORT_MIRROR_ENABLE_RX_MIRRORING BIT(1)
 # define LAN9303_SWE_PORT_MIRROR_ENABLE_TX_MIRRORING BIT(0)
 #define LAN9303_SWE_INGRESS_PORT_TYPE 0x1847
+#define LAN9303_SWE_BCST_THROT 0x1848
 #define LAN9303_BM_CFG 0x1c00
 #define LAN9303_BM_EGRSS_PORT_TYPE 0x1c0c
 # define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT2 (BIT(17) | BIT(16))
@@ -426,6 +427,20 @@ static int lan9303_read_switch_reg(struct lan9303 *chip, u16 regnum, u32 *val)
 	return ret;
 }
 
+static int lan9303_write_switch_reg_mask(
+	struct lan9303 *chip, u16 regnum, u32 val, u32 mask)
+{
+	int ret;
+	u32 reg;
+
+	ret = lan9303_read_switch_reg(chip, regnum, &reg);
+	if (ret)
+		return ret;
+	reg = (reg & ~mask) | val;
+
+	return lan9303_write_switch_reg(chip, regnum, reg);
+}
+
 static int lan9303_detect_phy_setup(struct lan9303 *chip)
 {
 	int reg;
@@ -614,6 +629,66 @@ static int lan9303_check_device(struct lan9303 *chip)
 	return 0;
 }
 
+/* ---------------------- Sysfs on slave port --------------------------*/
+/*13.4.3.23 Switch Engine Broadcast Throttling Register (SWE_BCST_THROT)*/
+static ssize_t
+swe_bcst_throt_show(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	struct dsa_port *dp = dsa_net_device_to_dsa_port(to_net_dev(dev));
+	struct lan9303 *chip = dp->ds->priv;
+	int port = dp->index;
+	int reg;
+
+	if (lan9303_read_switch_reg(chip, LAN9303_SWE_BCST_THROT, &reg))
+		return 0;
+
+	reg = (reg >> (9 * port)) & 0x1ff; /*extract port N*/
+	if (reg & 0x100)
+		reg &= 0xff; /* remove enable bit */
+	else
+		reg = 0;     /* not enabled*/
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", reg);
+}
+
+static ssize_t
+swe_bcst_throt_store(struct device *dev, struct device_attribute *attr,
+		     const char *buf, size_t len)
+{
+	struct dsa_port *dp = dsa_net_device_to_dsa_port(to_net_dev(dev));
+	struct lan9303 *chip = dp->ds->priv;
+	int port = dp->index;
+	int ret;
+	unsigned long level;
+
+	ret = kstrtoul(buf, 0, &level);
+	if (ret)
+		return ret;
+	level &= 0xff; /* ensure valid range */
+	if (level)
+		level |= 0x100; /* Set enable bit  */
+
+	ret = lan9303_write_switch_reg_mask(chip, LAN9303_SWE_BCST_THROT,
+					    level << (9 * port),
+					    0x1ff << (9 * port));
+	if (ret)
+		return ret;
+	return len;
+}
+
+static DEVICE_ATTR_RW(swe_bcst_throt);
+
+static struct attribute *lan9303_attrs[] = {
+	&dev_attr_swe_bcst_throt.attr,
+	NULL
+};
+
+static struct attribute_group lan9303_group = {
+	.name = "lan9303",
+	.attrs = lan9303_attrs,
+};
+
 /* ---------------------------- DSA -----------------------------------*/
 
 static enum dsa_tag_protocol lan9303_get_tag_protocol(struct dsa_switch *ds)
@@ -787,6 +862,11 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
 	switch (port) {
 	case 1:
 	case 2:
+		/* lan9303_setup is too early to attach sysfs nodes... */
+		if (sysfs_create_group(
+				&ds->ports[port].netdev->dev.kobj,
+				&lan9303_group))
+			dev_dbg(chip->dev, "cannot create sysfs group\n");
 		return lan9303_enable_packet_processing(chip, port);
 	default:
 		dev_dbg(chip->dev,
@@ -805,6 +885,9 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
 	switch (port) {
 	case 1:
 	case 2:
+		sysfs_remove_group(&ds->ports[port].netdev->dev.kobj,
+				   &lan9303_group);
+
 		lan9303_disable_packet_processing(chip, port);
 		lan9303_phy_write(ds, chip->phy_addr_sel_strap + port,
 				  MII_BMCR, BMCR_PDOWN);
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1697402 — Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node swe_bcst_throt

On Tue, Jul 25, 2017 at 06:15:49PM +0200, Egil Hjelmeland wrote:
> Allowing per-port access to Switch Engine Broadcast Throttling Register

Hi Egil

In general, we are against using sysfs. If there is a generic
mechanism, that applies for all sorts of network interfaces, it should
be used instead of sysfs.

Is this intended to reduce the effect of broadcast storms?

   Andrew

[toc] | [prev] | [next] | [standalone]

#1697890 — Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node swe_bcst_throt

On 26. juli 2017 19:14, Andrew Lunn wrote:
> On Tue, Jul 25, 2017 at 06:15:49PM +0200, Egil Hjelmeland wrote:
>> Allowing per-port access to Switch Engine Broadcast Throttling Register
> 
> Hi Egil
> 
> In general, we are against using sysfs. If there is a generic
> mechanism, that applies for all sorts of network interfaces, it should
> be used instead of sysfs.
> 
> Is this intended to reduce the effect of broadcast storms?
> 
>     Andrew
> 

Yes, this setting can be used to reduce effect of broadcast storms.

I knew you all dislike using sysfs. Still I had a hope you could accept
the dsa_net_device_to_dsa_port() function it the previous patch.
I feel it would be convenient to be able to add driver sysfs nodes in
private branches for own debug and fine tuning, without having to patch
the networking core.



Egil

[toc] | [prev] | [next] | [standalone]

#1697982 — Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node swe_bcst_throt

> Yes, this setting can be used to reduce effect of broadcast storms.

So one way to solve this is to teach the software bridge about
broadcast storm control. Put some rate limiting into its broadcast
flood handling. Then add a switchdev call which passes this down into
the switch.

Or look at doing it via TC. It is just a filter selecting broadcast
traffic and applying some shaping, which is what TC is all about.

Generic solutions which can be used by all switches are likely to be
accepted.

	Andrew

[toc] | [prev] | [next] | [standalone]

#1697987 — Re: [PATCH net-next v2 06/10] net: dsa: lan9303: added sysfs node swe_bcst_throt

Thu, Jul 27, 2017 at 03:26:25PM CEST, andrew@lunn.ch wrote:
>> Yes, this setting can be used to reduce effect of broadcast storms.
>
>So one way to solve this is to teach the software bridge about
>broadcast storm control. Put some rate limiting into its broadcast
>flood handling. Then add a switchdev call which passes this down into
>the switch.
>
>Or look at doing it via TC. It is just a filter selecting broadcast
>traffic and applying some shaping, which is what TC is all about.

Ack. This is probably the way to do this. Definitelly no sysfs knob...

>
>Generic solutions which can be used by all switches are likely to be
>accepted.
>
>	Andrew

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web