[PATCH net-next 0/4] MT7530 interrupt support

[PATCH net-next 0/4] MT7530 interrupt support

[DENG Qingfang]

Add support for MT7530 interrupt controller.

DENG Qingfang (4):
  net: phy: add MediaTek PHY driver
  net: dsa: mt7530: add interrupt support
  dt-bindings: net: dsa: add MT7530 interrupt controller binding
  staging: mt7621-dts: enable MT7530 interrupt controller

 .../devicetree/bindings/net/dsa/mt7530.txt    |   6 +
 drivers/net/dsa/Kconfig                       |   1 +
 drivers/net/dsa/mt7530.c                      | 264 ++++++++++++++++--
 drivers/net/dsa/mt7530.h                      |  20 +-
 drivers/net/phy/Kconfig                       |   5 +
 drivers/net/phy/Makefile                      |   1 +
 drivers/net/phy/mediatek.c                    | 112 ++++++++
 drivers/staging/mt7621-dts/mt7621.dtsi        |   4 +
 8 files changed, 385 insertions(+), 28 deletions(-)
 create mode 100644 drivers/net/phy/mediatek.c

-- 
2.25.1

[PATCH net-next 1/4] net: phy: add MediaTek PHY driver

[DENG Qingfang]

Add support for MediaTek PHYs found in MT7530 and MT7531 switches.
The initialization procedure is from the vendor driver, but due to lack
of documentation, the function of some register values remains unknown.

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
RFC v4 -> PATCH v1:
- No changes.

 drivers/net/phy/Kconfig    |   5 ++
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/mediatek.c | 112 +++++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 drivers/net/phy/mediatek.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 288bf405ebdb..9db39fb443e6 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -207,6 +207,11 @@ config MARVELL_88X2222_PHY
 	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
 	  Transceiver.
 
+config MEDIATEK_PHY
+	tristate "MediaTek PHYs"
+	help
+	  Supports the MediaTek switch integrated PHYs.
+
 config MICREL_PHY
 	tristate "Micrel PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index bcda7ed2455d..ab512cb3592b 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_LXT_PHY)		+= lxt.o
 obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
 obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
 obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
+obj-$(CONFIG_MEDIATEK_PHY)	+= mediatek.o
 obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
 obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_MICREL_PHY)	+= micrel.o
diff --git a/drivers/net/phy/mediatek.c b/drivers/net/phy/mediatek.c
new file mode 100644
index 000000000000..86e6c466b0e9
--- /dev/null
+++ b/drivers/net/phy/mediatek.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+
+#define MTK_EXT_PAGE_ACCESS		0x1f
+#define MTK_PHY_PAGE_STANDARD		0x0000
+#define MTK_PHY_PAGE_EXTENDED		0x0001
+#define MTK_PHY_PAGE_EXTENDED_2		0x0002
+#define MTK_PHY_PAGE_EXTENDED_3		0x0003
+#define MTK_PHY_PAGE_EXTENDED_2A30	0x2a30
+#define MTK_PHY_PAGE_EXTENDED_52B5	0x52b5
+
+static int mtk_phy_read_page(struct phy_device *phydev)
+{
+	return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
+}
+
+static int mtk_phy_write_page(struct phy_device *phydev, int page)
+{
+	return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
+}
+
+static void mtk_phy_config_init(struct phy_device *phydev)
+{
+	/* Disable EEE */
+	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+
+	/* Enable HW auto downshift */
+	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
+
+	/* Increase SlvDPSready time */
+	phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
+	__phy_write(phydev, 0x10, 0xafae);
+	__phy_write(phydev, 0x12, 0x2f);
+	__phy_write(phydev, 0x10, 0x8fae);
+	phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
+
+	/* Adjust 100_mse_threshold */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x123, 0xffff);
+
+	/* Disable mcc */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0xa6, 0x300);
+}
+
+static int mt7530_phy_config_init(struct phy_device *phydev)
+{
+	mtk_phy_config_init(phydev);
+
+	/* Increase post_update_timer */
+	phy_write_paged(phydev, MTK_PHY_PAGE_EXTENDED_3, 0x11, 0x4b);
+
+	return 0;
+}
+
+static int mt7531_phy_config_init(struct phy_device *phydev)
+{
+	if (phydev->interface != PHY_INTERFACE_MODE_INTERNAL)
+		return -EINVAL;
+
+	mtk_phy_config_init(phydev);
+
+	/* PHY link down power saving enable */
+	phy_set_bits(phydev, 0x17, BIT(4));
+	phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, 0xc6, 0x300);
+
+	/* Set TX Pair delay selection */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x13, 0x404);
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x14, 0x404);
+
+	return 0;
+}
+
+static struct phy_driver mtk_phy_driver[] = {
+	{
+		PHY_ID_MATCH_EXACT(0x03a29412),
+		.name		= "MediaTek MT7530 PHY",
+		.config_init	= mt7530_phy_config_init,
+		/* Interrupts are handled by the switch, not the PHY
+		 * itself.
+		 */
+		.config_intr	= genphy_no_config_intr,
+		.handle_interrupt = genphy_handle_interrupt_no_ack,
+		.read_page	= mtk_phy_read_page,
+		.write_page	= mtk_phy_write_page,
+	},
+	{
+		PHY_ID_MATCH_EXACT(0x03a29441),
+		.name		= "MediaTek MT7531 PHY",
+		.config_init	= mt7531_phy_config_init,
+		/* Interrupts are handled by the switch, not the PHY
+		 * itself.
+		 */
+		.config_intr	= genphy_no_config_intr,
+		.handle_interrupt = genphy_handle_interrupt_no_ack,
+		.read_page	= mtk_phy_read_page,
+		.write_page	= mtk_phy_write_page,
+	},
+};
+
+module_phy_driver(mtk_phy_driver);
+
+static struct mdio_device_id __maybe_unused mtk_phy_tbl[] = {
+	{ PHY_ID_MATCH_VENDOR(0x03a29400) },
+	{ }
+};
+
+MODULE_DESCRIPTION("MediaTek switch integrated PHY driver");
+MODULE_AUTHOR("DENG, Qingfang <dqfext@gmail.com>");
+MODULE_LICENSE("GPL");
+
+MODULE_DEVICE_TABLE(mdio, mtk_phy_tbl);
-- 
2.25.1

Re: [PATCH net-next 1/4] net: phy: add MediaTek PHY driver

[Andrew Lunn]

