Netdev List
 help / color / mirror / Atom feed
* [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Tristram.Ha @ 2017-09-07 21:09 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121901@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

The KSZ tail tag code can be used by different KSZ switch drivers.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c index 010ca0a..d5faf14 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -13,35 +13,21 @@
 #include <linux/slab.h>
 #include <net/dsa.h>
 #include "dsa_priv.h"
-
-/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
- * ---------------------------------------------------------------------------
- * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
- * ---------------------------------------------------------------------------
- * tag0 : Prioritization (not used now)
- * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
- *
- * For Egress (KSZ -> Host), 1 byte is added before FCS.
- * ---------------------------------------------------------------------------
- * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
- * ---------------------------------------------------------------------------
- * tag0 : zero-based value represents port
- *	  (eg, 0x00=port1, 0x02=port3, 0x06=port7)
- */
-
-#define	KSZ_INGRESS_TAG_LEN	2
-#define	KSZ_EGRESS_TAG_LEN	1
+#include "../../drivers/net/dsa/microchip/ksz_priv.h"
 
 static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)  {
 	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->dp->ds;
+	struct ksz_device *swdev = ds->priv;
 	struct sk_buff *nskb;
+	int len;
 	int padlen;
-	u8 *tag;
 
 	padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
 
-	if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
+	len = swdev->dev_ops->get_tx_len(swdev);
+	if (skb_tailroom(skb) >= padlen + len) {
 		/* Let dsa_slave_xmit() free skb */
 		if (__skb_put_padto(skb, skb->len + padlen, false))
 			return NULL;
@@ -49,7 +35,7 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 		nskb = skb;
 	} else {
 		nskb = alloc_skb(NET_IP_ALIGN + skb->len +
-				 padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
+				 padlen + len, GFP_ATOMIC);
 		if (!nskb)
 			return NULL;
 		skb_reserve(nskb, NET_IP_ALIGN);
@@ -70,9 +56,7 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 		consume_skb(skb);
 	}
 
-	tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
-	tag[0] = 0;
-	tag[1] = 1 << p->dp->index; /* destination port */
+	swdev->dev_ops->add_tail_tag(swdev, nskb, p->dp->index);
 
 	return nskb;
 }
@@ -83,16 +67,16 @@ static struct sk_buff *ksz_rcv(struct sk_buff *skb, struct net_device *dev,
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
 	struct dsa_switch *ds = cpu_dp->ds;
-	u8 *tag;
+	struct ksz_device *swdev = ds->priv;
+	int len;
 	int source_port;
 
-	tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
+	len = swdev->dev_ops->get_tail_tag(swdev, skb, &source_port);
 
-	source_port = tag[0] & 7;
 	if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
 		return NULL;
 
-	pskb_trim_rcsum(skb, skb->len - KSZ_EGRESS_TAG_LEN);
+	pskb_trim_rcsum(skb, skb->len - len);
 
 	skb->dev = ds->ports[source_port].netdev;

^ permalink raw reply

* [PATCH RFC 5/6] Switch SPI driver calls its own driver switch register function
From: Tristram.Ha @ 2017-09-07 21:09 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411218F4@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

SPI driver calls own specific switch register function.
Shutdown callback function is added to reset switch to default state.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
index c519469..d03eb83 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -25,6 +25,8 @@
 
 #include "ksz_priv.h"
 
+int ksz9477_switch_register(struct ksz_device *dev);
+
 /* SPI frame opcodes */
 #define KS_SPIOP_RD			3
 #define KS_SPIOP_WR			2
@@ -174,7 +176,7 @@ static int ksz_spi_probe(struct spi_device *spi)
 	if (spi->dev.platform_data)
 		dev->pdata = spi->dev.platform_data;
 
-	ret = ksz_switch_register(dev);
+	ret = ksz9477_switch_register(dev);
 	if (ret)
 		return ret;
 
@@ -193,6 +195,14 @@ static int ksz_spi_remove(struct spi_device *spi)
 	return 0;
 }
 
+static void ksz_spi_shutdown(struct spi_device *spi) {
+	struct ksz_device *dev = spi_get_drvdata(spi);
+
+	if (dev)
+		dev->dev_ops->reset(dev);
+}
+
 static const struct of_device_id ksz_dt_ids[] = {
 	{ .compatible = "microchip,ksz9477" },
 	{},
@@ -207,6 +217,7 @@ static int ksz_spi_remove(struct spi_device *spi)
 	},
 	.probe	= ksz_spi_probe,
 	.remove	= ksz_spi_remove,
+	.shutdown = ksz_spi_shutdown,
 };
 
 module_spi_driver(ksz_spi_driver);

^ permalink raw reply related

* [PATCH RFC 4/6] The common header ksz_priv.h does not contain chip specific data
From: Tristram.Ha @ 2017-09-07 21:09 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411218E7@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

The header ksz_priv.h is used by all KSZ switch drivers so chip specific data are removed and commonly used variables are added.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 2a98dbd..343b509 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -25,14 +25,56 @@
 #include <linux/etherdevice.h>
 #include <net/dsa.h>
 
-#include "ksz_9477_reg.h"
-
 struct ksz_io_ops;
 
 struct vlan_table {
 	u32 table[3];
 };
 
