| Message ID | 20210429062130.29403-1-dqfext@gmail.com |
|---|---|
| Headers | show |
| Series | MT7530 interrupt support | expand |
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
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
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
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>
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>
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>
Please fix this: error: the following would cause module name conflict: drivers/net/phy/mediatek.ko drivers/usb/musb/mediatek.ko Thanks.
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.
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
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?
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
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.
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
> 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
> 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
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
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
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); > +}
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>
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>
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
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.