On Thu, Apr 29, 2021 at 02:21:27PM +0800, DENG Qingfang wrote:
> Add support for MediaTek PHYs found in MT7530 and MT7531 switches.
> The initialization procedure is from the vendor driver, but due to lack
> of documentation, the function of some register values remains unknown.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

> +
> +	/* Enable HW auto downshift */
> +	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));

As a follow up patch, you could add support for controlling this via a
PHY tunable.

    Andrew

Re: [PATCH net-next 1/4] net: phy: add MediaTek PHY driver

[Florian Fainelli]

On 4/28/2021 11:21 PM, DENG Qingfang wrote:
> Add support for MediaTek PHYs found in MT7530 and MT7531 switches.
> The initialization procedure is from the vendor driver, but due to lack
> of documentation, the function of some register values remains unknown.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 1/4] net: phy: add MediaTek PHY driver

[Vladimir Oltean]

On Thu, Apr 29, 2021 at 02:21:27PM +0800, DENG Qingfang wrote:
> Add support for MediaTek PHYs found in MT7530 and MT7531 switches.
> The initialization procedure is from the vendor driver, but due to lack
> of documentation, the function of some register values remains unknown.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> RFC v4 -> PATCH v1:
> - No changes.
> 
>  drivers/net/phy/Kconfig    |   5 ++
>  drivers/net/phy/Makefile   |   1 +
>  drivers/net/phy/mediatek.c | 112 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 118 insertions(+)
>  create mode 100644 drivers/net/phy/mediatek.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 288bf405ebdb..9db39fb443e6 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -207,6 +207,11 @@ config MARVELL_88X2222_PHY
>  	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
>  	  Transceiver.
>  
> +config MEDIATEK_PHY
> +	tristate "MediaTek PHYs"
> +	help
> +	  Supports the MediaTek switch integrated PHYs.
> +
>  config MICREL_PHY
>  	tristate "Micrel PHYs"
>  	help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index bcda7ed2455d..ab512cb3592b 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_LXT_PHY)		+= lxt.o
>  obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
>  obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
>  obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
> +obj-$(CONFIG_MEDIATEK_PHY)	+= mediatek.o
>  obj-$(CONFIG_MESON_GXL_PHY)	+= meson-gxl.o
>  obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
>  obj-$(CONFIG_MICREL_PHY)	+= micrel.o
> diff --git a/drivers/net/phy/mediatek.c b/drivers/net/phy/mediatek.c
> new file mode 100644
> index 000000000000..86e6c466b0e9
> --- /dev/null
> +++ b/drivers/net/phy/mediatek.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#include <linux/bitfield.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +
> +#define MTK_EXT_PAGE_ACCESS		0x1f
> +#define MTK_PHY_PAGE_STANDARD		0x0000
> +#define MTK_PHY_PAGE_EXTENDED		0x0001
> +#define MTK_PHY_PAGE_EXTENDED_2		0x0002
> +#define MTK_PHY_PAGE_EXTENDED_3		0x0003
> +#define MTK_PHY_PAGE_EXTENDED_2A30	0x2a30
> +#define MTK_PHY_PAGE_EXTENDED_52B5	0x52b5
> +
> +static int mtk_phy_read_page(struct phy_device *phydev)
> +{
> +	return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
> +}
> +
> +static int mtk_phy_write_page(struct phy_device *phydev, int page)
> +{
> +	return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
> +}
> +
> +static void mtk_phy_config_init(struct phy_device *phydev)
> +{
> +	/* Disable EEE */
> +	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);

Is there any ERR around this (i.e. is the feature universally broken or
just on your setup)? There is a function called of_set_phy_eee_broken,
you can let the PHY core disable EEE advertisement based on device tree.

> +
> +	/* Enable HW auto downshift */
> +	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
> +
> +	/* Increase SlvDPSready time */
> +	phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
> +	__phy_write(phydev, 0x10, 0xafae);
> +	__phy_write(phydev, 0x12, 0x2f);
> +	__phy_write(phydev, 0x10, 0x8fae);
> +	phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
> +
> +	/* Adjust 100_mse_threshold */
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x123, 0xffff);
> +
> +	/* Disable mcc */
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0xa6, 0x300);
> +}

[PATCH net-next 2/4] net: dsa: mt7530: add interrupt support

[DENG Qingfang]

Add support for MT7530 interrupt controller to handle internal PHYs.
In order to assign an IRQ number to each PHY, the registration of MDIO bus
is also done in this driver.

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
RFC v4 -> PATCH v1:
- Cosmetic fixes.

 drivers/net/dsa/Kconfig  |   1 +
 drivers/net/dsa/mt7530.c | 264 +++++++++++++++++++++++++++++++++++----
 drivers/net/dsa/mt7530.h |  20 ++-
 3 files changed, 257 insertions(+), 28 deletions(-)

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index a5f1aa911fe2..264384449f09 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -36,6 +36,7 @@ config NET_DSA_LANTIQ_GSWIP
 config NET_DSA_MT7530
 	tristate "MediaTek MT753x and MT7621 Ethernet switch support"
 	select NET_DSA_TAG_MTK
+	select MEDIATEK_PHY
 	help
 	  This enables support for the MediaTek MT7530, MT7531, and MT7621
 	  Ethernet switch chips.
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 96f7c9eede35..db838343fb05 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -10,6 +10,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/of_platform.h>
@@ -596,18 +597,14 @@ mt7530_mib_reset(struct dsa_switch *ds)
 	mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
 }
 
-static int mt7530_phy_read(struct dsa_switch *ds, int port, int regnum)
+static int mt7530_phy_read(struct mt7530_priv *priv, int port, int regnum)
 {
-	struct mt7530_priv *priv = ds->priv;
-
 	return mdiobus_read_nested(priv->bus, port, regnum);
 }
 
-static int mt7530_phy_write(struct dsa_switch *ds, int port, int regnum,
+static int mt7530_phy_write(struct mt7530_priv *priv, int port, int regnum,
 			    u16 val)
 {
-	struct mt7530_priv *priv = ds->priv;
-
 	return mdiobus_write_nested(priv->bus, port, regnum, val);
 }
 
@@ -785,9 +782,8 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 }
 
 static int
-mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
+mt7531_ind_phy_read(struct mt7530_priv *priv, int port, int regnum)
 {
-	struct mt7530_priv *priv = ds->priv;
 	int devad;
 	int ret;
 
@@ -803,10 +799,9 @@ mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
 }
 
 static int
