2023-03-28 22:19:49

by Daniel Golle

[permalink] [raw]
Subject: [RFC PATCH net-next v2 0/2] net: dsa: add support for MT7988

The MediaTek MT7988 SoC comes with a built-in switch very similar to
previous MT7530 and MT7531. However, the switch address space is mapped
into the SoCs memory space rather than being connected via MDIO.
Using MMIO simplifies register access and also removes the need for a bus
lock, and for that reason also makes interrupt handling more light-weight.

Note that this is different from previous SoCs like MT7621 and MT7623N
which also came with an integrated MT7530-like switch which yet had to be
accessed via MDIO.

Split-off the part of the driver registering an MDIO driver, then add
another module acting as MMIO/platform driver.

Changes since initial RFC:
* use regmap for register access and move register access to bus-
specific driver
* move initialization of MT7531 SGMII PCS to MDIO driver

Daniel Golle (2):
net: dsa: mt7530: split-off MDIO driver
net: dsa: mt7530: introduce MMIO driver for MT7988 SoC

drivers/net/dsa/Kconfig | 16 +-
drivers/net/dsa/Makefile | 4 +-
drivers/net/dsa/mt7530-mdio.c | 309 ++++++++++++++++++++++
drivers/net/dsa/mt7530-mmio.c | 143 ++++++++++
drivers/net/dsa/mt7530.c | 479 ++++++++++++++--------------------
drivers/net/dsa/mt7530.h | 38 +--
6 files changed, 686 insertions(+), 303 deletions(-)
create mode 100644 drivers/net/dsa/mt7530-mdio.c
create mode 100644 drivers/net/dsa/mt7530-mmio.c

--
2.39.2


2023-03-28 22:20:49

by Daniel Golle

[permalink] [raw]
Subject: [RFC PATCH net-next v2 1/2] net: dsa: mt7530: split-off MDIO driver

In order to support the built-in switch of some MediaTek SoCs we need
to use MMIO instead of MDIO to access the switch.
Prepare this be splitting-off the part of the driver registering an
MDIO driver, so we can add another module acting as MMIO/platform driver.

Signed-off-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/Kconfig | 8 +-
drivers/net/dsa/Makefile | 3 +-
drivers/net/dsa/mt7530-mdio.c | 309 ++++++++++++++++++++++++++++++++++
drivers/net/dsa/mt7530.c | 286 ++++---------------------------
drivers/net/dsa/mt7530.h | 24 +--
5 files changed, 356 insertions(+), 274 deletions(-)
create mode 100644 drivers/net/dsa/mt7530-mdio.c

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 6b45fa8b69078..0d9d605b3882c 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -35,9 +35,15 @@ config NET_DSA_LANTIQ_GSWIP
the xrx200 / VR9 SoC.