+enum {
+	PHY_NO_FLOW_CTRL,
+	PHY_FLOW_CTRL,
+	PHY_TX_ONLY,
+	PHY_RX_ONLY
+};
+
+struct ksz_port_mib {
+	u8 cnt_ptr;
+	u8 ctrl;
+	unsigned long time;
+
+	struct ksz_port_mib_info {
+		u64 counter;
+		u8 read_cnt;
+		u8 read_max;
+	} *info;
+};
+
+struct ksz_port {
+	u16 member;
+	u16 vid_member;
+	int stp_state;
+	u16 speed;
+	u8 duplex;
+	u8 advertised;
+	u8 flow_ctrl;
+	u8 link;
+	u8 partner;
+
+	u32 on:1;			/* port is not disabled by hardware */
+	u32 fiber:1;			/* port is fiber */
+	u32 sgmii:1;			/* port is SGMII */
+	u32 force:1;
+	u32 link_down:1;		/* link just goes down */
+	u32 link_up:1;			/* link is up */
+
+	struct ksz_port_mib mib;
+};
+
+#define USE_FEWER_PORTS			BIT(18)
+
+#define PTP_TAG				BIT(29)
+
 struct ksz_device {
 	struct dsa_switch *ds;
 	struct ksz_platform_data *pdata;
@@ -43,6 +85,7 @@ struct ksz_device {
 	struct mutex alu_mutex;		/* ALU access */
 	struct mutex vlan_mutex;	/* vlan access */
 	const struct ksz_io_ops *ops;
+	const struct ksz_dev_ops *dev_ops;
 
 	struct device *dev;
 
@@ -56,10 +99,32 @@ struct ksz_device {
 	int cpu_port;			/* port connected to CPU */
 	int cpu_ports;			/* port bitmap can be cpu port */
 	int port_cnt;
+	int mib_cnt;
+	int mib_port_cnt;
+	int last_port;			/* ports after that not used */
+	int interface;
 
 	struct vlan_table *vlan_cache;
 
-	u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
+	u8 *txbuf;
+
+	struct ksz_port *ports;
+	struct timer_list mib_read_timer;
+	struct work_struct mib_read;
+	spinlock_t mib_read_lock;	/* use to update read_cnt */
+	unsigned long MIB_READ_INTERVAL;
+	u32 features;
+	u32 overrides;
+	u16 br_member;
+	u16 member;
+	u16 live_ports;
+	u16 on_ports;			/* ports enabled by DSA */
+	u16 rx_ports;
+	u16 tx_ports;
+	u16 mirror_rx;
+	u16 mirror_tx;
+	u16 HOST_MASK;
+	u16 PORT_MASK;
 };
 
 struct ksz_io_ops {
@@ -75,12 +140,30 @@ struct ksz_io_ops {
 			  u16 *value);
 	int (*phy_write16)(struct ksz_device *dev, int addr, int reg,
 			   u16 value);
+	unsigned int (*get)(struct ksz_device *dev, u32 reg, void *data,
+			    unsigned int len);
+	unsigned int (*set)(struct ksz_device *dev, u32 reg, void *data,
+			    unsigned int len);
+};
+
+struct ksz_dev_ops {
+	u32 (*get_port_addr)(int port, int offset);
+	int (*reset)(struct ksz_device *dev);
+	int (*get_rx_len)(struct ksz_device *dev);
+	int (*get_tx_len)(struct ksz_device *dev);
+	void (*add_tail_tag)(struct ksz_device *dev, struct sk_buff *skb,
+			     int port);
+	int (*get_tail_tag)(struct ksz_device *dev, struct sk_buff *skb,
+			    int *port);
+	int (*detect)(struct ksz_device *dev);
+	int (*init)(struct ksz_device *dev);
+	void (*exit)(struct ksz_device *dev);
 };
 
 struct ksz_device *ksz_switch_alloc(struct device *base,
 				    const struct ksz_io_ops *ops, void *priv); -int ksz_switch_detect(struct ksz_device *dev); -int ksz_switch_register(struct ksz_device *dev);
+int ksz_switch_register(struct ksz_device *dev,
+			const struct ksz_dev_ops *ops);
 void ksz_switch_remove(struct ksz_device *dev);
 
 static inline int ksz_read8(struct ksz_device *dev, u32 reg, u8 *val) @@ -174,37 +257,37 @@ static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)  static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
 			      u8 *data)
 {
-	ksz_read8(dev, PORT_CTRL_ADDR(port, offset), data);
+	ksz_read8(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
 static inline void ksz_pread16(struct ksz_device *dev, int port, int offset,
 			       u16 *data)
 {
-	ksz_read16(dev, PORT_CTRL_ADDR(port, offset), data);
+	ksz_read16(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
 static inline void ksz_pread32(struct ksz_device *dev, int port, int offset,
 			       u32 *data)
 {
-	ksz_read32(dev, PORT_CTRL_ADDR(port, offset), data);
+	ksz_read32(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
 static inline void ksz_pwrite8(struct ksz_device *dev, int port, int offset,
 			       u8 data)
 {
-	ksz_write8(dev, PORT_CTRL_ADDR(port, offset), data);
+	ksz_write8(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
 static inline void ksz_pwrite16(struct ksz_device *dev, int port, int offset,
 				u16 data)
 {
-	ksz_write16(dev, PORT_CTRL_ADDR(port, offset), data);
+	ksz_write16(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
 static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
 				u32 data)
 {
-	ksz_write32(dev, PORT_CTRL_ADDR(port, offset), data);
+	ksz_write32(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
 #endif

^ permalink raw reply related

* [PATCH RFC 3/6] The file ksz_common.c contains common code shared by all KSZ switch drivers
From: Tristram.Ha @ 2017-09-07 21:09 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411218DC@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

The file ksz_common.c only holds common code used by all KSZ switch drivers.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 56cd6d3..bebcc65 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -25,1114 +25,12 @@
 #include <linux/phy.h>
 #include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
+#include <linux/of_net.h>
 #include <net/dsa.h>
 #include <net/switchdev.h>
 
 #include "ksz_priv.h"
 
-static const struct {
-	int index;
-	char string[ETH_GSTRING_LEN];
-} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
-	{ 0x00, "rx_hi" },
-	{ 0x01, "rx_undersize" },
-	{ 0x02, "rx_fragments" },
-	{ 0x03, "rx_oversize" },
-	{ 0x04, "rx_jabbers" },
-	{ 0x05, "rx_symbol_err" },
-	{ 0x06, "rx_crc_err" },
-	{ 0x07, "rx_align_err" },
-	{ 0x08, "rx_mac_ctrl" },
-	{ 0x09, "rx_pause" },
-	{ 0x0A, "rx_bcast" },
-	{ 0x0B, "rx_mcast" },
-	{ 0x0C, "rx_ucast" },
-	{ 0x0D, "rx_64_or_less" },
-	{ 0x0E, "rx_65_127" },
-	{ 0x0F, "rx_128_255" },
-	{ 0x10, "rx_256_511" },
-	{ 0x11, "rx_512_1023" },
-	{ 0x12, "rx_1024_1522" },
-	{ 0x13, "rx_1523_2000" },
-	{ 0x14, "rx_2001" },
-	{ 0x15, "tx_hi" },
-	{ 0x16, "tx_late_col" },
-	{ 0x17, "tx_pause" },
-	{ 0x18, "tx_bcast" },
-	{ 0x19, "tx_mcast" },
-	{ 0x1A, "tx_ucast" },
-	{ 0x1B, "tx_deferred" },
-	{ 0x1C, "tx_total_col" },
-	{ 0x1D, "tx_exc_col" },
-	{ 0x1E, "tx_single_col" },
-	{ 0x1F, "tx_mult_col" },
-	{ 0x80, "rx_total" },
-	{ 0x81, "tx_total" },
-	{ 0x82, "rx_discards" },
-	{ 0x83, "tx_discards" },
-};
-
-static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set) -{
-	u8 data;
-
-	ksz_read8(dev, addr, &data);
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-	ksz_write8(dev, addr, data);
-}
-
-static void ksz_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set) -{
-	u32 data;
-
-	ksz_read32(dev, addr, &data);
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-	ksz_write32(dev, addr, data);
-}
-
-static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
-			 bool set)
-{
-	u32 addr;
-	u8 data;
-
-	addr = PORT_CTRL_ADDR(port, offset);
-	ksz_read8(dev, addr, &data);
-
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-
-	ksz_write8(dev, addr, data);
-}
-
-static void ksz_port_cfg32(struct ksz_device *dev, int port, int offset,
-			   u32 bits, bool set)
-{
-	u32 addr;
-	u32 data;
-
-	addr = PORT_CTRL_ADDR(port, offset);
-	ksz_read32(dev, addr, &data);
-
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-
-	ksz_write32(dev, addr, data);
-}
-
-static int wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton, int timeout) -{
-	u8 data;
-
-	do {
-		ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
-static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table) -{
-	struct ksz_device *dev = ds->priv;
-	int ret;
-
-	mutex_lock(&dev->vlan_mutex);
-
-	ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
-	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
-
-	/* wait to be cleared */
-	ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
-	if (ret < 0) {
-		dev_dbg(dev->dev, "Failed to read vlan table\n");
-		goto exit;
-	}
-
-	ksz_read32(dev, REG_SW_VLAN_ENTRY__4, &vlan_table[0]);
-	ksz_read32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, &vlan_table[1]);
-	ksz_read32(dev, REG_SW_VLAN_ENTRY_PORTS__4, &vlan_table[2]);
-
-	ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
-
-exit:
-	mutex_unlock(&dev->vlan_mutex);
-
-	return ret;
-}
-
-static int set_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table) -{
-	struct ksz_device *dev = ds->priv;
-	int ret;
-
-	mutex_lock(&dev->vlan_mutex);
-
-	ksz_write32(dev, REG_SW_VLAN_ENTRY__4, vlan_table[0]);
-	ksz_write32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, vlan_table[1]);
-	ksz_write32(dev, REG_SW_VLAN_ENTRY_PORTS__4, vlan_table[2]);
-
-	ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
-	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
-
-	/* wait to be cleared */
-	ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
-	if (ret < 0) {
-		dev_dbg(dev->dev, "Failed to write vlan table\n");
-		goto exit;
-	}
-
-	ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
-
-	/* update vlan cache table */
-	dev->vlan_cache[vid].table[0] = vlan_table[0];
-	dev->vlan_cache[vid].table[1] = vlan_table[1];
-	dev->vlan_cache[vid].table[2] = vlan_table[2];
-
-exit:
-	mutex_unlock(&dev->vlan_mutex);
-
-	return ret;
-}
-
-static void read_table(struct dsa_switch *ds, u32 *table) -{
-	struct ksz_device *dev = ds->priv;
-
-	ksz_read32(dev, REG_SW_ALU_VAL_A, &table[0]);
-	ksz_read32(dev, REG_SW_ALU_VAL_B, &table[1]);
-	ksz_read32(dev, REG_SW_ALU_VAL_C, &table[2]);
-	ksz_read32(dev, REG_SW_ALU_VAL_D, &table[3]);
-}
-
-static void write_table(struct dsa_switch *ds, u32 *table) -{
-	struct ksz_device *dev = ds->priv;
-
-	ksz_write32(dev, REG_SW_ALU_VAL_A, table[0]);
-	ksz_write32(dev, REG_SW_ALU_VAL_B, table[1]);
-	ksz_write32(dev, REG_SW_ALU_VAL_C, table[2]);
-	ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]);
-}
-
-static int wait_alu_ready(struct ksz_device *dev, u32 waiton, int timeout) -{
-	u32 data;
-
-	do {
-		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
-static int wait_alu_sta_ready(struct ksz_device *dev, u32 waiton, int timeout) -{
-	u32 data;
-
-	do {
-		ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
-static int ksz_reset_switch(struct dsa_switch *ds) -{
-	struct ksz_device *dev = ds->priv;
-	u8 data8;
-	u16 data16;
-	u32 data32;
-
-	/* reset switch */
-	ksz_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
-
-	/* turn off SPI DO Edge select */
-	ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
-	data8 &= ~SPI_AUTO_EDGE_DETECTION;
-	ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
-
-	/* default configuration */
-	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
-	data8 = SW_AGING_ENABLE | SW_LINK_AUTO_AGING |
-	      SW_SRC_ADDR_FILTER | SW_FLUSH_STP_TABLE | SW_FLUSH_MSTP_TABLE;
-	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
-
-	/* disable interrupts */
-	ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
-	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
-	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
-
-	/* set broadcast storm protection 10% rate */
-	ksz_read16(dev, REG_SW_MAC_CTRL_2, &data16);
-	data16 &= ~BROADCAST_STORM_RATE;
-	data16 |= (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100;
-	ksz_write16(dev, REG_SW_MAC_CTRL_2, data16);
-
-	return 0;
-}
-
-static void port_setup(struct ksz_device *dev, int port, bool cpu_port) -{
-	u8 data8;
-	u16 data16;
-
-	/* enable tag tail for host port */
-	if (cpu_port)
-		ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_TAIL_TAG_ENABLE,
-			     true);
-
-	ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK, false);
-
-	/* set back pressure */
-	ksz_port_cfg(dev, port, REG_PORT_MAC_CTRL_1, PORT_BACK_PRESSURE, true);
-
-	/* set flow control */
-	ksz_port_cfg(dev, port, REG_PORT_CTRL_0,
-		     PORT_FORCE_TX_FLOW_CTRL | PORT_FORCE_RX_FLOW_CTRL, true);
-
-	/* enable broadcast storm limit */
-	ksz_port_cfg(dev, port, P_BCAST_STORM_CTRL, PORT_BROADCAST_STORM, true);
-
-	/* disable DiffServ priority */
-	ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_DIFFSERV_PRIO_ENABLE, false);
-
-	/* replace priority */
-	ksz_port_cfg(dev, port, REG_PORT_MRI_MAC_CTRL, PORT_USER_PRIO_CEILING,
-		     false);
-	ksz_port_cfg32(dev, port, REG_PORT_MTI_QUEUE_CTRL_0__4,
-		       MTI_PVID_REPLACE, false);
-
-	/* enable 802.1p priority */
-	ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_PRIO_ENABLE, true);
-
-	/* configure MAC to 1G & RGMII mode */
-	ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
-	data8 |= PORT_RGMII_ID_EG_ENABLE;
-	data8 &= ~PORT_MII_NOT_1GBIT;
-	data8 &= ~PORT_MII_SEL_M;
-	data8 |= PORT_RGMII_SEL;
-	ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
-
-	/* clear pending interrupts */
-	ksz_pread16(dev, port, REG_PORT_PHY_INT_ENABLE, &data16);
-}
-
-static void ksz_config_cpu_port(struct dsa_switch *ds) -{
-	struct ksz_device *dev = ds->priv;
-	int i;
-
-	ds->num_ports = dev->port_cnt;
-
-	for (i = 0; i < ds->num_ports; i++) {
-		if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
-			dev->cpu_port = i;
-
-			/* enable cpu port */
-			port_setup(dev, i, true);
-		}
-	}
-}
-
-static int ksz_setup(struct dsa_switch *ds) -{
-	struct ksz_device *dev = ds->priv;
-	int ret = 0;
-
-	dev->vlan_cache = devm_kcalloc(dev->dev, sizeof(struct vlan_table),
-				       dev->num_vlans, GFP_KERNEL);
-	if (!dev->vlan_cache)
-		return -ENOMEM;
-
-	ret = ksz_reset_switch(ds);
-	if (ret) {
-		dev_err(ds->dev, "failed to reset switch\n");
-		return ret;
-	}
-
-	/* accept packet up to 2000bytes */
-	ksz_cfg(dev, REG_SW_MAC_CTRL_1, SW_LEGAL_PACKET_DISABLE, true);
-
-	ksz_config_cpu_port(ds);
-
-	ksz_cfg(dev, REG_SW_MAC_CTRL_1, MULTICAST_STORM_DISABLE, true);
-
-	/* queue based egress rate limit */
-	ksz_cfg(dev, REG_SW_MAC_CTRL_5, SW_OUT_RATE_LIMIT_QUEUE_BASED, true);
-
-	/* start switch */
-	ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
-
-	return 0;
-}
-
-static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds) -{
-	return DSA_TAG_PROTO_KSZ;
-}
-
-static int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg) -{
-	struct ksz_device *dev = ds->priv;
-	u16 val = 0;
-
-	ksz_pread16(dev, addr, 0x100 + (reg << 1), &val);
-
-	return val;
-}
-
-static int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val) -{
-	struct ksz_device *dev = ds->priv;
-
-	ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val);
-
-	return 0;
-}
-
-static int ksz_enable_port(struct dsa_switch *ds, int port,
-			   struct phy_device *phy)
-{
-	struct ksz_device *dev = ds->priv;
-
-	/* setup slave port */
-	port_setup(dev, port, false);
-
-	return 0;
-}
-
-static void ksz_disable_port(struct dsa_switch *ds, int port,
-			     struct phy_device *phy)
-{
-	struct ksz_device *dev = ds->priv;
-
-	/* there is no port disable */
-	ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK, true);
-}
-
-static int ksz_sset_count(struct dsa_switch *ds) -{
-	return TOTAL_SWITCH_COUNTER_NUM;
-}
-
-static void ksz_get_strings(struct dsa_switch *ds, int port, uint8_t *buf) -{
-	int i;
-
-	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
-		memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
-		       ETH_GSTRING_LEN);
-	}
-}
-
-static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
-				  uint64_t *buf)
-{
-	struct ksz_device *dev = ds->priv;
-	int i;
-	u32 data;
-	int timeout;
-
-	mutex_lock(&dev->stats_mutex);
-
-	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
-		data = MIB_COUNTER_READ;
-		data |= ((mib_names[i].index & 0xFF) << MIB_COUNTER_INDEX_S);
-		ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
-
-		timeout = 1000;
-		do {
-			ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
-				    &data);
-			usleep_range(1, 10);
-			if (!(data & MIB_COUNTER_READ))
-				break;
-		} while (timeout-- > 0);
-
-		/* failed to read MIB. get out of loop */
-		if (!timeout) {
-			dev_dbg(dev->dev, "Failed to get MIB\n");
-			break;
-		}
-
-		/* count resets upon read */
-		ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
-
-		dev->mib_value[i] += (uint64_t)data;
-		buf[i] = dev->mib_value[i];
-	}
-
-	mutex_unlock(&dev->stats_mutex);
-}
-
-static void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8 state) -{
-	struct ksz_device *dev = ds->priv;
-	u8 data;
-
-	ksz_pread8(dev, port, P_STP_CTRL, &data);
-	data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
-
-	switch (state) {
-	case BR_STATE_DISABLED:
-		data |= PORT_LEARN_DISABLE;
-		break;
-	case BR_STATE_LISTENING:
-		data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
-		break;
-	case BR_STATE_LEARNING:
-		data |= PORT_RX_ENABLE;
-		break;
-	case BR_STATE_FORWARDING:
-		data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
-		break;
-	case BR_STATE_BLOCKING:
-		data |= PORT_LEARN_DISABLE;
-		break;
-	default:
-		dev_err(ds->dev, "invalid STP state: %d\n", state);
-		return;
-	}
-
-	ksz_pwrite8(dev, port, P_STP_CTRL, data);
-}
-
-static void ksz_port_fast_age(struct dsa_switch *ds, int port) -{
-	struct ksz_device *dev = ds->priv;
-	u8 data8;
-
-	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
-	data8 |= SW_FAST_AGING;
-	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
-
-	data8 &= ~SW_FAST_AGING;
-	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
-}
-
-static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port, bool flag) -{
-	struct ksz_device *dev = ds->priv;
-
-	if (flag) {
-		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
-			     PORT_VLAN_LOOKUP_VID_0, true);
-		ksz_cfg32(dev, REG_SW_QM_CTRL__4, UNICAST_VLAN_BOUNDARY, true);
-		ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE, true);
-	} else {
-		ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE, false);
-		ksz_cfg32(dev, REG_SW_QM_CTRL__4, UNICAST_VLAN_BOUNDARY, false);
-		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
-			     PORT_VLAN_LOOKUP_VID_0, false);
-	}
-
-	return 0;
-}
-
-static int ksz_port_vlan_prepare(struct dsa_switch *ds, int port,
-				 const struct switchdev_obj_port_vlan *vlan,
-				 struct switchdev_trans *trans)
-{
-	/* nothing needed */
-
-	return 0;
-}
-
-static void ksz_port_vlan_add(struct dsa_switch *ds, int port,
-			      const struct switchdev_obj_port_vlan *vlan,
-			      struct switchdev_trans *trans)
-{
-	struct ksz_device *dev = ds->priv;
-	u32 vlan_table[3];
-	u16 vid;
-	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
-
-	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
-		if (get_vlan_table(ds, vid, vlan_table)) {
-			dev_dbg(dev->dev, "Failed to get vlan table\n");
-			return;
-		}
-
-		vlan_table[0] = VLAN_VALID | (vid & VLAN_FID_M);
-		if (untagged)
-			vlan_table[1] |= BIT(port);
-		else
-			vlan_table[1] &= ~BIT(port);
-		vlan_table[1] &= ~(BIT(dev->cpu_port));
-
-		vlan_table[2] |= BIT(port) | BIT(dev->cpu_port);
-
-		if (set_vlan_table(ds, vid, vlan_table)) {
-			dev_dbg(dev->dev, "Failed to set vlan table\n");
-			return;
-		}
-
-		/* change PVID */
-		if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
-			ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID, vid);
-	}
-}
-
-static int ksz_port_vlan_del(struct dsa_switch *ds, int port,
-			     const struct switchdev_obj_port_vlan *vlan)
-{
-	struct ksz_device *dev = ds->priv;
-	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
-	u32 vlan_table[3];
-	u16 vid;
-	u16 pvid;
-
-	ksz_pread16(dev, port, REG_PORT_DEFAULT_VID, &pvid);
-	pvid = pvid & 0xFFF;
-
-	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
-		if (get_vlan_table(ds, vid, vlan_table)) {
-			dev_dbg(dev->dev, "Failed to get vlan table\n");
-			return -ETIMEDOUT;
-		}
-
-		vlan_table[2] &= ~BIT(port);
-
-		if (pvid == vid)
-			pvid = 1;
-
-		if (untagged)
-			vlan_table[1] &= ~BIT(port);
-
-		if (set_vlan_table(ds, vid, vlan_table)) {
-			dev_dbg(dev->dev, "Failed to set vlan table\n");
-			return -ETIMEDOUT;
-		}
-	}
-
-	ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID, pvid);
-
-	return 0;
-}
-
-struct alu_struct {
-	/* entry 1 */
-	u8	is_static:1;
-	u8	is_src_filter:1;
-	u8	is_dst_filter:1;
-	u8	prio_age:3;
-	u32	_reserv_0_1:23;
-	u8	mstp:3;
-	/* entry 2 */
-	u8	is_override:1;
-	u8	is_use_fid:1;
-	u32	_reserv_1_1:23;
-	u8	port_forward:7;
-	/* entry 3 & 4*/
-	u32	_reserv_2_1:9;
-	u8	fid:7;
-	u8	mac[ETH_ALEN];
-};
-
-static int ksz_port_fdb_add(struct dsa_switch *ds, int port,
-			    const unsigned char *addr, u16 vid)
-{
-	struct ksz_device *dev = ds->priv;
-	u32 alu_table[4];
-	u32 data;
-	int ret = 0;
-
-	mutex_lock(&dev->alu_mutex);
-
-	/* find any entry with mac & vid */
-	data = vid << ALU_FID_INDEX_S;
-	data |= ((addr[0] << 8) | addr[1]);
-	ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
-
-	data = ((addr[2] << 24) | (addr[3] << 16));
-	data |= ((addr[4] << 8) | addr[5]);
-	ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
-
-	/* start read operation */
-	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
-
-	/* wait to be finished */
-	ret = wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0) {
-		dev_dbg(dev->dev, "Failed to read ALU\n");
-		goto exit;
-	}
-
-	/* read ALU entry */
-	read_table(ds, alu_table);
-
-	/* update ALU entry */
-	alu_table[0] = ALU_V_STATIC_VALID;
-	alu_table[1] |= BIT(port);
-	if (vid)
-		alu_table[1] |= ALU_V_USE_FID;
-	alu_table[2] = (vid << ALU_V_FID_S);
-	alu_table[2] |= ((addr[0] << 8) | addr[1]);
-	alu_table[3] = ((addr[2] << 24) | (addr[3] << 16));
-	alu_table[3] |= ((addr[4] << 8) | addr[5]);
-
-	write_table(ds, alu_table);
-
-	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
-
-	/* wait to be finished */
-	ret = wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0)
-		dev_dbg(dev->dev, "Failed to write ALU\n");
-
-exit:
-	mutex_unlock(&dev->alu_mutex);
-
-	return ret;
-}
-
-static int ksz_port_fdb_del(struct dsa_switch *ds, int port,
-			    const unsigned char *addr, u16 vid)
-{
-	struct ksz_device *dev = ds->priv;
-	u32 alu_table[4];
-	u32 data;
-	int ret = 0;
-
-	mutex_lock(&dev->alu_mutex);
-
-	/* read any entry with mac & vid */
-	data = vid << ALU_FID_INDEX_S;
-	data |= ((addr[0] << 8) | addr[1]);
-	ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
-
-	data = ((addr[2] << 24) | (addr[3] << 16));
-	data |= ((addr[4] << 8) | addr[5]);
-	ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
-
-	/* start read operation */
-	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
-
-	/* wait to be finished */
-	ret = wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0) {
-		dev_dbg(dev->dev, "Failed to read ALU\n");
-		goto exit;
-	}
-
-	ksz_read32(dev, REG_SW_ALU_VAL_A, &alu_table[0]);
-	if (alu_table[0] & ALU_V_STATIC_VALID) {
-		ksz_read32(dev, REG_SW_ALU_VAL_B, &alu_table[1]);
-		ksz_read32(dev, REG_SW_ALU_VAL_C, &alu_table[2]);
-		ksz_read32(dev, REG_SW_ALU_VAL_D, &alu_table[3]);
-
-		/* clear forwarding port */
-		alu_table[2] &= ~BIT(port);
-
-		/* if there is no port to forward, clear table */
-		if ((alu_table[2] & ALU_V_PORT_MAP) == 0) {
-			alu_table[0] = 0;
-			alu_table[1] = 0;
-			alu_table[2] = 0;
-			alu_table[3] = 0;
-		}
-	} else {
-		alu_table[0] = 0;
-		alu_table[1] = 0;
-		alu_table[2] = 0;
-		alu_table[3] = 0;
-	}
-
-	write_table(ds, alu_table);
-
-	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
-
-	/* wait to be finished */
-	ret = wait_alu_ready(dev, ALU_START, 1000);
-	if (ret < 0)
-		dev_dbg(dev->dev, "Failed to write ALU\n");
-
-exit:
-	mutex_unlock(&dev->alu_mutex);
-
-	return ret;
-}
-
-static void convert_alu(struct alu_struct *alu, u32 *alu_table) -{
-	alu->is_static = !!(alu_table[0] & ALU_V_STATIC_VALID);
-	alu->is_src_filter = !!(alu_table[0] & ALU_V_SRC_FILTER);
-	alu->is_dst_filter = !!(alu_table[0] & ALU_V_DST_FILTER);
-	alu->prio_age = (alu_table[0] >> ALU_V_PRIO_AGE_CNT_S) &
-			ALU_V_PRIO_AGE_CNT_M;
-	alu->mstp = alu_table[0] & ALU_V_MSTP_M;
-
-	alu->is_override = !!(alu_table[1] & ALU_V_OVERRIDE);
-	alu->is_use_fid = !!(alu_table[1] & ALU_V_USE_FID);
-	alu->port_forward = alu_table[1] & ALU_V_PORT_MAP;
-
-	alu->fid = (alu_table[2] >> ALU_V_FID_S) & ALU_V_FID_M;
-
-	alu->mac[0] = (alu_table[2] >> 8) & 0xFF;
-	alu->mac[1] = alu_table[2] & 0xFF;
-	alu->mac[2] = (alu_table[3] >> 24) & 0xFF;
-	alu->mac[3] = (alu_table[3] >> 16) & 0xFF;
-	alu->mac[4] = (alu_table[3] >> 8) & 0xFF;
-	alu->mac[5] = alu_table[3] & 0xFF;
-}
-
-static int ksz_port_fdb_dump(struct dsa_switch *ds, int port,
-			     dsa_fdb_dump_cb_t *cb, void *data)
-{
-	struct ksz_device *dev = ds->priv;
-	int ret = 0;
-	u32 ksz_data;
-	u32 alu_table[4];
-	struct alu_struct alu;
-	int timeout;
-
-	mutex_lock(&dev->alu_mutex);
-
-	/* start ALU search */
-	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_START | ALU_SEARCH);
-
-	do {
-		timeout = 1000;
-		do {
-			ksz_read32(dev, REG_SW_ALU_CTRL__4, &ksz_data);
-			if ((ksz_data & ALU_VALID) || !(ksz_data & ALU_START))
-				break;
-			usleep_range(1, 10);
-		} while (timeout-- > 0);
-
-		if (!timeout) {
-			dev_dbg(dev->dev, "Failed to search ALU\n");
-			ret = -ETIMEDOUT;
-			goto exit;
-		}
-
-		/* read ALU table */
-		read_table(ds, alu_table);
-
-		convert_alu(&alu, alu_table);
-
-		if (alu.port_forward & BIT(port)) {
-			ret = cb(alu.mac, alu.fid, alu.is_static, data);
-			if (ret)
-				goto exit;
-		}
-	} while (ksz_data & ALU_START);
-
-exit:
-
-	/* stop ALU search */
-	ksz_write32(dev, REG_SW_ALU_CTRL__4, 0);
-
-	mutex_unlock(&dev->alu_mutex);
-
-	return ret;
-}
-
-static int ksz_port_mdb_prepare(struct dsa_switch *ds, int port,
-				const struct switchdev_obj_port_mdb *mdb,
-				struct switchdev_trans *trans)
-{
-	/* nothing to do */
-	return 0;
-}
-
-static void ksz_port_mdb_add(struct dsa_switch *ds, int port,
-			     const struct switchdev_obj_port_mdb *mdb,
-			     struct switchdev_trans *trans)
-{
-	struct ksz_device *dev = ds->priv;
-	u32 static_table[4];
-	u32 data;
-	int index;
-	u32 mac_hi, mac_lo;
-
-	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
-	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
-	mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
-
-	mutex_lock(&dev->alu_mutex);
-
-	for (index = 0; index < dev->num_statics; index++) {
-		/* find empty slot first */
-		data = (index << ALU_STAT_INDEX_S) |
-			ALU_STAT_READ | ALU_STAT_START;
-		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
-
-		/* wait to be finished */
-		if (wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0) {
-			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
-			goto exit;
-		}
-
-		/* read ALU static table */
-		read_table(ds, static_table);
-
-		if (static_table[0] & ALU_V_STATIC_VALID) {
-			/* check this has same vid & mac address */
-			if (((static_table[2] >> ALU_V_FID_S) == (mdb->vid)) &&
-			    ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
-			    (static_table[3] == mac_lo)) {
-				/* found matching one */
-				break;
-			}
-		} else {
-			/* found empty one */
-			break;
-		}
-	}
-
-	/* no available entry */
-	if (index == dev->num_statics)
-		goto exit;
-
-	/* add entry */
-	static_table[0] = ALU_V_STATIC_VALID;
-	static_table[1] |= BIT(port);
-	if (mdb->vid)
-		static_table[1] |= ALU_V_USE_FID;
-	static_table[2] = (mdb->vid << ALU_V_FID_S);
-	static_table[2] |= mac_hi;
-	static_table[3] = mac_lo;
-
-	write_table(ds, static_table);
-
-	data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
-	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
-
-	/* wait to be finished */
-	if (wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0)
-		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
-
-exit:
-	mutex_unlock(&dev->alu_mutex);
-}
-
-static int ksz_port_mdb_del(struct dsa_switch *ds, int port,
-			    const struct switchdev_obj_port_mdb *mdb)
-{
-	struct ksz_device *dev = ds->priv;
-	u32 static_table[4];
-	u32 data;
-	int index;
-	int ret = 0;
-	u32 mac_hi, mac_lo;
-
-	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
-	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
-	mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
-
-	mutex_lock(&dev->alu_mutex);
-
-	for (index = 0; index < dev->num_statics; index++) {
-		/* find empty slot first */
-		data = (index << ALU_STAT_INDEX_S) |
-			ALU_STAT_READ | ALU_STAT_START;
-		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
-
-		/* wait to be finished */
-		ret = wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
-		if (ret < 0) {
-			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
-			goto exit;
-		}
-
-		/* read ALU static table */
-		read_table(ds, static_table);
-
-		if (static_table[0] & ALU_V_STATIC_VALID) {
-			/* check this has same vid & mac address */
-
-			if (((static_table[2] >> ALU_V_FID_S) == (mdb->vid)) &&
-			    ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
-			    (static_table[3] == mac_lo)) {
-				/* found matching one */
-				break;
-			}
-		}
-	}
-
-	/* no available entry */
-	if (index == dev->num_statics) {
-		ret = -EINVAL;
-		goto exit;
-	}
-
-	/* clear port */
-	static_table[1] &= ~BIT(port);
-
-	if ((static_table[1] & ALU_V_PORT_MAP) == 0) {
-		/* delete entry */
-		static_table[0] = 0;
-		static_table[1] = 0;
-		static_table[2] = 0;
-		static_table[3] = 0;
-	}
-
-	write_table(ds, static_table);
-
-	data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
-	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
-
-	/* wait to be finished */
-	ret = wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
-	if (ret < 0)
-		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
-
-exit:
-	mutex_unlock(&dev->alu_mutex);
-
-	return ret;
-}
-
-static int ksz_port_mirror_add(struct dsa_switch *ds, int port,
-			       struct dsa_mall_mirror_tc_entry *mirror,
-			       bool ingress)
-{
-	struct ksz_device *dev = ds->priv;
-
-	if (ingress)
-		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX, true);
-	else
-		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX, true);
-
-	ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_SNIFFER, false);
-
-	/* configure mirror port */
-	ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
-		     PORT_MIRROR_SNIFFER, true);
-
-	ksz_cfg(dev, S_MIRROR_CTRL, SW_MIRROR_RX_TX, false);
-
-	return 0;
-}
-
-static void ksz_port_mirror_del(struct dsa_switch *ds, int port,
-				struct dsa_mall_mirror_tc_entry *mirror)
-{
-	struct ksz_device *dev = ds->priv;
-	u8 data;
-
-	if (mirror->ingress)
-		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX, false);
-	else
-		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX, false);
-
-	ksz_pread8(dev, port, P_MIRROR_CTRL, &data);
-
-	if (!(data & (PORT_MIRROR_RX | PORT_MIRROR_TX)))
-		ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
-			     PORT_MIRROR_SNIFFER, false);
-}
-
-static const struct dsa_switch_ops ksz_switch_ops = {
-	.get_tag_protocol	= ksz_get_tag_protocol,
-	.setup			= ksz_setup,
-	.phy_read		= ksz_phy_read16,
-	.phy_write		= ksz_phy_write16,
-	.port_enable		= ksz_enable_port,
-	.port_disable		= ksz_disable_port,
-	.get_strings		= ksz_get_strings,
-	.get_ethtool_stats	= ksz_get_ethtool_stats,
-	.get_sset_count		= ksz_sset_count,
-	.port_stp_state_set	= ksz_port_stp_state_set,
-	.port_fast_age		= ksz_port_fast_age,
-	.port_vlan_filtering	= ksz_port_vlan_filtering,
-	.port_vlan_prepare	= ksz_port_vlan_prepare,
-	.port_vlan_add		= ksz_port_vlan_add,
-	.port_vlan_del		= ksz_port_vlan_del,
-	.port_fdb_dump		= ksz_port_fdb_dump,
-	.port_fdb_add		= ksz_port_fdb_add,
-	.port_fdb_del		= ksz_port_fdb_del,
-	.port_mdb_prepare       = ksz_port_mdb_prepare,
-	.port_mdb_add           = ksz_port_mdb_add,
-	.port_mdb_del           = ksz_port_mdb_del,
-	.port_mirror_add	= ksz_port_mirror_add,
-	.port_mirror_del	= ksz_port_mirror_del,
-};
-
-struct ksz_chip_data {
-	u32 chip_id;
-	const char *dev_name;
-	int num_vlans;
-	int num_alus;
-	int num_statics;
-	int cpu_ports;
-	int port_cnt;
-};
-
-static const struct ksz_chip_data ksz_switch_chips[] = {
-	{
-		.chip_id = 0x00947700,
-		.dev_name = "KSZ9477",
-		.num_vlans = 4096,
-		.num_alus = 4096,
-		.num_statics = 16,
-		.cpu_ports = 0x7F,	/* can be configured as cpu port */
-		.port_cnt = 7,		/* total physical port count */
-	},
-};
-
-static int ksz_switch_init(struct ksz_device *dev) -{
-	int i;
-
-	mutex_init(&dev->reg_mutex);
-	mutex_init(&dev->stats_mutex);
-	mutex_init(&dev->alu_mutex);
-	mutex_init(&dev->vlan_mutex);
-
-	dev->ds->ops = &ksz_switch_ops;
-
-	for (i = 0; i < ARRAY_SIZE(ksz_switch_chips); i++) {
-		const struct ksz_chip_data *chip = &ksz_switch_chips[i];
-
-		if (dev->chip_id == chip->chip_id) {
-			dev->name = chip->dev_name;
-			dev->num_vlans = chip->num_vlans;
-			dev->num_alus = chip->num_alus;
-			dev->num_statics = chip->num_statics;
-			dev->port_cnt = chip->port_cnt;
-			dev->cpu_ports = chip->cpu_ports;
-
-			break;
-		}
-	}
-
-	/* no switch found */
-	if (!dev->port_cnt)
-		return -ENODEV;
-
-	return 0;
-}
-
 struct ksz_device *ksz_switch_alloc(struct device *base,
 				    const struct ksz_io_ops *ops,
 				    void *priv)