-mt7531_ind_phy_write(struct dsa_switch *ds, int port, int regnum,
+mt7531_ind_phy_write(struct mt7530_priv *priv, int port, int regnum,
 		     u16 data)
 {
-	struct mt7530_priv *priv = ds->priv;
 	int devad;
 	int ret;
 
@@ -822,6 +817,22 @@ mt7531_ind_phy_write(struct dsa_switch *ds, int port, int regnum,
 	return ret;
 }
 
+static int
+mt753x_phy_read(struct mii_bus *bus, int port, int regnum)
+{
+	struct mt7530_priv *priv = bus->priv;
+
+	return priv->info->phy_read(priv, port, regnum);
+}
+
+static int
+mt753x_phy_write(struct mii_bus *bus, int port, int regnum, u16 val)
+{
+	struct mt7530_priv *priv = bus->priv;
+
+	return priv->info->phy_write(priv, port, regnum, val);
+}
+
 static void
 mt7530_get_strings(struct dsa_switch *ds, int port, u32 stringset,
 		   uint8_t *data)
@@ -1828,6 +1839,210 @@ mt7530_setup_gpio(struct mt7530_priv *priv)
 }
 #endif /* CONFIG_GPIOLIB */
 
+static irqreturn_t
+mt7530_irq_thread_fn(int irq, void *dev_id)
+{
+	struct mt7530_priv *priv = dev_id;
+	bool handled = false;
+	u32 val;
+	int p;
+
+	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);
+
+	for (p = 0; p < MT7530_NUM_PHYS; p++) {
+		if (BIT(p) & val) {
+			unsigned int irq;
+
+			irq = irq_find_mapping(priv->irq_domain, p);
+			handle_nested_irq(irq);
+			handled = true;
+		}
+	}
+
+	return IRQ_RETVAL(handled);
+}
+
+static void
+mt7530_irq_mask(struct irq_data *d)
+{
+	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+	priv->irq_enable &= ~BIT(d->hwirq);
+}
+
+static void
+mt7530_irq_unmask(struct irq_data *d)
+{
+	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+	priv->irq_enable |= BIT(d->hwirq);
+}
+
+static void
+mt7530_irq_bus_lock(struct irq_data *d)
+{
+	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+	mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
+}
+
+static void
+mt7530_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
+
+	mt7530_mii_write(priv, MT7530_SYS_INT_EN, priv->irq_enable);
+	mutex_unlock(&priv->bus->mdio_lock);
+}
+
+static struct irq_chip mt7530_irq_chip = {
+	.name = KBUILD_MODNAME,
+	.irq_mask = mt7530_irq_mask,
+	.irq_unmask = mt7530_irq_unmask,
+	.irq_bus_lock = mt7530_irq_bus_lock,
+	.irq_bus_sync_unlock = mt7530_irq_bus_sync_unlock,
+};
+
+static int
+mt7530_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, &mt7530_irq_chip, handle_simple_irq);
+	irq_set_nested_thread(irq, true);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops mt7530_irq_domain_ops = {
+	.map = mt7530_irq_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static void
+mt7530_setup_mdio_irq(struct mt7530_priv *priv)
+{
+	struct dsa_switch *ds = priv->ds;
+	int p;
+
+	for (p = 0; p < MT7530_NUM_PHYS; p++) {
+		if (BIT(p) & ds->phys_mii_mask) {
+			unsigned int irq;
+
+			irq = irq_create_mapping(priv->irq_domain, p);
+			ds->slave_mii_bus->irq[p] = irq;
+		}
+	}
+}
+
+static int
+mt7530_setup_irq(struct mt7530_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	if (!of_property_read_bool(np, "interrupt-controller")) {
+		dev_info(dev, "no interrupt support\n");
+		return 0;
+	}
+
+	priv->irq = of_irq_get(np, 0);
+	if (priv->irq <= 0) {
+		dev_err(dev, "failed to get parent IRQ: %d\n", priv->irq);
+		return priv->irq ? : -EINVAL;
+	}
+
+	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;
+	}
+
+	/* This register must be set for MT7530 to properly fire interrupts */
+	if (priv->id != ID_MT7531)
+		mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL);
+
+	ret = request_threaded_irq(priv->irq, NULL, mt7530_irq_thread_fn,
+				   IRQF_ONESHOT, KBUILD_MODNAME, priv);
+	if (ret) {
+		irq_domain_remove(priv->irq_domain);
+		dev_err(dev, "failed to request IRQ: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void
+mt7530_free_mdio_irq(struct mt7530_priv *priv)
+{
+	int p;
+
+	for (p = 0; p < MT7530_NUM_PHYS; p++) {
+		if (BIT(p) & priv->ds->phys_mii_mask) {
+			unsigned int irq;
+
+			irq = irq_find_mapping(priv->irq_domain, p);
+			irq_dispose_mapping(irq);
+		}
+	}
+}
+
+static void
+mt7530_free_irq_common(struct mt7530_priv *priv)
+{
+	free_irq(priv->irq, priv);
+	irq_domain_remove(priv->irq_domain);
+}
+
+static void
+mt7530_free_irq(struct mt7530_priv *priv)
+{
+	mt7530_free_mdio_irq(priv);
+	mt7530_free_irq_common(priv);
+}
+
+static int
+mt7530_setup_mdio(struct mt7530_priv *priv)
+{
+	struct dsa_switch *ds = priv->ds;
+	struct device *dev = priv->dev;
+	struct mii_bus *bus;
+	static int idx;
+	int ret;
+
+	bus = devm_mdiobus_alloc(dev);
+	if (!bus)
+		return -ENOMEM;
+
+	ds->slave_mii_bus = bus;
+	bus->priv = priv;
+	bus->name = KBUILD_MODNAME "-mii";
+	snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);
+	bus->read = mt753x_phy_read;
+	bus->write = mt753x_phy_write;
+	bus->parent = dev;
+	bus->phy_mask = ~ds->phys_mii_mask;
+
+	if (priv->irq)
+		mt7530_setup_mdio_irq(priv);
+
+	ret = mdiobus_register(bus);
+	if (ret) {
+		dev_err(dev, "failed to register MDIO bus: %d\n", ret);
+		if (priv->irq)
+			mt7530_free_mdio_irq(priv);
+	}
+
+	return ret;
+}
+
 static int
 mt7530_setup(struct dsa_switch *ds)
 {
@@ -2791,24 +3006,20 @@ static int
 mt753x_setup(struct dsa_switch *ds)
 {
 	struct mt7530_priv *priv = ds->priv;
+	int ret = priv->info->sw_setup(ds);
 
-	return priv->info->sw_setup(ds);
-}
-
-static int
-mt753x_phy_read(struct dsa_switch *ds, int port, int regnum)
-{
-	struct mt7530_priv *priv = ds->priv;
+	if (ret)
+		return ret;
 
-	return priv->info->phy_read(ds, port, regnum);
-}
+	ret = mt7530_setup_irq(priv);
+	if (ret)
+		return ret;
 
-static int
-mt753x_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
-{
-	struct mt7530_priv *priv = ds->priv;
+	ret = mt7530_setup_mdio(priv);
+	if (ret && priv->irq)
+		mt7530_free_irq_common(priv);
 
-	return priv->info->phy_write(ds, port, regnum, val);
+	return ret;
 }
 
 static int mt753x_get_mac_eee(struct dsa_switch *ds, int port,
@@ -2845,8 +3056,6 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.get_tag_protocol	= mtk_get_tag_protocol,
 	.setup			= mt753x_setup,
 	.get_strings		= mt7530_get_strings,
-	.phy_read		= mt753x_phy_read,
-	.phy_write		= mt753x_phy_write,
 	.get_ethtool_stats	= mt7530_get_ethtool_stats,
 	.get_sset_count		= mt7530_get_sset_count,
 	.set_ageing_time	= mt7530_set_ageing_time,
@@ -3029,6 +3238,9 @@ mt7530_remove(struct mdio_device *mdiodev)
 		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);
 }
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 0204da486f3a..334d610a503d 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -7,6 +7,7 @@
 #define __MT7530_H
 
 #define MT7530_NUM_PORTS		7
+#define MT7530_NUM_PHYS			5
 #define MT7530_CPU_PORT			6
 #define MT7530_NUM_FDB_RECORDS		2048
 #define MT7530_ALL_MEMBERS		0xff
@@ -393,6 +394,12 @@ enum mt7531_sgmii_force_duplex {
 #define  SYS_CTRL_SW_RST		BIT(1)
 #define  SYS_CTRL_REG_RST		BIT(0)
 
+/* Register for system interrupt */
+#define MT7530_SYS_INT_EN		0x7008
+
+/* Register for system interrupt status */
+#define MT7530_SYS_INT_STS		0x700c
+
 /* Register for PHY Indirect Access Control */
 #define MT7531_PHY_IAC			0x701C
 #define  MT7531_PHY_ACS_ST		BIT(31)
@@ -714,6 +721,8 @@ static const char *p5_intf_modes(unsigned int p5_interface)
 	}
 }
 