config NET_DSA_MT7530
- tristate "MediaTek MT7530 and MT7531 Ethernet switch support"
+ tristate "MediaTek Ethernet switch support"
select NET_DSA_TAG_MTK
select MEDIATEK_GE_PHY
+ help
+ This enables support for the MediaTek Ethernet switch chips.
+
+config NET_DSA_MT7530_MDIO
+ tristate "MediaTek MT7530 and MT7531 Ethernet switch support"
+ select NET_DSA_MT7530
select PCS_MTK_LYNXI
help
This enables support for the MediaTek MT7530 and MT7531 Ethernet
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 16eb879e0cb4d..2b072d574ed02 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -6,7 +6,8 @@ ifdef CONFIG_NET_DSA_LOOP
obj-$(CONFIG_FIXED_PHY) += dsa_loop_bdinfo.o
endif
obj-$(CONFIG_NET_DSA_LANTIQ_GSWIP) += lantiq_gswip.o
-obj-$(CONFIG_NET_DSA_MT7530) += mt7530.o
+obj-$(CONFIG_NET_DSA_MT7530) += mt7530.o
+obj-$(CONFIG_NET_DSA_MT7530_MDIO) += mt7530-mdio.o
obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
obj-$(CONFIG_NET_DSA_RZN1_A5PSW) += rzn1_a5psw.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
new file mode 100644
index 0000000000000..203dc28697204
--- /dev/null
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/gpio/consumer.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/pcs/pcs-mtk-lynxi.h>
+#include <linux/of_irq.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <net/dsa.h>
+
+#include "mt7530.h"
+
+static const struct of_device_id mt7530_of_match[] = {
+ { .compatible = "mediatek,mt7621", .data = &mt753x_table[ID_MT7621], },
+ { .compatible = "mediatek,mt7530", .data = &mt753x_table[ID_MT7530], },
+ { .compatible = "mediatek,mt7531", .data = &mt753x_table[ID_MT7531], },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mt7530_of_match);
+
+static int mt7530_regmap_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct mii_bus *bus = context;
+ 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);
+
+ *val = (hi << 16) | (lo & 0xffff);
+
+ return 0;
+};
+
+static int mt7530_regmap_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct mii_bus *bus = context;
+ 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 const struct regmap_bus mt7530_mdio_regmap_bus = {
+ .reg_write = mt7530_regmap_write,
+ .reg_read = mt7530_regmap_read,
+};
+
+static void
+mt7530_mdio_regmap_lock(void *mdio_lock)
+{
+ mutex_lock_nested(mdio_lock, MDIO_MUTEX_NESTED);
+}
+
+static void
+mt7530_mdio_regmap_unlock(void *mdio_lock)
+{
+ mutex_unlock(mdio_lock);
+}
+
+static int
+mt7531_create_sgmii(struct mt7530_priv *priv)
+{
+ struct regmap_config *mt7531_pcs_config[2];
+ struct phylink_pcs *pcs;
+ struct regmap *regmap;
+ int i, ret = 0;
+
+ for (i = 0; i < 2; i++) {
+ mt7531_pcs_config[i] = devm_kzalloc(priv->dev,
+ sizeof(struct regmap_config),
+ GFP_KERNEL);
+ if (!mt7531_pcs_config[i]) {
+ ret = -ENOMEM;
+ break;
+ }
+
+ mt7531_pcs_config[i]->name = i ? "port6" : "port5";
+ mt7531_pcs_config[i]->reg_bits = 16;
+ mt7531_pcs_config[i]->val_bits = 32;
+ mt7531_pcs_config[i]->reg_stride = 4;
+ mt7531_pcs_config[i]->reg_base = MT7531_SGMII_REG_BASE(5 + i);
+ mt7531_pcs_config[i]->max_register = 0x17c;
+ mt7531_pcs_config[i]->lock = mt7530_mdio_regmap_lock;
+ mt7531_pcs_config[i]->unlock = mt7530_mdio_regmap_unlock;
+ mt7531_pcs_config[i]->lock_arg = &priv->bus->mdio_lock;
+
+ regmap = devm_regmap_init(priv->dev,
+ &mt7530_mdio_regmap_bus, priv->bus,
+ mt7531_pcs_config[i]);
+ if (IS_ERR(regmap)) {
+ ret = PTR_ERR(regmap);
+ break;
+ }
+ pcs = mtk_pcs_lynxi_create(priv->dev, regmap,
+ MT7531_PHYA_CTRL_SIGNAL3, 0);
+ if (!pcs) {
+ ret = -ENXIO;
+ break;
+ }
+ priv->ports[5 + i].sgmii_pcs = pcs;
+ }
+
+ if (ret && i)
+ mtk_pcs_lynxi_destroy(priv->ports[5].sgmii_pcs);
+
+ return ret;
+}
+
+static int
+mt7530_probe(struct mdio_device *mdiodev)
+{
+ static struct regmap_config *sw_regmap_config;
+ struct mt7530_priv *priv;
+ struct device_node *dn;
+ int ret;
+
+ dn = mdiodev->dev.of_node;
+
+ priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
+ if (!priv->ds)
+ return -ENOMEM;
+
+ priv->ds->dev = &mdiodev->dev;
+ priv->ds->num_ports = MT7530_NUM_PORTS;
+
+ /* 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);
+ }
+ }
+
+ /* Get the hardware identifier from the devicetree node.
+ * We will need it for some of the clock and regulator setup.
+ */
+ priv->info = of_device_get_match_data(&mdiodev->dev);
+ if (!priv->info)
+ return -EINVAL;
+
+ /* Sanity check if these required device operations are filled
+ * properly.
+ */
+ if (!priv->info->sw_setup || !priv->info->pad_setup ||
+ !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
+ !priv->info->mac_port_get_caps ||
+ !priv->info->mac_port_config)
+ return -EINVAL;
+
+ priv->id = priv->info->id;
+
+ if (priv->id == ID_MT7530) {
+ 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);
+
+ sw_regmap_config = devm_kzalloc(&mdiodev->dev, sizeof(*sw_regmap_config), GFP_KERNEL);
+ if (!sw_regmap_config)
+ return -ENOMEM;
+
+ sw_regmap_config->name = "switch";
+ sw_regmap_config->reg_bits = 16;
+ sw_regmap_config->val_bits = 32;
+ sw_regmap_config->reg_stride = 4;
+ sw_regmap_config->reg_base = 0x0;
+ sw_regmap_config->max_register = 0x7ffc;
+ sw_regmap_config->disable_locking = true;
+ priv->regmap = devm_regmap_init(priv->dev, &mt7530_mdio_regmap_bus,
+ priv->bus, sw_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ if (priv->id == ID_MT7531) {
+ ret = mt7531_create_sgmii(priv);
+ if (ret)
+ return ret;
+ }
+
+ return dsa_register_switch(priv->ds);
+}
+
+static void
+mt7530_remove(struct mdio_device *mdiodev)
+{
+ struct mt7530_priv *priv = dev_get_drvdata(&mdiodev->dev);
+ int ret = 0, i;
+
+ if (!priv)
+ return;
+
+ 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);
+
+ if (priv->irq)
+ mt7530_free_irq(priv);
+
+ dsa_unregister_switch(priv->ds);
+
+ for (i = 0; i < 2; ++i)
+ mtk_pcs_lynxi_destroy(priv->ports[5 + i].sgmii_pcs);
+
+ mutex_destroy(&priv->reg_mutex);
+}
+
+static void mt7530_shutdown(struct mdio_device *mdiodev)
+{
+ struct mt7530_priv *priv = dev_get_drvdata(&mdiodev->dev);
+
+ if (!priv)
+ return;
+
+ dsa_switch_shutdown(priv->ds);
+
+ dev_set_drvdata(&mdiodev->dev, NULL);
+}
+
+static struct mdio_driver mt7530_mdio_driver = {
+ .probe = mt7530_probe,
+ .remove = mt7530_remove,
+ .shutdown = mt7530_shutdown,
+ .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 (MDIO)");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a0d99af897ace..ca379f4d3cd32 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -14,7 +14,6 @@
#include <linux/of_mdio.h>
#include <linux/of_net.h>
#include <linux/of_platform.h>
-#include <linux/pcs/pcs-mtk-lynxi.h>
#include <linux/phylink.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
@@ -185,54 +184,20 @@ core_clear(struct mt7530_priv *priv, u32 reg, u32 val)
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;
+ return regmap_write(priv->regmap, reg, val);
}

static u32
mt7530_mii_read(struct mt7530_priv *priv, u32 reg)
{
- struct mii_bus *bus = priv->bus;
- u16 page, r, lo, hi;
+ u32 val;
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");
+ ret = regmap_read(priv->regmap, reg, &val);
+ if (!ret)
+ return val;
+ else
return ret;
- }
-
- lo = bus->read(bus, 0x1f, r);
- hi = bus->read(bus, 0x1f, 0x10);
-
- return (hi << 16) | (lo & 0xffff);
}

static void
@@ -282,14 +247,10 @@ 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);
+ regmap_update_bits(priv->regmap, reg, mask, set);

mutex_unlock(&bus->mdio_lock);
}
@@ -297,7 +258,7 @@ mt7530_rmw(struct mt7530_priv *priv, u32 reg,
static void
mt7530_set(struct mt7530_priv *priv, u32 reg, u32 val)
{
- mt7530_rmw(priv, reg, 0, val);
+ mt7530_rmw(priv, reg, val, val);
}

static void
@@ -921,6 +882,24 @@ mt7530_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
return 0;
}