@@ -1159,53 +57,43 @@ struct ksz_device *ksz_switch_alloc(struct device *base,  }  EXPORT_SYMBOL(ksz_switch_alloc);
 
-int ksz_switch_detect(struct ksz_device *dev) -{
-	u8 data8;
-	u32 id32;
-	int ret;
-
-	/* turn off SPI DO Edge select */
-	ret = ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
-	if (ret)
-		return ret;
-
-	data8 &= ~SPI_AUTO_EDGE_DETECTION;
-	ret = ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
-	if (ret)
-		return ret;
-
-	/* read chip id */
-	ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
-	if (ret)
-		return ret;
-
-	dev->chip_id = id32;
-
-	return 0;
-}
-EXPORT_SYMBOL(ksz_switch_detect);
-
-int ksz_switch_register(struct ksz_device *dev)
+int ksz_switch_register(struct ksz_device *dev,
+			const struct ksz_dev_ops *ops)
 {
 	int ret;
 
 	if (dev->pdata)
 		dev->chip_id = dev->pdata->chip_id;
 
-	if (ksz_switch_detect(dev))
+	dev->dev_ops = ops;
+
+	if (dev->dev_ops->detect(dev))
 		return -EINVAL;
 
-	ret = ksz_switch_init(dev);
+	ret = dev->dev_ops->init(dev);
 	if (ret)
 		return ret;
 
-	return dsa_register_switch(dev->ds);
+	dev->interface = PHY_INTERFACE_MODE_MII;
+	if (dev->dev->of_node) {
+		ret = of_get_phy_mode(dev->dev->of_node);
+		if (ret >= 0)
+			dev->interface = ret;
+	}
+
+	ret = dsa_register_switch(dev->ds);
+	if (ret) {
+		dev->dev_ops->exit(dev);
+		return ret;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(ksz_switch_register);
 
 void ksz_switch_remove(struct ksz_device *dev)  {
+	dev->dev_ops->exit(dev);
 	dsa_unregister_switch(dev->ds);
 }
 EXPORT_SYMBOL(ksz_switch_remove);

^ permalink raw reply related

* [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Tristram.Ha @ 2017-09-07 21:09 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411218CF@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Create new ksz9477.c file from original ksz_common.c.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
new file mode 100644
index 0000000..bc722b4
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -0,0 +1,1317 @@
+/*
+ * Microchip switch driver main logic
+ *
+ * Copyright (C) 2017
+ *
+ * Permission to use, copy, modify, and/or distribute this software for 
+any
+ * purpose with or without fee is hereby granted, provided that the 
+above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL 
+WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE 
+FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY 
+DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN 
+AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT 
+OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_data/microchip-ksz.h>
+#include <linux/phy.h>
+#include <linux/etherdevice.h>
+#include <linux/if_bridge.h>
+#include <net/dsa.h>
+#include <net/switchdev.h>
+
+#include "ksz_priv.h"
+#include "ksz_9477_reg.h"
+
+static const struct {
+	int index;
+	char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+	{ 0x00, "rx_hi" },
+	{ 0x01, "rx_undersize" },
+	{ 0x02, "rx_fragments" },
+	{ 0x03, "rx_oversize" },
+	{ 0x04, "rx_jabbers" },
+	{ 0x05, "rx_symbol_err" },
+	{ 0x06, "rx_crc_err" },
+	{ 0x07, "rx_align_err" },
+	{ 0x08, "rx_mac_ctrl" },
+	{ 0x09, "rx_pause" },
+	{ 0x0A, "rx_bcast" },
+	{ 0x0B, "rx_mcast" },
+	{ 0x0C, "rx_ucast" },
+	{ 0x0D, "rx_64_or_less" },
+	{ 0x0E, "rx_65_127" },
+	{ 0x0F, "rx_128_255" },
+	{ 0x10, "rx_256_511" },
+	{ 0x11, "rx_512_1023" },
+	{ 0x12, "rx_1024_1522" },
+	{ 0x13, "rx_1523_2000" },
+	{ 0x14, "rx_2001" },
+	{ 0x15, "tx_hi" },
+	{ 0x16, "tx_late_col" },
+	{ 0x17, "tx_pause" },
+	{ 0x18, "tx_bcast" },
+	{ 0x19, "tx_mcast" },
+	{ 0x1A, "tx_ucast" },
+	{ 0x1B, "tx_deferred" },
+	{ 0x1C, "tx_total_col" },
+	{ 0x1D, "tx_exc_col" },
+	{ 0x1E, "tx_single_col" },
+	{ 0x1F, "tx_mult_col" },
+	{ 0x80, "rx_total" },
+	{ 0x81, "tx_total" },
+	{ 0x82, "rx_discards" },
+	{ 0x83, "tx_discards" },
+};
+
+static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool 
+set) {
+	u8 data;
+
+	ksz_read8(dev, addr, &data);
+	if (set)
+		data |= bits;
+	else
+		data &= ~bits;
+	ksz_write8(dev, addr, data);
+}
+
+static void ksz_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool 
+set) {
+	u32 data;
+
+	ksz_read32(dev, addr, &data);
+	if (set)
+		data |= bits;
+	else
+		data &= ~bits;
+	ksz_write32(dev, addr, data);
+}
+
+static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
+			 bool set)
+{
+	u32 addr;
+	u8 data;
+
+	addr = PORT_CTRL_ADDR(port, offset);
+	ksz_read8(dev, addr, &data);
+
+	if (set)
+		data |= bits;
+	else
+		data &= ~bits;
+
+	ksz_write8(dev, addr, data);
+}
+
+static void ksz_port_cfg32(struct ksz_device *dev, int port, int offset,
+			   u32 bits, bool set)
+{
+	u32 addr;
+	u32 data;
+
+	addr = PORT_CTRL_ADDR(port, offset);
+	ksz_read32(dev, addr, &data);
+
+	if (set)
+		data |= bits;
+	else
+		data &= ~bits;
+
+	ksz_write32(dev, addr, data);
+}
+
+static int wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton, int 
+timeout) {
+	u8 data;
+
+	do {
+		ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
+		if (!(data & waiton))
+			break;
+		usleep_range(1, 10);
+	} while (timeout-- > 0);
+
+	if (timeout <= 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 
+*vlan_table) {
+	struct ksz_device *dev = ds->priv;
+	int ret;
+
+	mutex_lock(&dev->vlan_mutex);
+
+	ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
+	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
+
+	/* wait to be cleared */
+	ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
+	if (ret < 0) {
+		dev_dbg(dev->dev, "Failed to read vlan table\n");
+		goto exit;
+	}
+
+	ksz_read32(dev, REG_SW_VLAN_ENTRY__4, &vlan_table[0]);
+	ksz_read32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, &vlan_table[1]);
+	ksz_read32(dev, REG_SW_VLAN_ENTRY_PORTS__4, &vlan_table[2]);
+
+	ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
+
+exit:
+	mutex_unlock(&dev->vlan_mutex);
+
+	return ret;
+}
+
+static int set_vlan_table(struct dsa_switch *ds, u16 vid, u32 
+*vlan_table) {
+	struct ksz_device *dev = ds->priv;
+	int ret;
+
+	mutex_lock(&dev->vlan_mutex);
+
+	ksz_write32(dev, REG_SW_VLAN_ENTRY__4, vlan_table[0]);
+	ksz_write32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, vlan_table[1]);
+	ksz_write32(dev, REG_SW_VLAN_ENTRY_PORTS__4, vlan_table[2]);
+
+	ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
+	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
+
+	/* wait to be cleared */
+	ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
+	if (ret < 0) {
+		dev_dbg(dev->dev, "Failed to write vlan table\n");
+		goto exit;
+	}
+
+	ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
+
+	/* update vlan cache table */
+	dev->vlan_cache[vid].table[0] = vlan_table[0];
+	dev->vlan_cache[vid].table[1] = vlan_table[1];
+	dev->vlan_cache[vid].table[2] = vlan_table[2];
+
+exit:
+	mutex_unlock(&dev->vlan_mutex);
+
+	return ret;
+}
+
+static void read_table(struct dsa_switch *ds, u32 *table) {
+	struct ksz_device *dev = ds->priv;
+
+	ksz_read32(dev, REG_SW_ALU_VAL_A, &table[0]);
+	ksz_read32(dev, REG_SW_ALU_VAL_B, &table[1]);
+	ksz_read32(dev, REG_SW_ALU_VAL_C, &table[2]);
+	ksz_read32(dev, REG_SW_ALU_VAL_D, &table[3]); }
+
+static void write_table(struct dsa_switch *ds, u32 *table) {
+	struct ksz_device *dev = ds->priv;
+
+	ksz_write32(dev, REG_SW_ALU_VAL_A, table[0]);
+	ksz_write32(dev, REG_SW_ALU_VAL_B, table[1]);
+	ksz_write32(dev, REG_SW_ALU_VAL_C, table[2]);
+	ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]); }
+
+static int wait_alu_ready(struct ksz_device *dev, u32 waiton, int 
+timeout) {
+	u32 data;
+
+	do {
+		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
+		if (!(data & waiton))
+			break;
+		usleep_range(1, 10);
+	} while (timeout-- > 0);
+
+	if (timeout <= 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int wait_alu_sta_ready(struct ksz_device *dev, u32 waiton, int 
+timeout) {
+	u32 data;
+
+	do {
+		ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
+		if (!(data & waiton))
+			break;
+		usleep_range(1, 10);
+	} while (timeout-- > 0);
+
+	if (timeout <= 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int ksz_reset_switch(struct ksz_device *dev) {
+	u8 data8;
+	u16 data16;
+	u32 data32;
+
+	/* reset switch */
+	ksz_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
+
+	/* turn off SPI DO Edge select */
+	ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
+	data8 &= ~SPI_AUTO_EDGE_DETECTION;
+	ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
+
+	/* default configuration */
+	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
+	data8 = SW_AGING_ENABLE | SW_LINK_AUTO_AGING |
+	      SW_SRC_ADDR_FILTER | SW_FLUSH_STP_TABLE | SW_FLUSH_MSTP_TABLE;
+	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
+
+	/* disable interrupts */
+	ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
+	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
+	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
+
+	/* set broadcast storm protection 10% rate */
+	ksz_read16(dev, REG_SW_MAC_CTRL_2, &data16);
+	data16 &= ~BROADCAST_STORM_RATE;
+	data16 |= (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100;
+	ksz_write16(dev, REG_SW_MAC_CTRL_2, data16);
+
+	return 0;
+}
+
+static void port_setup(struct ksz_device *dev, int port, bool cpu_port) 
+{
+	u8 data8;
+	u16 data16;
+
+	/* enable tag tail for host port */
+	if (cpu_port)
+		ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_TAIL_TAG_ENABLE,
+			     true);
+
+	ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK, false);
+
+	/* set back pressure */
+	ksz_port_cfg(dev, port, REG_PORT_MAC_CTRL_1, PORT_BACK_PRESSURE, 
+true);
+
+	/* set flow control */
+	ksz_port_cfg(dev, port, REG_PORT_CTRL_0,
+		     PORT_FORCE_TX_FLOW_CTRL | PORT_FORCE_RX_FLOW_CTRL, true);
+
+	/* enable broadcast storm limit */
+	ksz_port_cfg(dev, port, P_BCAST_STORM_CTRL, PORT_BROADCAST_STORM, 
+true);
+
+	/* disable DiffServ priority */
+	ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_DIFFSERV_PRIO_ENABLE, 
+false);
+
+	/* replace priority */
+	ksz_port_cfg(dev, port, REG_PORT_MRI_MAC_CTRL, PORT_USER_PRIO_CEILING,
+		     false);
+	ksz_port_cfg32(dev, port, REG_PORT_MTI_QUEUE_CTRL_0__4,
+		       MTI_PVID_REPLACE, false);
+
+	/* enable 802.1p priority */
+	ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_PRIO_ENABLE, true);
+
+	/* configure MAC to 1G & RGMII mode */
+	ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
+	data8 |= PORT_RGMII_ID_EG_ENABLE;
+	data8 &= ~PORT_MII_NOT_1GBIT;
+	data8 &= ~PORT_MII_SEL_M;
+	data8 |= PORT_RGMII_SEL;
+	ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
+
+	/* clear pending interrupts */
+	ksz_pread16(dev, port, REG_PORT_PHY_INT_ENABLE, &data16); }
+
+static void ksz_config_cpu_port(struct dsa_switch *ds) {
+	struct ksz_device *dev = ds->priv;
+	int i;
+
+	ds->num_ports = dev->port_cnt;
+
+	for (i = 0; i < ds->num_ports; i++) {
+		if (dsa_is_cpu_port(ds, i) && (dev->cpu_ports & (1 << i))) {
+			dev->cpu_port = i;
+
+			/* enable cpu port */
+			port_setup(dev, i, true);
+			dev->HOST_MASK = (1 << dev->cpu_port);
+			dev->PORT_MASK |= dev->HOST_MASK;
+		}
+	}
+}
+
+static int ksz_setup(struct dsa_switch *ds) {
+	struct ksz_device *dev = ds->priv;
+	int ret = 0;
+
+	dev->vlan_cache = devm_kcalloc(dev->dev, sizeof(struct vlan_table),
+				       dev->num_vlans, GFP_KERNEL);
+	if (!dev->vlan_cache)
+		return -ENOMEM;
+
+	ret = ksz_reset_switch(dev);
+	if (ret) {
+		dev_err(ds->dev, "failed to reset switch\n");
+		return ret;
+	}
+
+	/* accept packet up to 2000bytes */
+	ksz_cfg(dev, REG_SW_MAC_CTRL_1, SW_LEGAL_PACKET_DISABLE, true);
+
+	ksz_config_cpu_port(ds);
+
+	ksz_cfg(dev, REG_SW_MAC_CTRL_1, MULTICAST_STORM_DISABLE, true);
+
+	/* queue based egress rate limit */
+	ksz_cfg(dev, REG_SW_MAC_CTRL_5, SW_OUT_RATE_LIMIT_QUEUE_BASED, true);
+
+	/* start switch */
+	ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
+
+	return 0;
+}
+
+static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch 
+*ds) {
+	return DSA_TAG_PROTO_KSZ;
+}
+
+static int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg) {
+	struct ksz_device *dev = ds->priv;
+	u16 val = 0;
+
+	ksz_pread16(dev, addr, 0x100 + (reg << 1), &val);
+
+	return val;
+}
+
+static int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, 
+u16 val) {
+	struct ksz_device *dev = ds->priv;
+
+	ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val);
+
+	return 0;
+}
+
+static int ksz_enable_port(struct dsa_switch *ds, int port,
+			   struct phy_device *phy)
+{
+	struct ksz_device *dev = ds->priv;
+
+	/* setup slave port */
+	port_setup(dev, port, false);
+
+	return 0;
+}
+
+static void ksz_disable_port(struct dsa_switch *ds, int port,
+			     struct phy_device *phy)
+{
+	struct ksz_device *dev = ds->priv;
+
+	/* there is no port disable */
+	ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK, true); }
+
+static int ksz_sset_count(struct dsa_switch *ds) {
+	return TOTAL_SWITCH_COUNTER_NUM;
+}
+
+static void ksz_get_strings(struct dsa_switch *ds, int port, uint8_t 
+*buf) {
+	int i;
+
+	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
+		memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
+		       ETH_GSTRING_LEN);
+	}
+}
+
+static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
+				  uint64_t *buf)
+{
+	struct ksz_device *dev = ds->priv;
+	int i;
+	u32 data;
+	int timeout;
+
+	mutex_lock(&dev->stats_mutex);
+
+	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
+		data = MIB_COUNTER_READ;
+		data |= ((mib_names[i].index & 0xFF) << MIB_COUNTER_INDEX_S);
+		ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
+
+		timeout = 1000;
+		do {
+			ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
+				    &data);
+			usleep_range(1, 10);
+			if (!(data & MIB_COUNTER_READ))
+				break;
+		} while (timeout-- > 0);
+
+		/* failed to read MIB. get out of loop */
+		if (!timeout) {
+			dev_dbg(dev->dev, "Failed to get MIB\n");
+			break;
+		}
+
+		/* count resets upon read */
+		ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
+
+		dev->ports[port].mib.info[i].counter += (uint64_t)data;
+		buf[i] = dev->ports[port].mib.info[i].counter;
+	}
+
+	mutex_unlock(&dev->stats_mutex);
+}
+
+static void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8 
+state) {
+	struct ksz_device *dev = ds->priv;
+	u8 data;
+
+	ksz_pread8(dev, port, P_STP_CTRL, &data);
+	data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
+
+	switch (state) {
+	case BR_STATE_DISABLED:
+		data |= PORT_LEARN_DISABLE;
+		break;
+	case BR_STATE_LISTENING:
+		data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
+		break;
+	case BR_STATE_LEARNING:
+		data |= PORT_RX_ENABLE;
+		break;
+	case BR_STATE_FORWARDING:
+		data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
+		break;
+	case BR_STATE_BLOCKING:
+		data |= PORT_LEARN_DISABLE;
+		break;
+	default:
+		dev_err(ds->dev, "invalid STP state: %d\n", state);
+		return;
+	}
+
+	ksz_pwrite8(dev, port, P_STP_CTRL, data); }
+
+static void ksz_port_fast_age(struct dsa_switch *ds, int port) {
+	struct ksz_device *dev = ds->priv;
+	u8 data8;
+
+	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
+	data8 |= SW_FAST_AGING;
+	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
+
+	data8 &= ~SW_FAST_AGING;
+	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8); }
+
+static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port, 
+bool flag) {
+	struct ksz_device *dev = ds->priv;
+
+	if (flag) {
+		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
+			     PORT_VLAN_LOOKUP_VID_0, true);
+		ksz_cfg32(dev, REG_SW_QM_CTRL__4, UNICAST_VLAN_BOUNDARY, true);
+		ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE, true);
+	} else {
+		ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE, false);
+		ksz_cfg32(dev, REG_SW_QM_CTRL__4, UNICAST_VLAN_BOUNDARY, false);
+		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
+			     PORT_VLAN_LOOKUP_VID_0, false);
+	}
+
+	return 0;
+}
+
+static int ksz_port_vlan_prepare(struct dsa_switch *ds, int port,
+				 const struct switchdev_obj_port_vlan *vlan,
+				 struct switchdev_trans *trans)
+{
+	/* nothing needed */
+
+	return 0;
+}
+
+static void ksz_port_vlan_add(struct dsa_switch *ds, int port,
+			      const struct switchdev_obj_port_vlan *vlan,
+			      struct switchdev_trans *trans) {
+	struct ksz_device *dev = ds->priv;
+	u32 vlan_table[3];
+	u16 vid;
+	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
+		if (get_vlan_table(ds, vid, vlan_table)) {
+			dev_dbg(dev->dev, "Failed to get vlan table\n");
+			return;
+		}
+
+		vlan_table[0] = VLAN_VALID | (vid & VLAN_FID_M);
+		if (untagged)
+			vlan_table[1] |= BIT(port);
+		else
+			vlan_table[1] &= ~BIT(port);
+		vlan_table[1] &= ~(BIT(dev->cpu_port));
+
+		vlan_table[2] |= BIT(port) | BIT(dev->cpu_port);
+
+		if (set_vlan_table(ds, vid, vlan_table)) {
+			dev_dbg(dev->dev, "Failed to set vlan table\n");
+			return;
+		}
+
+		/* change PVID */
+		if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
+			ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID, vid);
+	}
+}
+
+static int ksz_port_vlan_del(struct dsa_switch *ds, int port,
+			     const struct switchdev_obj_port_vlan *vlan) {
+	struct ksz_device *dev = ds->priv;
+	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+	u32 vlan_table[3];
+	u16 vid;
+	u16 pvid;
+
+	ksz_pread16(dev, port, REG_PORT_DEFAULT_VID, &pvid);
+	pvid = pvid & 0xFFF;
+
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
+		if (get_vlan_table(ds, vid, vlan_table)) {
+			dev_dbg(dev->dev, "Failed to get vlan table\n");
+			return -ETIMEDOUT;
+		}
+
+		vlan_table[2] &= ~BIT(port);
+
+		if (pvid == vid)
+			pvid = 1;
+
+		if (untagged)
+			vlan_table[1] &= ~BIT(port);
+
+		if (set_vlan_table(ds, vid, vlan_table)) {
+			dev_dbg(dev->dev, "Failed to set vlan table\n");
+			return -ETIMEDOUT;
+		}
+	}
+
+	ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID, pvid);
+
+	return 0;
+}
+
+struct alu_struct {
+	/* entry 1 */
+	u8	is_static:1;
+	u8	is_src_filter:1;
+	u8	is_dst_filter:1;
+	u8	prio_age:3;
+	u32	_reserv_0_1:23;
+	u8	mstp:3;
+	/* entry 2 */
+	u8	is_override:1;
+	u8	is_use_fid:1;
+	u32	_reserv_1_1:23;
+	u8	port_forward:7;
+	/* entry 3 & 4*/
+	u32	_reserv_2_1:9;
+	u8	fid:7;
+	u8	mac[ETH_ALEN];
+};
+
+static int ksz_port_fdb_add(struct dsa_switch *ds, int port,
+			    const unsigned char *addr, u16 vid) {
+	struct ksz_device *dev = ds->priv;
+	u32 alu_table[4];
+	u32 data;
+	int ret = 0;
+
+	mutex_lock(&dev->alu_mutex);
+
+	/* find any entry with mac & vid */
+	data = vid << ALU_FID_INDEX_S;
+	data |= ((addr[0] << 8) | addr[1]);
+	ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
+
+	data = ((addr[2] << 24) | (addr[3] << 16));
+	data |= ((addr[4] << 8) | addr[5]);
+	ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
+
+	/* start read operation */
+	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
+
+	/* wait to be finished */
+	ret = wait_alu_ready(dev, ALU_START, 1000);
+	if (ret < 0) {
+		dev_dbg(dev->dev, "Failed to read ALU\n");
+		goto exit;
+	}
+
+	/* read ALU entry */
+	read_table(ds, alu_table);
+
+	/* update ALU entry */
+	alu_table[0] = ALU_V_STATIC_VALID;
+	alu_table[1] |= BIT(port);
+	if (vid)
+		alu_table[1] |= ALU_V_USE_FID;
+	alu_table[2] = (vid << ALU_V_FID_S);
+	alu_table[2] |= ((addr[0] << 8) | addr[1]);
+	alu_table[3] = ((addr[2] << 24) | (addr[3] << 16));
+	alu_table[3] |= ((addr[4] << 8) | addr[5]);
+
+	write_table(ds, alu_table);
+
+	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
+
+	/* wait to be finished */
+	ret = wait_alu_ready(dev, ALU_START, 1000);
+	if (ret < 0)
+		dev_dbg(dev->dev, "Failed to write ALU\n");
+
+exit:
+	mutex_unlock(&dev->alu_mutex);
+
+	return ret;
+}
+
+static int ksz_port_fdb_del(struct dsa_switch *ds, int port,
+			    const unsigned char *addr, u16 vid) {
+	struct ksz_device *dev = ds->priv;
+	u32 alu_table[4];
+	u32 data;
+	int ret = 0;
+
+	mutex_lock(&dev->alu_mutex);
+
+	/* read any entry with mac & vid */
+	data = vid << ALU_FID_INDEX_S;
+	data |= ((addr[0] << 8) | addr[1]);
+	ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
+
+	data = ((addr[2] << 24) | (addr[3] << 16));
+	data |= ((addr[4] << 8) | addr[5]);
+	ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
+
+	/* start read operation */
+	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
+
+	/* wait to be finished */
+	ret = wait_alu_ready(dev, ALU_START, 1000);
+	if (ret < 0) {
+		dev_dbg(dev->dev, "Failed to read ALU\n");
+		goto exit;
+	}
+
+	ksz_read32(dev, REG_SW_ALU_VAL_A, &alu_table[0]);
+	if (alu_table[0] & ALU_V_STATIC_VALID) {
+		ksz_read32(dev, REG_SW_ALU_VAL_B, &alu_table[1]);
+		ksz_read32(dev, REG_SW_ALU_VAL_C, &alu_table[2]);
+		ksz_read32(dev, REG_SW_ALU_VAL_D, &alu_table[3]);
+
+		/* clear forwarding port */
+		alu_table[2] &= ~BIT(port);
+
+		/* if there is no port to forward, clear table */
+		if ((alu_table[2] & ALU_V_PORT_MAP) == 0) {
+			alu_table[0] = 0;
+			alu_table[1] = 0;
+			alu_table[2] = 0;
+			alu_table[3] = 0;
+		}
+	} else {
+		alu_table[0] = 0;
+		alu_table[1] = 0;
+		alu_table[2] = 0;
+		alu_table[3] = 0;
+	}
+
+	write_table(ds, alu_table);
+
+	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
+
+	/* wait to be finished */
+	ret = wait_alu_ready(dev, ALU_START, 1000);
+	if (ret < 0)
+		dev_dbg(dev->dev, "Failed to write ALU\n");
+
+exit:
+	mutex_unlock(&dev->alu_mutex);
+
+	return ret;
+}
+
+static void convert_alu(struct alu_struct *alu, u32 *alu_table) {
+	alu->is_static = !!(alu_table[0] & ALU_V_STATIC_VALID);
+	alu->is_src_filter = !!(alu_table[0] & ALU_V_SRC_FILTER);
+	alu->is_dst_filter = !!(alu_table[0] & ALU_V_DST_FILTER);
+	alu->prio_age = (alu_table[0] >> ALU_V_PRIO_AGE_CNT_S) &
+			ALU_V_PRIO_AGE_CNT_M;
+	alu->mstp = alu_table[0] & ALU_V_MSTP_M;
+
+	alu->is_override = !!(alu_table[1] & ALU_V_OVERRIDE);
+	alu->is_use_fid = !!(alu_table[1] & ALU_V_USE_FID);
+	alu->port_forward = alu_table[1] & ALU_V_PORT_MAP;
+
+	alu->fid = (alu_table[2] >> ALU_V_FID_S) & ALU_V_FID_M;
+
+	alu->mac[0] = (alu_table[2] >> 8) & 0xFF;
+	alu->mac[1] = alu_table[2] & 0xFF;
+	alu->mac[2] = (alu_table[3] >> 24) & 0xFF;
+	alu->mac[3] = (alu_table[3] >> 16) & 0xFF;
+	alu->mac[4] = (alu_table[3] >> 8) & 0xFF;
+	alu->mac[5] = alu_table[3] & 0xFF;
+}
+
+static int ksz_port_fdb_dump(struct dsa_switch *ds, int port,
+			     dsa_fdb_dump_cb_t *cb, void *data) {
+	struct ksz_device *dev = ds->priv;
+	int ret = 0;
+	u32 ksz_data;
+	u32 alu_table[4];
+	struct alu_struct alu;
+	int timeout;
+
+	mutex_lock(&dev->alu_mutex);
+
+	/* start ALU search */
+	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_START | ALU_SEARCH);
+
+	do {
+		timeout = 1000;
+		do {
+			ksz_read32(dev, REG_SW_ALU_CTRL__4, &ksz_data);
+			if ((ksz_data & ALU_VALID) || !(ksz_data & ALU_START))
+				break;
+			usleep_range(1, 10);
+		} while (timeout-- > 0);
+
+		if (!timeout) {
+			dev_dbg(dev->dev, "Failed to search ALU\n");
+			ret = -ETIMEDOUT;
+			goto exit;
+		}
+
+		/* read ALU table */
+		read_table(ds, alu_table);
+
+		convert_alu(&alu, alu_table);
+
+		if (alu.port_forward & BIT(port)) {
+			ret = cb(alu.mac, alu.fid, alu.is_static, data);
+			if (ret)
+				goto exit;
+		}
+	} while (ksz_data & ALU_START);
+
+exit:
+
+	/* stop ALU search */
+	ksz_write32(dev, REG_SW_ALU_CTRL__4, 0);
+
+	mutex_unlock(&dev->alu_mutex);
+
+	return ret;
+}
+
+static int ksz_port_mdb_prepare(struct dsa_switch *ds, int port,
+				const struct switchdev_obj_port_mdb *mdb,
+				struct switchdev_trans *trans)
+{
+	/* nothing to do */
+	return 0;
+}
+
+static void ksz_port_mdb_add(struct dsa_switch *ds, int port,
+			     const struct switchdev_obj_port_mdb *mdb,
+			     struct switchdev_trans *trans)
+{
+	struct ksz_device *dev = ds->priv;
+	u32 static_table[4];
+	u32 data;
+	int index;
+	u32 mac_hi, mac_lo;
+
+	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
+	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
+	mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
+
+	mutex_lock(&dev->alu_mutex);
+
+	for (index = 0; index < dev->num_statics; index++) {
+		/* find empty slot first */
+		data = (index << ALU_STAT_INDEX_S) |
+			ALU_STAT_READ | ALU_STAT_START;
+		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+
+		/* wait to be finished */
+		if (wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0) {
+			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
+			goto exit;
+		}
+
+		/* read ALU static table */
+		read_table(ds, static_table);
+
+		if (static_table[0] & ALU_V_STATIC_VALID) {
+			/* check this has same vid & mac address */
+			if (((static_table[2] >> ALU_V_FID_S) == mdb->vid) &&
+			    ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
+			    static_table[3] == mac_lo) {
+				/* found matching one */
+				break;
+			}
+		} else {
+			/* found empty one */
+			break;
+		}
+	}
+
+	/* no available entry */
+	if (index == dev->num_statics)
+		goto exit;
+
+	/* add entry */
+	static_table[0] = ALU_V_STATIC_VALID;
+	static_table[1] |= BIT(port);
+	if (mdb->vid)
+		static_table[1] |= ALU_V_USE_FID;
+	static_table[2] = (mdb->vid << ALU_V_FID_S);
+	static_table[2] |= mac_hi;
+	static_table[3] = mac_lo;
+
+	write_table(ds, static_table);
+
+	data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
+	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+
+	/* wait to be finished */
+	if (wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0)
+		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
+
+exit:
+	mutex_unlock(&dev->alu_mutex);
+}
+
+static int ksz_port_mdb_del(struct dsa_switch *ds, int port,
+			    const struct switchdev_obj_port_mdb *mdb) {
+	struct ksz_device *dev = ds->priv;
+	u32 static_table[4];
+	u32 data;
+	int index;
+	int ret = 0;
+	u32 mac_hi, mac_lo;
+
+	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
+	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
+	mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
+
+	mutex_lock(&dev->alu_mutex);
+
+	for (index = 0; index < dev->num_statics; index++) {
+		/* find empty slot first */
+		data = (index << ALU_STAT_INDEX_S) |
+			ALU_STAT_READ | ALU_STAT_START;
+		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+
+		/* wait to be finished */
+		ret = wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
+		if (ret < 0) {
+			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
+			goto exit;
+		}
+
+		/* read ALU static table */
+		read_table(ds, static_table);
+
+		if (static_table[0] & ALU_V_STATIC_VALID) {
+			/* check this has same vid & mac address */
+
+			if (((static_table[2] >> ALU_V_FID_S) == mdb->vid) &&
+			    ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
+			    static_table[3] == mac_lo) {
+				/* found matching one */
+				break;
+			}
+		}
+	}
+
+	/* no available entry */
+	if (index == dev->num_statics) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	/* clear port */
+	static_table[1] &= ~BIT(port);
+
+	if ((static_table[1] & ALU_V_PORT_MAP) == 0) {
+		/* delete entry */
+		static_table[0] = 0;
+		static_table[1] = 0;
+		static_table[2] = 0;
+		static_table[3] = 0;
+	}
+
+	write_table(ds, static_table);
+
+	data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
+	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
+
+	/* wait to be finished */
+	ret = wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
+	if (ret < 0)
+		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
+
+exit:
+	mutex_unlock(&dev->alu_mutex);
+
+	return ret;
+}
+
+static int ksz_port_mirror_add(struct dsa_switch *ds, int port,
+			       struct dsa_mall_mirror_tc_entry *mirror,
+			       bool ingress)
+{
+	struct ksz_device *dev = ds->priv;
+
+	if (ingress)
+		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX, true);
+	else
+		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX, true);
+
+	ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_SNIFFER, false);
+
+	/* configure mirror port */
+	ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
+		     PORT_MIRROR_SNIFFER, true);
+
+	ksz_cfg(dev, S_MIRROR_CTRL, SW_MIRROR_RX_TX, false);
+
+	return 0;
+}
+
+static void ksz_port_mirror_del(struct dsa_switch *ds, int port,
+				struct dsa_mall_mirror_tc_entry *mirror) {
+	struct ksz_device *dev = ds->priv;
+	u8 data;
+
+	if (mirror->ingress)
+		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX, false);
+	else
+		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX, false);
+
+	ksz_pread8(dev, port, P_MIRROR_CTRL, &data);
+
+	if (!(data & (PORT_MIRROR_RX | PORT_MIRROR_TX)))
+		ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
+			     PORT_MIRROR_SNIFFER, false);
+}
+
+static const struct dsa_switch_ops ksz_switch_ops = {
+	.get_tag_protocol	= ksz_get_tag_protocol,
+	.setup			= ksz_setup,
+	.phy_read		= ksz_phy_read16,
+	.phy_write		= ksz_phy_write16,
+	.port_enable		= ksz_enable_port,
+	.port_disable		= ksz_disable_port,
+	.get_strings		= ksz_get_strings,
+	.get_ethtool_stats	= ksz_get_ethtool_stats,
+	.get_sset_count		= ksz_sset_count,
+	.port_stp_state_set	= ksz_port_stp_state_set,
+	.port_fast_age		= ksz_port_fast_age,
+	.port_vlan_filtering	= ksz_port_vlan_filtering,
+	.port_vlan_prepare	= ksz_port_vlan_prepare,
+	.port_vlan_add		= ksz_port_vlan_add,
+	.port_vlan_del		= ksz_port_vlan_del,
+	.port_fdb_dump		= ksz_port_fdb_dump,
+	.port_fdb_add		= ksz_port_fdb_add,
+	.port_fdb_del		= ksz_port_fdb_del,
+	.port_mdb_prepare       = ksz_port_mdb_prepare,
+	.port_mdb_add           = ksz_port_mdb_add,
+	.port_mdb_del           = ksz_port_mdb_del,
+	.port_mirror_add	= ksz_port_mirror_add,
+	.port_mirror_del	= ksz_port_mirror_del,
+};
+
+static u32 ksz_get_port_addr(int port, int offset) {
+	return PORT_CTRL_ADDR(port, offset);
+}
+
+/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
+ * 
+-----------------------------------------------------------------------
+----
+ * 
+DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4by
+tes)
+ * 
+-----------------------------------------------------------------------
+----
+ * tag0 : Prioritization (not used now)
+ * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 
+0x10=port5)
+ *
+ * For Egress (KSZ -> Host), 1 byte is added before FCS.
+ * 
+-----------------------------------------------------------------------
+----
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * 
+-----------------------------------------------------------------------
+----
+ * tag0 : zero-based value represents port
+ *	  (eg, 0x00=port1, 0x02=port3, 0x06=port7)
+ */
+
+#define	KSZ_INGRESS_TAG_LEN	2
+#define	KSZ_EGRESS_TAG_LEN	1
+#define KSZ_PTP_TAG_LEN		4
+#define KSZ_PTP_TAG_INDICATION	0x80
+
+/**
+ * This information needs to be communicated to the MAC driver so that 
+it can
+ * receive the frames correctly from the switch as some MAC drivers 
+adhere to
+ * receive exactly 1518 bytes.
+ */
+static int ksz_get_rx_len(struct ksz_device *dev) {
+	int len = KSZ_EGRESS_TAG_LEN;
+
+	/* PTP function is turned on. */
+	if (dev->overrides & PTP_TAG)
+		len += KSZ_PTP_TAG_LEN;
+	return len;
+}
+
+/**
+ * This information tells how many more bytes need to be allocated to
+ * accommodate the tail tag.
+ */
+static int ksz_get_tx_len(struct ksz_device *dev) {
+	int len = KSZ_INGRESS_TAG_LEN;
+
+	/* PTP function is turned on. */
+	if (dev->overrides & PTP_TAG)
+		len += KSZ_PTP_TAG_LEN;
+	return len;
+}
+
+static void ksz_add_tail_tag(struct ksz_device *dev, struct sk_buff *skb,
+			     int port)
+{
+	struct {
+		u32 timestamp;
+		u16 ports;
+	} tx_tag;
+	u8 *trailer;
+	u8 *tag;
+	int len = KSZ_INGRESS_TAG_LEN;
+	int ptp_len = 0;
+
+	port = 1 << port;
+
+	/* PTP function is turned on. */
+	if (dev->overrides & PTP_TAG)
+		ptp_len = KSZ_PTP_TAG_LEN;
+	len += ptp_len;
+	trailer = skb_put(skb, len);
+	memset(&tx_tag, 0, sizeof(tx_tag));
+	tx_tag.ports = port & dev->PORT_MASK;
+	tx_tag.ports = cpu_to_be16(tx_tag.ports);
+	tag = (u8 *)&tx_tag;
+	memcpy(trailer, &tag[4 - ptp_len], ptp_len + KSZ_INGRESS_TAG_LEN); }
+
+static int ksz_get_tail_tag(struct ksz_device *dev, struct sk_buff *skb,
+			    int *port)
+{
+	u8 *trailer;
+	int len = KSZ_EGRESS_TAG_LEN;
+
+	trailer = skb_tail_pointer(skb) - len;
+	if (*trailer & KSZ_PTP_TAG_INDICATION)
+		len += KSZ_PTP_TAG_LEN;
+	*trailer &= ~KSZ_PTP_TAG_INDICATION;
+	*port = *trailer;
+	if (*port >= 0 && *port < dev->mib_port_cnt) {
+		struct ksz_port *p = &dev->ports[*port];
+
+		/**
+		 * Switch is forwarding when port membership includes other
+		 * ports.
+		 */
+		if (p->member & ~((1 << *port) | dev->HOST_MASK))
+			skb->offload_fwd_mark = 1;
+	}
+	return len;
+}
+
+static int ksz_switch_detect(struct ksz_device *dev) {
+	u8 data8;
+	u32 id32;
+	int ret;
+
+	/* turn off SPI DO Edge select */
+	ret = ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
+	if (ret)
+		return ret;
+
+	data8 &= ~SPI_AUTO_EDGE_DETECTION;
+	ret = ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
+	if (ret)
+		return ret;
+
+	/* read chip id */
+	ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
+	if (ret)
+		return ret;
+
+	/* Number of ports can be reduced depending on chip. */
+	dev->mib_port_cnt = TOTAL_PORT_NUM;
+
+	dev->chip_id = id32;
+
+	dev->mib_cnt = TOTAL_SWITCH_COUNTER_NUM;
+
+	/* Try to read MIB counters in all ports every second. */
+	dev->MIB_READ_INTERVAL = msecs_to_jiffies(1000 / dev->mib_port_cnt);
+
+	return 0;
+}
+
+struct ksz_chip_data {
+	u32 chip_id;
+	const char *dev_name;
+	int num_vlans;
+	int num_alus;
+	int num_statics;
+	int cpu_ports;
+	int port_cnt;
+};
+
+static const struct ksz_chip_data ksz_switch_chips[] = {
+	{
+		.chip_id = 0x00947700,
+		.dev_name = "KSZ9477",
+		.num_vlans = 4096,
+		.num_alus = 4096,
+		.num_statics = 16,
+		.cpu_ports = 0x7F,	/* can be configured as cpu port */
+		.port_cnt = 7,		/* total physical port count */
+	},
+};
+
+static int ksz_switch_init(struct ksz_device *dev) {
+	int i;
+
+	mutex_init(&dev->reg_mutex);
+	mutex_init(&dev->stats_mutex);
+	mutex_init(&dev->alu_mutex);
+	mutex_init(&dev->vlan_mutex);
+
+	dev->ds->ops = &ksz_switch_ops;
+
+	for (i = 0; i < ARRAY_SIZE(ksz_switch_chips); i++) {
+		const struct ksz_chip_data *chip = &ksz_switch_chips[i];
+
+		if (dev->chip_id == chip->chip_id) {
+			dev->name = chip->dev_name;
+			dev->num_vlans = chip->num_vlans;
+			dev->num_alus = chip->num_alus;
+			dev->num_statics = chip->num_statics;
+			dev->port_cnt = chip->port_cnt;
+			dev->cpu_ports = chip->cpu_ports;
+
+			break;
+		}
+	}
+
+	/* no switch found */
+	if (!dev->port_cnt)
+		return -ENODEV;
+
+	dev->PORT_MASK = (1 << dev->port_cnt) - 1;
+	dev->PORT_MASK |= dev->HOST_MASK;
+
+	i = dev->mib_port_cnt;
+	dev->ports = devm_kzalloc(dev->dev, sizeof(struct ksz_port) * i,
+				  GFP_KERNEL);
+	if (!dev->ports)
+		return -ENOMEM;
+	for (i = 0; i < dev->mib_port_cnt; i++) {
+		dev->ports[i].mib.info =
+			devm_kzalloc(dev->dev,
+				     sizeof(struct ksz_port_mib_info) *
+				     TOTAL_SWITCH_COUNTER_NUM, GFP_KERNEL);
+		if (!dev->ports[i].mib.info)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void ksz_switch_exit(struct ksz_device *dev) { }
+
+static const struct ksz_dev_ops ksz9477_dev_ops = {
+	.get_port_addr = ksz_get_port_addr,
+	.reset = ksz_reset_switch,
+	.get_rx_len = ksz_get_rx_len,
+	.get_tx_len = ksz_get_tx_len,
+	.add_tail_tag = ksz_add_tail_tag,
+	.get_tail_tag = ksz_get_tail_tag,
+	.detect = ksz_switch_detect,
+	.init = ksz_switch_init,
+	.exit = ksz_switch_exit,
+};
+
+int ksz9477_switch_register(struct ksz_device *dev) {
+	return ksz_switch_register(dev, &ksz9477_dev_ops); } 
+EXPORT_SYMBOL(ksz9477_switch_register);
+
+MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>"); 
+MODULE_DESCRIPTION("Microchip KSZ Series Switch DSA Driver"); 
+MODULE_LICENSE("GPL");

^ permalink raw reply related

* [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Tristram.Ha @ 2017-09-07 21:08 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411218C4@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Break ksz_common.c into 2 files so that the common code can be used by other KSZ switch drivers.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile
index ed335e2..0961c30 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,2 @@
-obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ)	        += ksz9477.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)	+= ksz_spi.o

^ permalink raw reply related

* [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.
From: Tristram.Ha @ 2017-09-07 21:08 UTC (permalink / raw)
  To: andrew, muvarov, pavel, nathan.leigh.conrad, vivien.didelot,
	f.fainelli, netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411218B7@CHN-SV-EXMX02.mchp-main.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

This series of patches is to modify the original KSZ9477 DSA driver so that other KSZ switch drivers can be added and use the common code.

This patch set is against net-next.

 drivers/net/dsa/microchip/Makefile     |    2 +-
 drivers/net/dsa/microchip/ksz9477.c    | 1317 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.c | 1156 +---------------------------
 drivers/net/dsa/microchip/ksz_priv.h   |  105 ++-
 drivers/net/dsa/microchip/ksz_spi.c    |   13 +-
 net/dsa/tag_ksz.c                      |   40 +-
 6 files changed, 1458 insertions(+), 1175 deletions(-)  create mode 100644 drivers/net/dsa/microchip/ksz9477.c

^ permalink raw reply

* Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter chain
From: Jiri Pirko @ 2017-09-07 20:52 UTC (permalink / raw)
  To: Cong Wang; +Cc: Linux Kernel Network Developers, Jakub Kicinski, Jiri Pirko
In-Reply-To: <CAM_iQpXsYc1cfZmuFKGH1O-FPJr9+NJqZTTB5z1oJVGYgvkV+Q@mail.gmail.com>

Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:
>On Wed, Sep 6, 2017 at 11:32 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Thu, Sep 07, 2017 at 06:26:07AM CEST, xiyou.wangcong@gmail.com wrote:
>>>This patch fixes the following madness of tc filter chain:
>>
>> Could you avoid expressive words like "madness" and such?
>> Please be technical.
>>
>
>If the following 2a) 2b) 2c) 2d) are not enough to show the madness,
>I don't know any other to show it. Madness is for code, not for you
>or any other person, so 100% technical.