+struct mt7530_priv;
+
 /* struct mt753x_info -	This is the main data structure for holding the specific
  *			part for each supported device
  * @sw_setup:		Holding the handler to a device initialization
@@ -738,8 +747,8 @@ struct mt753x_info {
 	enum mt753x_id id;
 
 	int (*sw_setup)(struct dsa_switch *ds);
-	int (*phy_read)(struct dsa_switch *ds, int port, int regnum);
-	int (*phy_write)(struct dsa_switch *ds, int port, int regnum, u16 val);
+	int (*phy_read)(struct mt7530_priv *priv, int port, int regnum);
+	int (*phy_write)(struct mt7530_priv *priv, int port, int regnum, u16 val);
 	int (*pad_setup)(struct dsa_switch *ds, phy_interface_t interface);
 	int (*cpu_port_config)(struct dsa_switch *ds, int port);
 	bool (*phy_mode_supported)(struct dsa_switch *ds, int port,
@@ -773,6 +782,10 @@ struct mt753x_info {
  *			registers
  * @p6_interface	Holding the current port 6 interface
  * @p5_intf_sel:	Holding the current port 5 interface select
+ *
+ * @irq:		IRQ number of the switch
+ * @irq_domain:		IRQ domain of the switch irq_chip
+ * @irq_enable:		IRQ enable bits, synced to SYS_INT_EN
  */
 struct mt7530_priv {
 	struct device		*dev;
@@ -794,6 +807,9 @@ struct mt7530_priv {
 	struct mt7530_port	ports[MT7530_NUM_PORTS];
 	/* protect among processes for registers access*/
 	struct mutex reg_mutex;
+	int irq;
+	struct irq_domain *irq_domain;
+	u32 irq_enable;
 };
 
 struct mt7530_hw_vlan_entry {
-- 
2.25.1

Re: [PATCH net-next 2/4] net: dsa: mt7530: add interrupt support

[Andrew Lunn]

On Thu, Apr 29, 2021 at 02:21:28PM +0800, DENG Qingfang wrote:
> Add support for MT7530 interrupt controller to handle internal PHYs.
> In order to assign an IRQ number to each PHY, the registration of MDIO bus
> is also done in this driver.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/4] net: dsa: mt7530: add interrupt support

[Florian Fainelli]

On 4/28/2021 11:21 PM, DENG Qingfang wrote:
> Add support for MT7530 interrupt controller to handle internal PHYs.
> In order to assign an IRQ number to each PHY, the registration of MDIO bus
> is also done in this driver.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---

[snip]

> +static int
> +mt7530_setup_irq(struct mt7530_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +
> +	if (!of_property_read_bool(np, "interrupt-controller")) {
> +		dev_info(dev, "no interrupt support\n");
> +		return 0;
> +	}
> +
> +	priv->irq = of_irq_get(np, 0);

Using platform_get_irq() may be a bit nicer to avoid using too many
OF-centric APIs, but this does not have to be changed right now.
Likewise for the interrupt-controller above.

> +	if (priv->irq <= 0) {
> +		dev_err(dev, "failed to get parent IRQ: %d\n", priv->irq);
> +		return priv->irq ? : -EINVAL;
> +	}
> +
> +	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;
> +	}
> +
> +	/* This register must be set for MT7530 to properly fire interrupts */
> +	if (priv->id != ID_MT7531)
> +		mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL);
> +
> +	ret = request_threaded_irq(priv->irq, NULL, mt7530_irq_thread_fn,
> +				   IRQF_ONESHOT, KBUILD_MODNAME, priv);

Maybe dev_name() would be more unique in case a system happens to have
more switches in the future so you can easily differentiate them.