+static const char *p5_intf_modes(unsigned int p5_interface)
+{
+ switch (p5_interface) {
+ case P5_DISABLED:
+ return "DISABLED";
+ case P5_INTF_SEL_PHY_P0:
+ return "PHY P0";
+ case P5_INTF_SEL_PHY_P4:
+ return "PHY P4";
+ case P5_INTF_SEL_GMAC5:
+ return "GMAC5";
+ case P5_INTF_SEL_GMAC5_SGMII:
+ return "GMAC5_SGMII";
+ default:
+ return "unknown";
+ }
+}
+
static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
@@ -2067,12 +2046,13 @@ mt7530_free_irq_common(struct mt7530_priv *priv)
irq_domain_remove(priv->irq_domain);
}

-static void
+void
mt7530_free_irq(struct mt7530_priv *priv)
{
mt7530_free_mdio_irq(priv);
mt7530_free_irq_common(priv);
}
+EXPORT_SYMBOL_GPL(mt7530_free_irq);

static int
mt7530_setup_mdio(struct mt7530_priv *priv)
@@ -2895,57 +2875,10 @@ static const struct phylink_pcs_ops mt7530_pcs_ops = {
.pcs_an_restart = mt7530_pcs_an_restart,
};

-static int mt7530_regmap_read(void *context, unsigned int reg, unsigned int *val)
-{
- struct mt7530_priv *priv = context;
-
- *val = mt7530_read(priv, reg);
- return 0;
-};
-
-static int mt7530_regmap_write(void *context, unsigned int reg, unsigned int val)
-{
- struct mt7530_priv *priv = context;
-
- mt7530_write(priv, reg, val);
- return 0;
-};
-
-static int mt7530_regmap_update_bits(void *context, unsigned int reg,
- unsigned int mask, unsigned int val)
-{
- struct mt7530_priv *priv = context;
-
- mt7530_rmw(priv, reg, mask, val);
- return 0;
-};
-
-static const struct regmap_bus mt7531_regmap_bus = {
- .reg_write = mt7530_regmap_write,
- .reg_read = mt7530_regmap_read,
- .reg_update_bits = mt7530_regmap_update_bits,
-};
-
-#define MT7531_PCS_REGMAP_CONFIG(_name, _reg_base) \
- { \
- .name = _name, \
- .reg_bits = 16, \
- .val_bits = 32, \
- .reg_stride = 4, \
- .reg_base = _reg_base, \
- .max_register = 0x17c, \
- }
-
-static const struct regmap_config mt7531_pcs_config[] = {
- MT7531_PCS_REGMAP_CONFIG("port5", MT7531_SGMII_REG_BASE(5)),
- MT7531_PCS_REGMAP_CONFIG("port6", MT7531_SGMII_REG_BASE(6)),
-};
-
static int
mt753x_setup(struct dsa_switch *ds)
{
struct mt7530_priv *priv = ds->priv;
- struct regmap *regmap;
int i, ret;

/* Initialise the PCS devices */
@@ -2967,16 +2900,6 @@ mt753x_setup(struct dsa_switch *ds)
if (ret && priv->irq)
mt7530_free_irq_common(priv);

- if (priv->id == ID_MT7531)
- for (i = 0; i < 2; i++) {
- regmap = devm_regmap_init(ds->dev,
- &mt7531_regmap_bus, priv,
- &mt7531_pcs_config[i]);
- priv->ports[5 + i].sgmii_pcs =
- mtk_pcs_lynxi_create(ds->dev, regmap,
- MT7531_PHYA_CTRL_SIGNAL3, 0);
- }
-
return ret;
}

@@ -3010,7 +2933,7 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
return 0;
}

-static const struct dsa_switch_ops mt7530_switch_ops = {
+const struct dsa_switch_ops mt7530_switch_ops = {
.get_tag_protocol = mtk_get_tag_protocol,
.setup = mt753x_setup,
.get_strings = mt7530_get_strings,
@@ -3044,8 +2967,9 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
.get_mac_eee = mt753x_get_mac_eee,
.set_mac_eee = mt753x_set_mac_eee,
};
+EXPORT_SYMBOL_GPL(mt7530_switch_ops);

-static const struct mt753x_info mt753x_table[] = {
+const struct mt753x_info mt753x_table[] = {
[ID_MT7621] = {
.id = ID_MT7621,
.pcs_ops = &mt7530_pcs_ops,
@@ -3084,153 +3008,7 @@ static const struct mt753x_info mt753x_table[] = {
.mac_port_config = mt7531_mac_config,
},
};
-
-static const struct of_device_id mt7530_of_match[] = {
- { .compatible = "mediatek,mt7621", .data = &mt753x_table[ID_MT7621], },
- { .compatible = "mediatek,mt7530", .data = &mt753x_table[ID_MT7530], },
- { .compatible = "mediatek,mt7531", .data = &mt753x_table[ID_MT7531], },
- { /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, mt7530_of_match);
-
-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 = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds), GFP_KERNEL);
- if (!priv->ds)
- return -ENOMEM;
-
- priv->ds->dev = &mdiodev->dev;
- priv->ds->num_ports = MT7530_NUM_PORTS;
-
- /* 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);
- }
- }
-
- /* Get the hardware identifier from the devicetree node.
- * We will need it for some of the clock and regulator setup.
- */
- priv->info = of_device_get_match_data(&mdiodev->dev);
- if (!priv->info)
- return -EINVAL;
-
- /* Sanity check if these required device operations are filled
- * properly.
- */
- if (!priv->info->sw_setup || !priv->info->pad_setup ||
- !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
- !priv->info->mac_port_get_caps ||
- !priv->info->mac_port_config)
- return -EINVAL;
-
- priv->id = priv->info->id;
-
- if (priv->id == ID_MT7530) {
- 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);
-}
-
-static void
-mt7530_remove(struct mdio_device *mdiodev)
-{
- struct mt7530_priv *priv = dev_get_drvdata(&mdiodev->dev);
- int ret = 0, i;
-
- if (!priv)
- return;
-
- 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);
-
- if (priv->irq)
- mt7530_free_irq(priv);
-
- dsa_unregister_switch(priv->ds);
-
- for (i = 0; i < 2; ++i)
- mtk_pcs_lynxi_destroy(priv->ports[5 + i].sgmii_pcs);
-
- mutex_destroy(&priv->reg_mutex);
-}
-
-static void mt7530_shutdown(struct mdio_device *mdiodev)
-{
- struct mt7530_priv *priv = dev_get_drvdata(&mdiodev->dev);
-
- if (!priv)
- return;
-
- dsa_switch_shutdown(priv->ds);
-
- dev_set_drvdata(&mdiodev->dev, NULL);
-}
-
-static struct mdio_driver mt7530_mdio_driver = {
- .probe = mt7530_probe,
- .remove = mt7530_remove,
- .shutdown = mt7530_shutdown,
- .mdiodrv.driver = {
- .name = "mt7530",
- .of_match_table = mt7530_of_match,
- },
-};
-
-mdio_module_driver(mt7530_mdio_driver);
+EXPORT_SYMBOL_GPL(mt753x_table);

