2017-03-21 09:35:24

by Sean Wang

[permalink] [raw]
Subject: [PATCH net-next v2 0/5] net-next: dsa: add Mediatek MT7530 support

From: Sean Wang <[email protected]>

MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
Mediatek router platforms such as MT7623A or MT7623N which includes 7-port
Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY. Among these ports,
The port from 0 to 4 are the user ports connecting with the remote devices
while the port 5 and 6 are the CPU ports connecting into Mediatek Ethernet
GMAC.

The patch series integrated Mediatek MT7530 into DSA support which
includes the most of the essential callbacks such as tag insertion for
port distinguishing, port control, bridge offloading, STP setup and
ethtool operations to allow DSA to model each user port into independently
standalone netdevice as the other DSA driver had done.

Changes since v1:
- rebased into 4.11-rc1
- refined binding document including below five items
- changed the type of mediatek,mcm into bool
- used reset controller binding for MCM reset and removed "mediatek,ethsys"
property from binding
- reused CPU port's ethernet Phandle instead of creating new one and removed
"mediatek,ethernet" property from binding
- aligned naming for GPIO reset with dsa/marvell.txt
- added phy-mode as required property child nodes within ports container
- handled gpio reset with devm_gpiod_* API
- refined comment words
- removed condition for CDM setting since the setup looks both fine for all cases
- allowed of_find_net_device_by_node() working with pointing the device node into
real netdev instance
- fixed Kbuild warnings

Sean Wang (5):
dt-bindings: net: dsa: add Mediatek MT7530 binding
net-next: dsa: add Mediatek tag RX/TX handler
net-next: ethernet: mediatek: add CDM able to recognize the tag for
DSA
net-next: ethernet: mediatek: add device_node of GMAC pointing into
the netdev instance
net-next: dsa: add dsa support for Mediatek MT7530 switch

.../devicetree/bindings/net/dsa/mt7530.txt | 92 ++
drivers/net/dsa/Kconfig | 8 +
drivers/net/dsa/Makefile | 2 +-
drivers/net/dsa/mt7530.c | 1172 ++++++++++++++++++++
drivers/net/dsa/mt7530.h | 382 +++++++
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 8 +
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 +
include/net/dsa.h | 1 +
net/dsa/Kconfig | 2 +
net/dsa/Makefile | 1 +
net/dsa/dsa.c | 3 +
net/dsa/dsa_priv.h | 3 +
net/dsa/tag_mtk.c | 117 ++
13 files changed, 1794 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt
create mode 100644 drivers/net/dsa/mt7530.c
create mode 100644 drivers/net/dsa/mt7530.h
create mode 100644 net/dsa/tag_mtk.c

--
1.9.1


2017-03-21 09:35:32

by Sean Wang

[permalink] [raw]
Subject: [PATCH net-next v2 1/5] dt-bindings: net: dsa: add Mediatek MT7530 binding

From: Sean Wang <[email protected]>

Add device-tree binding for Mediatek MT7530 switch.