> +	if (ret) {

Can you call mt7530_free_irq() to avoid the error repetition?

> +		irq_domain_remove(priv->irq_domain);
> +		dev_err(dev, "failed to request IRQ: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void
> +mt7530_free_mdio_irq(struct mt7530_priv *priv)
> +{
> +	int p;
> +
> +	for (p = 0; p < MT7530_NUM_PHYS; p++) {
> +		if (BIT(p) & priv->ds->phys_mii_mask) {
> +			unsigned int irq;
> +
> +			irq = irq_find_mapping(priv->irq_domain, p);
> +			irq_dispose_mapping(irq);
> +		}
> +	}
> +}
> +
> +static void
> +mt7530_free_irq_common(struct mt7530_priv *priv)
> +{
> +	free_irq(priv->irq, priv);
> +	irq_domain_remove(priv->irq_domain);
> +}
> +
> +static void
> +mt7530_free_irq(struct mt7530_priv *priv)
> +{
> +	mt7530_free_mdio_irq(priv);
> +	mt7530_free_irq_common(priv);
> +}
> +
> +static int
> +mt7530_setup_mdio(struct mt7530_priv *priv)
> +{
> +	struct dsa_switch *ds = priv->ds;
> +	struct device *dev = priv->dev;
> +	struct mii_bus *bus;
> +	static int idx;
> +	int ret;
> +
> +	bus = devm_mdiobus_alloc(dev);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	ds->slave_mii_bus = bus;
> +	bus->priv = priv;
> +	bus->name = KBUILD_MODNAME "-mii";
> +	snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);

Likewise using dev_name() here would provide an unique name in case you
have multiple switches.

Feel free to address my comments later, they do not seem to be blocking:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 2/4] net: dsa: mt7530: add interrupt support

[Vladimir Oltean]

On Thu, Apr 29, 2021 at 02:21:28PM +0800, DENG Qingfang wrote:
> Add support for MT7530 interrupt controller to handle internal PHYs.
> In order to assign an IRQ number to each PHY, the registration of MDIO bus
> is also done in this driver.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> RFC v4 -> PATCH v1:
> - Cosmetic fixes.
> 
>  drivers/net/dsa/Kconfig  |   1 +
>  drivers/net/dsa/mt7530.c | 264 +++++++++++++++++++++++++++++++++++----
>  drivers/net/dsa/mt7530.h |  20 ++-
>  3 files changed, 257 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index a5f1aa911fe2..264384449f09 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -36,6 +36,7 @@ config NET_DSA_LANTIQ_GSWIP
>  config NET_DSA_MT7530
>  	tristate "MediaTek MT753x and MT7621 Ethernet switch support"
>  	select NET_DSA_TAG_MTK
> +	select MEDIATEK_PHY

I'm pretty much the last person you'd want to ask for Kconfig advice,
but I think that you should only select the Kconfig options which have
no dependencies of their own, and MEDIATEK_PHY isn't like that. Do you
ensure through some other way that its dependencies are always satisfied?

>  	help
>  	  This enables support for the MediaTek MT7530, MT7531, and MT7621
>  	  Ethernet switch chips.
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 96f7c9eede35..db838343fb05 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -10,6 +10,7 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/of_irq.h>
>  #include <linux/of_mdio.h>
>  #include <linux/of_net.h>
>  #include <linux/of_platform.h>
> @@ -596,18 +597,14 @@ mt7530_mib_reset(struct dsa_switch *ds)
>  	mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
>  }
>  
> -static int mt7530_phy_read(struct dsa_switch *ds, int port, int regnum)
> +static int mt7530_phy_read(struct mt7530_priv *priv, int port, int regnum)
>  {
> -	struct mt7530_priv *priv = ds->priv;
> -
>  	return mdiobus_read_nested(priv->bus, port, regnum);
>  }
>  
> -static int mt7530_phy_write(struct dsa_switch *ds, int port, int regnum,
> +static int mt7530_phy_write(struct mt7530_priv *priv, int port, int regnum,
>  			    u16 val)
>  {
> -	struct mt7530_priv *priv = ds->priv;
> -
>  	return mdiobus_write_nested(priv->bus, port, regnum, val);
>  }
>  
> @@ -785,9 +782,8 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
>  }
>  
>  static int
> -mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
> +mt7531_ind_phy_read(struct mt7530_priv *priv, int port, int regnum)
>  {
> -	struct mt7530_priv *priv = ds->priv;
>  	int devad;
>  	int ret;
>  
> @@ -803,10 +799,9 @@ mt7531_ind_phy_read(struct dsa_switch *ds, int port, int regnum)
>  }
>  
>  static int
> -mt7531_ind_phy_write(struct dsa_switch *ds, int port, int regnum,
> +mt7531_ind_phy_write(struct mt7530_priv *priv, int port, int regnum,
>  		     u16 data)
>  {
> -	struct mt7530_priv *priv = ds->priv;
>  	int devad;
>  	int ret;
>  
> @@ -822,6 +817,22 @@ mt7531_ind_phy_write(struct dsa_switch *ds, int port, int regnum,
>  	return ret;
>  }
>  
> +static int
> +mt753x_phy_read(struct mii_bus *bus, int port, int regnum)
> +{
> +	struct mt7530_priv *priv = bus->priv;
> +
> +	return priv->info->phy_read(priv, port, regnum);
> +}
> +
> +static int
> +mt753x_phy_write(struct mii_bus *bus, int port, int regnum, u16 val)
> +{
> +	struct mt7530_priv *priv = bus->priv;
> +
> +	return priv->info->phy_write(priv, port, regnum, val);
> +}
> +
>  static void
>  mt7530_get_strings(struct dsa_switch *ds, int port, u32 stringset,
>  		   uint8_t *data)
> @@ -1828,6 +1839,210 @@ mt7530_setup_gpio(struct mt7530_priv *priv)
>  }
>  #endif /* CONFIG_GPIOLIB */
>  
> +static irqreturn_t
> +mt7530_irq_thread_fn(int irq, void *dev_id)
> +{
> +	struct mt7530_priv *priv = dev_id;
> +	bool handled = false;
> +	u32 val;
> +	int p;
> +
> +	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);
> +
> +	for (p = 0; p < MT7530_NUM_PHYS; p++) {
> +		if (BIT(p) & val) {
> +			unsigned int irq;
> +
> +			irq = irq_find_mapping(priv->irq_domain, p);
> +			handle_nested_irq(irq);
> +			handled = true;
> +		}
> +	}
> +
> +	return IRQ_RETVAL(handled);
> +}
> +
> +static void
> +mt7530_irq_mask(struct irq_data *d)
> +{
> +	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
> +
> +	priv->irq_enable &= ~BIT(d->hwirq);
> +}
> +
> +static void
> +mt7530_irq_unmask(struct irq_data *d)
> +{
> +	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
> +
> +	priv->irq_enable |= BIT(d->hwirq);
> +}
> +
> +static void
> +mt7530_irq_bus_lock(struct irq_data *d)
> +{
> +	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
> +
> +	mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
> +}
> +
> +static void
> +mt7530_irq_bus_sync_unlock(struct irq_data *d)
> +{
> +	struct mt7530_priv *priv = irq_data_get_irq_chip_data(d);
> +
> +	mt7530_mii_write(priv, MT7530_SYS_INT_EN, priv->irq_enable);
> +	mutex_unlock(&priv->bus->mdio_lock);
> +}
> +
> +static struct irq_chip mt7530_irq_chip = {
> +	.name = KBUILD_MODNAME,
> +	.irq_mask = mt7530_irq_mask,
> +	.irq_unmask = mt7530_irq_unmask,
> +	.irq_bus_lock = mt7530_irq_bus_lock,
> +	.irq_bus_sync_unlock = mt7530_irq_bus_sync_unlock,
> +};
> +
> +static int
> +mt7530_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, &mt7530_irq_chip, handle_simple_irq);
> +	irq_set_nested_thread(irq, true);
> +	irq_set_noprobe(irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops mt7530_irq_domain_ops = {
> +	.map = mt7530_irq_map,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +static void
> +mt7530_setup_mdio_irq(struct mt7530_priv *priv)
> +{
> +	struct dsa_switch *ds = priv->ds;
> +	int p;
> +
> +	for (p = 0; p < MT7530_NUM_PHYS; p++) {
> +		if (BIT(p) & ds->phys_mii_mask) {
> +			unsigned int irq;
> +
> +			irq = irq_create_mapping(priv->irq_domain, p);
> +			ds->slave_mii_bus->irq[p] = irq;
> +		}
> +	}
> +}
> +
> +static int
> +mt7530_setup_irq(struct mt7530_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +
> +	if (!of_property_read_bool(np, "interrupt-controller")) {
> +		dev_info(dev, "no interrupt support\n");
> +		return 0;
> +	}
> +
> +	priv->irq = of_irq_get(np, 0);
> +	if (priv->irq <= 0) {
> +		dev_err(dev, "failed to get parent IRQ: %d\n", priv->irq);
> +		return priv->irq ? : -EINVAL;
> +	}
> +
> +	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;
> +	}
> +
> +	/* This register must be set for MT7530 to properly fire interrupts */
> +	if (priv->id != ID_MT7531)
> +		mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL);
> +
> +	ret = request_threaded_irq(priv->irq, NULL, mt7530_irq_thread_fn,
> +				   IRQF_ONESHOT, KBUILD_MODNAME, priv);
> +	if (ret) {
> +		irq_domain_remove(priv->irq_domain);
> +		dev_err(dev, "failed to request IRQ: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void
> +mt7530_free_mdio_irq(struct mt7530_priv *priv)
> +{
> +	int p;
> +
> +	for (p = 0; p < MT7530_NUM_PHYS; p++) {
> +		if (BIT(p) & priv->ds->phys_mii_mask) {
> +			unsigned int irq;
> +
> +			irq = irq_find_mapping(priv->irq_domain, p);
> +			irq_dispose_mapping(irq);
> +		}
> +	}
> +}
> +
> +static void
> +mt7530_free_irq_common(struct mt7530_priv *priv)
> +{
> +	free_irq(priv->irq, priv);
> +	irq_domain_remove(priv->irq_domain);
> +}
> +
> +static void
> +mt7530_free_irq(struct mt7530_priv *priv)
> +{
> +	mt7530_free_mdio_irq(priv);
> +	mt7530_free_irq_common(priv);
> +}
> +
> +static int
> +mt7530_setup_mdio(struct mt7530_priv *priv)
> +{
> +	struct dsa_switch *ds = priv->ds;
> +	struct device *dev = priv->dev;
> +	struct mii_bus *bus;
> +	static int idx;

Interesting use of "static".

> +	int ret;
> +
> +	bus = devm_mdiobus_alloc(dev);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	ds->slave_mii_bus = bus;
> +	bus->priv = priv;
> +	bus->name = KBUILD_MODNAME "-mii";
> +	snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);
> +	bus->read = mt753x_phy_read;
> +	bus->write = mt753x_phy_write;
> +	bus->parent = dev;
> +	bus->phy_mask = ~ds->phys_mii_mask;
> +
> +	if (priv->irq)
> +		mt7530_setup_mdio_irq(priv);
> +
> +	ret = mdiobus_register(bus);
> +	if (ret) {
> +		dev_err(dev, "failed to register MDIO bus: %d\n", ret);
> +		if (priv->irq)
> +			mt7530_free_mdio_irq(priv);
> +	}
> +
> +	return ret;
> +}
> +

Looks good.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

[PATCH net-next 3/4] dt-bindings: net: dsa: add MT7530 interrupt controller binding

[DENG Qingfang]

Add device tree binding to support MT7530 interrupt controller.

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
RFC v4 -> PATCH v1:
- No changes.

 Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
index de04626a8e9d..892b1570c496 100644
--- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
+++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
@@ -81,6 +81,12 @@ Optional properties:
 - gpio-controller: Boolean; if defined, MT7530's LED controller will run on
 	GPIO mode.
 - #gpio-cells: Must be 2 if gpio-controller is defined.
+- interrupt-controller: Boolean; Enables the internal interrupt controller.
+
+If interrupt-controller is defined, the following property is required.
+
+- #interrupt-cells: Must be 1.
+- interrupts: Parent interrupt for the interrupt controller.
 
 See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of additional
 required, optional properties and how the integrated switch subnodes must
-- 
2.25.1

Re: [PATCH net-next 3/4] dt-bindings: net: dsa: add MT7530 interrupt controller binding

[Andrew Lunn]

On Thu, Apr 29, 2021 at 02:21:29PM +0800, DENG Qingfang wrote:
> Add device tree binding to support MT7530 interrupt controller.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 3/4] dt-bindings: net: dsa: add MT7530 interrupt controller binding

[Florian Fainelli]

On 4/28/2021 11:21 PM, DENG Qingfang wrote:
> Add device tree binding to support MT7530 interrupt controller.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> RFC v4 -> PATCH v1:
> - No changes.
> 
>  Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
> index de04626a8e9d..892b1570c496 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mt7530.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
> @@ -81,6 +81,12 @@ Optional properties:
>  - gpio-controller: Boolean; if defined, MT7530's LED controller will run on
>  	GPIO mode.
>  - #gpio-cells: Must be 2 if gpio-controller is defined.
> +- interrupt-controller: Boolean; Enables the internal interrupt controller.
> +
> +If interrupt-controller is defined, the following property is required.

s/is/are/

It would be good to follow up with a converstion of this binding file to
YAML eventually.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 3/4] dt-bindings: net: dsa: add MT7530 interrupt controller binding