MODULE_AUTHOR("Sean Wang <[email protected]>");
MODULE_DESCRIPTION("Driver for Mediatek MT7530 Switch");
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index c5d29f3fc1d80..6ac7f503dce2f 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -682,24 +682,6 @@ enum p5_interface_select {
P5_INTF_SEL_GMAC5_SGMII,
};

-static const char *p5_intf_modes(unsigned int p5_interface)
-{
- switch (p5_interface) {
- case P5_DISABLED:
- return "DISABLED";
- case P5_INTF_SEL_PHY_P0:
- return "PHY P0";
- case P5_INTF_SEL_PHY_P4:
- return "PHY P4";
- case P5_INTF_SEL_GMAC5:
- return "GMAC5";
- case P5_INTF_SEL_GMAC5_SGMII:
- return "GMAC5_SGMII";
- default:
- return "unknown";
- }
-}
-
struct mt7530_priv;

struct mt753x_pcs {
@@ -777,6 +759,7 @@ struct mt7530_priv {
struct reset_control *rstc;
struct regulator *core_pwr;
struct regulator *io_pwr;
+ struct regmap *regmap;
struct gpio_desc *reset;
const struct mt753x_info *info;
unsigned int id;
@@ -830,4 +813,9 @@ static inline void INIT_MT7530_DUMMY_POLL(struct mt7530_dummy_poll *p,
p->reg = reg;
}

+void mt7530_free_irq(struct mt7530_priv *priv);
+
+extern const struct dsa_switch_ops mt7530_switch_ops;
+extern const struct mt753x_info mt753x_table[];
+
#endif /* __MT7530_H */
--
2.39.2

2023-03-28 22:22:11

by Daniel Golle

[permalink] [raw]
Subject: [RFC PATCH net-next v2 2/2] net: dsa: mt7530: introduce MMIO driver for MT7988 SoC

The MediaTek MT7988 SoC comes with a built-in switch very similar to
previous MT7530 and MT7531. However, the switch address space is mapped
into the SoCs memory space rather than being connected via MDIO.
Using MMIO simplifies register access and also removes the need for a bus
lock, and for that reason also makes interrupt handling more light-weight.

Note that this is different from previous SoCs like MT7621 and MT7623N
which also came with an integrated MT7530-like switch which yet had to be
accessed via MDIO.

Signed-off-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/Kconfig | 8 ++
drivers/net/dsa/Makefile | 1 +
drivers/net/dsa/mt7530-mmio.c | 143 +++++++++++++++++++++++++
drivers/net/dsa/mt7530.c | 195 +++++++++++++++++++++++++++++-----
drivers/net/dsa/mt7530.h | 14 +--
5 files changed, 331 insertions(+), 30 deletions(-)
create mode 100644 drivers/net/dsa/mt7530-mmio.c

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 0d9d605b3882c..70b7bd4d81b7b 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -50,6 +50,14 @@ config NET_DSA_MT7530_MDIO
switch chips. Multi-chip module MT7530 in MT7621AT, MT7621DAT,
MT7621ST and MT7623AI SoCs is supported.

+config NET_DSA_MT7530_MMIO
+ tristate "MediaTek MT7988 built-in Ethernet switch support"
+ select NET_DSA_MT7530
+ depends on HAS_IOMEM
+ help
+ This enables support for the built-in Ethernet switch of the
+ MediaTek MT7988 SoC.
+
config NET_DSA_MV88E6060
tristate "Marvell 88E6060 ethernet switch chip support"
select NET_DSA_TAG_TRAILER
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 2b072d574ed02..7c763578b297f 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -8,6 +8,7 @@ endif
obj-$(CONFIG_NET_DSA_LANTIQ_GSWIP) += lantiq_gswip.o
obj-$(CONFIG_NET_DSA_MT7530) += mt7530.o
obj-$(CONFIG_NET_DSA_MT7530_MDIO) += mt7530-mdio.o
+obj-$(CONFIG_NET_DSA_MT7530_MMIO) += mt7530-mmio.o
obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
obj-$(CONFIG_NET_DSA_RZN1_A5PSW) += rzn1_a5psw.o
obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
new file mode 100644
index 0000000000000..6e73845626918
--- /dev/null
+++ b/drivers/net/dsa/mt7530-mmio.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <net/dsa.h>
+
+#include "mt7530.h"
+
+static const struct of_device_id mt7988_of_match[] = {
+ { .compatible = "mediatek,mt7988-switch", .data = &mt753x_table[ID_MT7988], },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mt7988_of_match);
+
+static int
+mt7988_probe(struct platform_device *pdev)
+{
+ static struct regmap_config *sw_regmap_config;
+ struct mt7530_priv *priv;
+ void __iomem *base_addr;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ base_addr = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base_addr)) {
+ dev_err(&pdev->dev, "cannot request I/O memory space\n");
+ return -ENXIO;
+ }
+
+ sw_regmap_config = devm_kzalloc(&pdev->dev, sizeof(*sw_regmap_config), GFP_KERNEL);
+ if (!sw_regmap_config)
+ return -ENOMEM;
+
+ sw_regmap_config->name = "switch";
+ sw_regmap_config->reg_bits = 16;
+ sw_regmap_config->val_bits = 32;
+ sw_regmap_config->reg_stride = 4;
+ sw_regmap_config->reg_base = 0x0;
+ sw_regmap_config->max_register = 0x7ffc;
+ priv->regmap = devm_regmap_init_mmio(&pdev->dev, base_addr, sw_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ priv->ds = devm_kzalloc(&pdev->dev, sizeof(*priv->ds), GFP_KERNEL);
+ if (!priv->ds)
+ return -ENOMEM;
+
+ priv->ds->dev = &pdev->dev;
+ priv->ds->num_ports = MT7530_NUM_PORTS;
+
+ priv->rstc = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(priv->rstc)) {
+ dev_err(&pdev->dev, "couldn't get reset line\n");
+ return PTR_ERR(priv->rstc);
+ }
+
+ /* Get the hardware identifier from the devicetree node.
+ * We will need it for some of the clock and regulator setup.
+ */
+ priv->info = of_device_get_match_data(&pdev->dev);
+ if (!priv->info)
+ return -EINVAL;
+
+ /* Sanity check if these required device operations are filled
+ * properly.
+ */
+ if (!priv->info->sw_setup || !priv->info->pad_setup ||
+ !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
+ !priv->info->mac_port_get_caps ||
+ !priv->info->mac_port_config)
+ return -EINVAL;
+
+ priv->id = priv->info->id;
+ priv->bus = NULL;
+ priv->dev = &pdev->dev;
+ priv->ds->priv = priv;
+ priv->ds->ops = &mt7530_switch_ops;
+ mutex_init(&priv->reg_mutex);
+ dev_set_drvdata(&pdev->dev, priv);
+
+ return dsa_register_switch(priv->ds);
+}
+
+static int
+mt7988_remove(struct platform_device *pdev)
+{
+ struct mt7530_priv *priv = platform_get_drvdata(pdev);
+ int ret = 0;
+
+ if (!priv)
+ return 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);
+
+ if (priv->irq)
+ mt7530_free_irq(priv);
+
+ dsa_unregister_switch(priv->ds);
+
+ mutex_destroy(&priv->reg_mutex);
+
+ return 0;
+}
+
+static void mt7988_shutdown(struct platform_device *pdev)
+{
+ struct mt7530_priv *priv = platform_get_drvdata(pdev);
+
+ if (!priv)
+ return;
+
+ dsa_switch_shutdown(priv->ds);
+
+ dev_set_drvdata(&pdev->dev, NULL);
+}
+
+static struct platform_driver mt7988_platform_driver = {
+ .probe = mt7988_probe,
+ .remove = mt7988_remove,
+ .shutdown = mt7988_shutdown,
+ .driver = {
+ .name = "mt7988-switch",
+ .of_match_table = mt7988_of_match,
+ },
+};
+module_platform_driver(mt7988_platform_driver);
+
+MODULE_AUTHOR("Sean Wang <[email protected]>");
+MODULE_DESCRIPTION("Driver for Mediatek MT7530 Switch (MMIO)");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ca379f4d3cd32..2013aa42f73a7 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -117,6 +117,9 @@ core_write_mmd_indirect(struct mt7530_priv *priv, int prtad,
struct mii_bus *bus = priv->bus;
int ret;

+ if (!bus)
+ return 0;
+
/* Write the desired MMD Devad */
ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
if (ret < 0)
@@ -146,11 +149,13 @@ 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);
+ if (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);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);
}