We hav eto agree to disagree. You want to be expressive, apparently
there is no way to talk you out of that. Sigh...


>
>>
>>>
>>>1) tcf_chain_destroy() is called by both tcf_block_put() and
>>>   tcf_chain_put().  tcf_chain_put() is correctly refcnt'ed and paired
>>>   with tcf_chain_get(), but tcf_block_put() is not, it should be paired
>>>   with tcf_block_get() which means we still need to decrease the refcnt.
>>>   Think it in another way: if we call tcf_bock_put() immediately after
>>>   tcf_block_get(), could we get effectively a nop? This causes a memory
>>>   leak as reported by Jakub.
>>>
>>>2) tp proto should hold a refcnt to the chain too. This significantly
>>>   simplifies the logic:
>>>
>>>2a) Chain 0 is no longer special, it is created and refcnted by tp
>>>    like any other chains. All the ugliness in tcf_chain_put() can be
>>>    gone!
>>>
>>>2b) No need to handle the flushing oddly, because block still holds
>>>    chain 0, it can not be released, this guarantees block is the last
>>>    user.
>>>
>>>2c) The race condition with RCU callbacks is easier to handle with just
>>>    a rcu_barrier()! Much easier to understand, nothing to hide! Thanks
>>>    to the previous patch. Please see also the comments in code.
>>>
>>>2d) Make the code understandable by humans, much less error-prone.
>>>
>>>Fixes: 744a4cf63e52 ("net: sched: fix use after free when tcf_chain_destroy is called multiple times")
>>>Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
>>>Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>>>Cc: Jiri Pirko <jiri@mellanox.com>
>>>Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>>---
>>> net/sched/cls_api.c | 38 ++++++++++++++++++++++----------------
>>> 1 file changed, 22 insertions(+), 16 deletions(-)
>>>
>>>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>>>index 6c5ea84d2682..e9060dc36519 100644
>>>--- a/net/sched/cls_api.c
>>>+++ b/net/sched/cls_api.c
>>>@@ -209,21 +209,20 @@ static void tcf_chain_flush(struct tcf_chain *chain)
>>>               RCU_INIT_POINTER(*chain->p_filter_chain, NULL);
>>>       while ((tp = rtnl_dereference(chain->filter_chain)) != NULL) {
>>>               RCU_INIT_POINTER(chain->filter_chain, tp->next);
>>>+              tcf_chain_put(chain);
>>>               tcf_proto_destroy(tp);
>>>       }
>>> }
>>>
>>> static void tcf_chain_destroy(struct tcf_chain *chain)
>>> {
>>>-      /* May be already removed from the list by the previous call. */
>>>-      if (!list_empty(&chain->list))
>>>-              list_del_init(&chain->list);
>>>+      list_del(&chain->list);
>>>+      kfree(chain);
>>>+}
>>>
>>>-      /* There might still be a reference held when we got here from
>>>-       * tcf_block_put. Wait for the user to drop reference before free.
>>>-       */
>>>-      if (!chain->refcnt)
>>>-              kfree(chain);
>>>+static void tcf_chain_hold(struct tcf_chain *chain)
>>>+{
>>>+      ++chain->refcnt;
>>> }
>>>
>>> struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>>>@@ -233,7 +232,7 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>>>
>>>       list_for_each_entry(chain, &block->chain_list, list) {
>>>               if (chain->index == chain_index) {
>>>-                      chain->refcnt++;
>>>+                      tcf_chain_hold(chain);
>>>                       return chain;
>>>               }
>>>       }
>>>@@ -246,10 +245,7 @@ EXPORT_SYMBOL(tcf_chain_get);
>>>
>>> void tcf_chain_put(struct tcf_chain *chain)
>>> {
>>>-      /* Destroy unused chain, with exception of chain 0, which is the
>>>-       * default one and has to be always present.
>>>-       */
>>>-      if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)
>>>+      if (--chain->refcnt == 0)
>>
>> Okay, so you take the reference for every goto_chain action and every
>> tp, right? Note that for chain 0, you hold one more reference (due to
>> the creation). That is probably ok as we need chain 0 not to go away
>> even if all tps and goto_chain actions are gone.
>
>Yeah, this is the core of the patch.
>
>>
>>
>>>               tcf_chain_destroy(chain);
>>> }
>>> EXPORT_SYMBOL(tcf_chain_put);
>>>@@ -294,10 +290,18 @@ void tcf_block_put(struct tcf_block *block)
>>>       if (!block)
>>>               return;
>>>
>>>-      list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
>>>+      /* Standalone actions are not allowed to jump to any chain, and
>>>+       * bound actions should be all removed after flushing. However,
>>>+       * filters are destroyed in RCU callbacks, we have to flush and wait
>>>+       * for them before releasing this refcnt, otherwise we race with RCU
>>>+       * callbacks!!!
>>
>> Why the "!!!"? Please avoid that. Not necessary at all.
>
>
>Because we don't have lock here, we have to pay more extra
>attention to it. Playing list without lock is like playing fire. I use "!!!" to
>draw people's attention when they touch it, perhaps "XXX" is better?
>
>
>>
>>
>>>+       */
>>>+      list_for_each_entry(chain, &block->chain_list, list)
>>>               tcf_chain_flush(chain);
>>>-              tcf_chain_destroy(chain);
>>>-      }
>>>+      rcu_barrier();
>>
>> This actually tries to fix another bug I discovered yesterday. Good.
>>
>
>This on the other hand proves we should make the code clean
>and understandable asap, not just wait for net-next.
>
>
>
>>
>>>+
>>>+      list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
>>>+              tcf_chain_put(chain);
>>
>> Which reference are you putting here? For chain 0, that is the original
>> reference due to creation from block_get. But how about the other
>> chains? If you do flush all in the previous list iteration, they are
>> removed there. Also note that they are removed from the list while
>> iterating it.
>
>
>Yes it is for chain 0, because block holds a reference to chain 0 during
>creation. Non-0 chains are created with refcnt==1 too but paired with
>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0
>not special w.r.t. refcnt.