[Rob Herring]

On Thu, 29 Apr 2021 14:21:29 +0800, DENG Qingfang wrote:
> Add device tree binding to support MT7530 interrupt controller.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> RFC v4 -> PATCH v1:
> - No changes.
> 
>  Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH net-next 4/4] staging: mt7621-dts: enable MT7530 interrupt controller

[DENG Qingfang]

Enable MT7530 interrupt controller in the MT7621 SoC.

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
RFC v4 -> PATCH v1:
- No changes.

 drivers/staging/mt7621-dts/mt7621.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 16fc94f65486..0f7e487883a5 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -447,6 +447,10 @@ switch0: switch0@0 {
 				mediatek,mcm;
 				resets = <&rstctrl 2>;
 				reset-names = "mcm";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+				interrupt-parent = <&gic>;
+				interrupts = <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>;
 
 				ports {
 					#address-cells = <1>;
-- 
2.25.1

Re: [PATCH net-next 4/4] staging: mt7621-dts: enable MT7530 interrupt controller

[Andrew Lunn]

On Thu, Apr 29, 2021 at 02:21:30PM +0800, DENG Qingfang wrote:
> Enable MT7530 interrupt controller in the MT7621 SoC.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 4/4] staging: mt7621-dts: enable MT7530 interrupt controller

[Florian Fainelli]

On 4/28/2021 11:21 PM, DENG Qingfang wrote:
> Enable MT7530 interrupt controller in the MT7621 SoC.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next 4/4] staging: mt7621-dts: enable MT7530 interrupt controller