static void
@@ -159,6 +164,9 @@ core_rmw(struct mt7530_priv *priv, u32 reg, u32 mask, u32 set)
struct mii_bus *bus = priv->bus;
u32 val;

+ if (!bus)
+ return;
+
mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

val = core_read_mmd_indirect(priv, reg, MDIO_MMD_VEND2);
@@ -205,11 +213,13 @@ 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);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

mt7530_mii_write(priv, reg, val);

- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);
}

static u32
@@ -221,14 +231,16 @@ _mt7530_unlocked_read(struct mt7530_dummy_poll *p)
static u32
_mt7530_read(struct mt7530_dummy_poll *p)
{
- struct mii_bus *bus = p->priv->bus;
+ struct mii_bus *bus = p->priv->bus;
u32 val;

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

val = mt7530_mii_read(p->priv, p->reg);

- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);

return val;
}
@@ -248,11 +260,13 @@ mt7530_rmw(struct mt7530_priv *priv, u32 reg,
{
struct mii_bus *bus = priv->bus;

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

regmap_update_bits(priv->regmap, reg, mask, set);

- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);
}

static void
@@ -603,7 +617,8 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,

INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
!(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -636,7 +651,8 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,

ret = val & MT7531_MDIO_RW_DATA_MASK;
out:
- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);

return ret;
}
@@ -652,7 +668,8 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,

INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
!(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -684,7 +701,8 @@ mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
}

out:
- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);

return ret;
}
@@ -699,7 +717,8 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)

INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

ret = readx_poll_timeout(_mt7530_unlocked_read, &p, val,
!(val & MT7531_PHY_ACS_ST), 20, 100000);
@@ -722,7 +741,8 @@ mt7531_ind_c22_phy_read(struct mt7530_priv *priv, int port, int regnum)

ret = val & MT7531_MDIO_RW_DATA_MASK;
out:
- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);

return ret;
}
@@ -738,7 +758,8 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,

INIT_MT7530_DUMMY_POLL(&p, priv, MT7531_PHY_IAC);

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

ret = readx_poll_timeout(_mt7530_unlocked_read, &p, reg,
!(reg & MT7531_PHY_ACS_ST), 20, 100000);
@@ -760,7 +781,8 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
}

out:
- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);

return ret;
}
@@ -1070,7 +1092,8 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
if (!dsa_is_cpu_port(ds, port))
return 0;

- mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (bus)
+ mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

val = mt7530_mii_read(priv, MT7530_GMACCR);
val &= ~MAX_RX_PKT_LEN_MASK;
@@ -1091,7 +1114,8 @@ mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)