Cc: [email protected]
Signed-off-by: Sean Wang <[email protected]>
---
.../devicetree/bindings/net/dsa/mt7530.txt | 92 ++++++++++++++++++++++
1 file changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
new file mode 100644
index 0000000..a9bc27b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
@@ -0,0 +1,92 @@
+Mediatek MT7530 Ethernet switch
+================================
+
+Required properties:
+
+- compatible: Must be compatible = "mediatek,mt7530";
+- #address-cells: Must be 1.
+- #size-cells: Must be 0.
+- mediatek,mcm: Boolean; if defined, indicates that either MT7530 is the part
+ on multi-chip module belong to MT7623A has or the remotely standalone
+ chip as the function MT7623N reference board provided for.
+- core-supply: Phandle to the regulator node necessary for the core power.
+- io-supply: Phandle to the regulator node necessary for the I/O power.
+ See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
+ for details for the regulator setup on these boards.
+
+If the property mediatek,mcm isn't defined, following property is required
+
+- reset-gpios: Should be a gpio specifier for a reset line.
+
+Else, following properties are required
+
+- resets : Phandle pointing to the system reset controller with
+ line index for the ethsys.
+- reset-names : Should be set to "mcm".
+
+Required properties for the child nodes within ports container:
+
+- reg: Port address described must be 6 for CPU port and from 0 to 5 for
+ user ports.
+- phy-mode: String, must be either "trgmii" or "rgmii" for port labeled
+ "cpu".
+
+See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional
+required, optional properties and how the integrated switch subnodes must
+be specified.
+
+Example:
+
+ &mdio0 {
+ switch@0 {
+ compatible = "mediatek,mt7530";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ core-supply = <&mt6323_vpa_reg>;
+ io-supply = <&mt6323_vemc3v3_reg>;
+ reset-gpios = <&pio 33 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ port@0 {
+ reg = <0>;
+ label = "lan0";
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "lan1";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "lan2";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "lan3";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "wan";
+ };
+
+ port@6 {
+ reg = <6>;
+ label = "cpu";
+ ethernet = <&gmac0>;
+ phy-mode = "trgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+ };
+ };
+ };
--
1.9.1

2017-03-21 09:36:59

by Sean Wang

[permalink] [raw]
Subject: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

From: Sean Wang <[email protected]>

MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
Mediatek router platforms such as MT7623A or MT7623N platform which
includes 7-port Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY.
Among these ports, The port from 0 to 4 are the user ports connecting
with the remote devices while the port 5 and 6 are the CPU ports
connecting into Mediatek Ethernet GMAC.

For port 6, it can communicate with the CPU via Mediatek Ethernet GMAC
through either the TRGMII or RGMII which could be controlled by phy-mode
in the dt-bindings to specify which mode is preferred to use. And for
port 5, only RGMII can be specified. However, currently, only port 6 is
being supported in this DSA driver.

The driver is made with the reference to qca8k and other existing DSA
driver. The most of the essential callbacks of the DSA are already
support in the driver, including tag insert for user port distinguishing,
port control, bridge offloading, STP setup and ethtool operation to allow
DSA to model each user port into a standalone netdevice as the other DSA
driver had done.

Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Landen Chao <[email protected]>
---
drivers/net/dsa/Kconfig | 8 +
drivers/net/dsa/Makefile | 2 +-
drivers/net/dsa/mt7530.c | 1172 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mt7530.h | 382 +++++++++++++++
4 files changed, 1563 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/dsa/mt7530.c
create mode 100644 drivers/net/dsa/mt7530.h

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 0659846..5b322b4 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -34,4 +34,12 @@ config NET_DSA_QCA8K
This enables support for the Qualcomm Atheros QCA8K Ethernet
switch chips.

+config NET_DSA_MT7530
+ tristate "Mediatek MT7530 Ethernet switch support"
+ depends on NET_DSA
+ select NET_DSA_TAG_MTK
+ ---help---
+ This enables support for the Mediatek MT7530 Ethernet switch
+ chip.
+
endmenu
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index a3c9416..8e629c1 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -2,6 +2,6 @@ obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
obj-$(CONFIG_NET_DSA_BCM_SF2) += bcm-sf2.o
bcm-sf2-objs := bcm_sf2.o bcm_sf2_cfp.o
obj-$(CONFIG_NET_DSA_QCA8K) += qca8k.o
-
+obj-$(CONFIG_NET_DSA_MT7530) += mt7530.o
obj-y += b53/
obj-y += mv88e6xxx/
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
new file mode 100644
index 0000000..99e5485
--- /dev/null
+++ b/drivers/net/dsa/mt7530.c
@@ -0,0 +1,1172 @@
+/*
+ * Mediatek MT7530 DSA Switch driver
+ * Copyright (C) 2017 Sean Wang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/etherdevice.h>
+#include <linux/if_bridge.h>
+#include <linux/mdio.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of_gpio.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+#include <linux/of_platform.h>
+#include <linux/phy.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <net/dsa.h>
+#include <net/switchdev.h>
+
+#include "mt7530.h"
+
+/* String, offset, and register size in bytes if different from 4 bytes */
+static const struct mt7530_mib_desc mt7530_mib[] = {
+ MIB_DESC(1, 0x00, "TxDrop"),
+ MIB_DESC(1, 0x04, "TxCrcErr"),
+ MIB_DESC(1, 0x08, "TxUnicast"),
+ MIB_DESC(1, 0x0c, "TxMulticast"),
+ MIB_DESC(1, 0x10, "TxBroadcast"),
+ MIB_DESC(1, 0x14, "TxCollision"),
+ MIB_DESC(1, 0x18, "TxSingleCollision"),
+ MIB_DESC(1, 0x1c, "TxMultipleCollision"),
+ MIB_DESC(1, 0x20, "TxDeferred"),
+ MIB_DESC(1, 0x24, "TxLateCollision"),
+ MIB_DESC(1, 0x28, "TxExcessiveCollistion"),
+ MIB_DESC(1, 0x2c, "TxPause"),
+ MIB_DESC(1, 0x30, "TxPktSz64"),
+ MIB_DESC(1, 0x34, "TxPktSz65To127"),
+ MIB_DESC(1, 0x38, "TxPktSz128To255"),
+ MIB_DESC(1, 0x3c, "TxPktSz256To511"),
+ MIB_DESC(1, 0x40, "TxPktSz512To1023"),
+ MIB_DESC(1, 0x44, "Tx1024ToMax"),
+ MIB_DESC(2, 0x48, "TxBytes"),
+ MIB_DESC(1, 0x60, "RxDrop"),
+ MIB_DESC(1, 0x64, "RxFiltering"),
+ MIB_DESC(1, 0x6c, "RxMulticast"),
+ MIB_DESC(1, 0x70, "RxBroadcast"),
+ MIB_DESC(1, 0x74, "RxAlignErr"),
+ MIB_DESC(1, 0x78, "RxCrcErr"),
+ MIB_DESC(1, 0x7c, "RxUnderSizeErr"),
+ MIB_DESC(1, 0x80, "RxFragErr"),
+ MIB_DESC(1, 0x84, "RxOverSzErr"),
+ MIB_DESC(1, 0x88, "RxJabberErr"),
+ MIB_DESC(1, 0x8c, "RxPause"),
+ MIB_DESC(1, 0x90, "RxPktSz64"),
+ MIB_DESC(1, 0x94, "RxPktSz65To127"),
+ MIB_DESC(1, 0x98, "RxPktSz128To255"),
+ MIB_DESC(1, 0x9c, "RxPktSz256To511"),
+ MIB_DESC(1, 0xa0, "RxPktSz512To1023"),
+ MIB_DESC(1, 0xa4, "RxPktSz1024ToMax"),
+ MIB_DESC(2, 0xa8, "RxBytes"),
+ MIB_DESC(1, 0xb0, "RxCtrlDrop"),
+ MIB_DESC(1, 0xb4, "RxIngressDrop"),
+ MIB_DESC(1, 0xb8, "RxArlDrop"),
+};
+
+static int
+mt7623_trgmii_write(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ int ret;
+
+ ret = regmap_write(priv->ethernet, TRGMII_BASE(reg), val);
+ if (ret < 0)
+ dev_err(priv->dev,
+ "failed to priv write register\n");
+ return ret;
+}
+
+static u32
+mt7623_trgmii_read(struct mt7530_priv *priv, u32 reg)
+{
+ int ret;
+ u32 val;
+
+ ret = regmap_read(priv->ethernet, TRGMII_BASE(reg), &val);
+ if (ret < 0) {
+ dev_err(priv->dev,
+ "failed to priv read register\n");
+ return ret;
+ }
+
+ return val;
+}
+
+static void
+mt7623_trgmii_rmw(struct mt7530_priv *priv, u32 reg,
+ u32 mask, u32 set)
+{
+ u32 val;
+
+ val = mt7623_trgmii_read(priv, reg);
+ val &= ~mask;
+ val |= set;
+ mt7623_trgmii_write(priv, reg, val);
+}
+
+static void
+mt7623_trgmii_set(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ mt7623_trgmii_rmw(priv, reg, 0, val);
+}
+
+static void
+mt7623_trgmii_clear(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ mt7623_trgmii_rmw(priv, reg, val, 0);
+}
+
+static int
+core_read_mmd_indirect(struct mt7530_priv *priv, int prtad, int devad)
+{
+ struct mii_bus *bus = priv->bus;
+ int value, ret;
+
+ /* Write the desired MMD Devad */
+ ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
+ if (ret < 0)
+ goto err;
+
+ /* Write the desired MMD register address */
+ ret = bus->write(bus, 0, MII_MMD_DATA, prtad);
+ if (ret < 0)
+ goto err;
+
+ /* Select the Function : DATA with no post increment */
+ ret = bus->write(bus, 0, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+ if (ret < 0)
+ goto err;
+
+ /* Read the content of the MMD's selected register */
+ value = bus->read(bus, 0, MII_MMD_DATA);
+
+ return value;
+err:
+ dev_err(&bus->dev, "failed to read mmd register\n");
+
+ return ret;
+}
+
+static int
+core_write_mmd_indirect(struct mt7530_priv *priv, int prtad,
+ int devad, u32 data)
+{
+ struct mii_bus *bus = priv->bus;
+ int ret;
+
+ /* Write the desired MMD Devad */
+ ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
+ if (ret < 0)
+ goto err;
+
+ /* Write the desired MMD register address */
+ ret = bus->write(bus, 0, MII_MMD_DATA, prtad);
+ if (ret < 0)
+ goto err;
+
+ /* Select the Function : DATA with no post increment */
+ ret = bus->write(bus, 0, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+ if (ret < 0)
+ goto err;
+
+ /* Write the data into MMD's selected register */
+ ret = bus->write(bus, 0, MII_MMD_DATA, data);
+err:
+ if (ret < 0)
+ dev_err(&bus->dev,
+ "failed to write mmd register\n");
+ return ret;
+}
+
+static void
+core_write(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ struct mii_bus *bus = priv->bus;
+
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+ core_write_mmd_indirect(priv, reg, MDIO_MMD_VEND2, val);
+
+ mutex_unlock(&bus->mdio_lock);
+}
+
+static void
+core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
+{
+ struct mii_bus *bus = priv->bus;
+ u32 val;
+
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+ val = core_read_mmd_indirect(priv, reg, MDIO_MMD_VEND2);
+ val &= ~mask;
+ val |= set;
+ core_write_mmd_indirect(priv, reg, MDIO_MMD_VEND2, val);
+
+ mutex_unlock(&bus->mdio_lock);
+}
+
+static void
+core_set(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ core_rmw(priv, reg, 0, val);
+}
+
+static void
+core_clear(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ core_rmw(priv, reg, val, 0);
+}
+
+static int
+mt7530_mii_write(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ struct mii_bus *bus = priv->bus;
+ u16 page, r, lo, hi;
+ int ret;
+
+ page = (reg >> 6) & 0x3ff;
+ r = (reg >> 2) & 0xf;
+ lo = val & 0xffff;
+ hi = val >> 16;
+
+ /* MT7530 uses 31 as the pseudo port */
+ ret = bus->write(bus, 0x1f, 0x1f, page);
+ if (ret < 0)
+ goto err;
+
+ ret = bus->write(bus, 0x1f, r, lo);
+ if (ret < 0)
+ goto err;
+
+ ret = bus->write(bus, 0x1f, 0x10, hi);
+err:
+ if (ret < 0)
+ dev_err(&bus->dev,
+ "failed to write mt7530 register\n");
+ return ret;
+}
+
+static u32
+mt7530_mii_read(struct mt7530_priv *priv, u32 reg)
+{
+ struct mii_bus *bus = priv->bus;
+ u16 page, r, lo, hi;
+ int ret;
+
+ page = (reg >> 6) & 0x3ff;
+ r = (reg >> 2) & 0xf;
+
+ /* MT7530 uses 31 as the pseudo port */
+ ret = bus->write(bus, 0x1f, 0x1f, page);
+ if (ret < 0) {
+ dev_err(&bus->dev,
+ "failed to read mt7530 register\n");
+ return ret;
+ }
+
+ lo = bus->read(bus, 0x1f, r);
+ hi = bus->read(bus, 0x1f, 0x10);
+
+ return (hi << 16) | (lo & 0xffff);
+}
+
+static void
+mt7530_write(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ struct mii_bus *bus = priv->bus;
+
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+ mt7530_mii_write(priv, reg, val);
+
+ mutex_unlock(&bus->mdio_lock);
+}
+
+static u32
+mt7530_read(struct mt7530_priv *priv, u32 reg)
+{
+ struct mii_bus *bus = priv->bus;
+ u32 val;
+
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+ val = mt7530_mii_read(priv, reg);
+
+ mutex_unlock(&bus->mdio_lock);
+
+ return val;
+}
+
+static void
+mt7530_rmw(struct mt7530_priv *priv, u32 reg,
+ u32 mask, u32 set)
+{
+ struct mii_bus *bus = priv->bus;
+ u32 val;
+
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+
+ val = mt7530_mii_read(priv, reg);
+ val &= ~mask;
+ val |= set;
+ mt7530_mii_write(priv, reg, val);
+
+ mutex_unlock(&bus->mdio_lock);
+}
+
+static void
+mt7530_set(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ mt7530_rmw(priv, reg, 0, val);
+}
+
+static void
+mt7530_clear(struct mt7530_priv *priv, u32 reg, u32 val)
+{
+ mt7530_rmw(priv, reg, val, 0);
+}
+
+static int
+mt7530_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
+{
+ struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
+
+ /* BIT(15) is used as indication for pseudo registers
+ * which would be translated into the general MDIO
+ * access to leverage the unique regmap sys interface.
+ */
+ if (reg & BIT(15))
+ *val = mdiobus_read_nested(priv->bus,
+ (reg & 0xf00) >> 8,
+ (reg & 0xff) >> 2);
+ else
+ *val = mt7530_read(priv, reg);
+
+ return 0;
+}
+
+static int
+mt7530_regmap_write(void *ctx, uint32_t reg, uint32_t val)
+{
+ struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
+
+ if (reg & BIT(15))
+ mdiobus_write_nested(priv->bus,
+ (reg & 0xf00) >> 8,
+ (reg & 0xff) >> 2, val);
+ else
+ mt7530_write(priv, reg, val);
+
+ return 0;
+}
+
+static const struct regmap_range mt7530_readable_ranges[] = {
+ regmap_reg_range(0x0000, 0x00ac), /* Global control */
+ regmap_reg_range(0x2000, 0x202c), /* Port Control - P0 */
+ regmap_reg_range(0x2100, 0x212c), /* Port Control - P1 */
+ regmap_reg_range(0x2200, 0x222c), /* Port Control - P2 */
+ regmap_reg_range(0x2300, 0x232c), /* Port Control - P3 */
+ regmap_reg_range(0x2400, 0x242c), /* Port Control - P4 */
+ regmap_reg_range(0x2500, 0x252c), /* Port Control - P5 */
+ regmap_reg_range(0x2600, 0x262c), /* Port Control - P6 */
+ regmap_reg_range(0x30e0, 0x30f8), /* Port MAC - SYS */
+ regmap_reg_range(0x3000, 0x3014), /* Port MAC - P0 */
+ regmap_reg_range(0x3100, 0x3114), /* Port MAC - P1 */
+ regmap_reg_range(0x3200, 0x3214), /* Port MAC - P2*/
+ regmap_reg_range(0x3300, 0x3314), /* Port MAC - P3*/
+ regmap_reg_range(0x3400, 0x3414), /* Port MAC - P4 */
+ regmap_reg_range(0x3500, 0x3514), /* Port MAC - P5 */
+ regmap_reg_range(0x3600, 0x3614), /* Port MAC - P6 */
+ regmap_reg_range(0x4000, 0x40d4), /* MIB - P0 */
+ regmap_reg_range(0x4100, 0x41d4), /* MIB - P1 */
+ regmap_reg_range(0x4200, 0x42d4), /* MIB - P2 */
+ regmap_reg_range(0x4300, 0x43d4), /* MIB - P3 */
+ regmap_reg_range(0x4400, 0x44d4), /* MIB - P4 */
+ regmap_reg_range(0x4500, 0x45d4), /* MIB - P5 */
+ regmap_reg_range(0x4600, 0x46d4), /* MIB - P6 */
+ regmap_reg_range(0x4fe0, 0x4ff4), /* SYS */
+ regmap_reg_range(0x7000, 0x700c), /* SYS 2 */
+ regmap_reg_range(0x7018, 0x7028), /* SYS 3 */
+ regmap_reg_range(0x7800, 0x7830), /* SYS 4 */
+ regmap_reg_range(0x7a00, 0x7a7c), /* TRGMII */
+ regmap_reg_range(0x8000, 0x8078), /* Psedo address for Phy - P0 */
+ regmap_reg_range(0x8100, 0x8178), /* Psedo address for Phy - P1 */
+ regmap_reg_range(0x8200, 0x8278), /* Psedo address for Phy - P2 */
+ regmap_reg_range(0x8300, 0x8378), /* Psedo address for Phy - P3 */
+ regmap_reg_range(0x8400, 0x8478), /* Psedo address for Phy - P4 */
+};
+
+static const struct regmap_access_table mt7530_readable_table = {
+ .yes_ranges = mt7530_readable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(mt7530_readable_ranges),
+};
+
+static struct regmap_config mt7530_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = 0x8478,
+ .reg_read = mt7530_regmap_read,
+ .reg_write = mt7530_regmap_write,
+ .rd_table = &mt7530_readable_table,
+};
+
+static int
+mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
+{
+ u32 reg;
+ int ret;
+
+ /* Set the command operating upon the MAC address entries */
+ reg = ATC_BUSY | ATC_MAT(0) | cmd;
+ mt7530_write(priv, MT7530_ATC, reg);
+
+ /* Wait for completion */
+ ret = wait_condition_timeout(
+ !(mt7530_read(priv, MT7530_ATC) & ATC_BUSY), 20);
+ if (ret < 0) {
+ dev_err(priv->dev, "cmd = %x timeout\n", cmd);
+ return -EIO;
+ }
+
+ /* Additional sanity for read command if the specified
+ * entry is invalid
+ */
+ reg = mt7530_read(priv, MT7530_ATC);
+ if ((cmd == MT7530_FDB_READ) && (reg & ATC_INVALID))
+ return -EINVAL;
+
+ if (rsp)
+ *rsp = reg;
+
+ return 0;
+}
+
+static void
+mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
+{
+ u32 reg[3];
+ int i;
+
+ /* Read from ARL table into an array */
+ for (i = 0; i < 3; i++) {
+ reg[i] = mt7530_read(priv, MT7530_TSRA1 + (i * 4));
+
+ dev_dbg(priv->dev, "%s(%d) reg[%d]=0x%x\n",
+ __func__, __LINE__, i, reg[i]);
+ }
+
+ /* vid - 11:0 on reg[1] */
+ fdb->vid = (reg[1] >> 0) & 0xfff;
+ /* aging - 31:24 on reg[2] */
+ fdb->aging = (reg[2] >> 24) & 0xff;
+ /* portmask - 11:4 on reg[2] */
+ fdb->port_mask = (reg[2] >> 4) & 0xff;
+ /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
+ fdb->mac[0] = (reg[0] >> 24) & 0xff;
+ fdb->mac[1] = (reg[0] >> 16) & 0xff;
+ fdb->mac[2] = (reg[0] >> 8) & 0xff;
+ fdb->mac[3] = (reg[0] >> 0) & 0xff;
+ fdb->mac[4] = (reg[1] >> 24) & 0xff;
+ fdb->mac[5] = (reg[1] >> 16) & 0xff;
+ /* noarp - 3:2 on reg[2] */
+ fdb->noarp = ((reg[2] >> 2) & 0x3) == STATIC_ENT;
+}
+
+static void
+mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
+ u8 port_mask, const u8 *mac,
+ u8 aging, u8 type)
+{
+ u32 reg[3] = { 0 };
+ int i;
+
+ /* vid - 11:0 on reg[1] */
+ reg[1] |= (vid & 0xfff) << 0;
+ /* aging - 31:25 on reg[2] */
+ reg[2] |= (aging & 0xff) << 24;
+ /* portmask - 11:4 on reg[2] */
+ reg[2] |= (port_mask & 0xff) << 4;
+ /* type - 3 indicate that entry is static wouldn't
+ * be aged out and 0 specified as erasing an entry
+ */
+ reg[2] |= (type & 0x3) << 2;
+ /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
+ reg[1] |= mac[5] << 16;
+ reg[1] |= mac[4] << 24;
+ reg[0] |= mac[3] << 0;
+ reg[0] |= mac[2] << 8;
+ reg[0] |= mac[1] << 16;
+ reg[0] |= mac[0] << 24;
+
+ /* Wrirte array into the ARL table */
+ for (i = 0; i < 3; i++)
+ mt7530_write(priv, MT7530_ATA1 + (i * 4), reg[i]);
+}
+
+static int
+mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
+{
+ struct mt7530_priv *priv = ds->priv;
+ u32 ncpo1, ssc_delta, trgint, i;
+
+ switch (mode) {
+ case PHY_INTERFACE_MODE_RGMII:
+ trgint = 0;
+ ncpo1 = 0x0c80;
+ ssc_delta = 0x87;
+ break;
+ case PHY_INTERFACE_MODE_TRGMII:
+ trgint = 1;
+ ncpo1 = 0x1400;
+ ssc_delta = 0x57;
+ break;
+ default:
+ pr_err("xMII mode %d not supported\n", mode);
+ return -EINVAL;
+ }
+
+ mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
+ P6_INTF_MODE(trgint));
+
+ /* Lower Tx Driving */
+ for (i = 0 ; i < 6 ; i++)
+ mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
+ TD_DM_DRVP(8) | TD_DM_DRVN(8));
+
+ /* Setup MT7530 core clock */
+ if (!trgint) {
+ /* Disable MT7530 core clock */
+ core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
+
+ /* Disable MT7530 PLL, since phy_device has not yet been
+ * created when this function is called. So we provide
+ * core_write_mmd_indirect to complete this function
+ */
+ core_write_mmd_indirect(priv,
+ CORE_GSWPLL_GRP1,
+ MDIO_MMD_VEND2,
+ 0);
+
+ /* Setup MT7530 core clock into 500Mhz */
+ core_write(priv, CORE_GSWPLL_GRP2,
+ RG_GSWPLL_POSDIV_500M(1) |
+ RG_GSWPLL_FBKDIV_500M(25));
+
+ /* Enable MT7530 PLL */
+ core_write(priv, CORE_GSWPLL_GRP1,
+ RG_GSWPLL_EN_PRE |
+ RG_GSWPLL_POSDIV_200M(2) |
+ RG_GSWPLL_FBKDIV_200M(32));
+
+ /* Enable MT7530 core clock */
+ core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
+ }
+
+ /* Setup the MT7530 TRGMII Tx Clock */
+ core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
+ core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
+ core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
+ core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
+ core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta));
+ core_write(priv, CORE_PLL_GROUP4,
+ RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN |
+ RG_SYSPLL_BIAS_LPF_EN);
+ core_write(priv, CORE_PLL_GROUP2,
+ RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN |
+ RG_SYSPLL_POSDIV(1));
+ core_write(priv, CORE_PLL_GROUP7,
+ RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
+ RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
+ core_set(priv, CORE_TRGMII_GSW_CLK_CG,
+ REG_GSWCK_EN | REG_TRGMIICK_EN);
+
+ if (!trgint)
+ for (i = 0 ; i < 5 ; i++)
+ mt7530_rmw(priv, MT7530_TRGMII_RD(i),
+ RD_TAP_MASK, RD_TAP(16));
+ else
+ mt7623_trgmii_set(priv, GSW_INTF_MODE, INTF_MODE_TRGMII);
+
+ return 0;
+}
+
+static int
+mt7623_pad_clk_setup(struct dsa_switch *ds)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int i;
+
+ for (i = 0 ; i < 6; i++)
+ mt7623_trgmii_write(priv, GSW_TRGMII_TD_ODT(i),
+ TD_DM_DRVP(8) | TD_DM_DRVN(8));
+
+ mt7623_trgmii_set(priv, GSW_TRGMII_RCK_CTRL, RX_RST | RXC_DQSISEL);
+ mt7623_trgmii_clear(priv, GSW_TRGMII_RCK_CTRL, RX_RST);
+
+ return 0;
+}
+
+static void
+mt7530_mib_reset(struct dsa_switch *ds)
+{
+ struct mt7530_priv *priv = ds->priv;
+
+ mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_FLUSH);
+ mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
+}
+
+static void
+mt7530_port_set_status(struct dsa_switch *ds, int port, int enable)
+{
+ struct mt7530_priv *priv = ds->priv;
+ u32 mask = PMCR_TX_EN | PMCR_RX_EN;
+
+ mutex_lock(&priv->reg_mutex);
+ if (enable)
+ mt7530_set(priv, MT7530_PMCR_P(port), mask);
+ else
+ mt7530_clear(priv, MT7530_PMCR_P(port), mask);
+ mutex_unlock(&priv->reg_mutex);
+}
+
+static int
+mt7530_setup(struct dsa_switch *ds)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int ret, i, phy_mode;
+ u8 cpup_mask = 0;
+ u32 id, val;
+ struct regmap *regmap;
+ struct device_node *dn;
+
+ /* Make sure that cpu port specfied on the dt is appropriate */
+ if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
+ dev_err(priv->dev, "port not matched with the CPU port\n");
+ return -EINVAL;
+ }
+
+ /* The parent node of master_netdev which holds the common system
+ * controller also is the container for two GMACs nodes representing
+ * as two netdev instances.
+ */
+ dn = ds->master_netdev->dev.of_node->parent;
+ priv->ethernet = syscon_node_to_regmap(dn);
+ if (IS_ERR(priv->ethernet))
+ return PTR_ERR(priv->ethernet);
+
+ regmap = devm_regmap_init(ds->dev, NULL, priv,
+ &mt7530_regmap_config);
+ if (IS_ERR(regmap))
+ dev_warn(priv->dev, "phy regmap initialization failed");
+
+ phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
+ if (phy_mode < 0) {
+ dev_err(priv->dev, "Can't find phy-mode for master device\n");
+ return phy_mode;
+ }
+ dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
+
+ regulator_set_voltage(priv->core_pwr, 1000000, 1000000);
+ ret = regulator_enable(priv->core_pwr);
+ if (ret < 0) {
+ dev_err(priv->dev,
+ "Failed to enable core power: %d\n", ret);
+ return ret;
+ }
+
+ regulator_set_voltage(priv->io_pwr, 3300000, 3300000);
+ ret = regulator_enable(priv->io_pwr);
+ if (ret < 0) {
+ dev_err(priv->dev, "Failed to enable io pwr: %d\n",
+ ret);
+ return ret;
+ }
+
+ /* Reset whole chip through gpio pin or memory-mapped registers for
+ * different type of hardware
+ */
+ if (priv->mcm) {
+ reset_control_assert(priv->rstc);
+ usleep_range(1000, 1100);
+ reset_control_deassert(priv->rstc);
+ } else {
+ gpiod_set_value_cansleep(priv->reset, 0);
+ usleep_range(1000, 1100);
+ gpiod_set_value_cansleep(priv->reset, 1);
+ }
+
+ /* Wait until the reset completion */
+ ret = wait_condition_timeout(mt7530_read(priv, MT7530_HWTRAP) != 0,
+ 1000);
+ if (ret < 0) {
+ dev_err(priv->dev, "reset timeout\n");
+ return ret;
+ }
+
+ id = mt7530_read(priv, MT7530_CREV);
+ id >>= CHIP_NAME_SHIFT;
+ if (id != MT7530_ID)
+ return -ENODEV;
+
+ /* Reset the switch through internal reset */
+ mt7530_write(priv, MT7530_SYS_CTRL,
+ SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
+ SYS_CTRL_REG_RST);
+
+ /* Enable Port 6 only, P5 as GMAC5 which currently is not supported */
+ val = mt7530_read(priv, MT7530_MHWTRAP);
+ val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
+ val |= MHWTRAP_MANUAL;
+ mt7530_write(priv, MT7530_MHWTRAP, val);
+
+ ret = mt7530_pad_clk_setup(ds, phy_mode);
+ if (ret < 0)
+ return ret;
+
+ /* Enable and reset MIB counters */
+ mt7530_mib_reset(ds);
+
+ /* Disable forwarding by default on all ports */
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ mt7530_write(priv, MT7530_PCR_P(i), PCR_MATRIX_INIT);
+
+ mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK);
+
+ /* Fabric setup for the cpu port */
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ if (dsa_is_cpu_port(ds, i)) {
+ /* Enable Mediatek header mode on the cpu port */
+ mt7530_write(priv, MT7530_PVC_P(i),
+ PORT_SPEC_TAG);
+
+ /* Setup the MAC by default for the cpu port */
+ mt7530_write(priv, MT7530_PMCR_P(i), PMCR_CPUP_LINK);
+
+ /* Disable auto learning on the cpu port */
+ mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
+
+ /* Unknown unicast frame fordwarding to the cpu port */
+ mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(i)));
+
+ /* CPU port gets connected to all user ports of
+ * the switch
+ */
+ mt7530_write(priv, MT7530_PCR_P(i),
+ PCR_MATRIX(ds->enabled_port_mask));
+
+ cpup_mask |= BIT(i);
+ }
+
+ /* Fabric setup for the all user ports */
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ if (ds->enabled_port_mask & BIT(i)) {
+ /* Setup the MAC by default for all user ports */
+ mt7530_write(priv, MT7530_PMCR_P(i),
+ PMCR_USERP_LINK);
+
+ /* The user port gets connected to the cpu port only */
+ mt7530_write(priv, MT7530_PCR_P(i),
+ PCR_MATRIX(cpup_mask));
+ }
+
+ /* Flush the FDB table */
+ ret = mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, 0);
+ if (ret < 0)
+ return ret;
+
+ /* Setup RX circuit, relevant PAD and driving on the host which must
+ * be placed after the setup on the device side is all finished.
+ */
+ ret = mt7623_pad_clk_setup(ds);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int mt7530_phy_read(struct dsa_switch *ds, int port, int regnum)
+{
+ struct mt7530_priv *priv = ds->priv;
+
+ return mdiobus_read_nested(priv->bus, port, regnum);
+}
+
+int mt7530_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
+{
+ struct mt7530_priv *priv = ds->priv;
+
+ return mdiobus_write_nested(priv->bus, port, regnum, val);
+}
+
+static void
+mt7530_get_strings(struct dsa_switch *ds, int port, uint8_t *data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(mt7530_mib); i++)
+ strncpy(data + i * ETH_GSTRING_LEN, mt7530_mib[i].name,
+ ETH_GSTRING_LEN);
+}
+
+static void
+mt7530_get_ethtool_stats(struct dsa_switch *ds, int port,
+ uint64_t *data)
+{
+ struct mt7530_priv *priv = ds->priv;
+ const struct mt7530_mib_desc *mib;
+ u32 reg, i;
+ u64 hi;
+
+ for (i = 0; i < ARRAY_SIZE(mt7530_mib); i++) {
+ mib = &mt7530_mib[i];
+ reg = MT7530_PORT_MIB_COUNTER(port) + mib->offset;
+
+ data[i] = mt7530_read(priv, reg);
+ if (mib->size == 2) {
+ hi = mt7530_read(priv, reg + 4);
+ data[i] |= hi << 32;
+ }
+ }
+}
+
+static int
+mt7530_get_sset_count(struct dsa_switch *ds)
+{
+ return ARRAY_SIZE(mt7530_mib);
+}
+
+static int
+mt7530_port_enable(struct dsa_switch *ds, int port,
+ struct phy_device *phy)
+{
+ mt7530_port_set_status(ds, port, 1);
+
+ return 0;
+}
+
+static void
+mt7530_port_disable(struct dsa_switch *ds, int port,
+ struct phy_device *phy)
+{
+ mt7530_port_set_status(ds, port, 0);
+}
+
+static void
+mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
+{
+ struct mt7530_priv *priv = ds->priv;
+ u32 stp_state;
+
+ switch (state) {
+ case BR_STATE_DISABLED:
+ stp_state = MT7530_STP_DISABLED;
+ break;
+ case BR_STATE_BLOCKING:
+ stp_state = MT7530_STP_BLOCKING;
+ break;
+ case BR_STATE_LISTENING:
+ stp_state = MT7530_STP_LISTENING;
+ break;
+ case BR_STATE_LEARNING:
+ stp_state = MT7530_STP_LEARNING;
+ break;
+ case BR_STATE_FORWARDING:
+ default:
+ stp_state = MT7530_STP_FORWARDING;
+ break;
+ }
+
+ mt7530_rmw(priv, MT7530_SSP_P(port), FID_PST_MASK, stp_state);
+}
+
+static int
+mt7530_port_bridge_join(struct dsa_switch *ds, int port,
+ struct net_device *bridge)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int port_bitmap = BIT(MT7530_CPU_PORT);
+ int i;
+
+ mutex_lock(&priv->reg_mutex);
+
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ if (ds->enabled_port_mask & BIT(i)) {
+ if (ds->ports[i].bridge_dev != bridge)
+ continue;
+ /* Add this port to the port maxtrix of the
+ * other ports in the bridge
+ */
+ mt7530_set(priv, MT7530_PCR_P(i),
+ PCR_MATRIX(BIT(port)));
+
+ if (i != port)
+ port_bitmap |= BIT(i);
+ }
+
+ /* Add all other ports to this port matrix */
+ mt7530_rmw(priv, MT7530_PCR_P(port),
+ PCR_MATRIX_MASK, PCR_MATRIX(port_bitmap));
+
+ mutex_unlock(&priv->reg_mutex);
+
+ return 0;
+}
+
+static void
+mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
+ struct net_device *bridge)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int i;
+
+ mutex_lock(&priv->reg_mutex);
+
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ if (ds->enabled_port_mask & BIT(i)) {
+ if (ds->ports[i].bridge_dev != bridge)
+ continue;
+ /* Remove this port from the port maxtrix
+ * of the other ports in the bridge.
+ */
+ mt7530_clear(priv, MT7530_PCR_P(i),
+ PCR_MATRIX(BIT(port)));
+ }
+
+ /* Set the cpu port to be the only one in the port matrix of
+ * this port.
+ */
+ mt7530_rmw(priv, MT7530_PCR_P(port),
+ PCR_MATRIX_MASK,
+ PCR_MATRIX(BIT(MT7530_CPU_PORT)));
+
+ mutex_unlock(&priv->reg_mutex);
+}
+
+static int
+mt7530_port_fdb_prepare(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_fdb *fdb,
+ struct switchdev_trans *trans)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int ret;
+
+ /* Because auto-learned entries shares the same FDB table.
+ * an entry is reserved with no port_mask to make sure fdb_add
+ * is called while the entry is still available.
+ */
+ mutex_lock(&priv->reg_mutex);
+ mt7530_fdb_write(priv, fdb->vid, 0, fdb->addr, -1, STATIC_ENT);
+ ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, 0);
+ mutex_unlock(&priv->reg_mutex);
+
+ return ret;
+}
+
+static void
+mt7530_port_fdb_add(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_fdb *fdb,
+ struct switchdev_trans *trans)
+{
+ struct mt7530_priv *priv = ds->priv;
+ u8 port_mask = BIT(port);
+
+ mutex_lock(&priv->reg_mutex);
+ mt7530_fdb_write(priv, fdb->vid, port_mask, fdb->addr, -1, STATIC_ENT);
+ mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, 0);
+ mutex_unlock(&priv->reg_mutex);
+}
+
+static int
+mt7530_port_fdb_del(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_fdb *fdb)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int ret;
+ u8 port_mask = BIT(port);
+
+ mutex_lock(&priv->reg_mutex);
+ mt7530_fdb_write(priv, fdb->vid, port_mask, fdb->addr, -1, STATIC_EMP);
+ ret = mt7530_fdb_cmd(priv, MT7530_FDB_WRITE, 0);
+ mutex_unlock(&priv->reg_mutex);
+
+ return ret;
+}
+
+static int
+mt7530_port_fdb_dump(struct dsa_switch *ds, int port,
+ struct switchdev_obj_port_fdb *fdb,
+ int (*cb)(struct switchdev_obj *obj))
+{
+ struct mt7530_priv *priv = ds->priv;
+ struct mt7530_fdb _fdb = { 0 };
+ int cnt = MT7530_NUM_FDB_RECORDS;
+ int ret = 0;
+ u32 rsp = 0;
+
+ mutex_lock(&priv->reg_mutex);
+
+ ret = mt7530_fdb_cmd(priv, MT7530_FDB_START, &rsp);
+ if (ret < 0)
+ goto err;
+
+ do {
+ if (rsp & ATC_SRCH_HIT) {
+ mt7530_fdb_read(priv, &_fdb);
+ if (_fdb.port_mask & BIT(port)) {
+ ether_addr_copy(fdb->addr, _fdb.mac);
+ fdb->vid = _fdb.vid;
+ fdb->ndm_state = _fdb.noarp ?
+ NUD_NOARP : NUD_REACHABLE;
+ ret = cb(&fdb->obj);
+ if (ret < 0)
+ break;
+ }
+ }
+ } while (--cnt &&
+ !(rsp & ATC_SRCH_END) &&
+ !mt7530_fdb_cmd(priv, MT7530_FDB_NEXT, &rsp));
+err:
+ mutex_unlock(&priv->reg_mutex);
+
+ return 0;
+}
+
+static enum dsa_tag_protocol
+mtk_get_tag_protocol(struct dsa_switch *ds)
+{
+ return DSA_TAG_PROTO_MTK;
+}
+
+static struct dsa_switch_ops mt7530_switch_ops = {
+ .get_tag_protocol = mtk_get_tag_protocol,
+ .setup = mt7530_setup,
+ .get_strings = mt7530_get_strings,
+ .phy_read = mt7530_phy_read,
+ .phy_write = mt7530_phy_write,
+ .get_ethtool_stats = mt7530_get_ethtool_stats,
+ .get_sset_count = mt7530_get_sset_count,
+ .port_enable = mt7530_port_enable,
+ .port_disable = mt7530_port_disable,
+ .port_stp_state_set = mt7530_stp_state_set,
+ .port_bridge_join = mt7530_port_bridge_join,
+ .port_bridge_leave = mt7530_port_bridge_leave,
+ .port_fdb_prepare = mt7530_port_fdb_prepare,
+ .port_fdb_add = mt7530_port_fdb_add,
+ .port_fdb_del = mt7530_port_fdb_del,
+ .port_fdb_dump = mt7530_port_fdb_dump,
+};
+
+static int
+mt7530_probe(struct mdio_device *mdiodev)
+{
+ struct mt7530_priv *priv;
+ struct device_node *dn;
+
+ dn = mdiodev->dev.of_node;
+
+ priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
+ if (!priv->ds)
+ return -ENOMEM;
+
+ /* Use medatek,mcm property to distinguish hardware type that would
+ * casues a little bit differences on power-on sequence.
+ */
+ priv->mcm = of_property_read_bool(dn, "mediatek,mcm");
+ if (priv->mcm) {
+ dev_info(&mdiodev->dev, "MT7530 adapts as multi-chip module\n");
+
+ priv->rstc = devm_reset_control_get(&mdiodev->dev, "mcm");
+ if (IS_ERR(priv->rstc)) {
+ dev_err(&mdiodev->dev, "Couldn't get our reset line\n");
+ return PTR_ERR(priv->rstc);
+ }
+ }
+
+ priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core");
+ if (IS_ERR(priv->core_pwr))
+ return PTR_ERR(priv->core_pwr);
+
+ priv->io_pwr = devm_regulator_get(&mdiodev->dev, "io");
+ if (IS_ERR(priv->io_pwr))
+ return PTR_ERR(priv->io_pwr);
+
+ /* Not MCM that indicates switch works as the remote standalone
+ * integrated circuit so the GPIO pin would be used to complete
+ * the reset, otherwise memory-mapped register accessing used
+ * through syscon provides in the case of MCM.
+ */
+ if (!priv->mcm) {
+ priv->reset = devm_gpiod_get_optional(&mdiodev->dev, "reset",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(priv->reset)) {
+ dev_err(&mdiodev->dev, "Couldn't get our reset line\n");
+ return PTR_ERR(priv->reset);
+ }
+ }
+
+ priv->bus = mdiodev->bus;
+ priv->dev = &mdiodev->dev;
+ priv->ds->priv = priv;
+ priv->ds->ops = &mt7530_switch_ops;
+ mutex_init(&priv->reg_mutex);
+ dev_set_drvdata(&mdiodev->dev, priv);
+
+ return dsa_register_switch(priv->ds, &mdiodev->dev);
+}
+
+static void
+mt7530_remove(struct mdio_device *mdiodev)
+{
+ struct mt7530_priv *priv = dev_get_drvdata(&mdiodev->dev);
+ int ret = 0;
+
+ ret = regulator_disable(priv->core_pwr);
+ if (ret < 0)
+ dev_err(priv->dev,
+ "Failed to disable core power: %d\n", ret);
+
+ ret = regulator_disable(priv->io_pwr);
+ if (ret < 0)
+ dev_err(priv->dev, "Failed to disable io pwr: %d\n",
+ ret);
+
+ dsa_unregister_switch(priv->ds);
+ mutex_destroy(&priv->reg_mutex);
+}
+
+static const struct of_device_id mt7530_of_match[] = {
+ { .compatible = "mediatek,mt7530" },
+ { /* sentinel */ },
+};
+
+static struct mdio_driver mt7530_mdio_driver = {
+ .probe = mt7530_probe,
+ .remove = mt7530_remove,
+ .mdiodrv.driver = {
+ .name = "mt7530",
+ .of_match_table = mt7530_of_match,
+ },
+};
+
+mdio_module_driver(mt7530_mdio_driver);
+
+MODULE_AUTHOR("Sean Wang <[email protected]>");
+MODULE_DESCRIPTION("Driver for Mediatek MT7530 Switch");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:mediatek-mt7530");
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
new file mode 100644
index 0000000..ba1e1cd
--- /dev/null
+++ b/drivers/net/dsa/mt7530.h
@@ -0,0 +1,382 @@
+/*
+ * Copyright (C) 2017 Sean Wang <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __MT7530_H
+#define __MT7530_H
+
+#define MT7530_NUM_PORTS 7
+#define MT7530_CPU_PORT 6
+#define MT7530_NUM_FDB_RECORDS 2048
+
+#define TRGMII_BASE(x) (0x10000 + (x))
+
+/* Registers to ethsys access */
+#define ETHSYS_CLKCFG0 0x2c
+#define ETHSYS_TRGMII_CLK_SEL362_5 BIT(11)
+
+#define SYSC_REG_RSTCTRL 0x34
+#define RESET_MCM BIT(2)
+
+/* Registers to mac forward conrol for unknown frames */
+#define MT7530_MFC 0x10
+#define BC_FFP(x) (((x) & 0xff) << 24)
+#define UNM_FFP(x) (((x) & 0xff) << 16)
+#define UNU_FFP(x) (((x) & 0xff) << 8)
+#define UNU_FFP_MASK UNU_FFP(~0)
+
+/* Registers for address table access */
+
+#define MT7530_ATA1 0x74
+#define STATIC_EMP 0
+#define STATIC_ENT 3
+#define MT7530_ATA2 0x78
+
+/* Register for address table write data */
+#define MT7530_ATWD 0x7c
+
+/* Register for address table control */
+#define MT7530_ATC 0x80
+#define ATC_HASH (((x) & 0xfff) << 16)
+#define ATC_BUSY BIT(15)
+#define ATC_SRCH_END BIT(14)
+#define ATC_SRCH_HIT BIT(13)
+#define ATC_INVALID BIT(12)
+#define ATC_MAT(x) (((x) & 0xf) << 8)
+#define ATC_MAT_MACTAB ATC_MAT(0)
+
+enum mt7530_fdb_cmd {
+ MT7530_FDB_READ = 0,
+ MT7530_FDB_WRITE = 1,
+ MT7530_FDB_FLUSH = 2,
+ MT7530_FDB_START = 4,
+ MT7530_FDB_NEXT = 5,
+};
+
+/* Registers for table search read address */
+#define MT7530_TSRA1 0x84
+#define MT7530_TSRA2 0x88
+#define MT7530_ATRD 0x8C
+
+/* Register for vlan table control */
+#define MT7530_VTCR 0x90
+#define VTCR_BUSY BIT(31)
+#define VTCR_FUNC (((x) & 0xf) << 12)
+#define VTCR_FUNC_RD_VID 0x1
+#define VTCR_FUNC_WR_VID 0x2
+#define VTCR_FUNC_INV_VID 0x3
+#define VTCR_FUNC_VAL_VID 0x4
+#define VTCR_VID ((x) & 0xfff)
+
+/* Register for setup vlan and acl write data */
+#define MT7530_VAWD1 0x94
+#define PORT_STAG BIT(31)
+#define IVL_MAC BIT(30)
+#define PORT_MEM(x) (((x) & 0xff) << 16)
+#define VALID BIT(1)
+
+#define MT7530_VAWD2 0x98
+
+/* Register for port STP state control */
+#define MT7530_SSP_P(x) (0x2000 + ((x) * 0x100))
+#define FID_PST(x) ((x) & 0x3)
+#define FID_PST_MASK FID_PST(0x3)
+
+enum mt7530_stp_state {
+ MT7530_STP_DISABLED = 0,
+ MT7530_STP_BLOCKING = 1,
+ MT7530_STP_LISTENING = 1,
+ MT7530_STP_LEARNING = 2,
+ MT7530_STP_FORWARDING = 3
+};
+
+/* Registor for port control */
+#define MT7530_PCR_P(x) (0x2004 + ((x) * 0x100))
+#define PORT_VLAN(x) ((x) & 0x3)
+#define PCR_MATRIX(x) (((x) & 0xff) << 16)
+#define PORT_PRI(x) (((x) & 0x7) << 24)
+#define EG_TAG(x) (((x) & 0x3) << 28)
+#define PCR_MATRIX_MASK PCR_MATRIX(0xff)
+#define PCR_MATRIX_INIT PCR_MATRIX(0)
+
+/* Register for port security control */
+#define MT7530_PSC_P(x) (0x200c + ((x) * 0x100))
+#define SA_DIS BIT(4)
+
+/* Register for port vlan control */
+#define MT7530_PVC_P(x) (0x2010 + ((x) * 0x100))
+#define PORT_SPEC_TAG BIT(5)
+#define VLAN_ATTR(x) (((x) & 0x3) << 6)
+#define STAG_VPID (((x) & 0xffff) << 16)
+
+/* Register for port port-and-protocol based vlan 1 control */
+#define MT7530_PPBV1_P(x) (0x2014 + ((x) * 0x100))
+
+/* Register for port MAC control register */
+#define MT7530_PMCR_P(x) (0x3000 + ((x) * 0x100))
+#define PMCR_IFG_XMIT(x) (((x) & 0x3) << 18)
+#define PMCR_MAC_MODE BIT(16)
+#define PMCR_FORCE_MODE BIT(15)
+#define PMCR_TX_EN BIT(14)
+#define PMCR_RX_EN BIT(13)
+#define PMCR_BACKOFF_EN BIT(9)
+#define PMCR_BACKPR_EN BIT(8)
+#define PMCR_TX_FC_EN BIT(5)
+#define PMCR_RX_FC_EN BIT(4)
+#define PMCR_FORCE_SPEED_1000 BIT(3)
+#define PMCR_FORCE_FDX BIT(1)
+#define PMCR_FORCE_LNK BIT(0)
+#define PMCR_COMMON_LINK (PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | \
+ PMCR_BACKOFF_EN | PMCR_BACKPR_EN | \
+ PMCR_TX_EN | PMCR_RX_EN | \
+ PMCR_TX_FC_EN | PMCR_RX_FC_EN)
+#define PMCR_CPUP_LINK (PMCR_COMMON_LINK | PMCR_FORCE_MODE | \
+ PMCR_FORCE_SPEED_1000 | \
+ PMCR_FORCE_FDX | \
+ PMCR_FORCE_LNK)
+#define PMCR_USERP_LINK PMCR_COMMON_LINK
+#define PMCR_FIXED_LINK (PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | \
+ PMCR_FORCE_MODE | PMCR_TX_EN | \
+ PMCR_RX_EN | PMCR_BACKPR_EN | \
+ PMCR_BACKOFF_EN | \
+ PMCR_FORCE_SPEED_1000 | \
+ PMCR_FORCE_FDX | \
+ PMCR_FORCE_LNK)
+#define PMCR_FIXED_LINK_FC (PMCR_FIXED_LINK | \
+ PMCR_TX_FC_EN | PMCR_RX_FC_EN)
+
+#define MT7530_PMSR_P(x) (0x3008 + (x) * 0x100)
+
+/* Register for MIB */
+#define MT7530_PORT_MIB_COUNTER(x) (0x4000 + (x) * 0x100)
+#define MT7530_MIB_CCR 0x4fe0
+#define CCR_MIB_ENABLE BIT(31)
+#define CCR_RX_OCT_CNT_GOOD BIT(7)
+#define CCR_RX_OCT_CNT_BAD BIT(6)
+#define CCR_TX_OCT_CNT_GOOD BIT(5)
+#define CCR_TX_OCT_CNT_BAD BIT(4)
+#define CCR_MIB_FLUSH (CCR_RX_OCT_CNT_GOOD | \
+ CCR_RX_OCT_CNT_BAD | \
+ CCR_TX_OCT_CNT_GOOD | \
+ CCR_TX_OCT_CNT_BAD)
+#define CCR_MIB_ACTIVATE (CCR_MIB_ENABLE | \
+ CCR_RX_OCT_CNT_GOOD | \
+ CCR_RX_OCT_CNT_BAD | \
+ CCR_TX_OCT_CNT_GOOD | \
+ CCR_TX_OCT_CNT_BAD)
+/* Register for system reset */
+#define MT7530_SYS_CTRL 0x7000
+#define SYS_CTRL_PHY_RST BIT(2)
+#define SYS_CTRL_SW_RST BIT(1)
+#define SYS_CTRL_REG_RST BIT(0)
+
+/* Register for hw trap status */
+#define MT7530_HWTRAP 0x7800
+
+/* Register for hw trap modification */
+#define MT7530_MHWTRAP 0x7804
+#define MHWTRAP_MANUAL BIT(16)
+#define MHWTRAP_P5_MAC_SEL BIT(13)
+#define MHWTRAP_P6_DIS BIT(8)
+#define MHWTRAP_P5_RGMII_MODE BIT(7)
+#define MHWTRAP_P5_DIS BIT(6)
+#define MHWTRAP_PHY_ACCESS BIT(5)
+
+/* Regiser for TOP signal control */
+#define MT7530_TOP_SIG_CTRL 0x7808
+#define TOP_SIG_CTRL_NORMAL (BIT(17) | BIT(16))
+
+#define MT7530_IO_DRV_CR 0x7810
+#define P5_IO_CLK_DRV(x) ((x) & 0x3)
+#define P5_IO_DATA_DRV(x) (((x) & 0x3) << 4)
+
+#define MT7530_P6ECR 0x7830
+#define P6_INTF_MODE_MASK 0x3
+#define P6_INTF_MODE(x) ((x) & 0x3)
+
+/* Registers for TRGMII on the both side */
+#define MT7530_TRGMII_RCK_CTRL 0x7a00
+#define GSW_TRGMII_RCK_CTRL 0x300
+#define RX_RST BIT(31)
+#define RXC_DQSISEL BIT(30)
+#define DQSI1_TAP_MASK (0x7f << 8)
+#define DQSI0_TAP_MASK 0x7f
+#define DQSI1_TAP(x) (((x) & 0x7f) << 8)
+#define DQSI0_TAP(x) ((x) & 0x7f)
+
+#define MT7530_TRGMII_RCK_RTT 0x7a04
+#define GSW_TRGMII_RCK_RTT 0x304
+#define DQS1_GATE BIT(31)
+#define DQS0_GATE BIT(30)
+
+#define MT7530_TRGMII_RD(x) (0x7a10 + (x) * 8)
+#define GSW_TRGMII_RD(x) (0x310 + (x) * 8)
+#define BSLIP_EN BIT(31)
+#define EDGE_CHK BIT(30)
+#define RD_TAP_MASK 0x7f
+#define RD_TAP(x) ((x) & 0x7f)
+
+#define GSW_TRGMII_TXCTRL 0x340
+#define MT7530_TRGMII_TXCTRL 0x7a40
+#define TRAIN_TXEN BIT(31)
+#define TXC_INV BIT(30)
+#define TX_RST BIT(28)
+
+#define MT7530_TRGMII_TD_ODT(i) (0x7a54 + 8 * (i))
+#define GSW_TRGMII_TD_ODT(i) (0x354 + 8 * (i))
+#define TD_DM_DRVP(x) ((x) & 0xf)
+#define TD_DM_DRVN(x) (((x) & 0xf) << 4)
+
+#define GSW_INTF_MODE 0x390
+#define INTF_MODE_TRGMII BIT(1)
+
+#define MT7530_TRGMII_TCK_CTRL 0x7a78
+#define TCK_TAP(x) (((x) & 0xf) << 8)
+
+#define MT7530_P5RGMIIRXCR 0x7b00
+#define CSR_RGMII_EDGE_ALIGN BIT(8)
+#define CSR_RGMII_RXC_0DEG_CFG(x) ((x) & 0xf)
+
+#define MT7530_P5RGMIITXCR 0x7b04
+#define CSR_RGMII_TXC_CFG(x) ((x) & 0x1f)
+
+#define MT7530_CREV 0x7ffc
+#define CHIP_NAME_SHIFT 16
+#define MT7530_ID 0x7530
+
+/* Registers for core PLL access through mmd indirect */
+#define CORE_PLL_GROUP2 0x401
+#define RG_SYSPLL_EN_NORMAL BIT(15)
+#define RG_SYSPLL_VODEN BIT(14)
+#define RG_SYSPLL_LF BIT(13)
+#define RG_SYSPLL_RST_DLY(x) (((x) & 0x3) << 12)
+#define RG_SYSPLL_LVROD_EN BIT(10)
+#define RG_SYSPLL_PREDIV(x) (((x) & 0x3) << 8)
+#define RG_SYSPLL_POSDIV(x) (((x) & 0x3) << 5)
+#define RG_SYSPLL_FBKSEL BIT(4)
+#define RT_SYSPLL_EN_AFE_OLT BIT(0)
+
+#define CORE_PLL_GROUP4 0x403
+#define RG_SYSPLL_DDSFBK_EN BIT(12)
+#define RG_SYSPLL_BIAS_EN BIT(11)
+#define RG_SYSPLL_BIAS_LPF_EN BIT(10)
+
+#define CORE_PLL_GROUP5 0x404
+#define RG_LCDDS_PCW_NCPO1(x) ((x) & 0xffff)
+
+#define CORE_PLL_GROUP6 0x405
+#define RG_LCDDS_PCW_NCPO0(x) ((x) & 0xffff)
+
+#define CORE_PLL_GROUP7 0x406
+#define RG_LCDDS_PWDB BIT(15)
+#define RG_LCDDS_ISO_EN BIT(13)
+#define RG_LCCDS_C(x) (((x) & 0x7) << 4)
+#define RG_LCDDS_PCW_NCPO_CHG BIT(3)
+
+#define CORE_PLL_GROUP10 0x409
+#define RG_LCDDS_SSC_DELTA(x) ((x) & 0xfff)
+
+#define CORE_PLL_GROUP11 0x40a
+#define RG_LCDDS_SSC_DELTA1(x) ((x) & 0xfff)
+
+#define CORE_GSWPLL_GRP1 0x40d
+#define RG_GSWPLL_PREDIV(x) (((x) & 0x3) << 14)
+#define RG_GSWPLL_POSDIV_200M(x) (((x) & 0x3) << 12)
+#define RG_GSWPLL_EN_PRE BIT(11)
+#define RG_GSWPLL_FBKSEL BIT(10)
+#define RG_GSWPLL_BP BIT(9)
+#define RG_GSWPLL_BR BIT(8)
+#define RG_GSWPLL_FBKDIV_200M(x) ((x) & 0xff)
+
+#define CORE_GSWPLL_GRP2 0x40e
+#define RG_GSWPLL_POSDIV_500M(x) (((x) & 0x3) << 8)
+#define RG_GSWPLL_FBKDIV_500M(x) ((x) & 0xff)
+
+#define CORE_TRGMII_GSW_CLK_CG 0x410
+#define REG_GSWCK_EN BIT(0)
+#define REG_TRGMIICK_EN BIT(1)
+
+#define wait_condition_timeout(condition, timeout) \
+({ \
+ long __ret = 0; \
+ unsigned long toj; \
+ toj = jiffies + msecs_to_jiffies(timeout); \
+ \
+ for ( ; !(condition) && !time_after_eq(jiffies, toj) ; ) \
+ cond_resched(); \
+ \
+ if (time_after_eq(jiffies, toj)) \
+ __ret = -ETIMEDOUT; \
+ __ret; \
+})
+
+#define MIB_DESC(_s, _o, _n) \
+ { \
+ .size = (_s), \
+ .offset = (_o), \
+ .name = (_n), \
+ }
+
+struct mt7530_mib_desc {
+ unsigned int size;
+ unsigned int offset;
+ const char *name;
+};
+
+struct mt7530_fdb {
+ u16 vid;
+ u8 port_mask;
+ u8 aging;
+ u8 mac[6];
+ bool noarp;
+};
+
+/* struct mt7530_priv - This is the main datasructure for holding the state
+ * of the driver
+ * @dev: The device pointer
+ * @ds: The pointer to the dsa core structure
+ * @bus: The bus used for the device and built-in PHY
+ * @ethsys: The regmap used for enabling the necessary PLL
+ * @ethernet: The regmap used for access TRGMII-based registers
+ * @core_pwr: The power supplied into the core
+ * @io_pwr: The power supplied into the I/O
+ * @mcm: Flag for distinguishing if standalone IC or module
+ * coupling
+ * @reset: The descriptor for GPIO line tied to its reset pin
+ * @phy_mode: The xMII for cpu port used
+ * @ports: Holding the state amongs ports
+ * @reg_mutex: The lock for protecting among process accessing
+ * registers
+ */
+struct mt7530_priv {
+ struct device *dev;
+ struct dsa_switch *ds;
+ struct mii_bus *bus;
+ struct reset_control *rstc;
+ struct regmap *ethernet;
+ struct regulator *core_pwr;
+ struct regulator *io_pwr;
+ bool mcm;
+ struct gpio_desc *reset;
+ int phy_mode;
+
+ /* protect among processes for registers access*/
+ struct mutex reg_mutex;
+};
+
+struct mt7530_hw_stats {
+ const char *string;
+ u16 reg;
+ u8 sizeof_stat;
+};
+
+#endif /* __MT7530_H */
--
1.9.1