[Vladimir Oltean]

On Thu, Apr 29, 2021 at 02:21:30PM +0800, DENG Qingfang wrote:
> Enable MT7530 interrupt controller in the MT7621 SoC.
> 
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> RFC v4 -> PATCH v1:
> - No changes.
> 
>  drivers/staging/mt7621-dts/mt7621.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
> index 16fc94f65486..0f7e487883a5 100644
> --- a/drivers/staging/mt7621-dts/mt7621.dtsi
> +++ b/drivers/staging/mt7621-dts/mt7621.dtsi
> @@ -447,6 +447,10 @@ switch0: switch0@0 {
>  				mediatek,mcm;
>  				resets = <&rstctrl 2>;
>  				reset-names = "mcm";
> +				interrupt-controller;
> +				#interrupt-cells = <1>;
> +				interrupt-parent = <&gic>;
> +				interrupts = <GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>;
>  
>  				ports {
>  					#address-cells = <1>;
> -- 
> 2.25.1
> 

I don't remember if I mentioned this before, but a short-hand way of
expressing this is using:

	interrupts-extended = <&gic GIC_SHARED 23 IRQ_TYPE_LEVEL_HIGH>;

but the entire drivers/staging/mt7621-dts/mt7621.dtsi file uses
interrupt-parent, so this is fine.

Also, I panicked for a second thinking that this is the ARM GIC which
supports the GIC_SHARED flag, but I see that mt7621 is MIPS.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Re: [PATCH net-next 0/4] MT7530 interrupt support

[David Miller]

Please fix this:

error: the following would cause module name conflict:
  drivers/net/phy/mediatek.ko
  drivers/usb/musb/mediatek.ko


Thanks.

Re: [PATCH net-next 0/4] MT7530 interrupt support

[DENG Qingfang]

On Thu, Apr 29, 2021 at 05:08:15PM -0700, David Miller wrote:
> 
> Please fix this:
> 
> error: the following would cause module name conflict:
>   drivers/net/phy/mediatek.ko
>   drivers/usb/musb/mediatek.ko

So I still have to rename the PHY driver..
Andrew, what is your suggestion? Is mediatek_phy.c okay?

> 
> 
> Thanks.

Re: [PATCH net-next 0/4] MT7530 interrupt support

[Andrew Lunn]

On Fri, Apr 30, 2021 at 10:38:39AM +0800, DENG Qingfang wrote:
> On Thu, Apr 29, 2021 at 05:08:15PM -0700, David Miller wrote:
> > 
> > Please fix this:
> > 
> > error: the following would cause module name conflict:
> >   drivers/net/phy/mediatek.ko
> >   drivers/usb/musb/mediatek.ko
> 
> So I still have to rename the PHY driver..
> Andrew, what is your suggestion? Is mediatek_phy.c okay?

mediatek_phy.c gets you into trouble with the generic PHY drivers.
Most Ethernet PHY drivers have the model number in the file name. Does
the PHY have its own name/numbering, or is it always considered part
of the switch?  If the PHY has an identity of its own, use
that. Otherwise maybe mediatek75xx.c?

      Andrew

Aw: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Frank Wunderlich]

Hi,

i prepare rename of musb-driver to musb_mtk.ko as this is also not the best name.

if this module does only handle mt753x i would suggest

phy_mt753x.{c,ko}

regards Frank


> Gesendet: Freitag, 30. April 2021 um 14:24 Uhr
> Von: "Andrew Lunn" <andrew@lunn.ch>

> mediatek_phy.c gets you into trouble with the generic PHY drivers.
> Most Ethernet PHY drivers have the model number in the file name. Does
> the PHY have its own name/numbering, or is it always considered part
> of the switch?  If the PHY has an identity of its own, use
> that. Otherwise maybe mediatek75xx.c?

Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Andrew Lunn]

On Fri, Apr 30, 2021 at 02:37:16PM +0200, Frank Wunderlich wrote:
> Hi,
> 
> i prepare rename of musb-driver to musb_mtk.ko as this is also not the best name.
> 
> if this module does only handle mt753x i would suggest
> 
> phy_mt753x.{c,ko}

Hi Frank

That name will cause confusion with generic PHY drivers. They all seem
to use phy- as a file name prefix. At the moment, no Ethernet PHY
driver has phy as filename prefix or suffix.

       Andrew

Aw: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Frank Wunderlich]

Hi

which ethernet-phy drivers do you mean?
i found only combined phy-drivers with ethernet-handling (marvell,xilinx,mscc,ti) having the "phy-" prefix too

as i see something like this in drivers/phy/intel/

phy-intel-keembay-emmc.c
phy-intel-keembay-usb.c

maybe this in drivers/phy/mediatek/

phy-mediatek-mt753x.c

? maybe with additional -eth, but this seems redundant, as mt753x is an ethernet-switch

or like the other mtk-phys