mt7530_mii_write(priv, MT7530_GMACCR, val);

- mutex_unlock(&bus->mdio_lock);
+ if (bus)
+ mutex_unlock(&bus->mdio_lock);

return 0;
}
@@ -1892,10 +1916,14 @@ mt7530_irq_thread_fn(int irq, void *dev_id)
u32 val;
int p;

- mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
+ if (priv->bus)
+ mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
+
val = mt7530_mii_read(priv, MT7530_SYS_INT_STS);
mt7530_mii_write(priv, MT7530_SYS_INT_STS, val);
- mutex_unlock(&priv->bus->mdio_lock);
+
+ if (priv->bus)
+ mutex_unlock(&priv->bus->mdio_lock);

for (p = 0; p < MT7530_NUM_PHYS; p++) {
if (BIT(p) & val) {
@@ -1968,6 +1996,47 @@ static const struct irq_domain_ops mt7530_irq_domain_ops = {
.xlate = irq_domain_xlate_onecell,
};

+static void
+mt7988_irq_mask(struct irq_data *d)
+{
+ struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+ priv->irq_enable &= ~BIT(d->hwirq);
+ mt7530_mii_write(priv, MT7530_SYS_INT_EN, priv->irq_enable);
+}
+
+static void
+mt7988_irq_unmask(struct irq_data *d)
+{
+ struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+ priv->irq_enable |= BIT(d->hwirq);
+ mt7530_mii_write(priv, MT7530_SYS_INT_EN, priv->irq_enable);
+}
+
+static struct irq_chip mt7988_irq_chip = {
+ .name = KBUILD_MODNAME,
+ .irq_mask = mt7988_irq_mask,
+ .irq_unmask = mt7988_irq_unmask,
+};
+
+static int
+mt7988_irq_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_data(irq, domain->host_data);
+ irq_set_chip_and_handler(irq, &mt7988_irq_chip, handle_simple_irq);
+ irq_set_nested_thread(irq, true);
+ irq_set_noprobe(irq);
+
+ return 0;
+}
+
+static const struct irq_domain_ops mt7988_irq_domain_ops = {
+ .map = mt7988_irq_map,
+ .xlate = irq_domain_xlate_onecell,
+};
+
static void
mt7530_setup_mdio_irq(struct mt7530_priv *priv)
{
@@ -2002,8 +2071,15 @@ mt7530_setup_irq(struct mt7530_priv *priv)
return priv->irq ? : -EINVAL;
}

- priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
- &mt7530_irq_domain_ops, priv);
+ if (priv->id == ID_MT7988)
+ priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
+ &mt7988_irq_domain_ops,
+ priv);
+ else
+ priv->irq_domain = irq_domain_add_linear(np, MT7530_NUM_PHYS,
+ &mt7530_irq_domain_ops,
+ priv);
+
if (!priv->irq_domain) {
dev_err(dev, "failed to create IRQ domain\n");
return -ENOMEM;
@@ -2588,6 +2664,8 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
case PHY_INTERFACE_MODE_NA:
case PHY_INTERFACE_MODE_1000BASEX:
case PHY_INTERFACE_MODE_2500BASEX:
+ case PHY_INTERFACE_MODE_USXGMII:
+ case PHY_INTERFACE_MODE_10GKR:
/* handled in SGMII PCS driver */
return 0;
default:
@@ -2712,7 +2790,9 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port,
* variants.
*/
if (interface == PHY_INTERFACE_MODE_TRGMII ||
- (phy_interface_mode_is_8023z(interface))) {
+ interface == PHY_INTERFACE_MODE_USXGMII ||
+ interface == PHY_INTERFACE_MODE_10GKR ||
+ phy_interface_mode_is_8023z(interface)) {
speed = SPEED_1000;
duplex = DUPLEX_FULL;
}
@@ -2933,6 +3013,60 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
return 0;
}

+static int mt7988_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
+{
+ return 0;
+}
+
+static int mt7988_setup(struct dsa_switch *ds)
+{
+ struct mt7530_priv *priv = ds->priv;
+ u32 unused_pm = 0;
+ int i;
+
+ /* Reset the switch */
+ reset_control_assert(priv->rstc);
+ udelay(20);
+ reset_control_deassert(priv->rstc);
+ udelay(20);
+
+ /* Reset the switch PHYs */
+ mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
+
+ /* BPDU to CPU port */
+ mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
+ BIT(MT7988_CPU_PORT));
+ mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
+ MT753X_BPDU_CPU_ONLY);
+
+ /* Enable and reset MIB counters */
+ mt7530_mib_reset(ds);
+
+ for (i = 0; i < MT7530_NUM_PORTS; i++) {
+ /* Disable forwarding by default on all ports */
+ mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
+ PCR_MATRIX_CLR);
+
+ mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);
+
+ if (dsa_is_unused_port(ds, i))
+ unused_pm |= BIT(i);
+ else if (dsa_is_cpu_port(ds, i))
+ mt753x_cpu_port_enable(ds, i);
+ else
+ mt7530_port_disable(ds, i);
+
+ /* Enable consistent egress tag */
+ mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
+ PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
+ }
+
+ ds->configure_vlan_while_not_filtering = true;
+
+ /* Flush the FDB table */
+ return mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);
+}
+
const struct dsa_switch_ops mt7530_switch_ops = {
.get_tag_protocol = mtk_get_tag_protocol,
.setup = mt753x_setup,
@@ -3007,6 +3141,19 @@ const struct mt753x_info mt753x_table[] = {
.mac_port_get_caps = mt7531_mac_port_get_caps,
.mac_port_config = mt7531_mac_config,
},
+ [ID_MT7988] = {
+ .id = ID_MT7988,
+ .pcs_ops = &mt7530_pcs_ops,
+ .sw_setup = mt7988_setup,
+ .phy_read_c22 = mt7531_ind_c22_phy_read,
+ .phy_write_c22 = mt7531_ind_c22_phy_write,
+ .phy_read_c45 = mt7531_ind_c45_phy_read,
+ .phy_write_c45 = mt7531_ind_c45_phy_write,
+ .pad_setup = mt7988_pad_setup,
+ .cpu_port_config = mt7531_cpu_port_config,
+ .mac_port_get_caps = mt7531_mac_port_get_caps,
+ .mac_port_config = mt7531_mac_config,
+ },
};
EXPORT_SYMBOL_GPL(mt753x_table);

diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 6ac7f503dce2f..e3dafaff0ce18 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -11,6 +11,8 @@
#define MT7530_NUM_FDB_RECORDS 2048
#define MT7530_ALL_MEMBERS 0xff

+#define MT7988_CPU_PORT 6
+
#define MTK_HDR_LEN 4
#define MT7530_MAX_MTU (15 * 1024 - ETH_HLEN - ETH_FCS_LEN - MTK_HDR_LEN)

@@ -18,6 +20,7 @@ enum mt753x_id {
ID_MT7530 = 0,
ID_MT7621 = 1,
ID_MT7531 = 2,
+ ID_MT7988 = 3,
};

#define NUM_TRGMII_CTRL 5
@@ -54,11 +57,11 @@ enum mt753x_id {
#define MT7531_MIRROR_PORT_SET(x) (((x) & MIRROR_MASK) << 16)
#define MT7531_CPU_PMAP_MASK GENMASK(7, 0)

-#define MT753X_MIRROR_REG(id) (((id) == ID_MT7531) ? \
+#define MT753X_MIRROR_REG(id) ((((id) == ID_MT7531) || ((id) == ID_MT7988)) ? \
MT7531_CFC : MT7530_MFC)
-#define MT753X_MIRROR_EN(id) (((id) == ID_MT7531) ? \
+#define MT753X_MIRROR_EN(id) ((((id) == ID_MT7531) || ((id) == ID_MT7988)) ? \
MT7531_MIRROR_EN : MIRROR_EN)
-#define MT753X_MIRROR_MASK(id) (((id) == ID_MT7531) ? \
+#define MT753X_MIRROR_MASK(id) ((((id) == ID_MT7531) || ((id) == ID_MT7988)) ? \
MT7531_MIRROR_MASK : MIRROR_MASK)

/* Registers for BPDU and PAE frame control*/
@@ -295,9 +298,8 @@ enum mt7530_vlan_port_acc_frm {
MT7531_FORCE_DPX | \
MT7531_FORCE_RX_FC | \
MT7531_FORCE_TX_FC)
-#define PMCR_FORCE_MODE_ID(id) (((id) == ID_MT7531) ? \
- MT7531_FORCE_MODE : \
- PMCR_FORCE_MODE)
+#define PMCR_FORCE_MODE_ID(id) ((((id) == ID_MT7531) || ((id) == ID_MT7988)) ? \
+ MT7531_FORCE_MODE : PMCR_FORCE_MODE)
#define PMCR_LINK_SETTINGS_MASK (PMCR_TX_EN | PMCR_FORCE_SPEED_1000 | \
PMCR_RX_EN | PMCR_FORCE_SPEED_100 | \
PMCR_TX_FC_EN | PMCR_RX_FC_EN | \
--
2.39.2

2023-03-28 22:39:17

by Andrew Lunn

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v2 1/2] net: dsa: mt7530: split-off MDIO driver

On Tue, Mar 28, 2023 at 11:16:51PM +0100, Daniel Golle wrote:
> In order to support the built-in switch of some MediaTek SoCs we need
> to use MMIO instead of MDIO to access the switch.
> Prepare this be splitting-off the part of the driver registering an
> MDIO driver, so we can add another module acting as MMIO/platform driver.
>
> Signed-off-by: Daniel Golle <[email protected]>

Hi Daniel

This is getting better.

Please try to split this patch up. Ideally you want lots of small
patches which are obviously correct. So maybe refactor the creation of
the PCS as a patch. Move p5_intf_modes() as a patch, etc.

Also, look at mt7530_probe() and mt7988_probe() side by side and see
what is common and can be put into one shared function in the
core. Same for remove. It could be that creating that helper is a
patch done before adding mt7530-mdio.c.

Andrew

2023-03-28 23:01:39

by Andrew Lunn

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v2 2/2] net: dsa: mt7530: introduce MMIO driver for MT7988 SoC

> @@ -146,11 +149,13 @@ 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);
> + if (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);
> + if (bus)
> + mutex_unlock(&bus->mdio_lock);
> }

All this if (bus) is pretty ugly.

First off, what is this mutex actually protecting? And why is the same
protection not required for MMIO?

If you have a convincing argument the mutex is not needed, please add
two helpers:

mt7530_mutex_lock(struct mt7530_priv *priv)
mt7530_mutex_unlock(struct mt7530_priv *priv)

and hide the if inside that. As part of the commit message, explain
why the mutex is not needed for MDIO.

Adding these helpers and changing all calls can be a preparation
patch. It is the sort of patch which should be obviously correct.

> @@ -2588,6 +2664,8 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> case PHY_INTERFACE_MODE_NA:
> case PHY_INTERFACE_MODE_1000BASEX:
> case PHY_INTERFACE_MODE_2500BASEX:
> + case PHY_INTERFACE_MODE_USXGMII:
> + case PHY_INTERFACE_MODE_10GKR:
> /* handled in SGMII PCS driver */
> return 0;
> default:
> @@ -2712,7 +2790,9 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port,
> * variants.
> */
> if (interface == PHY_INTERFACE_MODE_TRGMII ||
> - (phy_interface_mode_is_8023z(interface))) {
> + interface == PHY_INTERFACE_MODE_USXGMII ||
> + interface == PHY_INTERFACE_MODE_10GKR ||
> + phy_interface_mode_is_8023z(interface)) {
> speed = SPEED_1000;
> duplex = DUPLEX_FULL;
> }

This looks like something which should be in a separate patch.

