2021-04-29 06:22:41

by Qingfang Deng

[permalink] [raw]
Subject: [PATCH net-next 0/4] MT7530 interrupt support

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


2021-04-29 06:23:00

by Qingfang Deng

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

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 <[email protected]>
---
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 <[email protected]>");
+MODULE_LICENSE("GPL");
+
+MODULE_DEVICE_TABLE(mdio, mtk_phy_tbl);
--
2.25.1

2021-04-29 06:23:09

by Qingfang Deng

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

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 <[email protected]>
---
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

2021-04-29 06:24:49

by Qingfang Deng

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

Add device tree binding to support MT7530 interrupt controller.

Signed-off-by: DENG Qingfang <[email protected]>
---
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

2021-04-29 06:24:49

by Qingfang Deng

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

Enable MT7530 interrupt controller in the MT7621 SoC.

Signed-off-by: DENG Qingfang <[email protected]>
---
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

2021-04-29 21:28:19

by Andrew Lunn

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

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 <[email protected]>

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

> +
> + /* 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

2021-04-29 21:32:17

by Andrew Lunn

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

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 <[email protected]>

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

Andrew

2021-04-29 21:32:36

by Andrew Lunn

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

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 <[email protected]>

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

Andrew

2021-04-29 21:33:38

by Andrew Lunn

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

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 <[email protected]>

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

Andrew

2021-04-29 23:33:25

by Florian Fainelli

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



On 4/28/2021 11:21 PM, DENG Qingfang wrote:
> Enable MT7530 interrupt controller in the MT7621 SoC.
>
> Signed-off-by: DENG Qingfang <[email protected]>

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

2021-04-29 23:35:29

by Florian Fainelli

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



On 4/28/2021 11:21 PM, DENG Qingfang wrote:
> Add device tree binding to support MT7530 interrupt controller.
>
> Signed-off-by: DENG Qingfang <[email protected]>
> ---
> 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 <[email protected]>
--
Florian

2021-04-29 23:42:07

by Florian Fainelli

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



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 <[email protected]>
> ---

[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 <[email protected]>
--
Florian

2021-04-29 23:43:24

by Florian Fainelli

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



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 <[email protected]>

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

2021-04-30 00:12:53

by David Miller

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


Please fix this:

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


Thanks.

2021-04-30 02:43:26

by Qingfang Deng

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

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.

2021-04-30 12:28:07

by Andrew Lunn

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

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

2021-04-30 12:41:32

by Frank Wunderlich

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

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" <[email protected]>

> 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?

2021-04-30 12:46:49

by Andrew Lunn

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

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

2021-04-30 12:58:42

by Frank Wunderlich

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

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" <[email protected]>
> 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.

2021-04-30 13:15:26

by Andrew Lunn

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

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

2021-04-30 15:05:10

by Frank Wunderlich

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

> Gesendet: Freitag, 30. April 2021 um 15:13 Uhr
> Von: "Andrew Lunn" <[email protected]>
> > 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

2021-04-30 16:36:10

by Andrew Lunn

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

> 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

2021-05-03 19:37:15

by Rob Herring

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

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 <[email protected]>
> ---
> RFC v4 -> PATCH v1:
> - No changes.
>
> Documentation/devicetree/bindings/net/dsa/mt7530.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>

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

2021-05-05 09:39:14

by Landen Chao (趙皎宏)

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

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

2021-05-05 09:48:12

by Qingfang Deng

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

On Wed, May 5, 2021 at 5:31 PM Landen Chao <[email protected]> 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

2021-05-06 11:37:43

by Vladimir Oltean

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

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 <[email protected]>
> ---
> 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);
> +}

2021-05-06 12:15:32

by Vladimir Oltean

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

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-05-06 12:56:47

by Landen Chao (趙皎宏)

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

On Wed, 2021-05-05 at 17:43 +0800, DENG Qingfang wrote:
> On Wed, May 5, 2021 at 5:31 PM Landen Chao <[email protected]>
> 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

2021-05-06 18:14:04

by Vladimir Oltean

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

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-05-07 09:06:26

by Qingfang Deng

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

On Thu, May 6, 2021 at 8:54 PM Landen Chao <[email protected]> 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.