So you need to tcf_chain_put only chain 0 here, right? The rest of the
chains get destroyed by the previous list_for_each_entry iteration when
flush happens and actions destroy happens what decrements recnt to 0
there.

What do I miss, who would still hold reference for non-0 chains when all
tps and all goto_chain actions are gone?


>
>
>>
>> I believe that you need to add tcf_chain_hold(chain) to the start of the
>> previous list iteration to ensure all existing chains will stay, then
>> you can put them here.
>
>We don't need it, it is already perfectly paired with tcf_block_get().
>Think about the following:
>
>1)
>tcf_block_get(...); // create chain 0 with refcnt=1
>tcf_block_put(); // no tp, only chain 0 in chain_list
>// chain 0 is put, refcnt=0, it is gone
>
>2)
>tcf_block_get(); // create chain 0 with refcnt=1
>...
>tcf_chain_get(11); // create chain 11 with refcnt=1
>// one tp is inserted to chain 11, now refcnt==2
>// in tc_ctl_tfilter()
>tcf_chain_put(11); // paired with above get, refcnt==1
>...
>tcf_block_put(); // flush chain 11, tp removed, refcnt==0
>// put chain 0 too, paired with block get, refcnt == 0
>// both chain 0 and chain11 are gone
>
>
>
>>
>> Did you test this? I believe we need some simple test script.
>
>Of course I did. I verified memleak is gone and tested basic
>filters and gact actions with chain 0 and chain 11, everything
>works as expected, I also added a printk() to verify the chain
>is really gone as soon as all references are gone.
>
>I will contribute my test cases back after I figure out how.
>Also net-next is already closed.