2017-03-21 09:42:39

by Sean Wang

[permalink] [raw]
Subject: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance

From: Sean Wang <[email protected]>

the patch adds the setup of the corresponding device node of GMAC into the
netdev instance which could allow other modules such as DSA to find the
instance through the node in dt-bindings using of_find_net_device_by_node()
call.

Signed-off-by: Sean Wang <[email protected]>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index c21ed99..84b09a4 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2323,6 +2323,8 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;

eth->netdev[id]->irq = eth->irq[0];
+ eth->netdev[id]->dev.of_node = np;
+
return 0;

free_netdev:
--
1.9.1

2017-03-21 09:43:02

by Sean Wang

[permalink] [raw]
Subject: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA

From: Sean Wang <[email protected]>

The patch adds the setup for allowing CDM can recognize these packets with
carrying port-distinguishing tag. Otherwise, these tagging packets will be
handled incorrectly by CDM. The setup is working out for general untag
packets as well.

Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Landen Chao <[email protected]>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 ++++++
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 ++++
2 files changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 9e75768..c21ed99 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1846,6 +1846,12 @@ static int mtk_hw_init(struct mtk_eth *eth)
/* GE2, Force 1000M/FD, FC ON */
mtk_w32(eth, MAC_MCR_FIXED_LINK, MTK_MAC_MCR(1));