> +static int mt7988_setup(struct dsa_switch *ds)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + u32 unused_pm = 0;
> + int i;
> +
> + /* Reset the switch */
> + reset_control_assert(priv->rstc);
> + udelay(20);
> + reset_control_deassert(priv->rstc);
> + udelay(20);
> +
> + /* Reset the switch PHYs */
> + mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
> +
> + /* BPDU to CPU port */
> + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> + BIT(MT7988_CPU_PORT));
> + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> + MT753X_BPDU_CPU_ONLY);
> +
> + /* Enable and reset MIB counters */
> + mt7530_mib_reset(ds);
> +
> + for (i = 0; i < MT7530_NUM_PORTS; i++) {
> + /* Disable forwarding by default on all ports */
> + mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> + PCR_MATRIX_CLR);
> +
> + mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);
> +
> + if (dsa_is_unused_port(ds, i))
> + unused_pm |= BIT(i);
> + else if (dsa_is_cpu_port(ds, i))
> + mt753x_cpu_port_enable(ds, i);
> + else
> + mt7530_port_disable(ds, i);
> +
> + /* Enable consistent egress tag */
> + mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
> + PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> + }
> +
> + ds->configure_vlan_while_not_filtering = true;
> +
> + /* Flush the FDB table */
> + return mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);

Is this really specific to the mt7988?

Andrew

2023-03-28 23:13:53

by Daniel Golle

[permalink] [raw]
Subject: Re: [RFC PATCH net-next v2 2/2] net: dsa: mt7530: introduce MMIO driver for MT7988 SoC

On Wed, Mar 29, 2023 at 12:45:37AM +0200, Andrew Lunn wrote:
> > @@ -146,11 +149,13 @@ 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);
> > + if (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);
> > + if (bus)
> > + mutex_unlock(&bus->mdio_lock);
> > }
>
> All this if (bus) is pretty ugly.
>
> First off, what is this mutex actually protecting? And why is the same
> protection not required for MMIO?

The mutex is protecting the MDIO bus. Each access to any register of the
MT753x switch requires at least two operations on the MDIO bus.
Hence if there is congruent access, e.g. due to PCS or PHY polling, this
can mess up and interfere with another ongoing register access sequence.

You may argue that we should just use regmap's locking API for that, as
it is done also when creating the pcs-mtk-lynxi instance. However, some
things like interrupt handling require more complex (as in: not covered
by regmap_update_bits) pseudo-atomic operations, so the idea was to keep
the locking just like it was before for MDIO-connected MT753x switches
and provide a lock-less regmap replacing the former mt7530_mii_write() and
mt7530_mii_read() functions.

If we use MMIO we can directly access the 32-bit wide registers and hence
do not require locking.

>
> If you have a convincing argument the mutex is not needed, please add
> two helpers:
>
> mt7530_mutex_lock(struct mt7530_priv *priv)
> mt7530_mutex_unlock(struct mt7530_priv *priv)
>
> and hide the if inside that. As part of the commit message, explain
> why the mutex is not needed for MDIO.

Ok, will do.

>
> Adding these helpers and changing all calls can be a preparation
> patch. It is the sort of patch which should be obviously correct.

Understood.

>
> > @@ -2588,6 +2664,8 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> > case PHY_INTERFACE_MODE_NA:
> > case PHY_INTERFACE_MODE_1000BASEX:
> > case PHY_INTERFACE_MODE_2500BASEX:
> > + case PHY_INTERFACE_MODE_USXGMII:
> > + case PHY_INTERFACE_MODE_10GKR:
> > /* handled in SGMII PCS driver */
> > return 0;
> > default:
> > @@ -2712,7 +2790,9 @@ static void mt753x_phylink_mac_link_up(struct dsa_switch *ds, int port,
> > * variants.
> > */
> > if (interface == PHY_INTERFACE_MODE_TRGMII ||
> > - (phy_interface_mode_is_8023z(interface))) {
> > + interface == PHY_INTERFACE_MODE_USXGMII ||
> > + interface == PHY_INTERFACE_MODE_10GKR ||
> > + phy_interface_mode_is_8023z(interface)) {
> > speed = SPEED_1000;
> > duplex = DUPLEX_FULL;
> > }
>
> This looks like something which should be in a separate patch.

Ok, I will put it into a separate patch before adding support for
MT7988 which is the only switch supporting those modes (and also
requiring them).

>
> > +static int mt7988_setup(struct dsa_switch *ds)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + u32 unused_pm = 0;
> > + int i;
> > +
> > + /* Reset the switch */
> > + reset_control_assert(priv->rstc);
> > + udelay(20);
> > + reset_control_deassert(priv->rstc);
> > + udelay(20);
> > +
> > + /* Reset the switch PHYs */
> > + mt7530_write(priv, MT7530_SYS_CTRL, SYS_CTRL_PHY_RST);
> > +
> > + /* BPDU to CPU port */
> > + mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> > + BIT(MT7988_CPU_PORT));
> > + mt7530_rmw(priv, MT753X_BPC, MT753X_BPDU_PORT_FW_MASK,
> > + MT753X_BPDU_CPU_ONLY);
> > +
> > + /* Enable and reset MIB counters */
> > + mt7530_mib_reset(ds);
> > +
> > + for (i = 0; i < MT7530_NUM_PORTS; i++) {
> > + /* Disable forwarding by default on all ports */
> > + mt7530_rmw(priv, MT7530_PCR_P(i), PCR_MATRIX_MASK,
> > + PCR_MATRIX_CLR);
> > +
> > + mt7530_set(priv, MT7531_DBG_CNT(i), MT7531_DIS_CLR);
> > +
> > + if (dsa_is_unused_port(ds, i))
> > + unused_pm |= BIT(i);
> > + else if (dsa_is_cpu_port(ds, i))
> > + mt753x_cpu_port_enable(ds, i);
> > + else
> > + mt7530_port_disable(ds, i);
> > +
> > + /* Enable consistent egress tag */
> > + mt7530_rmw(priv, MT7530_PVC_P(i), PVC_EG_TAG_MASK,
> > + PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
> > + }
> > +
> > + ds->configure_vlan_while_not_filtering = true;
> > +
> > + /* Flush the FDB table */
> > + return mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);
>
> Is this really specific to the mt7988?

While the setup function is somehow similar to mt7530_setup or
mt7531_setup there are also differences. It would be possible to
split this more, so more common parts can be shared, such as the
loop over the ports which is the same as for MT7531.
The way initial reset is carried out as well as setting up the CPU
port is specific to MT7988.

>
> Andrew