^ permalink raw reply

* Re: [RFC PATCH v3 0/6] Configuring traffic classes via new hardware offload mechanism in tc/mqprio
From: Nambiar, Amritha @ 2017-09-07 20:09 UTC (permalink / raw)
  To: Florian Fainelli, intel-wired-lan, jeffrey.t.kirsher
  Cc: alexander.h.duyck, netdev
In-Reply-To: <3ecacf3d-1f02-fbc6-0e90-8e84dcd15a4e@gmail.com>

On 9/7/2017 11:34 AM, Florian Fainelli wrote:
> On 09/07/2017 04:00 AM, Amritha Nambiar wrote:
>> The following series introduces a new hardware offload mode in 
>> tc/mqprio where the TCs, the queue configurations and bandwidth 
>> rate limits are offloaded to the hardware. The existing mqprio 
>> framework is extended to configure the queue counts and layout and
>>  also added support for rate limiting. This is achieved through new
>>  netlink attributes for the 'mode' option which takes values such
>> as 'dcb' (default) and 'channel' and a 'shaper' option for QoS 
>> attributes such as bandwidth rate limits in hw mode 1.
> 
> So "dcb" defines a default priorities to queue mapping?

In the default offload implementation, only the basic hw offload was
supported factoring only the 'number of TCs' values. The rest of the
queue configuration was being ignored. This is the legacy behavior with
hw mode set to 1.
Example:
# tc qdisc add dev eth0 root mqprio num_tc 4  map 0 0 0 0 1 1 1 1 queues
4@0 4@4 hw 1
I just named this default behavior to 'dcb' while introducing a new
offload mechanism.

> 
>> Legacy devices can fall back to the existing setup supporting hw 
>> mode 1 without these additional options where only the TCs are 
>> offloaded and then the 'mode' and 'shaper' options defaults to DCB
>>  support.
> 
> That's the last part that confuses me, see below.

As I introduced new options for 'mode' and 'shaper', I set the defaults
for these options to 'dcb' so existing offloaders can continue to work
without supporting these new options. Patch 1 has a detailed description
on how this is done.

> 
>> The i40e driver enables the new mqprio hardware offload mechanism 
>> factoring the TCs, queue configuration and bandwidth rates by 
>> creating HW channel VSIs.
> 
> I am really confused by what you call hw_mode 1, as I understand it 
> there are really 3 different modes:

There are actually 2 modes now with 'hw' option set to 1, legacy/dcb and
channel.
> 
> - legacy: you don't define any traffic class mapping, but you can 
> still chain this scheduler with a match + action (like what 
> Documentation/networking/multiqueue.txt) you can optionally also add 
> "shaper" arguments, but there should not be any default DCB queue 
> mapping either?
> 
> - dcb: a default mapping for traffic classes to queues is defined, 
> optional "shaper" arguments

The legacy mode now becomes the dcb mode. In this mode, although the TC
values, the queue configurations, prio-tc-mapping are all offloaded to
the device, the existing implementation in current drivers support only
a basic hw offload factoring only the TC values.

Examples:
# ... num_tc 2  map 0 0 0 0 1 1 1 1 queues 4@0 4@4 hw 1
# ... num_tc 2  map 0 0 0 0 1 1 1 1 queues 4@0 4@4 hw 1 mode dcb
# ... num_tc 2  map 0 0 0 0 1 1 1 1 queues 4@0 4@4 hw 1 mode dcb\
  shaper dcb

> 
> - channel: (maybe calling that "custom_tc_map" would be clearer?) 
> where you express the exact traffic classes to queue mapping and 
> optional "shaper" arguments

In the channel mode, a full hw offload is supported, the TC values, the
queue configurations and additionally QoS attributes (optional) are all
used in the new implementation.

Examples:
# ... num_tc 2  map 0 0 0 0 1 1 1 1 queues 4@0 4@4 hw 1 mode channel
# ... num_tc 2  map 0 0 0 0 1 1 1 1 queues 4@0 4@4 hw 1 mode channel\
shaper bw_rlimit max_rate 4Gbit 5Gbit

> 
> I think that's what you are doing, but I just got confused by the 
> cover letter.
> 
>> 
>> In this new mode, the priority to traffic class mapping and the 
>> user specified queue ranges are used to configure the traffic class
>> when the 'mode' option is set to 'channel'. This is achieved by
>> creating HW channels(VSI). A new channel is created for each of the
>> traffic class configuration offloaded via mqprio framework except
>> for the first TC (TC0) which is for the main VSI. TC0 for the main
>> VSI is also reconfigured as per user provided queue parameters.
>> Finally, bandwidth rate limits are set on these traffic classes
>> through the shaper attribute by sending these rates in addition to
>> the number of TCs and the queue configurations.
>> 
>> Example: # tc qdisc add dev eth0 root mqprio num_tc 2 map 0 0 0 0 1
>> 1 1 1\ queues 4@0 4@4 hw 1 mode channel shaper bw_rlimit\
> 
> Do you see a case where you can declare a different number of
> traffic classes say 4 and map them onto just 2 hardware queues? If
> not, it seems a tiny bit redundant to have to specify both the map
> and the queue mapping should be sufficient, right?

This will be subjected to validation of the user input and will be
treated as invalid configuration. The 'map' specifies the mapping
between user priorities and traffic classes while the queue mapping is
the queue layout specifying the queue count and offsets.

> 
>> min_rate 1Gbit 2Gbit max_rate 4Gbit 5Gbit
>> 
>> To dump the bandwidth rates:
>> 
>> # tc qdisc show dev eth0
>> 
>> qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0 
>> queues:(0:3) (4:7) mode:channel shaper:bw_rlimit   min_rate:1Gbit 
>> 2Gbit   max_rate:4Gbit 5Gbit
> 
> I am not well versed into tc, but being able to specify "shaper" 
> arguments has actually value outside of just the multiq scheduler and
> it could probably be an action on its own?

The mqprio scheduler already supports configuring the traffic classes
and enabling support for HW shapers was just a matter to extending it
with new netlink based attributes.

> 
>> 
>> ---
>> 
>> Amritha Nambiar (6): mqprio: Introduce new hardware offload mode 
>> and shaper in mqprio i40e: Add macro for PF reset bit i40e: Add 
>> infrastructure for queue channel support i40e: Enable 'channel' 
>> mode in mqprio for TC configs i40e: Refactor VF BW rate limiting 
>> i40e: Add support setting TC max bandwidth rates
>> 
>> 
>> drivers/net/ethernet/intel/i40e/i40e.h             |   44 + 
>> drivers/net/ethernet/intel/i40e/i40e_debugfs.c     |    3 
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |    8 
>> drivers/net/ethernet/intel/i40e/i40e_main.c        | 1463 
>> +++++++++++++++++--- drivers/net/ethernet/intel/i40e/i40e_txrx.h | 
>> 2 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |   50 - 
>> include/net/pkt_cls.h                              |    9 
>> include/uapi/linux/pkt_sched.h                     |   32 
>> net/sched/sch_mqprio.c                             |  183 ++- 9 
>> files changed, 1551 insertions(+), 243 deletions(-)
>> 
>> --
>> 
> 
> 

^ permalink raw reply

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Guedes, Andre @ 2017-09-07 19:58 UTC (permalink / raw)
  To: Gomes, Vinicius, henrik@austad.us
  Cc: jiri@resnulli.us, jhs@mojatatu.com, Ong, Boon Leong,
	xiyou.wangcong@gmail.com, Sanchez-Palencia, Jesus,
	richardcochran@gmail.com, netdev@vger.kernel.org, Briano, Ivan,
	intel-wired-lan@lists.osuosl.org
In-Reply-To: <20170907053411.GA6580@sisyphus.home.austad.us>

[-- Attachment #1: Type: text/plain, Size: 3398 bytes --]

Hi Henrik,

Thanks for your feedback! I'll address some of your comments below.

On Thu, 2017-09-07 at 07:34 +0200, Henrik Austad wrote:
> > As for the shapers config interface:
> > 
> >  * CBS (802.1Qav)
> > 
> >    This patchset is proposing a new qdisc called 'cbs'. Its 'tc' cmd line
> > is:
> >    $ tc qdisc add dev IFACE parent ID cbs locredit N hicredit M sendslope S
> > \
> >      idleslope I
> 
> So this confuses me a bit, why specify sendSlope?
> 
>     sendSlope = portTransmitRate - idleSlope
> 
> and portTransmitRate is the speed of the MAC (which you get from the 
> driver). Adding sendSlope here is just redundant I think.

Yes, this was something we've spent quite a few time discussing before this RFC
series. After reading the Annex L from 802.1Q-2014 (operation of CBS algorithm)
so many times, we've came up with the rationale explained below.

The rationale here is that sendSlope is just another parameter from CBS
algorithm like idleSlope, hiCredit and loCredit. As such, its calculation
should be done at the same "layer" as the others parameters (in this case, user
space) in order to keep consistency. Moreover, in this design, the driver layer
is dead simple: all the device driver has to do is applying CBS parameters to
hardware. Having any CBS parameter calculation in the driver layer means all
device drivers must implement that calculation.

> Also, does this mean that when you create the qdisc, you have locked the 
> bandwidth for the scheduler? Meaning, if I later want to add another 
> stream that requires more bandwidth, I have to close all active streams, 
> reconfigure the qdisc and then restart?

If we want to reserve more bandwidth to "accommodate" a new stream, we don't
need to close all active streams. All we have to do is changing the CBS qdisc
and pass the new CBS parameters. Here is what the command-line would look like:

$ tc qdisc change dev enp0s4 parent 8001:5 cbs locredit -1470 hicredit 30
sendslope -980000 idleslope 20000

No application/stream is interrupted while new CBS parameters are applied.

> >    Note that the parameters for this qdisc are the ones defined by the
> >    802.1Q-2014 spec, so no hardware specific functionality is exposed here.
> 
> You do need to know if the link is brought up as 100 or 1000 though - which 
> the driver already knows.

User space knows that information via ethtool or /sys.

> > Testing this RFC
> > ================
> > 
> > For testing the patches of this RFC only, you can refer to the samples and
> > helper script being added to samples/tsn/ and the use the 'mqprio' qdisc to
> > setup the priorities to Tx queues mapping, together with the 'cbs' qdisc to
> > configure the HW shaper of the i210 controller:
> 
> I will test it, feedback will be provided soon! :)

That's great! Please let us know if you find any issue and thanks for you
support.

> > 8) You can also run a Talker for class B (prio 2 here)
> > $ ./talker -i enp3s0 -p 2
> > 
> >  * The bandwidth displayed on the listener output now should increase to
> > very
> >    close to the one configured for class A + class B.
> 
> Because you grab both class A *and* B, or because B will eat what A does 
> not use?

Because the listener application grabs both class A and B traffic.

Regards,

Andre

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3262 bytes --]

^ permalink raw reply

* [PATCH] net: tulip: Constify tulip_tbl
From: Kees Cook @ 2017-09-07 19:35 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Jarod Wilson, Gustavo A. R. Silva, linux-parisc,
	linux-kernel

It looks like all users of tulip_tbl are reads, so mark this table
as read-only.