+ /* Indicates CDM to parse the MTK special tag from CPU
+ * which also is working out for untag packets.
+ */
+ val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
+ mtk_w32(eth, val | MTK_CDMQ_STAG_EN, MTK_CDMQ_IG_CTRL);
+
/* Enable RX VLan Offloading */
mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 99b1c8e..996024d 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -70,6 +70,10 @@
/* Frame Engine Interrupt Grouping Register */
#define MTK_FE_INT_GRP 0x20

+/* CDMP Ingress Control Register */
+#define MTK_CDMQ_IG_CTRL 0x1400
+#define MTK_CDMQ_STAG_EN BIT(0)
+
/* CDMP Exgress Control Register */
#define MTK_CDMP_EG_CTRL 0x404

--
1.9.1

2017-03-21 09:42:59

by Sean Wang

[permalink] [raw]
Subject: [PATCH net-next v2 2/5] net-next: dsa: add Mediatek tag RX/TX handler

From: Sean Wang <[email protected]>

Add the support for the 4-bytes tag for DSA port distinguishing inserted
allowing receiving and transmitting the packet via the particular port.
The tag is being added after the source MAC address in the ethernet
header.

Signed-off-by: Sean Wang <[email protected]>
Signed-off-by: Landen Chao <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
include/net/dsa.h | 1 +
net/dsa/Kconfig | 2 +
net/dsa/Makefile | 1 +
net/dsa/dsa.c | 3 ++
net/dsa/dsa_priv.h | 3 ++
net/dsa/tag_mtk.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 127 insertions(+)
create mode 100644 net/dsa/tag_mtk.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4e13e69..3276547 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -31,6 +31,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_EDSA,
DSA_TAG_PROTO_BRCM,
DSA_TAG_PROTO_QCA,
+ DSA_TAG_PROTO_MTK,
DSA_TAG_LAST, /* MUST BE LAST */
};

diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 9649238..d78789b 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -31,4 +31,6 @@ config NET_DSA_TAG_TRAILER
config NET_DSA_TAG_QCA
bool

+config NET_DSA_TAG_MTK
+ bool
endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 31d3437..9b1d478 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -8,3 +8,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
+dsa_core-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index b6d4f6a..617f736 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -53,6 +53,9 @@ static struct sk_buff *dsa_slave_notag_xmit(struct sk_buff *skb,
#ifdef CONFIG_NET_DSA_TAG_QCA
[DSA_TAG_PROTO_QCA] = &qca_netdev_ops,
#endif
+#ifdef CONFIG_NET_DSA_TAG_MTK
+ [DSA_TAG_PROTO_MTK] = &mtk_netdev_ops,
+#endif
[DSA_TAG_PROTO_NONE] = &none_ops,
};

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 0706a51..2a31399 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -85,4 +85,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
/* tag_qca.c */
extern const struct dsa_device_ops qca_netdev_ops;

+/* tag_mtk.c */
+extern const struct dsa_device_ops mtk_netdev_ops;
+
#endif
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
new file mode 100644
index 0000000..833a9d6
--- /dev/null
+++ b/net/dsa/tag_mtk.c
@@ -0,0 +1,117 @@
+/*
+ * Mediatek DSA Tag support
+ * Copyright (C) 2017 Landen Chao <[email protected]>
+ * Sean Wang <[email protected]>
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/etherdevice.h>
+#include "dsa_priv.h"
+
+#define MTK_HDR_LEN 4
+#define MTK_HDR_RECV_SOURCE_PORT_MASK GENMASK(2, 0)
+#define MTK_HDR_XMIT_DP_BIT_MASK GENMASK(5, 0)
+
+static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct dsa_slave_priv *p = netdev_priv(dev);
+ u8 *mtk_tag;
+
+ if (skb_cow_head(skb, MTK_HDR_LEN) < 0)
+ goto out_free;
+
+ skb_push(skb, MTK_HDR_LEN);
+
+ memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN);
+
+ /* Build the tag after the MAC Source Address */
+ mtk_tag = skb->data + 2 * ETH_ALEN;
+ mtk_tag[0] = 0;
+ mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
+ mtk_tag[2] = 0;
+ mtk_tag[3] = 0;
+
+ return skb;
+
+out_free:
+ kfree_skb(skb);
+ return NULL;
+}
+
+static int mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
+ struct packet_type *pt, struct net_device *orig_dev)
+{
+ struct dsa_switch_tree *dst = dev->dsa_ptr;
+ struct dsa_switch *ds;
+ int port;
+ __be16 *phdr, hdr;
+
+ if (unlikely(!dst))
+ goto out_drop;
+
+ skb = skb_unshare(skb, GFP_ATOMIC);
+ if (!skb)
+ goto out;
+
+ if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN)))
+ goto out_drop;
+
+ /* The MTK header is added by the switch between src addr
+ * and ethertype at this point, skb->data points to 2 bytes
+ * after src addr so header should be 2 bytes right before.
+ */
+ phdr = (__be16 *)(skb->data - 2);
+ hdr = ntohs(*phdr);
+
+ /* Remove MTK tag and recalculate checksum. */
+ skb_pull_rcsum(skb, MTK_HDR_LEN);
+
+ memmove(skb->data - ETH_HLEN,
+ skb->data - ETH_HLEN - MTK_HDR_LEN,
+ 2 * ETH_ALEN);
+
+ /* This protocol doesn't support cascading multiple
+ * switches so it's safe to assume the switch is first
+ * in the tree.
+ */
+ ds = dst->ds[0];
+ if (!ds)
+ goto out_drop;
+
+ /* Get source port information */
+ port = (hdr & MTK_HDR_RECV_SOURCE_PORT_MASK);
+ if (!ds->ports[port].netdev)
+ goto out_drop;
+
+ /* Update skb & forward the frame accordingly */
+ skb_push(skb, ETH_HLEN);
+
+ skb->pkt_type = PACKET_HOST;
+ skb->dev = ds->ports[port].netdev;
+ skb->protocol = eth_type_trans(skb, skb->dev);
+
+ skb->dev->stats.rx_packets++;
+ skb->dev->stats.rx_bytes += skb->len;
+
+ netif_receive_skb(skb);
+
+ return 0;
+
+out_drop:
+ kfree_skb(skb);
+out:
+ return 0;
+}
+
+const struct dsa_device_ops mtk_netdev_ops = {
+ .xmit = mtk_tag_xmit,
+ .rcv = mtk_tag_rcv,
+};
--
1.9.1

2017-03-22 17:44:43

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA

On Tue, Mar 21, 2017 at 05:35:08PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> The patch adds the setup for allowing CDM can recognize these packets with
> carrying port-distinguishing tag. Otherwise, these tagging packets will be
> handled incorrectly by CDM. The setup is working out for general untag
> packets as well.
>
> Signed-off-by: Sean Wang <[email protected]>
> Signed-off-by: Landen Chao <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2017-03-22 18:19:00

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance

On Tue, Mar 21, 2017 at 05:35:09PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> the patch adds the setup of the corresponding device node of GMAC into the
> netdev instance which could allow other modules such as DSA to find the
> instance through the node in dt-bindings using of_find_net_device_by_node()
> call.
>
> Signed-off-by: Sean Wang <[email protected]>
> ---
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index c21ed99..84b09a4 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2323,6 +2323,8 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
> eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
>
> eth->netdev[id]->irq = eth->irq[0];
> + eth->netdev[id]->dev.of_node = np;
> +

Humm, O.K. This is not obvious, until you look at of_dev_node_match()
in net-sysfs.c.

Most Ethernet drivers don't set netdev.dev.of_node. But they do call
SET_NETDEV_DEV(), which sets netdev.dev.parent.

of_dev_node_match() first looks at netdev.dev.parent->of_node, and if that
does not match, then looks at netdev.dev.of_node.

For the mtk Ethernet driver, the parent is not going to work, because
of the sub devices. So netdev.dev.of_node does need to be set.

Reviewed-by: Andrew Lunn <[email protected]>

Andrew


2017-03-22 18:23:21

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance

On 03/21/2017 02:35 AM, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> the patch adds the setup of the corresponding device node of GMAC into the
> netdev instance which could allow other modules such as DSA to find the
> instance through the node in dt-bindings using of_find_net_device_by_node()
> call.
>
> Signed-off-by: Sean Wang <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2017-03-22 18:23:56

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA

On 03/21/2017 02:35 AM, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> The patch adds the setup for allowing CDM can recognize these packets with
> carrying port-distinguishing tag. Otherwise, these tagging packets will be
> handled incorrectly by CDM. The setup is working out for general untag
> packets as well.
>
> Signed-off-by: Sean Wang <[email protected]>
> Signed-off-by: Landen Chao <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2017-03-22 18:30:14

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

> +static int
> +core_read_mmd_indirect(struct mt7530_priv *priv, int prtad, int devad)
> +{
> + struct mii_bus *bus = priv->bus;
> + int value, ret;
> +
> + /* Write the desired MMD Devad */
> + ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
> + if (ret < 0)
> + goto err;
> +
> + /* Write the desired MMD register address */
> + ret = bus->write(bus, 0, MII_MMD_DATA, prtad);
> + if (ret < 0)
> + goto err;
> +
> + /* Select the Function : DATA with no post increment */
> + ret = bus->write(bus, 0, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> + if (ret < 0)
> + goto err;

Hi Sean

This appear to be a copy of mmd_phy_indirect(). There are patches from
Russell King which made this a public function. Once Russell patches
make net-next, it would be nice to use mmd_phy_indirect(). But that
might be as a follow up patch, rather than now, depending on the order
of acceptance of the patches.

Andrew

2017-03-22 18:39:32

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 03/21/2017 02:35 AM, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
> Mediatek router platforms such as MT7623A or MT7623N platform which
> includes 7-port Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY.
> Among these ports, The port from 0 to 4 are the user ports connecting
> with the remote devices while the port 5 and 6 are the CPU ports
> connecting into Mediatek Ethernet GMAC.
>
> For port 6, it can communicate with the CPU via Mediatek Ethernet GMAC
> through either the TRGMII or RGMII which could be controlled by phy-mode
> in the dt-bindings to specify which mode is preferred to use. And for
> port 5, only RGMII can be specified. However, currently, only port 6 is
> being supported in this DSA driver.
>
> The driver is made with the reference to qca8k and other existing DSA
> driver. The most of the essential callbacks of the DSA are already
> support in the driver, including tag insert for user port distinguishing,
> port control, bridge offloading, STP setup and ethtool operation to allow
> DSA to model each user port into a standalone netdevice as the other DSA
> driver had done.

Overall, this looks pretty nice and clean, a few comments below

>
> Signed-off-by: Sean Wang <[email protected]>
> Signed-off-by: Landen Chao <[email protected]>
> ---

> +static void
> +mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
> +{
> + u32 reg[3];
> + int i;
> +
> + /* Read from ARL table into an array */
> + for (i = 0; i < 3; i++) {
> + reg[i] = mt7530_read(priv, MT7530_TSRA1 + (i * 4));
> +
> + dev_dbg(priv->dev, "%s(%d) reg[%d]=0x%x\n",
> + __func__, __LINE__, i, reg[i]);
> + }
> +
> + /* vid - 11:0 on reg[1] */
> + fdb->vid = (reg[1] >> 0) & 0xfff;
> + /* aging - 31:24 on reg[2] */
> + fdb->aging = (reg[2] >> 24) & 0xff;
> + /* portmask - 11:4 on reg[2] */
> + fdb->port_mask = (reg[2] >> 4) & 0xff;
> + /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> + fdb->mac[0] = (reg[0] >> 24) & 0xff;
> + fdb->mac[1] = (reg[0] >> 16) & 0xff;
> + fdb->mac[2] = (reg[0] >> 8) & 0xff;
> + fdb->mac[3] = (reg[0] >> 0) & 0xff;
> + fdb->mac[4] = (reg[1] >> 24) & 0xff;
> + fdb->mac[5] = (reg[1] >> 16) & 0xff;
> + /* noarp - 3:2 on reg[2] */
> + fdb->noarp = ((reg[2] >> 2) & 0x3) == STATIC_ENT;

Could you add some definitions for the bits and masks that you are
shifting here?

> +}
> +
> +static void
> +mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
> + u8 port_mask, const u8 *mac,
> + u8 aging, u8 type)
> +{
> + u32 reg[3] = { 0 };
> + int i;
> +
> + /* vid - 11:0 on reg[1] */
> + reg[1] |= (vid & 0xfff) << 0;
> + /* aging - 31:25 on reg[2] */
> + reg[2] |= (aging & 0xff) << 24;
> + /* portmask - 11:4 on reg[2] */
> + reg[2] |= (port_mask & 0xff) << 4;
> + /* type - 3 indicate that entry is static wouldn't
> + * be aged out and 0 specified as erasing an entry
> + */
> + reg[2] |= (type & 0x3) << 2;
> + /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> + reg[1] |= mac[5] << 16;
> + reg[1] |= mac[4] << 24;
> + reg[0] |= mac[3] << 0;
> + reg[0] |= mac[2] << 8;
> + reg[0] |= mac[1] << 16;
> + reg[0] |= mac[0] << 24;
> +
> + /* Wrirte array into the ARL table */
> + for (i = 0; i < 3; i++)
> + mt7530_write(priv, MT7530_ATA1 + (i * 4), reg[i]);
> +}

Same here.

> +
> +static int
> +mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + u32 ncpo1, ssc_delta, trgint, i;
> +
> + switch (mode) {
> + case PHY_INTERFACE_MODE_RGMII:
> + trgint = 0;
> + ncpo1 = 0x0c80;
> + ssc_delta = 0x87;
> + break;
> + case PHY_INTERFACE_MODE_TRGMII:
> + trgint = 1;
> + ncpo1 = 0x1400;
> + ssc_delta = 0x57;
> + break;
> + default:
> + pr_err("xMII mode %d not supported\n", mode);
> + return -EINVAL;
> + }

You may be able to move this to an adjust_link callback that the PHY
library would call when the PHY gets setup and the port is finally used,
as opposed to doing this upfront during driver initialization.


> +mt7530_setup(struct dsa_switch *ds)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + int ret, i, phy_mode;
> + u8 cpup_mask = 0;
> + u32 id, val;
> + struct regmap *regmap;
> + struct device_node *dn;
> +
> + /* Make sure that cpu port specfied on the dt is appropriate */
> + if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
> + dev_err(priv->dev, "port not matched with the CPU port\n");
> + return -EINVAL;
> + }

This is kind of a hard error, in that case, a sensible thing to do could
be issue a warning to the user telling that the configuration does not
permit the use of Mediatek tags. Your get_tag_protocol() function could
then return DSA_TAG_PROTO_NONE here instead of DSA_TAG_PROTO_MTK. It
would still allow an user to utilize the switch, and we would know what
is wrong with the configuration/board setup though.


> +
> + /* The parent node of master_netdev which holds the common system
> + * controller also is the container for two GMACs nodes representing
> + * as two netdev instances.
> + */
> + dn = ds->master_netdev->dev.of_node->parent;
> + priv->ethernet = syscon_node_to_regmap(dn);
> + if (IS_ERR(priv->ethernet))
> + return PTR_ERR(priv->ethernet);
> +
> + regmap = devm_regmap_init(ds->dev, NULL, priv,
> + &mt7530_regmap_config);
> + if (IS_ERR(regmap))
> + dev_warn(priv->dev, "phy regmap initialization failed");
> +
> + phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> + if (phy_mode < 0) {
> + dev_err(priv->dev, "Can't find phy-mode for master device\n");
> + return phy_mode;
> + }
> + dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);

An adjust_link() callback which has a proper PHY device structure would
be more appropriate to look up the phy_mode rather than doing this here.

[snip]

> + mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK);
> +
> + /* Fabric setup for the cpu port */
> + for (i = 0; i < MT7530_NUM_PORTS; i++)

Are not you missing an opening parenthesis here in the for() statement?

> + if (dsa_is_cpu_port(ds, i)) {
> + /* Enable Mediatek header mode on the cpu port */
> + mt7530_write(priv, MT7530_PVC_P(i),
> + PORT_SPEC_TAG);
> +
> + /* Setup the MAC by default for the cpu port */
> + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_CPUP_LINK);
> +
> + /* Disable auto learning on the cpu port */
> + mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
> +
> + /* Unknown unicast frame fordwarding to the cpu port */
> + mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(i)));
> +
> + /* CPU port gets connected to all user ports of
> + * the switch
> + */
> + mt7530_write(priv, MT7530_PCR_P(i),
> + PCR_MATRIX(ds->enabled_port_mask));
> +
> + cpup_mask |= BIT(i);
> + }
> +
> + /* Fabric setup for the all user ports */
> + for (i = 0; i < MT7530_NUM_PORTS; i++)
> + if (ds->enabled_port_mask & BIT(i)) {
> + /* Setup the MAC by default for all user ports */
> + mt7530_write(priv, MT7530_PMCR_P(i),
> + PMCR_USERP_LINK);
> +
> + /* The user port gets connected to the cpu port only */
> + mt7530_write(priv, MT7530_PCR_P(i),
> + PCR_MATRIX(cpup_mask));
> + }

This should be moved to the port_enable() function.

[snip]

> +#define wait_condition_timeout(condition, timeout) \
> +({ \
> + long __ret = 0; \
> + unsigned long toj; \
> + toj = jiffies + msecs_to_jiffies(timeout); \
> + \
> + for ( ; !(condition) && !time_after_eq(jiffies, toj) ; ) \
> + cond_resched(); \
> + \
> + if (time_after_eq(jiffies, toj)) \
> + __ret = -ETIMEDOUT; \
> + __ret; \
> +})

Can you use read*_poll_timeout() instead of this?


> +/* struct mt7530_priv - This is the main datasructure for holding the state
> + * of the driver
> + * @dev: The device pointer
> + * @ds: The pointer to the dsa core structure
> + * @bus: The bus used for the device and built-in PHY
> + * @ethsys: The regmap used for enabling the necessary PLL
> + * @ethernet: The regmap used for access TRGMII-based registers
> + * @core_pwr: The power supplied into the core
> + * @io_pwr: The power supplied into the I/O
> + * @mcm: Flag for distinguishing if standalone IC or module
> + * coupling
> + * @reset: The descriptor for GPIO line tied to its reset pin
> + * @phy_mode: The xMII for cpu port used
> + * @ports: Holding the state amongs ports
> + * @reg_mutex: The lock for protecting among process accessing
> + * registers
> + */

Kudos for using kernel doc here.
--
Florian

2017-03-23 07:40:27

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

> +static int
> +mt7623_trgmii_write(struct mt7530_priv *priv, u32 reg, u32 val)
> +{
> + int ret;
> +
> + ret = regmap_write(priv->ethernet, TRGMII_BASE(reg), val);
> + if (ret < 0)
> + dev_err(priv->dev,
> + "failed to priv write register\n");
> + return ret;
> +}
> +
> +static u32
> +mt7623_trgmii_read(struct mt7530_priv *priv, u32 reg)
> +{
> + int ret;
> + u32 val;
> +
> + ret = regmap_read(priv->ethernet, TRGMII_BASE(reg), &val);
> + if (ret < 0) {
> + dev_err(priv->dev,
> + "failed to priv read register\n");
> + return ret;
> + }
> +
> + return val;
> +}

Hi Sean

These appear to be the only two accessors which use the regmap.

> +static int
> +mt7530_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> +{
> + struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
> +
> + /* BIT(15) is used as indication for pseudo registers
> + * which would be translated into the general MDIO
> + * access to leverage the unique regmap sys interface.
> + */
> + if (reg & BIT(15))
> + *val = mdiobus_read_nested(priv->bus,
> + (reg & 0xf00) >> 8,
> + (reg & 0xff) >> 2);
> + else
> + *val = mt7530_read(priv, reg);
> +
> + return 0;
> +}

.....

> +static const struct regmap_range mt7530_readable_ranges[] = {
> + regmap_reg_range(0x0000, 0x00ac), /* Global control */
> + regmap_reg_range(0x2000, 0x202c), /* Port Control - P0 */
> + regmap_reg_range(0x2100, 0x212c), /* Port Control - P1 */
> + regmap_reg_range(0x2200, 0x222c), /* Port Control - P2 */
> + regmap_reg_range(0x2300, 0x232c), /* Port Control - P3 */
> + regmap_reg_range(0x2400, 0x242c), /* Port Control - P4 */
> + regmap_reg_range(0x2500, 0x252c), /* Port Control - P5 */
> + regmap_reg_range(0x2600, 0x262c), /* Port Control - P6 */
> + regmap_reg_range(0x30e0, 0x30f8), /* Port MAC - SYS */
> + regmap_reg_range(0x3000, 0x3014), /* Port MAC - P0 */
> + regmap_reg_range(0x3100, 0x3114), /* Port MAC - P1 */
> + regmap_reg_range(0x3200, 0x3214), /* Port MAC - P2*/
> + regmap_reg_range(0x3300, 0x3314), /* Port MAC - P3*/
> + regmap_reg_range(0x3400, 0x3414), /* Port MAC - P4 */
> + regmap_reg_range(0x3500, 0x3514), /* Port MAC - P5 */
> + regmap_reg_range(0x3600, 0x3614), /* Port MAC - P6 */
> + regmap_reg_range(0x4000, 0x40d4), /* MIB - P0 */
> + regmap_reg_range(0x4100, 0x41d4), /* MIB - P1 */
> + regmap_reg_range(0x4200, 0x42d4), /* MIB - P2 */
> + regmap_reg_range(0x4300, 0x43d4), /* MIB - P3 */
> + regmap_reg_range(0x4400, 0x44d4), /* MIB - P4 */
> + regmap_reg_range(0x4500, 0x45d4), /* MIB - P5 */
> + regmap_reg_range(0x4600, 0x46d4), /* MIB - P6 */
> + regmap_reg_range(0x4fe0, 0x4ff4), /* SYS */
> + regmap_reg_range(0x7000, 0x700c), /* SYS 2 */
> + regmap_reg_range(0x7018, 0x7028), /* SYS 3 */
> + regmap_reg_range(0x7800, 0x7830), /* SYS 4 */
> + regmap_reg_range(0x7a00, 0x7a7c), /* TRGMII */
> + regmap_reg_range(0x8000, 0x8078), /* Psedo address for Phy - P0 */
> + regmap_reg_range(0x8100, 0x8178), /* Psedo address for Phy - P1 */
> + regmap_reg_range(0x8200, 0x8278), /* Psedo address for Phy - P2 */
> + regmap_reg_range(0x8300, 0x8378), /* Psedo address for Phy - P3 */
> + regmap_reg_range(0x8400, 0x8478), /* Psedo address for Phy - P4 */
> +};

It looks like your regmap accessor are only used for 0x7a00 to 0x7a7c.

It is not clear why you even bother with a regmap. If you have it, why
not use it for all registers within the regmap?

Andrew

2017-03-23 08:07:06

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

Hi Andrew,

The purpose for the regmap table registered is to

provide a way which helps us to look up a specific

register on the switch through regmap-debugfs.