phy-mtk-eth-mt7531.c (like phy-mtk-mipi-dsi.c,phy-mtk-hdmi.c,phy-mtk-hdmi-mt8173.c)

regards Frank


> Gesendet: Freitag, 30. April 2021 um 14:44 Uhr
> Von: "Andrew Lunn" <andrew@lunn.ch>
> That name will cause confusion with generic PHY drivers. They all seem
> to use phy- as a file name prefix. At the moment, no Ethernet PHY
> driver has phy as filename prefix or suffix.

Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Andrew Lunn]

On Fri, Apr 30, 2021 at 02:56:22PM +0200, Frank Wunderlich wrote:
> Hi
> 
> which ethernet-phy drivers do you mean?

drivers/net/phy.

> i found only combined phy-drivers with ethernet-handling (marvell,xilinx,mscc,ti) having the "phy-" prefix too
> 
> as i see something like this in drivers/phy/intel/
> 
> phy-intel-keembay-emmc.c
> phy-intel-keembay-usb.c
> 
> maybe this in drivers/phy/mediatek/
> 
> phy-mediatek-mt753x.c
> 
> ? maybe with additional -eth, but this seems redundant, as mt753x is an ethernet-switch
> 
> or like the other mtk-phys
> 
> phy-mtk-eth-mt7531.c (like phy-mtk-mipi-dsi.c,phy-mtk-hdmi.c,phy-mtk-hdmi-mt8173.c)

These are all examples of generic PHY drivers, not PHY drivers.

There is a lot of confusion between PHY drivers and generic PHY
drivers :-(

	Andrew

Aw: Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Frank Wunderlich]

> Gesendet: Freitag, 30. April 2021 um 15:13 Uhr
> Von: "Andrew Lunn" <andrew@lunn.ch>
> > which ethernet-phy drivers do you mean?
>
> drivers/net/phy.

ok, sorry :)

> These are all examples of generic PHY drivers, not PHY drivers.

> There is a lot of confusion between PHY drivers and generic PHY
> drivers :-(

right, there is (at least by me) a confusion about generic phy and phy (here net-only phy) drivers.

mhm, maybe the naming should differ if generic phy and net-phy are that different. i guess there is no way to merge the net phys to the generic phys (due to linking to the net device drivers) to have only 1 phy section, right?

but if phy- prefix is used by generic phys, maybe eth- or net- can be used here (maybe with "phy" added)

something like

eth-phy-mt753x.ko

else i have no idea now...my patch renaming the musb-module seems not to be accepted due to possible breakage

regards Frank

Re: Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Andrew Lunn]

> mhm, maybe the naming should differ if generic phy and net-phy are
> that different. i guess there is no way to merge the net phys to the
> generic phys (due to linking to the net device drivers) to have only
> 1 phy section, right?

phys and generic PHYs are very different things, completely different
API etc. They cannot be merged.

> but if phy- prefix is used by generic phys, maybe eth- or net- can be used here (maybe with "phy" added)
> 
> something like
> 
> eth-phy-mt753x.ko
> 
> else i have no idea now...my patch renaming the musb-module seems not
> to be accepted due to possible breakage

The usb module has been around for a long time, so it cannot be
changed. The phy driver is new, not in a released kernel. So we can
still rename it without causing problems.

I still want to understand the naming here. If you look at most
Ethernet switches with integrated PHYs, the PHYs have their own naming
scheme, separate from the switch, because they are independent IP. So
i would prefer this driver by named after the PHY name, not the switch
name. That might solve the naming conflict, mt123x for the PHY, mt7530
for the switch driver.

	Andrew

Re: Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Landen Chao]

On Fri, 2021-04-30 at 18:34 +0200, Andrew Lunn wrote:
> > mhm, maybe the naming should differ if generic phy and net-phy are
> > that different. i guess there is no way to merge the net phys to
> > the
> > generic phys (due to linking to the net device drivers) to have
> > only
> > 1 phy section, right?
> 
> phys and generic PHYs are very different things, completely different
> API etc. They cannot be merged.
> 
> > but if phy- prefix is used by generic phys, maybe eth- or net- can
> > be used here (maybe with "phy" added)
> > 
> > something like
> > 
> > eth-phy-mt753x.ko
How about using mediatek-ge.ko. 'ge' is the abbreviation of gigabit
Ethernet. Most mediatek products use the same gigabit Ethernet phy.

Landen
> > 
> > else i have no idea now...my patch renaming the musb-module seems
> > not
> > to be accepted due to possible breakage
> 
> The usb module has been around for a long time, so it cannot be
> changed. The phy driver is new, not in a released kernel. So we can
> still rename it without causing problems.
> 
> I still want to understand the naming here. If you look at most
> Ethernet switches with integrated PHYs, the PHYs have their own
> naming
> scheme, separate from the switch, because they are independent IP. So
> i would prefer this driver by named after the PHY name, not the
> switch
> name. That might solve the naming conflict, mt123x for the PHY,
> mt7530
> for the switch driver.
> 
> 	Andrew

Re: Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[DENG Qingfang]

On Wed, May 5, 2021 at 5:31 PM Landen Chao <landen.chao@mediatek.com> wrote:
> How about using mediatek-ge.ko. 'ge' is the abbreviation of gigabit
> Ethernet. Most mediatek products use the same gigabit Ethernet phy.

Well, MT7620's PHY is FE..

>
> Landen

Re: Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[Landen Chao]

On Wed, 2021-05-05 at 17:43 +0800, DENG Qingfang wrote:
> On Wed, May 5, 2021 at 5:31 PM Landen Chao <landen.chao@mediatek.com>
> wrote:
> > How about using mediatek-ge.ko. 'ge' is the abbreviation of gigabit
> > Ethernet. Most mediatek products use the same gigabit Ethernet phy.
> 
> Well, MT7620's PHY is FE..
MT7620's FE PHY HW is different from MT753x's GE PHY. Vendor registers
of these two PHY are totally different.
However, MT7620 internal switch MAC is quite the same as MT753x's MAC
except for VLAN table and other minor changes. The MAC part can be
reused. 

> 
> > 
> > Landen

Re: Re: Re: Re: [PATCH net-next 0/4] MT7530 interrupt support

[DENG Qingfang]

On Thu, May 6, 2021 at 8:54 PM Landen Chao <landen.chao@mediatek.com> wrote:
> MT7620's FE PHY HW is different from MT753x's GE PHY. Vendor registers
> of these two PHY are totally different.

Okay. So if the FE PHY is added later, it can be named mediatek-fe.c.