$ git grep tulip_tbl  # edited to avoid line-wraps...
interrupt.c: iowrite32(tulip_tbl[tp->chip_id].valid_intrs, ...
interrupt.c: iowrite32(tulip_tbl[tp->chip_id].valid_intrs&~RxPollInt, ...
interrupt.c: iowrite32(tulip_tbl[tp->chip_id].valid_intrs, ...
interrupt.c: iowrite32(tulip_tbl[tp->chip_id].valid_intrs | TimerInt,
pnic.c:      iowrite32(tulip_tbl[tp->chip_id].valid_intrs, ioaddr + CSR7);
tulip.h:     extern struct tulip_chip_table tulip_tbl[];
tulip_core.c:struct tulip_chip_table tulip_tbl[] = {
tulip_core.c:iowrite32(tulip_tbl[tp->chip_id].valid_intrs, ioaddr + CSR5);
tulip_core.c:iowrite32(tulip_tbl[tp->chip_id].valid_intrs, ioaddr + CSR7);
tulip_core.c:setup_timer(&tp->timer, tulip_tbl[tp->chip_id].media_timer,
tulip_core.c:const char *chip_name = tulip_tbl[chip_idx].chip_name;
tulip_core.c:if (pci_resource_len (pdev, 0) < tulip_tbl[chip_idx].io_size)
tulip_core.c:ioaddr =  pci_iomap(..., tulip_tbl[chip_idx].io_size);
tulip_core.c:tp->flags = tulip_tbl[chip_idx].flags;
tulip_core.c:setup_timer(&tp->timer, tulip_tbl[tp->chip_id].media_timer,
tulip_core.c:INIT_WORK(&tp->media_work, tulip_tbl[tp->chip_id].media_task);

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jarod Wilson <jarod@redhat.com>
Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Cc: netdev@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/net/ethernet/dec/tulip/tulip.h      | 2 +-
 drivers/net/ethernet/dec/tulip/tulip_core.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/tulip.h b/drivers/net/ethernet/dec/tulip/tulip.h
index 38431a155f09..06660dbc44b7 100644
--- a/drivers/net/ethernet/dec/tulip/tulip.h
+++ b/drivers/net/ethernet/dec/tulip/tulip.h
@@ -515,7 +515,7 @@ void comet_timer(unsigned long data);
 extern int tulip_debug;
 extern const char * const medianame[];
 extern const char tulip_media_cap[];
-extern struct tulip_chip_table tulip_tbl[];
+extern const struct tulip_chip_table tulip_tbl[];
 void oom_timer(unsigned long data);
 extern u8 t21040_csr13[];
 
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index 84394b43c0a1..851b6d1f5a42 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -138,7 +138,7 @@ static void tulip_timer(unsigned long data)
  * It is indexed via the values in 'enum chips'
  */
 
-struct tulip_chip_table tulip_tbl[] = {
+const struct tulip_chip_table tulip_tbl[] = {
   { }, /* placeholder for array, slot unused currently */
   { }, /* placeholder for array, slot unused currently */
 
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related

* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Greg KH @ 2017-09-07 19:34 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Nikita Yushchenko, Chris Healy
In-Reply-To: <20170828191748.19492-2-vivien.didelot@savoirfairelinux.com>

I agree you shouldn't be using debugfs for this, but in the future, if
you do write debugfs code, please take the following review into
account:

On Mon, Aug 28, 2017 at 03:17:39PM -0400, Vivien Didelot wrote:
> +static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
> +{
> +	struct dentry *dir;
> +	char name[32];
> +
> +	snprintf(name, sizeof(name), DSA_PORT_FMT, port);
> +
> +	dir = debugfs_create_dir(name, ds->debugfs_dir);
> +	if (IS_ERR_OR_NULL(dir))
> +		return -EFAULT;

You should _never_ care about the return value of a debugfs call, and
you should not need to ever propagate the error upward.  The api was
written to not need this.

Just call the function, and return, that's it.  If you need to save the
return value (i.e. it's a dentry), you also don't care, just save it and
pass it to some other debugfs call, and all will still be fine.  Your
code should never do anything different if a debugfs call succeeds or
fails.

> +static int dsa_debugfs_create_switch(struct dsa_switch *ds)
> +{
> +	char name[32];
> +	int i, err;
> +
> +	/* skip if there is no debugfs support */
> +	if (!dsa_debugfs_dir)
> +		return 0;

Again, you don't care, all of these functions should return void.

> +	snprintf(name, sizeof(name), DSA_SWITCH_FMT, ds->index);
> +
> +	ds->debugfs_dir = debugfs_create_dir(name, dsa_debugfs_dir);
> +	if (IS_ERR_OR_NULL(ds->debugfs_dir))
> +		return -EFAULT;

See, that's horrid, you should never need to make such a bad check.

Also, even if it were the correct way to do this you never return EFAULT
unless there is a memory copy error to/from userspace.  That is not the
case here, or in any of this code, right?

> +static void dsa_debugfs_destroy_switch(struct dsa_switch *ds)
> +{
> +	/* handles NULL */
> +	debugfs_remove_recursive(ds->debugfs_dir);

Of course it handles NULL, why comment that?  That's the whole goal of
debugfs, to be dirt simple, allow you to do anything you want, in almost
no lines of code.

Also, it will never be mounted on a "real" system, so you better not
rely on it for anything "real".

> +		err = dsa_debugfs_create_switch(ds);
> +		if (err) {
> +			pr_warn("DSA: failed to create debugfs interface for switch %d (%d)\n",
> +				ds->index, err);

Never complain to the syslog about a debugfs issue.

> +void dsa_debugfs_destroy_module(void)
> +{
> +	/* handles NULL */
> +	debugfs_remove_recursive(dsa_debugfs_dir);

again, of course it does, do you think we don't know how to write an
api?  :)

thanks,

greg k-h

^ permalink raw reply

* Re: [RFC PATCH v3 0/6] Configuring traffic classes via new hardware offload mechanism in tc/mqprio
From: Florian Fainelli @ 2017-09-07 18:34 UTC (permalink / raw)
  To: Amritha Nambiar, intel-wired-lan, jeffrey.t.kirsher
  Cc: alexander.h.duyck, netdev
In-Reply-To: <150478158684.24662.17975701233699487888.stgit@anamdev.jf.intel.com>

On 09/07/2017 04:00 AM, Amritha Nambiar wrote:
> The following series introduces a new hardware offload mode in
> tc/mqprio where the TCs, the queue configurations and
> bandwidth rate limits are offloaded to the hardware. The existing
> mqprio framework is extended to configure the queue counts and
> layout and also added support for rate limiting. This is achieved
> through new netlink attributes for the 'mode' option which takes
> values such as 'dcb' (default) and 'channel' and a 'shaper' option
> for QoS attributes such as bandwidth rate limits in hw mode 1.

So "dcb" defines a default priorities to queue mapping?

> Legacy devices can fall back to the existing setup supporting hw mode
> 1 without these additional options where only the TCs are offloaded
> and then the 'mode' and 'shaper' options defaults to DCB support.

That's the last part that confuses me, see below.

> The i40e driver enables the new mqprio hardware offload mechanism
> factoring the TCs, queue configuration and bandwidth rates by
> creating HW channel VSIs.

I am really confused by what you call hw_mode 1, as I understand it
there are really 3 different modes:

- legacy: you don't define any traffic class mapping, but you can still
chain this scheduler with a match + action (like what
Documentation/networking/multiqueue.txt) you can optionally also add
"shaper" arguments, but there should not be any default DCB queue
mapping either?

- dcb: a default mapping for traffic classes to queues is defined,
optional "shaper" arguments

- channel: (maybe calling that "custom_tc_map" would be clearer?) where
you express the exact traffic classes to queue mapping and optional
"shaper" arguments

I think that's what you are doing, but I just got confused by the cover
letter.

> 
> In this new mode, the priority to traffic class mapping and the
> user specified queue ranges are used to configure the traffic
> class when the 'mode' option is set to 'channel'. This is achieved by
> creating HW channels(VSI). A new channel is created for each of the
> traffic class configuration offloaded via mqprio framework except for
> the first TC (TC0) which is for the main VSI. TC0 for the main VSI is
> also reconfigured as per user provided queue parameters. Finally,
> bandwidth rate limits are set on these traffic classes through the
> shaper attribute by sending these rates in addition to the number of
> TCs and the queue configurations.
> 
> Example:
>     # tc qdisc add dev eth0 root mqprio num_tc 2 map 0 0 0 0 1 1 1 1\
>       queues 4@0 4@4 hw 1 mode channel shaper bw_rlimit\

Do you see a case where you can declare a different number of traffic
classes say 4 and map them onto just 2 hardware queues? If not, it seems
a tiny bit redundant to have to specify both the map and the queue
mapping should be sufficient, right?

>       min_rate 1Gbit 2Gbit max_rate 4Gbit 5Gbit
> 
>     To dump the bandwidth rates:
> 
>     # tc qdisc show dev eth0
> 
>     qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
>                  queues:(0:3) (4:7)
>                  mode:channel
>                  shaper:bw_rlimit   min_rate:1Gbit 2Gbit   max_rate:4Gbit 5Gbit

I am not well versed into tc, but being able to specify "shaper"
arguments has actually value outside of just the multiq scheduler and it
could probably be an action on its own?

> 
> ---
> 
> Amritha Nambiar (6):
>       mqprio: Introduce new hardware offload mode and shaper in mqprio
>       i40e: Add macro for PF reset bit
>       i40e: Add infrastructure for queue channel support
>       i40e: Enable 'channel' mode in mqprio for TC configs
>       i40e: Refactor VF BW rate limiting
>       i40e: Add support setting TC max bandwidth rates
> 
> 
>  drivers/net/ethernet/intel/i40e/i40e.h             |   44 +
>  drivers/net/ethernet/intel/i40e/i40e_debugfs.c     |    3 
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |    8 
>  drivers/net/ethernet/intel/i40e/i40e_main.c        | 1463 +++++++++++++++++---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h        |    2 
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |   50 -
>  include/net/pkt_cls.h                              |    9 
>  include/uapi/linux/pkt_sched.h                     |   32 
>  net/sched/sch_mqprio.c                             |  183 ++-
>  9 files changed, 1551 insertions(+), 243 deletions(-)
> 
> --
> 


-- 
Florian

^ permalink raw reply

* Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter chain
From: Cong Wang @ 2017-09-07 17:45 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jakub Kicinski, Jiri Pirko
In-Reply-To: <20170907063216.GB1967@nanopsycho>

On Wed, Sep 6, 2017 at 11:32 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Sep 07, 2017 at 06:26:07AM CEST, xiyou.wangcong@gmail.com wrote:
>>This patch fixes the following madness of tc filter chain:
>
> Could you avoid expressive words like "madness" and such?
> Please be technical.
>

If the following 2a) 2b) 2c) 2d) are not enough to show the madness,
I don't know any other to show it. Madness is for code, not for you
or any other person, so 100% technical.

>
>>
>>1) tcf_chain_destroy() is called by both tcf_block_put() and
>>   tcf_chain_put().  tcf_chain_put() is correctly refcnt'ed and paired
>>   with tcf_chain_get(), but tcf_block_put() is not, it should be paired
>>   with tcf_block_get() which means we still need to decrease the refcnt.
>>   Think it in another way: if we call tcf_bock_put() immediately after
>>   tcf_block_get(), could we get effectively a nop? This causes a memory
>>   leak as reported by Jakub.
>>
>>2) tp proto should hold a refcnt to the chain too. This significantly
>>   simplifies the logic:
>>
>>2a) Chain 0 is no longer special, it is created and refcnted by tp
>>    like any other chains. All the ugliness in tcf_chain_put() can be
>>    gone!
>>
>>2b) No need to handle the flushing oddly, because block still holds
>>    chain 0, it can not be released, this guarantees block is the last
>>    user.
>>
>>2c) The race condition with RCU callbacks is easier to handle with just
>>    a rcu_barrier()! Much easier to understand, nothing to hide! Thanks
>>    to the previous patch. Please see also the comments in code.
>>
>>2d) Make the code understandable by humans, much less error-prone.
>>
>>Fixes: 744a4cf63e52 ("net: sched: fix use after free when tcf_chain_destroy is called multiple times")
>>Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
>>Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>>Cc: Jiri Pirko <jiri@mellanox.com>
>>Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>---
>> net/sched/cls_api.c | 38 ++++++++++++++++++++++----------------
>> 1 file changed, 22 insertions(+), 16 deletions(-)
>>
>>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>>index 6c5ea84d2682..e9060dc36519 100644
>>--- a/net/sched/cls_api.c
>>+++ b/net/sched/cls_api.c
>>@@ -209,21 +209,20 @@ static void tcf_chain_flush(struct tcf_chain *chain)
>>               RCU_INIT_POINTER(*chain->p_filter_chain, NULL);
>>       while ((tp = rtnl_dereference(chain->filter_chain)) != NULL) {
>>               RCU_INIT_POINTER(chain->filter_chain, tp->next);
>>+              tcf_chain_put(chain);
>>               tcf_proto_destroy(tp);
>>       }
>> }
>>
>> static void tcf_chain_destroy(struct tcf_chain *chain)
>> {
>>-      /* May be already removed from the list by the previous call. */
>>-      if (!list_empty(&chain->list))
>>-              list_del_init(&chain->list);
>>+      list_del(&chain->list);
>>+      kfree(chain);
>>+}
>>
>>-      /* There might still be a reference held when we got here from
>>-       * tcf_block_put. Wait for the user to drop reference before free.
>>-       */
>>-      if (!chain->refcnt)
>>-              kfree(chain);
>>+static void tcf_chain_hold(struct tcf_chain *chain)
>>+{
>>+      ++chain->refcnt;
>> }
>>
>> struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>>@@ -233,7 +232,7 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>>
>>       list_for_each_entry(chain, &block->chain_list, list) {
>>               if (chain->index == chain_index) {
>>-                      chain->refcnt++;
>>+                      tcf_chain_hold(chain);
>>                       return chain;
>>               }
>>       }
>>@@ -246,10 +245,7 @@ EXPORT_SYMBOL(tcf_chain_get);
>>
>> void tcf_chain_put(struct tcf_chain *chain)
>> {
>>-      /* Destroy unused chain, with exception of chain 0, which is the
>>-       * default one and has to be always present.
>>-       */
>>-      if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)
>>+      if (--chain->refcnt == 0)
>
> Okay, so you take the reference for every goto_chain action and every
> tp, right? Note that for chain 0, you hold one more reference (due to
> the creation). That is probably ok as we need chain 0 not to go away
> even if all tps and goto_chain actions are gone.

Yeah, this is the core of the patch.

>
>
>>               tcf_chain_destroy(chain);
>> }
>> EXPORT_SYMBOL(tcf_chain_put);
>>@@ -294,10 +290,18 @@ void tcf_block_put(struct tcf_block *block)
>>       if (!block)
>>               return;
>>
>>-      list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
>>+      /* Standalone actions are not allowed to jump to any chain, and
>>+       * bound actions should be all removed after flushing. However,
>>+       * filters are destroyed in RCU callbacks, we have to flush and wait
>>+       * for them before releasing this refcnt, otherwise we race with RCU
>>+       * callbacks!!!
>
> Why the "!!!"? Please avoid that. Not necessary at all.


Because we don't have lock here, we have to pay more extra
attention to it. Playing list without lock is like playing fire. I use "!!!" to
draw people's attention when they touch it, perhaps "XXX" is better?


>
>
>>+       */
>>+      list_for_each_entry(chain, &block->chain_list, list)
>>               tcf_chain_flush(chain);
>>-              tcf_chain_destroy(chain);
>>-      }
>>+      rcu_barrier();
>
> This actually tries to fix another bug I discovered yesterday. Good.
>

This on the other hand proves we should make the code clean
and understandable asap, not just wait for net-next.



>
>>+
>>+      list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
>>+              tcf_chain_put(chain);
>
> Which reference are you putting here? For chain 0, that is the original
> reference due to creation from block_get. But how about the other
> chains? If you do flush all in the previous list iteration, they are
> removed there. Also note that they are removed from the list while
> iterating it.


Yes it is for chain 0, because block holds a reference to chain 0 during
creation. Non-0 chains are created with refcnt==1 too but paired with
tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0
not special w.r.t. refcnt.


>
> I believe that you need to add tcf_chain_hold(chain) to the start of the
> previous list iteration to ensure all existing chains will stay, then
> you can put them here.

We don't need it, it is already perfectly paired with tcf_block_get().
Think about the following:

1)
tcf_block_get(...); // create chain 0 with refcnt=1
tcf_block_put(); // no tp, only chain 0 in chain_list
// chain 0 is put, refcnt=0, it is gone

2)
tcf_block_get(); // create chain 0 with refcnt=1
...
tcf_chain_get(11); // create chain 11 with refcnt=1
// one tp is inserted to chain 11, now refcnt==2
// in tc_ctl_tfilter()
tcf_chain_put(11); // paired with above get, refcnt==1
...
tcf_block_put(); // flush chain 11, tp removed, refcnt==0
// put chain 0 too, paired with block get, refcnt == 0
// both chain 0 and chain11 are gone



>
> Did you test this? I believe we need some simple test script.

Of course I did. I verified memleak is gone and tested basic
filters and gact actions with chain 0 and chain 11, everything
works as expected, I also added a printk() to verify the chain
is really gone as soon as all references are gone.

I will contribute my test cases back after I figure out how.
Also net-next is already closed.

^ permalink raw reply

* Re: [Intel-wired-lan] [RFC PATCH v3 0/6] Configuring traffic classes via new hardware offload mechanism in tc/mqprio
From: Shannon Nelson @ 2017-09-07 17:38 UTC (permalink / raw)
  To: Nambiar, Amritha, intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
In-Reply-To: <bd18cda5-4827-2246-f736-630527a9fcbc@intel.com>

On 9/7/2017 10:22 AM, Nambiar, Amritha wrote:
> On 9/7/2017 9:45 AM, Shannon Nelson wrote:
[...]
>>
>> It would be nice to know what has changed since the last review, either
>> summarized here or in the individual patch files. 
[...]
> 
> For all those patch files that have changed since the last revision, I
> have captured all the new changes in the section titled "v3: " of each
> patch file. 

Mea culpa.

ugh.
sln

^ permalink raw reply

* Re: [Intel-wired-lan] [RFC PATCH v3 0/6] Configuring traffic classes via new hardware offload mechanism in tc/mqprio
From: Nambiar, Amritha @ 2017-09-07 17:22 UTC (permalink / raw)
  To: Shannon Nelson, intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
In-Reply-To: <a37ba2ca-145a-0228-961d-c805434e7d40@oracle.com>