And not all ranges of register is defined

so I only include the meaningful ones in a sparse way

for the table.

Sean


On Thu, 2017-03-23 at 08:22 +0100, Andrew Lunn wrote:
> > +static int
> > +mt7623_trgmii_write(struct mt7530_priv *priv, u32 reg, u32 val)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(priv->ethernet, TRGMII_BASE(reg), val);
> > + if (ret < 0)
> > + dev_err(priv->dev,
> > + "failed to priv write register\n");
> > + return ret;
> > +}
> > +
> > +static u32
> > +mt7623_trgmii_read(struct mt7530_priv *priv, u32 reg)
> > +{
> > + int ret;
> > + u32 val;
> > +
> > + ret = regmap_read(priv->ethernet, TRGMII_BASE(reg), &val);
> > + if (ret < 0) {
> > + dev_err(priv->dev,
> > + "failed to priv read register\n");
> > + return ret;
> > + }
> > +
> > + return val;
> > +}
>
> Hi Sean
>
> These appear to be the only two accessors which use the regmap.
>
> > +static int
> > +mt7530_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> > +{
> > + struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
> > +
> > + /* BIT(15) is used as indication for pseudo registers
> > + * which would be translated into the general MDIO
> > + * access to leverage the unique regmap sys interface.
> > + */
> > + if (reg & BIT(15))
> > + *val = mdiobus_read_nested(priv->bus,
> > + (reg & 0xf00) >> 8,
> > + (reg & 0xff) >> 2);
> > + else
> > + *val = mt7530_read(priv, reg);
> > +
> > + return 0;
> > +}
>
> .....
>
> > +static const struct regmap_range mt7530_readable_ranges[] = {
> > + regmap_reg_range(0x0000, 0x00ac), /* Global control */
> > + regmap_reg_range(0x2000, 0x202c), /* Port Control - P0 */
> > + regmap_reg_range(0x2100, 0x212c), /* Port Control - P1 */
> > + regmap_reg_range(0x2200, 0x222c), /* Port Control - P2 */
> > + regmap_reg_range(0x2300, 0x232c), /* Port Control - P3 */
> > + regmap_reg_range(0x2400, 0x242c), /* Port Control - P4 */
> > + regmap_reg_range(0x2500, 0x252c), /* Port Control - P5 */
> > + regmap_reg_range(0x2600, 0x262c), /* Port Control - P6 */
> > + regmap_reg_range(0x30e0, 0x30f8), /* Port MAC - SYS */
> > + regmap_reg_range(0x3000, 0x3014), /* Port MAC - P0 */
> > + regmap_reg_range(0x3100, 0x3114), /* Port MAC - P1 */
> > + regmap_reg_range(0x3200, 0x3214), /* Port MAC - P2*/
> > + regmap_reg_range(0x3300, 0x3314), /* Port MAC - P3*/
> > + regmap_reg_range(0x3400, 0x3414), /* Port MAC - P4 */
> > + regmap_reg_range(0x3500, 0x3514), /* Port MAC - P5 */
> > + regmap_reg_range(0x3600, 0x3614), /* Port MAC - P6 */
> > + regmap_reg_range(0x4000, 0x40d4), /* MIB - P0 */
> > + regmap_reg_range(0x4100, 0x41d4), /* MIB - P1 */
> > + regmap_reg_range(0x4200, 0x42d4), /* MIB - P2 */
> > + regmap_reg_range(0x4300, 0x43d4), /* MIB - P3 */
> > + regmap_reg_range(0x4400, 0x44d4), /* MIB - P4 */
> > + regmap_reg_range(0x4500, 0x45d4), /* MIB - P5 */
> > + regmap_reg_range(0x4600, 0x46d4), /* MIB - P6 */
> > + regmap_reg_range(0x4fe0, 0x4ff4), /* SYS */
> > + regmap_reg_range(0x7000, 0x700c), /* SYS 2 */
> > + regmap_reg_range(0x7018, 0x7028), /* SYS 3 */
> > + regmap_reg_range(0x7800, 0x7830), /* SYS 4 */
> > + regmap_reg_range(0x7a00, 0x7a7c), /* TRGMII */
> > + regmap_reg_range(0x8000, 0x8078), /* Psedo address for Phy - P0 */
> > + regmap_reg_range(0x8100, 0x8178), /* Psedo address for Phy - P1 */
> > + regmap_reg_range(0x8200, 0x8278), /* Psedo address for Phy - P2 */
> > + regmap_reg_range(0x8300, 0x8378), /* Psedo address for Phy - P3 */
> > + regmap_reg_range(0x8400, 0x8478), /* Psedo address for Phy - P4 */
> > +};
>
> It looks like your regmap accessor are only used for 0x7a00 to 0x7a7c.
>
> It is not clear why you even bother with a regmap. If you have it, why
> not use it for all registers within the regmap?
>
> Andrew


2017-03-23 14:09:18

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 2017-03-23 09:06, Sean Wang wrote:
> Hi Andrew,
>
> The purpose for the regmap table registered is to
>
> provide a way which helps us to look up a specific
>
> register on the switch through regmap-debugfs.
>
>
> And not all ranges of register is defined
>
> so I only include the meaningful ones in a sparse way
>
> for the table.
I think in that case it might be nice to make regmap support optional in
order to avoid pulling in bloat on platforms that don't need it.

- Felix


2017-03-23 14:26:05

by John Crispin

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch



On 23/03/17 15:09, Felix Fietkau wrote:
> On 2017-03-23 09:06, Sean Wang wrote:
>> Hi Andrew,
>>
>> The purpose for the regmap table registered is to
>>
>> provide a way which helps us to look up a specific
>>
>> register on the switch through regmap-debugfs.
>>
>>
>> And not all ranges of register is defined
>>
>> so I only include the meaningful ones in a sparse way
>>
>> for the table.
> I think in that case it might be nice to make regmap support optional in
> order to avoid pulling in bloat on platforms that don't need it.
>
> - Felix
>
The 2 relevant platforms are mips/ralink and arm/mediatek. both require
regmap for the eth_sysctl syscon if they want to utilize the mtk_soc_eth
driver which is a prereq for mt7530. so regmap cannot be optional here.

John


>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2017-03-23 14:30:26

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 2017-03-23 15:25, John Crispin wrote:
>
>
> On 23/03/17 15:09, Felix Fietkau wrote:
>> On 2017-03-23 09:06, Sean Wang wrote:
>>> Hi Andrew,
>>>
>>> The purpose for the regmap table registered is to
>>>
>>> provide a way which helps us to look up a specific
>>>
>>> register on the switch through regmap-debugfs.
>>>
>>>
>>> And not all ranges of register is defined
>>>
>>> so I only include the meaningful ones in a sparse way
>>>
>>> for the table.
>> I think in that case it might be nice to make regmap support optional in
>> order to avoid pulling in bloat on platforms that don't need it.
>>
>> - Felix
>>
> The 2 relevant platforms are mips/ralink and arm/mediatek. both require
> regmap for the eth_sysctl syscon if they want to utilize the mtk_soc_eth
> driver which is a prereq for mt7530. so regmap cannot be optional here.
Makes sense, thanks.

- Felix

2017-03-24 07:56:35

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On Thu, Mar 23, 2017 at 04:06:56PM +0800, Sean Wang wrote:
> Hi Andrew,
>
> The purpose for the regmap table registered is to
>
> provide a way which helps us to look up a specific
>
> register on the switch through regmap-debugfs.
>
>
> And not all ranges of register is defined
>
> so I only include the meaningful ones in a sparse way
>
> for the table.

Many of there registers can be dumped using existing tools. You can
dump the port registers using ethtool -r, if you implement
get_regs/get_regs_len in your driver. mii-tool can dump the PHY
registers.

What you cannot see are global registers, so i can understand the
usage of regmap-debugfs. However, i have a hard time with you only
actually using the regmap for get/set for one small subset of
registers. Either you need to use regmap to get/set all registers, or
you remove it from the mainline driver, and keep it as a private patch
which you use for your development work. For the Marvell driver we
have an out of tree patch which exports a log of information via
debugfs.

Andrew

2017-03-24 14:02:50

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

Hi Sean

> + regmap = devm_regmap_init(ds->dev, NULL, priv,
> + &mt7530_regmap_config);
> + if (IS_ERR(regmap))
> + dev_warn(priv->dev, "phy regmap initialization failed");
> +

Shouldn't this be a fatal error? If you keep going when there is an
error, what happens when you actually try to use the regmap?

> + phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> + if (phy_mode < 0) {
> + dev_err(priv->dev, "Can't find phy-mode for master device\n");
> + return phy_mode;
> + }
> + dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);

dev_dbg?

> +
> + id = mt7530_read(priv, MT7530_CREV);
> + id >>= CHIP_NAME_SHIFT;
> + if (id != MT7530_ID)
> + return -ENODEV;

It might be helpful to say what ID has been found, if it is not the
supported ID.

Andrew

2017-03-24 14:20:41

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On Tue, Mar 21, 2017 at 05:35:10PM +0800, [email protected] wrote:

Hi Sean

> + /* Lower Tx Driving */
> + for (i = 0 ; i < 6 ; i++)

Could MT7530_CPU_PORT be used here?

> + mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
> + TD_DM_DRVP(8) | TD_DM_DRVN(8));
> +
> + /* Setup MT7530 core clock */
> + if (!trgint) {
> + /* Disable MT7530 core clock */
> + core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> +
> + /* Disable MT7530 PLL, since phy_device has not yet been
> + * created when this function is called. So we provide
> + * core_write_mmd_indirect to complete this function
> + */
> + core_write_mmd_indirect(priv,
> + CORE_GSWPLL_GRP1,
> + MDIO_MMD_VEND2,
> + 0);
> +
> + /* Setup MT7530 core clock into 500Mhz */
> + core_write(priv, CORE_GSWPLL_GRP2,
> + RG_GSWPLL_POSDIV_500M(1) |
> + RG_GSWPLL_FBKDIV_500M(25));
> +
> + /* Enable MT7530 PLL */
> + core_write(priv, CORE_GSWPLL_GRP1,
> + RG_GSWPLL_EN_PRE |
> + RG_GSWPLL_POSDIV_200M(2) |
> + RG_GSWPLL_FBKDIV_200M(32));
> +
> + /* Enable MT7530 core clock */
> + core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> + }
> +
> + /* Setup the MT7530 TRGMII Tx Clock */
> + core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> + core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
> + core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
> + core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
> + core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta));
> + core_write(priv, CORE_PLL_GROUP4,
> + RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN |
> + RG_SYSPLL_BIAS_LPF_EN);
> + core_write(priv, CORE_PLL_GROUP2,
> + RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN |
> + RG_SYSPLL_POSDIV(1));
> + core_write(priv, CORE_PLL_GROUP7,
> + RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
> + RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
> + core_set(priv, CORE_TRGMII_GSW_CLK_CG,
> + REG_GSWCK_EN | REG_TRGMIICK_EN);
> +
> + if (!trgint)
> + for (i = 0 ; i < 5 ; i++)

Why only 5 here? All other similar loops are to 6. Replacing 5 with a
#define might help make this more readable.

> + mt7530_rmw(priv, MT7530_TRGMII_RD(i),
> + RD_TAP_MASK, RD_TAP(16));
> + else
> + mt7623_trgmii_set(priv, GSW_INTF_MODE, INTF_MODE_TRGMII);
> +
> + return 0;
> +}
> +
> +static int
> +mt7623_pad_clk_setup(struct dsa_switch *ds)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + int i;
> +
> + for (i = 0 ; i < 6; i++)

MT7530_CPU_PORT?

> +
> +/* Registers to mac forward conrol for unknown frames */

/conrol/control

> +
> +/* Registor for port control */

Register

> +/* Regiser for TOP signal control */

Register

> +/* struct mt7530_priv - This is the main datasructure for holding the state

data structure

> + * of the driver
> + * @dev: The device pointer
> + * @ds: The pointer to the dsa core structure
> + * @bus: The bus used for the device and built-in PHY
> + * @ethsys: The regmap used for enabling the necessary PLL
> + * @ethernet: The regmap used for access TRGMII-based registers
> + * @core_pwr: The power supplied into the core
> + * @io_pwr: The power supplied into the I/O
> + * @mcm: Flag for distinguishing if standalone IC or module
> + * coupling
> + * @reset: The descriptor for GPIO line tied to its reset pin
> + * @phy_mode: The xMII for cpu port used
> + * @ports: Holding the state amongs ports

among

Andrew

2017-03-24 16:16:53

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH net-next v2 1/5] dt-bindings: net: dsa: add Mediatek MT7530 binding

On Tue, Mar 21, 2017 at 05:35:06PM +0800, [email protected] wrote:
> From: Sean Wang <[email protected]>
>
> Add device-tree binding for Mediatek MT7530 switch.
>
> Cc: [email protected]
> Signed-off-by: Sean Wang <[email protected]>
> ---
> .../devicetree/bindings/net/dsa/mt7530.txt | 92 ++++++++++++++++++++++
> 1 file changed, 92 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

Acked-by: Rob Herring <[email protected]>

2017-03-28 05:51:53

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

Hi Florian,

Thank for taking your time on reviewing. Add comment as inline.

On Wed, 2017-03-22 at 11:39 -0700, Florian Fainelli wrote:
> On 03/21/2017 02:35 AM, [email protected] wrote:
> > From: Sean Wang <[email protected]>
> >
> > MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
> > Mediatek router platforms such as MT7623A or MT7623N platform which
> > includes 7-port Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY.
> > Among these ports, The port from 0 to 4 are the user ports connecting
> > with the remote devices while the port 5 and 6 are the CPU ports
> > connecting into Mediatek Ethernet GMAC.
> >
> > For port 6, it can communicate with the CPU via Mediatek Ethernet GMAC
> > through either the TRGMII or RGMII which could be controlled by phy-mode
> > in the dt-bindings to specify which mode is preferred to use. And for
> > port 5, only RGMII can be specified. However, currently, only port 6 is
> > being supported in this DSA driver.
> >
> > The driver is made with the reference to qca8k and other existing DSA
> > driver. The most of the essential callbacks of the DSA are already
> > support in the driver, including tag insert for user port distinguishing,
> > port control, bridge offloading, STP setup and ethtool operation to allow
> > DSA to model each user port into a standalone netdevice as the other DSA
> > driver had done.
>
> Overall, this looks pretty nice and clean, a few comments below
>
> >
> > Signed-off-by: Sean Wang <[email protected]>
> > Signed-off-by: Landen Chao <[email protected]>
> > ---
>
> > +static void
> > +mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
> > +{
> > + u32 reg[3];
> > + int i;
> > +
> > + /* Read from ARL table into an array */
> > + for (i = 0; i < 3; i++) {
> > + reg[i] = mt7530_read(priv, MT7530_TSRA1 + (i * 4));
> > +
> > + dev_dbg(priv->dev, "%s(%d) reg[%d]=0x%x\n",
> > + __func__, __LINE__, i, reg[i]);
> > + }
> > +
> > + /* vid - 11:0 on reg[1] */
> > + fdb->vid = (reg[1] >> 0) & 0xfff;
> > + /* aging - 31:24 on reg[2] */
> > + fdb->aging = (reg[2] >> 24) & 0xff;
> > + /* portmask - 11:4 on reg[2] */
> > + fdb->port_mask = (reg[2] >> 4) & 0xff;
> > + /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> > + fdb->mac[0] = (reg[0] >> 24) & 0xff;
> > + fdb->mac[1] = (reg[0] >> 16) & 0xff;
> > + fdb->mac[2] = (reg[0] >> 8) & 0xff;
> > + fdb->mac[3] = (reg[0] >> 0) & 0xff;
> > + fdb->mac[4] = (reg[1] >> 24) & 0xff;
> > + fdb->mac[5] = (reg[1] >> 16) & 0xff;
> > + /* noarp - 3:2 on reg[2] */
> > + fdb->noarp = ((reg[2] >> 2) & 0x3) == STATIC_ENT;
>
> Could you add some definitions for the bits and masks that you are
> shifting here?
>

Okay, I'll make into proper macro for readability

> > +}
> > +
> > +static void
> > +mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
> > + u8 port_mask, const u8 *mac,
> > + u8 aging, u8 type)
> > +{
> > + u32 reg[3] = { 0 };
> > + int i;
> > +
> > + /* vid - 11:0 on reg[1] */
> > + reg[1] |= (vid & 0xfff) << 0;
> > + /* aging - 31:25 on reg[2] */
> > + reg[2] |= (aging & 0xff) << 24;
> > + /* portmask - 11:4 on reg[2] */
> > + reg[2] |= (port_mask & 0xff) << 4;
> > + /* type - 3 indicate that entry is static wouldn't
> > + * be aged out and 0 specified as erasing an entry
> > + */
> > + reg[2] |= (type & 0x3) << 2;
> > + /* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> > + reg[1] |= mac[5] << 16;
> > + reg[1] |= mac[4] << 24;
> > + reg[0] |= mac[3] << 0;
> > + reg[0] |= mac[2] << 8;
> > + reg[0] |= mac[1] << 16;
> > + reg[0] |= mac[0] << 24;
> > +
> > + /* Wrirte array into the ARL table */
> > + for (i = 0; i < 3; i++)
> > + mt7530_write(priv, MT7530_ATA1 + (i * 4), reg[i]);
> > +}
>
> Same here.
>

As above. I will improve them.


> > +
> > +static int
> > +mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + u32 ncpo1, ssc_delta, trgint, i;
> > +
> > + switch (mode) {
> > + case PHY_INTERFACE_MODE_RGMII:
> > + trgint = 0;
> > + ncpo1 = 0x0c80;
> > + ssc_delta = 0x87;
> > + break;
> > + case PHY_INTERFACE_MODE_TRGMII:
> > + trgint = 1;
> > + ncpo1 = 0x1400;
> > + ssc_delta = 0x57;
> > + break;
> > + default:
> > + pr_err("xMII mode %d not supported\n", mode);
> > + return -EINVAL;
> > + }
>
> You may be able to move this to an adjust_link callback that the PHY
> library would call when the PHY gets setup and the port is finally used,
> as opposed to doing this upfront during driver initialization.
>


Good point. i will follow up


>
> > +mt7530_setup(struct dsa_switch *ds)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + int ret, i, phy_mode;
> > + u8 cpup_mask = 0;
> > + u32 id, val;
> > + struct regmap *regmap;
> > + struct device_node *dn;
> > +
> > + /* Make sure that cpu port specfied on the dt is appropriate */
> > + if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
> > + dev_err(priv->dev, "port not matched with the CPU port\n");
> > + return -EINVAL;
> > + }
>
> This is kind of a hard error, in that case, a sensible thing to do could
> be issue a warning to the user telling that the configuration does not
> permit the use of Mediatek tags. Your get_tag_protocol() function could
> then return DSA_TAG_PROTO_NONE here instead of DSA_TAG_PROTO_MTK. It
> would still allow an user to utilize the switch, and we would know what
> is wrong with the configuration/board setup though.
>

I seems making a hard requirement.

I will improve it with showing warning message if the number of cpu port
is not right.

It at least allows the other user ports can work in case cpu port
doesn't work.


> > +
> > + /* The parent node of master_netdev which holds the common system
> > + * controller also is the container for two GMACs nodes representing
> > + * as two netdev instances.
> > + */
> > + dn = ds->master_netdev->dev.of_node->parent;
> > + priv->ethernet = syscon_node_to_regmap(dn);
> > + if (IS_ERR(priv->ethernet))
> > + return PTR_ERR(priv->ethernet);
> > +
> > + regmap = devm_regmap_init(ds->dev, NULL, priv,
> > + &mt7530_regmap_config);
> > + if (IS_ERR(regmap))
> > + dev_warn(priv->dev, "phy regmap initialization failed");
> > +
> > + phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> > + if (phy_mode < 0) {
> > + dev_err(priv->dev, "Can't find phy-mode for master device\n");
> > + return phy_mode;
> > + }
> > + dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
>
> An adjust_link() callback which has a proper PHY device structure would
> be more appropriate to look up the phy_mode rather than doing this here.
>

As above. I will follow up

> [snip]
>
> > + mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK);
> > +
> > + /* Fabric setup for the cpu port */
> > + for (i = 0; i < MT7530_NUM_PORTS; i++)
>
> Are not you missing an opening parenthesis here in the for() statement?
>

It is missing because the following is single statement.
I admits it is better to add the parenthesis for readability, so I will
add it.

> > + if (dsa_is_cpu_port(ds, i)) {
> > + /* Enable Mediatek header mode on the cpu port */
> > + mt7530_write(priv, MT7530_PVC_P(i),
> > + PORT_SPEC_TAG);
> > +
> > + /* Setup the MAC by default for the cpu port */
> > + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_CPUP_LINK);
> > +
> > + /* Disable auto learning on the cpu port */
> > + mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
> > +
> > + /* Unknown unicast frame fordwarding to the cpu port */
> > + mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(i)));
> > +
> > + /* CPU port gets connected to all user ports of
> > + * the switch
> > + */
> > + mt7530_write(priv, MT7530_PCR_P(i),
> > + PCR_MATRIX(ds->enabled_port_mask));
> > +
> > + cpup_mask |= BIT(i);
> > + }
> > +
> > + /* Fabric setup for the all user ports */
> > + for (i = 0; i < MT7530_NUM_PORTS; i++)
> > + if (ds->enabled_port_mask & BIT(i)) {
> > + /* Setup the MAC by default for all user ports */
> > + mt7530_write(priv, MT7530_PMCR_P(i),
> > + PMCR_USERP_LINK);
> > +
> > + /* The user port gets connected to the cpu port only */
> > + mt7530_write(priv, MT7530_PCR_P(i),
> > + PCR_MATRIX(cpup_mask));
> > + }
>
> This should be moved to the port_enable() function.
>

okay. i will follow up.

> [snip]
*_poll_timeout
> > +#define wait_condition_timeout(condition, timeout) \
> > +({ \
> > + long __ret = 0; \
> > + unsigned long toj; \
> > + toj = jiffies + msecs_to_jiffies(timeout); \
> > + \
> > + for ( ; !(condition) && !time_after_eq(jiffies, toj) ; ) \
> > + cond_resched(); \
> > + \
> > + if (time_after_eq(jiffies, toj)) \
> > + __ret = -ETIMEDOUT; \
> > + __ret; \
> > +})
>
> Can you use read*_poll_timeout() instead of this?
>

I will try to reuse them

the function pointer for op in *_poll_timeout seems only accepting
only single argument for address. which implies I need creating another
function for mt7530_readX() used by *_poll_timeout and keeping another
static pointer to mt7530 internal for mt7530_readX() to work.


> > +/* struct mt7530_priv - This is the main datasructure for holding the state
> > + * of the driver
> > + * @dev: The device pointer
> > + * @ds: The pointer to the dsa core structure
> > + * @bus: The bus used for the device and built-in PHY
> > + * @ethsys: The regmap used for enabling the necessary PLL
> > + * @ethernet: The regmap used for access TRGMII-based registers
> > + * @core_pwr: The power supplied into the core
> > + * @io_pwr: The power supplied into the I/O
> > + * @mcm: Flag for distinguishing if standalone IC or module
> > + * coupling
> > + * @reset: The descriptor for GPIO line tied to its reset pin
> > + * @phy_mode: The xMII for cpu port used
> > + * @ports: Holding the state amongs ports
> > + * @reg_mutex: The lock for protecting among process accessing
> > + * registers
> > + */
>
> Kudos for using kernel doc here.


2017-03-28 06:05:29

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch


Hi Andrew,

Add comment as below inline


On Fri, 2017-03-24 at 15:02 +0100, Andrew Lunn wrote:
> Hi Sean
>
> > + regmap = devm_regmap_init(ds->dev, NULL, priv,
> > + &mt7530_regmap_config);
> > + if (IS_ERR(regmap))
> > + dev_warn(priv->dev, "phy regmap initialization failed");
> > +
>
> Shouldn't this be a fatal error? If you keep going when there is an
> error, what happens when you actually try to use the regmap?
>

Initial thought is that it is just for debugging purpose so if it fails,
it should break any functionality of switch and only have implication as
a warning message.

Anyway, i will remove it in the next one since it seems better being
kept in private place as you suggested in the previous mail.

> > + phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> > + if (phy_mode < 0) {
> > + dev_err(priv->dev, "Can't find phy-mode for master device\n");
> > + return phy_mode;
> > + }
> > + dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
>
> dev_dbg?
>

Sorry. i forgot turned it into dev_dbg

> > +
> > + id = mt7530_read(priv, MT7530_CREV);
> > + id >>= CHIP_NAME_SHIFT;
> > + if (id != MT7530_ID)
> > + return -ENODEV;
>
> It might be helpful to say what ID has been found, if it is not the
> supported ID.


I will fix it up to make users know what was going on

thanks for taking your time on those reviewing again!


> Andrew


2017-03-28 06:50:27

by Sean Wang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

Hi Andrew


On Fri, 2017-03-24 at 15:19 +0100, Andrew Lunn wrote:
> On Tue, Mar 21, 2017 at 05:35:10PM +0800, [email protected] wrote:
>
> Hi Sean
>
> > + /* Lower Tx Driving */
> > + for (i = 0 ; i < 6 ; i++)
>
> Could MT7530_CPU_PORT be used here?
>

I should create meaningful definition instead of avoiding hard coding

where 6 should be corrected into 5 which is meant for the number for

control register to configure TRGMII path


> > + mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
> > + TD_DM_DRVP(8) | TD_DM_DRVN(8));
> > +
> > + /* Setup MT7530 core clock */
> > + if (!trgint) {
> > + /* Disable MT7530 core clock */
> > + core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> > +
> > + /* Disable MT7530 PLL, since phy_device has not yet been
> > + * created when this function is called. So we provide
> > + * core_write_mmd_indirect to complete this function
> > + */
> > + core_write_mmd_indirect(priv,
> > + CORE_GSWPLL_GRP1,
> > + MDIO_MMD_VEND2,
> > + 0);
> > +
> > + /* Setup MT7530 core clock into 500Mhz */
> > + core_write(priv, CORE_GSWPLL_GRP2,
> > + RG_GSWPLL_POSDIV_500M(1) |
> > + RG_GSWPLL_FBKDIV_500M(25));
> > +
> > + /* Enable MT7530 PLL */
> > + core_write(priv, CORE_GSWPLL_GRP1,
> > + RG_GSWPLL_EN_PRE |
> > + RG_GSWPLL_POSDIV_200M(2) |
> > + RG_GSWPLL_FBKDIV_200M(32));
> > +
> > + /* Enable MT7530 core clock */
> > + core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> > + }
> > +
> > + /* Setup the MT7530 TRGMII Tx Clock */
> > + core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> > + core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
> > + core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
> > + core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
> > + core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta));
> > + core_write(priv, CORE_PLL_GROUP4,
> > + RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN |
> > + RG_SYSPLL_BIAS_LPF_EN);
> > + core_write(priv, CORE_PLL_GROUP2,
> > + RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN |
> > + RG_SYSPLL_POSDIV(1));
> > + core_write(priv, CORE_PLL_GROUP7,
> > + RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
> > + RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
> > + core_set(priv, CORE_TRGMII_GSW_CLK_CG,
> > + REG_GSWCK_EN | REG_TRGMIICK_EN);
> > +
> > + if (!trgint)
> > + for (i = 0 ; i < 5 ; i++)
>
> Why only 5 here? All other similar loops are to 6. Replacing 5 with a
> #define might help make this more readable.

the same as above, it should be corrected

> > + mt7530_rmw(priv, MT7530_TRGMII_RD(i),
> > + RD_TAP_MASK, RD_TAP(16));
> > + else
> > + mt7623_trgmii_set(priv, GSW_INTF_MODE, INTF_MODE_TRGMII);
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +mt7623_pad_clk_setup(struct dsa_switch *ds)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + int i;
> > +
> > + for (i = 0 ; i < 6; i++)
>
> MT7530_CPU_PORT?


the same as above , it should be corrected
> > +
> > +/* Registers to mac forward conrol for unknown frames */
>
> /conrol/control
>

will fix it up
> > +
> > +/* Registor for port control */
>
> Register
>

will fix it up
> > +/* Regiser for TOP signal control */
>
> Register
>

will fix it up
> > +/* struct mt7530_priv - This is the main datasructure for holding the state
>
> data structure
>
> > + * of the driver
> > + * @dev: The device pointer
> > + * @ds: The pointer to the dsa core structure
> > + * @bus: The bus used for the device and built-in PHY
> > + * @ethsys: The regmap used for enabling the necessary PLL
> > + * @ethernet: The regmap used for access TRGMII-based registers
> > + * @core_pwr: The power supplied into the core
> > + * @io_pwr: The power supplied into the I/O
> > + * @mcm: Flag for distinguishing if standalone IC or module
> > + * coupling
> > + * @reset: The descriptor for GPIO line tied to its reset pin
> > + * @phy_mode: The xMII for cpu port used
> > + * @ports: Holding the state amongs ports
>
> among

will fix it up

thank for your careful reviewing again!!



> Andrew