On 9/7/2017 9:45 AM, Shannon Nelson wrote:
> On 9/7/2017 4:00 AM, Amritha Nambiar wrote:
>> The following series introduces a new hardware offload mode in
>> tc/mqprio where the TCs, the queue configurations and
>> bandwidth rate limits are offloaded to the hardware. The existing
>> mqprio framework is extended to configure the queue counts and
>> layout and also added support for rate limiting. This is achieved
>> through new netlink attributes for the 'mode' option which takes
>> values such as 'dcb' (default) and 'channel' and a 'shaper' option
>> for QoS attributes such as bandwidth rate limits in hw mode 1.
>> Legacy devices can fall back to the existing setup supporting hw mode
>> 1 without these additional options where only the TCs are offloaded
>> and then the 'mode' and 'shaper' options defaults to DCB support.
>> The i40e driver enables the new mqprio hardware offload mechanism
>> factoring the TCs, queue configuration and bandwidth rates by
>> creating HW channel VSIs.
>>
>> In this new mode, the priority to traffic class mapping and the
>> user specified queue ranges are used to configure the traffic
>> class when the 'mode' option is set to 'channel'. This is achieved by
>> creating HW channels(VSI). A new channel is created for each of the
>> traffic class configuration offloaded via mqprio framework except for
>> the first TC (TC0) which is for the main VSI. TC0 for the main VSI is
>> also reconfigured as per user provided queue parameters. Finally,
>> bandwidth rate limits are set on these traffic classes through the
>> shaper attribute by sending these rates in addition to the number of
>> TCs and the queue configurations.
>>
>> Example:
>>      # tc qdisc add dev eth0 root mqprio num_tc 2 map 0 0 0 0 1 1 1 1\
>>        queues 4@0 4@4 hw 1 mode channel shaper bw_rlimit\
>>        min_rate 1Gbit 2Gbit max_rate 4Gbit 5Gbit
>>
>>      To dump the bandwidth rates:
>>
>>      # tc qdisc show dev eth0
>>
>>      qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
>>                   queues:(0:3) (4:7)
>>                   mode:channel
>>                   shaper:bw_rlimit   min_rate:1Gbit 2Gbit   max_rate:4Gbit 5Gbit
>>
>> ---
>>
>> Amritha Nambiar (6):
>>        mqprio: Introduce new hardware offload mode and shaper in mqprio
>>        i40e: Add macro for PF reset bit
>>        i40e: Add infrastructure for queue channel support
>>        i40e: Enable 'channel' mode in mqprio for TC configs
>>        i40e: Refactor VF BW rate limiting
>>        i40e: Add support setting TC max bandwidth rates
>>
> 
> It would be nice to know what has changed since the last review, either 
> summarized here or in the individual patch files.  This helps in knowing 
> how much attention should be given to this new set of patches, and 
> encourages further review.  I don't remember seeing any responses to my 
> previous comments, and it looks like not all of them were acted upon.

For all those patch files that have changed since the last revision, I
have captured all the new changes in the section titled "v3: " of each
patch file. Also, I had replied to most of your comments that they'll be
fixed in v3 and the one in the mqprio patch that error handling was
already being done and these responses can be verified in patchwork. I
missed to respond to your comment regarding supporting macvlan offloads
through these channels, that will not be a part of this series and will
be looked upon at a later time in a subsequent patch series.

> 
> sln
> 
>>
>>   drivers/net/ethernet/intel/i40e/i40e.h             |   44 +
>>   drivers/net/ethernet/intel/i40e/i40e_debugfs.c     |    3
>>   drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |    8
>>   drivers/net/ethernet/intel/i40e/i40e_main.c        | 1463 +++++++++++++++++---
>>   drivers/net/ethernet/intel/i40e/i40e_txrx.h        |    2
>>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |   50 -
>>   include/net/pkt_cls.h                              |    9
>>   include/uapi/linux/pkt_sched.h                     |   32
>>   net/sched/sch_mqprio.c                             |  183 ++-
>>   9 files changed, 1551 insertions(+), 243 deletions(-)
>>
>> --
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>>

^ permalink raw reply

* Re: Bad escapes in ip -online
From: Stephen Hemminger @ 2017-09-07 16:56 UTC (permalink / raw)
  To: John Kodis; +Cc: netdev
In-Reply-To: <b4972c84-c827-3a05-2267-bbcadf101260@nasa.gov>

On Thu, 7 Sep 2017 12:08:56 -0400
John Kodis <john.m.kodis@nasa.gov> wrote:

> The -online option to the 'ip link', 'ip addr', and perhaps others is 
> putting out a backslash in places where a newline character should go, 
> as so:
> 
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode 
> DEFAULT group default qlen 1000\    link/loopback 00:00:00:00:00:00 brd 
> 00:00:00:00:00:00
> 
> -- John.
> 

The backslash is the virtual line separator in the one line format.
It is has always been that way, and programs parse the oneline output.

^ permalink raw reply

* Re: nfp bpf offload add/replace
From: Jakub Kicinski @ 2017-09-07 16:52 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, mlxsw, Daniel Borkmann, Simon Horman
In-Reply-To: <20170907140503.GE1967@nanopsycho>

On Thu, 7 Sep 2017 16:05:03 +0200, Jiri Pirko wrote:
> Thu, Sep 07, 2017 at 03:44:12PM CEST, kubakici@wp.pl wrote:
> >On Thu, 7 Sep 2017 11:10:33 +0200, Jiri Pirko wrote:  
> >> Hi Kuba.
> >> 
> >> I'm looking into cls_bpf code and nfp_net_bpf_offload function in your
> >> driver. Why do you need TC_CLSBPF_ADD? Seems like TC_CLSBPF_REPLACE
> >> should be enough. It would make the cls_bpf code easier.
> >>
> >> Note that other cls just have replace/destroy (u32 too, as drivers
> >> handle NEW/REPLACE in one switch-case - will patch this).  
> >
> >Could we clarify what the REPLACE is actually supposed to do?  :)
> >
> >In the flower code and the REPLACE looks a lot like ADD on the
> >surface...  If change is called it will invoke REPLACE with the new
> >filter and then if there was an old filter, it will do DELETE.  Is my
> >understanding correct?  
> 
> Yes, correct.
> 
> >
> >If so I found this model of operation somehow confusing.  Plus the
> >management of flows may get slightly tricky if there is a possibility of
> >"replacing" a flow with an identical one.  Flower may make calls like
> >these:
> >
> >add flower vlan_id 100 action ...
> ># REPLACE vid 100 ...
> >change ... flower vlan_id 100 action ...
> ># REPLACE vid 100 ...
> ># DELETE  vid 100 ...  
> 
> Yes, that is the flow.
> 
> >
> >Doesn't this force driver/HW to implement refcounting on the rules?  
> 
> Why do you think so? There is a cookie that is passed from flower down
> and driver uses it to remove the entry.

Right, the key/mask combination doesn't have to be unique anyway...

> >On why I need the replace - BPF unlike other classifiers usually
> >installs a single program, I think offloading multiple TC filters is
> >questionable (people will use tailcalls instead most likely).  I want to
> >be able to implement atomic replace of that single program (i.e. not ADD
> >followed by DELETE) because that simplifies the driver quite a bit.  
> 
> Understood. So, looks like the REPLACE/DESTROY would be sufficient for
> bpf. ADD is not needed as it can be done by REPLACE-NULL, right?

Yes, or you could take it to the extreme ;)
 DESTROY == offload(old, NULL)
 ADD     == offload(NULL, new)
 REPLACE == offload(obj, new)

> On the other hand, the rest of the cls, namely flower, u32 and matchall
> need ADD/DESTROY as they don't really do no replacing.
> 
> Makes sense?

Ack, if you're unifying things, I don't mind how things are muxed as
long as atomic replace is possible.

FWIW cls_bpf doesn't pass old prog in REPLACE right now, but I have
patch to add it anyway since it simplifies the driver when maps are
involved.  I should probably stop looking at the .command completely,
just rely on new/old programs being populated.

^ permalink raw reply

* Re: [Intel-wired-lan] [RFC PATCH v3 0/6] Configuring traffic classes via new hardware offload mechanism in tc/mqprio
From: Shannon Nelson @ 2017-09-07 16:45 UTC (permalink / raw)
  To: Amritha Nambiar, intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
In-Reply-To: <150478158684.24662.17975701233699487888.stgit@anamdev.jf.intel.com>

On 9/7/2017 4:00 AM, Amritha Nambiar wrote:
> The following series introduces a new hardware offload mode in
> tc/mqprio where the TCs, the queue configurations and
> bandwidth rate limits are offloaded to the hardware. The existing
> mqprio framework is extended to configure the queue counts and
> layout and also added support for rate limiting. This is achieved
> through new netlink attributes for the 'mode' option which takes
> values such as 'dcb' (default) and 'channel' and a 'shaper' option
> for QoS attributes such as bandwidth rate limits in hw mode 1.
> Legacy devices can fall back to the existing setup supporting hw mode
> 1 without these additional options where only the TCs are offloaded
> and then the 'mode' and 'shaper' options defaults to DCB support.
> The i40e driver enables the new mqprio hardware offload mechanism
> factoring the TCs, queue configuration and bandwidth rates by
> creating HW channel VSIs.
> 
> In this new mode, the priority to traffic class mapping and the
> user specified queue ranges are used to configure the traffic
> class when the 'mode' option is set to 'channel'. This is achieved by
> creating HW channels(VSI). A new channel is created for each of the
> traffic class configuration offloaded via mqprio framework except for
> the first TC (TC0) which is for the main VSI. TC0 for the main VSI is
> also reconfigured as per user provided queue parameters. Finally,
> bandwidth rate limits are set on these traffic classes through the
> shaper attribute by sending these rates in addition to the number of
> TCs and the queue configurations.
> 
> Example:
>      # tc qdisc add dev eth0 root mqprio num_tc 2 map 0 0 0 0 1 1 1 1\
>        queues 4@0 4@4 hw 1 mode channel shaper bw_rlimit\
>        min_rate 1Gbit 2Gbit max_rate 4Gbit 5Gbit
> 
>      To dump the bandwidth rates:
> 
>      # tc qdisc show dev eth0
> 
>      qdisc mqprio 804a: root  tc 2 map 0 0 0 0 1 1 1 1 0 0 0 0 0 0 0 0
>                   queues:(0:3) (4:7)
>                   mode:channel
>                   shaper:bw_rlimit   min_rate:1Gbit 2Gbit   max_rate:4Gbit 5Gbit
> 
> ---
> 
> Amritha Nambiar (6):
>        mqprio: Introduce new hardware offload mode and shaper in mqprio
>        i40e: Add macro for PF reset bit
>        i40e: Add infrastructure for queue channel support
>        i40e: Enable 'channel' mode in mqprio for TC configs
>        i40e: Refactor VF BW rate limiting
>        i40e: Add support setting TC max bandwidth rates
> 

It would be nice to know what has changed since the last review, either 
summarized here or in the individual patch files.  This helps in knowing 
how much attention should be given to this new set of patches, and 
encourages further review.  I don't remember seeing any responses to my 
previous comments, and it looks like not all of them were acted upon.

sln

> 
>   drivers/net/ethernet/intel/i40e/i40e.h             |   44 +
>   drivers/net/ethernet/intel/i40e/i40e_debugfs.c     |    3
>   drivers/net/ethernet/intel/i40e/i40e_ethtool.c     |    8
>   drivers/net/ethernet/intel/i40e/i40e_main.c        | 1463 +++++++++++++++++---
>   drivers/net/ethernet/intel/i40e/i40e_txrx.h        |    2
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |   50 -
>   include/net/pkt_cls.h                              |    9
>   include/uapi/linux/pkt_sched.h                     |   32
>   net/sched/sch_mqprio.c                             |  183 ++-
>   9 files changed, 1551 insertions(+), 243 deletions(-)
> 
> --
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> 

^ permalink raw reply

* Re: pull-request: mac80211 2017-09-07
From: David Miller @ 2017-09-07 16:41 UTC (permalink / raw)
  To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20170907070939.3658-1-johannes@sipsolutions.net>

From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu,  7 Sep 2017 09:09:38 +0200

> During my long absence some things have accumulated, but there wasn't
> actually all that much that could've gone into the last cycle, and a
> fix or two was taken care of by others.
> 
> The most important thing here is probably the deadlock fix that a few
> people have run into on 4.13, but that was only identified now, and
> perhaps the 40 MHz fix from Emmanuel that helps avoid iwlwifi firmware
> crashes.
> 
> Please pull and let me know if there's any problem.

Pulled, thanks.

^ permalink raw reply

* Re: iwlwifi: mvm: only send LEDS_CMD when the FW supports it
From: Kalle Valo @ 2017-09-07 16:40 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: torvalds, linux-wireless, johannes, linux-kernel, akpm, netdev,
	davem, emmanuel.grumbach, Luca Coelho
In-Reply-To: <20170907075152.4522-1-luca@coelho.fi>

Luciano Coelho <luca@coelho.fi> wrote:

> From: Luca Coelho <luciano.coelho@intel.com>
> 
> The LEDS_CMD command is only supported in some newer FW versions
> (e.g. iwlwifi-8000C-31.ucode), so we can't send it to older versions
> (such as iwlwifi-8000C-27.ucode).
> 
> To fix this, check for a new bit in the FW capabilities TLV that tells
> when the command is supported.
> 
> Note that the current version of -31.ucode in linux-firmware.git
> (31.532993.0) does not have this capability bit set, so the LED won't
> work, even though this version should support it.  But we will update
> this firmware soon, so it won't be a problem anymore.
> 
> Fixes: 7089ae634c50 ("iwlwifi: mvm: use firmware LED command where applicable")
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

Patch applied to wireless-drivers.git, thanks.

2eabc84d2f8e iwlwifi: mvm: only send LEDS_CMD when the FW supports it

-- 
https://patchwork.kernel.org/patch/9941719/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* Re: [PATCH] iwlwifi: mvm: only send LEDS_CMD when the FW supports it
From: Kalle Valo @ 2017-09-07 16:36 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Luca Coelho, Linux Wireless List, Johannes Berg,
	Linux Kernel Mailing List, Andrew Morton, Network Development,
	David Miller, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <CA+55aFyeEwg7vd8FPHqG+4d7KbJ6ke4NDwhd+8y=_auRApyvmw@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, Sep 7, 2017 at 5:39 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Linus, do you want to apply this directly or should we take it via the
>> normal route (wireless-drivers -> net)? If your prefer the latter when
>> I'm planning to submit this to Dave in a day or two and expecting it to
>> get to your tree in about a week, depending of course what is Dave's
>> schedule.
>
> Since we have a workaround for the problem, let's just go through the
> regular channels. As long as I get the fix through David before the
> merge window closes, I'm happy.

Ok, I'll aim to send the pull request to Dave tomorrow.

-- 
Kalle Valo

^ permalink raw reply

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Henrik Austad @ 2017-09-07 16:18 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
	intel-wired-lan, andre.guedes, ivan.briano,
	jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170907155315.5gqy5e4susl25wa2@localhost>

[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]

On Thu, Sep 07, 2017 at 05:53:15PM +0200, Richard Cochran wrote:
> On Thu, Sep 07, 2017 at 05:27:51PM +0200, Henrik Austad wrote:
> > On Thu, Sep 07, 2017 at 02:40:18PM +0200, Richard Cochran wrote:
> > And if you want to this driver to act as a bridge, how do you accomodate 
> > change in network requirements? (i.e. how does this work with switchdev?)
> 
> To my understanding, this Qdisc idea provides QoS for the host's
> transmitted traffic, and nothing more.

Ok, then we're on the same page.

> > - Or am I overthinking this?
> 
> Being able to configure the external ports of a switchdev is probably
> a nice feature, but that is another story.  (But maybe I misunderstood
> the authors' intent!)

ok, chalk that one up for later perhaps

> > If you have more than 1 application in userspace that wants to send data 
> > using this scheduler, how do you ensure fair transmission of frames? (both 
> > how much bandwidth they use,
> 
> There are many ways to handle this, and we shouldn't put any of that
> policy into the kernel.  For example, there might be a monolithic
> application with configurable threads, or an allocation server that
> grants bandwidth to applications via IPC, or a multiplexing stream
> server like jack, pulse, etc, and so on...

true

> > but also ordering of frames from each application)
> 
> Not sure what you mean by this.

Fair enough, I'm not that good at making myself clear :)

Let's see if I can make a better attempt:

If you have 2 separate applications that have their own streams going to 
different endpoints - but both are in the same class, then they will 
share the qdisc bandwidth.

So application 
- A sends frame A1, A2, A3, .. An
- B sends B1, B2, .. Bn

What I was trying to describe was: if application A send 2 frames, and B 
sends 2 frames at the same time, then you would hope that the order would 
be A1, B1, A2, B2, and not A1, A2, B1, B2.

None of this would be a problem if you expect a *single* user, like the 
allocation server you described above. Again, I think this is just me 
overthinking the problem right now :)

> > Do you expect all of this to be handled in userspace?
> 
> Yes, I do.

ok, fair enough

Thanks for answering my questions!

-- 
Henrik Austad

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Bad escapes in ip -online
From: John Kodis @ 2017-09-07 16:08 UTC (permalink / raw)
  To: netdev

The -online option to the 'ip link', 'ip addr', and perhaps others is 
putting out a backslash in places where a newline character should go, 
as so:

1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode 
DEFAULT group default qlen 1000\    link/loopback 00:00:00:00:00:00 brd 
00:00:00:00:00:00

-- John.

^ permalink raw reply

* Re: [PATCH] iwlwifi: mvm: only send LEDS_CMD when the FW supports it
From: Linus Torvalds @ 2017-09-07 16:13 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Luca Coelho, Linux Wireless List, Johannes Berg,
	Linux Kernel Mailing List, Andrew Morton, Network Development,
	David Miller, Emmanuel Grumbach, Luca Coelho
In-Reply-To: <877exan0nq.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>

On Thu, Sep 7, 2017 at 5:39 AM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
> Linus, do you want to apply this directly or should we take it via the
> normal route (wireless-drivers -> net)? If your prefer the latter when
> I'm planning to submit this to Dave in a day or two and expecting it to
> get to your tree in about a week, depending of course what is Dave's
> schedule.

Since we have a workaround for the problem, let's just go through the
regular channels. As long as I get the fix through David before the
merge window closes, I'm happy.

              Linus

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox