| Message ID | 20220822092017.5671-1-arun.ramadoss@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC,net-next,v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection | expand |
> +static irqreturn_t lan937x_switch_irq_thread(int irq, void *dev_id) > +{ > + struct ksz_device *dev = dev_id; > + irqreturn_t result = IRQ_NONE; > + u32 data; > + int ret; > + > + /* Read global interrupt status register */ > + ret = ksz_read32(dev, REG_SW_INT_STATUS__4, &data); > + if (ret) > + return result; I don't think you can return negative error numbers here. > + > + if (data & POR_READY_INT) { > + ret = ksz_write32(dev, REG_SW_INT_STATUS__4, POR_READY_INT); > + if (ret) > + return result; Returning IRQ_NONE here seems wrong. You handle the interrupt, so should probably return IRQ_HANDLED. Andrew
On Mon, Aug 22, 2022 at 02:50:17PM +0530, Arun Ramadoss wrote: > This patch enables the interrupts for internal phy link detection for > LAN937x. The interrupt enable bits are active low. It first enables port > interrupt and then port phy interrupt. Also patch register the irq > thread and in the ISR routine it clears the POR_READY_STS bit. > POR_READY_STS bit is write one clear bit and all other bit in the > register are read only. Since phy interrupts are handled by the lan937x > phy layer, switch interrupt routine does not read the phy layer > interrupts. > +static irqreturn_t lan937x_switch_irq_thread(int irq, void *dev_id) > +{ > + struct ksz_device *dev = dev_id; > + irqreturn_t result = IRQ_NONE; > + u32 data; > + int ret; > + > + /* Read global interrupt status register */ > + ret = ksz_read32(dev, REG_SW_INT_STATUS__4, &data); > + if (ret) > + return result; > + > + if (data & POR_READY_INT) { > + ret = ksz_write32(dev, REG_SW_INT_STATUS__4, POR_READY_INT); > + if (ret) > + return result; > + } > + > + return result; > +} I don't understand how this all fits together. How do you get from this interrupt handler into the PHY interrupt handler? The hardware looks similar to the mv88e6xxx driver. You have a top level interrupt controller which indicates a port has some sort of interrupt handler. This is the mv88e6xxx_g1_irq_thread_work(). It finds which port triggered the interrupt and then hands the interrupt off to the nested interrupt handler. mv88e6xxx_g2_irq_thread_fn() is the nested per port interrupt handler. It reads the per port interrupt status register, find the interrupt handler and calls the nested interrupt handler. This all glues together because phylib does a request_threaded_irq() for the PHY interrupt, so this last nested interrupt handler is in phylib. Andrew
Hi Andrew, Thanks for the comments On Mon, 2022-08-22 at 15:12 +0200, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Mon, Aug 22, 2022 at 02:50:17PM +0530, Arun Ramadoss wrote: > > This patch enables the interrupts for internal phy link detection > > for > > LAN937x. The interrupt enable bits are active low. It first enables > > port > > interrupt and then port phy interrupt. Also patch register the irq > > thread and in the ISR routine it clears the POR_READY_STS bit. > > POR_READY_STS bit is write one clear bit and all other bit in the > > register are read only. Since phy interrupts are handled by the > > lan937x > > phy layer, switch interrupt routine does not read the phy layer > > interrupts. > > +static irqreturn_t lan937x_switch_irq_thread(int irq, void > > *dev_id) > > +{ > > + struct ksz_device *dev = dev_id; > > + irqreturn_t result = IRQ_NONE; > > + u32 data; > > + int ret; > > + > > + /* Read global interrupt status register */ > > + ret = ksz_read32(dev, REG_SW_INT_STATUS__4, &data); > > + if (ret) > > + return result; > > + > > + if (data & POR_READY_INT) { > > + ret = ksz_write32(dev, REG_SW_INT_STATUS__4, > > POR_READY_INT); > > + if (ret) > > + return result; > > + } > > + > > + return result; > > +} > > I don't understand how this all fits together. How do you get from > this interrupt handler into the PHY interrupt handler? > I used the same gpio line number of switch as the interrupt for internal phy. And when phy link up/down happens, it triggers both the switch and phy interrupt routine. Earlier I tried to find out how to link switch port interrupt to phylib but I could not get the logic. Only after reading your comment I come to know phy interrupt can be triggered from the switch interrupt handler. Today I went through the marvel implementation on interrupt handling but could not grasp fully. ( https://lore.kernel.org/all/1476640613-25365-3-git-send-email-andrew@lunn.ch/T/#m74a139d43ca64833832d86810e2f4474277d7bf2 ) I have not used irq_domain before. Can you please brief on how phy interrupt handler is called from chip.c & global2. The dts file I used for testing, spi1: spi@f8008000 { cs-gpios = <0>, <0>, <0>, <&pioC 28 0>; id = <1>; status = "okay"; lan9370: lan9370@3 { compatible = "microchip,lan9370"; reg = <3>; spi-max-frequency = <44000000>; interrupt-parent = <&pioB>; interrupts = <28 IRQ_TYPE_LEVEL_LOW>; interrupt-controller; status = "okay"; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0x0>; phy-handle = <&t1phy0>; phy-mode = "internal"; label = "lan1"; }; port@1 { reg = <0x1>; phy-handle = <&t1phy1>; phy-mode = "internal"; label = "lan2"; }; } } mdio { #address-cells = <1>; #size-cells = <0>; compatible = "microchip,lan937x-mdio"; t1phy0: ethernet-phy@0{ interrupt-parent = <&lan9370>; interrupts = <28 IRQ_TYPE_LEVEL_LOW>; reg = <0x0>; }; t1phy1: ethernet-phy@1{ interrupt-parent = <&lan9370>; interrupts = <28 IRQ_TYPE_LEVEL_LOW>; reg = <0x1>; }; } } Is there anything, I need to change in the dts file. > The hardware looks similar to the mv88e6xxx driver. You have a top > level interrupt controller which indicates a port has some sort of > interrupt handler. This is the mv88e6xxx_g1_irq_thread_work(). It > finds which port triggered the interrupt and then hands the interrupt > off to the nested interrupt handler. > > mv88e6xxx_g2_irq_thread_fn() is the nested per port interrupt > handler. It reads the per port interrupt status register, find the > interrupt handler and calls the nested interrupt handler. > > This all glues together because phylib does a request_threaded_irq() > for the PHY interrupt, so this last nested interrupt handler is in > phylib. > > Andrew
> I used the same gpio line number of switch as the interrupt for > internal phy. And when phy link up/down happens, it triggers both the > switch and phy interrupt routine. Ah, shared interrupt. O.K. > I have not used irq_domain before. Can you please brief on how phy > interrupt handler is called from chip.c & global2. There are two different ways this can all be glued together. Using irq_domain, each interrupt source becomes a full interrupt in Linux. You can see these individual interrupt sources in /proc/interrupts. You can use request_threaded_irq() on it. The mv88e6xxx driver is also an interrupt controller as well as an Ethernet switch. > The dts file I used for testing, > spi1: spi@f8008000 { > cs-gpios = <0>, <0>, <0>, <&pioC 28 0>; > id = <1>; > status = "okay"; > > lan9370: lan9370@3 { > compatible = "microchip,lan9370"; > reg = <3>; > spi-max-frequency = <44000000>; > interrupt-parent = <&pioB>; > interrupts = <28 IRQ_TYPE_LEVEL_LOW>; > interrupt-controller; > status = "okay"; > ports { > #address-cells = <1>; > #size-cells = <0>; > port@0 { > reg = <0x0>; > phy-handle = <&t1phy0>; > phy-mode = "internal"; > label = "lan1"; > }; > port@1 { > reg = <0x1>; > phy-handle = <&t1phy1>; > phy-mode = "internal"; > label = "lan2"; > }; > } > } > > mdio { > #address-cells = <1>; > #size-cells = <0>; > compatible = "microchip,lan937x-mdio"; > > t1phy0: ethernet-phy@0{ > interrupt-parent = <&lan9370>; > interrupts = <28 IRQ_TYPE_LEVEL_LOW>; So here you would use the interrupt number within the domain. Ideally you want port 0 to use interrupt 0. > reg = <0x0>; > }; > t1phy1: ethernet-phy@1{ > interrupt-parent = <&lan9370>; > interrupts = <28 IRQ_TYPE_LEVEL_LOW>; and here port 1 uses interrupt 1. > reg = <0x1>; > }; > } > } You can see this for an Marvell switch here: https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts#L93 Doing it this way is however very verbose. I later discovered a short cut: https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1164 by setting mdiobus->irq[] to the interrupt number, phylib will automatically use the correct interrupt without needing an DT. Andrew
Hi Andrew, Thanks for the reply. On Tue, 2022-08-23 at 16:33 +0200, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > > https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts#L93 > > Doing it this way is however very verbose. I later discovered a short > cut: > > https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1164 > > by setting mdiobus->irq[] to the interrupt number, phylib will > automatically use the correct interrupt without needing an DT. > > Andrew In LAN937x, register REG_SW_PORT_INT_MASK__4 BIT7:0 used to enable/disable the interrupts for each port. It is the global interrupt enable for ports. In turn each port has REG_PORT_INT_MASK register, which enable/disable interrupts like PTP, PHY (BIT 1). And in turn for each phy it has different interrupts like link up, link down etc. As per your suggestion, I enabled the global_irq domain for each port and port_irq domain for port 1 alone and the corresponding mdio irq is updated. The dts is updated with *only one user port*. And it is working as expected. How can I extend the above procedure for remaining ports 2 -8. Do I need to create the array of port_irq[8] domain. But when I analyzed code flow, if the port_irq_thread is triggered it has function parameter as irq only. From this irq how can we derive the parent irq i.e from which port is triggered. Only if I know the port, then I can read the corresponding status register and check if the interrupt is from phy or ptp etc. Can you suggest how to enable irq_domain for the each port and find out the phy interrupt. So that it can be extended further in our ptp interrupt based implementation. Below is the draft code on implementation of single port irq domain. diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 764ada3a0f42..225d65b4b43e 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -63,6 +63,13 @@ struct ksz_chip_data { bool internal_phy[KSZ_MAX_NUM_PORTS]; }; +struct ksz_irq { + u16 masked; + struct irq_chip chip; + struct irq_domain *domain; + int nirqs; +}; + struct ksz_port { bool remove_tag; int stp_state; @@ -98,6 +105,7 @@ struct ksz_device { struct regmap *regmap[3]; void *priv; + int irq; struct gpio_desc *reset_gpio; /* Optional reset GPIO */ @@ -119,6 +127,9 @@ struct ksz_device { u16 mirror_tx; u32 features; /* chip specific features */ u16 port_mask; + struct mutex lock_irq; + struct ksz_irq global_irq; + struct ksz_irq port_irq; }; /* List of supported models */ diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c index 05bd089795f8..7ba897b6f950 100644 --- a/drivers/net/dsa/microchip/ksz_spi.c +++ b/drivers/net/dsa/microchip/ksz_spi.c @@ -85,6 +85,8 @@ static int ksz_spi_probe(struct spi_device *spi) if (ret) return ret; + dev->irq = spi->irq; + ret = ksz_switch_register(dev); /* Main DSA driver may not be started yet. */ diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c index daedd2bf20c1..6216c34c8cba 100644 --- a/drivers/net/dsa/microchip/lan937x_main.c +++ b/drivers/net/dsa/microchip/lan937x_main.c @@ -10,6 +10,7 @@ #include <linux/of_mdio.h> #include <linux/if_bridge.h> #include <linux/if_vlan.h> +#include <linux/irqdomain.h> #include <linux/math.h> #include <net/dsa.h> #include <net/switchdev.h> @@ -171,6 +172,7 @@ static int lan937x_mdio_register(struct ksz_device *dev) struct device_node *mdio_np; struct mii_bus *bus; int ret; + int p; mdio_np = of_get_child_by_name(dev->dev->of_node, "mdio"); if (!mdio_np) { @@ -194,6 +196,16 @@ static int lan937x_mdio_register(struct ksz_device *dev) ds->slave_mii_bus = bus; + for (p = 0; p < 8; p++) { + if (BIT(p) & ds->phys_mii_mask) { + unsigned int irq; + + irq = irq_find_mapping(dev->port_irq.domain, 1); + ds->slave_mii_bus->irq[p] = irq; + } + } + + ret = devm_of_mdiobus_register(ds->dev, bus, mdio_np); if (ret) { dev_err(ds->dev, "unable to register MDIO bus %s\n", @@ -383,6 +395,291 @@ void lan937x_setup_rgmii_delay(struct ksz_device *dev, int port) } } +int lan937x_switch_init(struct ksz_device *dev) +{ + dev->port_mask = (1 << dev->info->port_cnt) - 1; + + return 0; +} + +void lan937x_switch_exit(struct ksz_device *dev) +{ + lan937x_reset_switch(dev); +} + +static void lan937x_global_irq_mask(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + unsigned int n = d->hwirq; + + dev->global_irq.masked |= (1 << n); +} + +static void lan937x_global_irq_unmask(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + unsigned int n = d->hwirq; + + dev->global_irq.masked &= ~(1 << n); +} + +static void lan937x_global_irq_bus_lock(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + + mutex_lock(&dev->lock_irq); +} + +static void lan937x_global_irq_bus_sync_unlock(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + int ret; + + ret = ksz_write32(dev, REG_SW_PORT_INT_MASK__4, dev- >global_irq.masked); + if (ret) + dev_err(dev->dev, "failed to change IRQ mask\n"); + + mutex_unlock(&dev->lock_irq); +} + +static const struct irq_chip lan937x_global_irq_chip = { + .name = "lan937x-global", + .irq_mask = lan937x_global_irq_mask, + .irq_unmask = lan937x_global_irq_unmask, + .irq_bus_lock = lan937x_global_irq_bus_lock, + .irq_bus_sync_unlock = lan937x_global_irq_bus_sync_unlock, +}; + + +static int lan937x_global_irq_domain_map(struct irq_domain *d, + unsigned int irq, + irq_hw_number_t hwirq) +{ + struct ksz_device *dev = d->host_data; + + irq_set_chip_data(irq, d->host_data); + irq_set_chip_and_handler(irq, &dev->global_irq.chip, handle_level_irq); + irq_set_noprobe(irq); + + return 0; +} + +static const struct irq_domain_ops lan937x_global_irq_domain_ops = { + .map = lan937x_global_irq_domain_map, + .xlate = irq_domain_xlate_twocell, +}; + +static void lan937x_global_irq_free(struct ksz_device *dev) +{ + int irq, virq; + + for (irq = 0; irq < 8; irq++) { + virq = irq_find_mapping(dev->global_irq.domain, irq); + irq_dispose_mapping(virq); + } + + irq_domain_remove(dev->global_irq.domain); +} + +static irqreturn_t lan937x_global_irq_thread_fn(int irq, void *dev_id) +{ + struct ksz_device *dev = dev_id; + unsigned int nhandled = 0; + unsigned int sub_irq; + unsigned int n; + u32 data; + int ret; + + ret = ksz_read32(dev, REG_SW_INT_STATUS__4, &data); + if (ret) + goto out; + + if (data & POR_READY_INT) { + ret = ksz_write32(dev, REG_SW_INT_STATUS__4, POR_READY_INT); + if (ret) + goto out; + } + + /* Read global interrupt status register */ + ret = ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data); + if (ret) + goto out; + + for (n = 0; n < dev->global_irq.nirqs; ++n) { + if (data & (1 << n)) { + sub_irq = irq_find_mapping(dev- >global_irq.domain, n); + handle_nested_irq(sub_irq); + ++nhandled; + } + } +out: + return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE); +} + +static int lan937x_global_irq_setup(struct ksz_device *dev) +{ + int ret, irq; + + dev->global_irq.nirqs = 8; + dev->global_irq.domain = irq_domain_add_simple( + NULL, dev->global_irq.nirqs, 0, + &lan937x_global_irq_domain_ops, dev); + if (!dev->global_irq.domain) + return -ENOMEM; + + for (irq = 0; irq < dev->global_irq.nirqs; irq++) + irq_create_mapping(dev->global_irq.domain, irq); + + dev->global_irq.chip = lan937x_global_irq_chip; + dev->global_irq.masked = ~0; + + ret = request_threaded_irq(dev->irq, NULL, + lan937x_global_irq_thread_fn, + IRQF_ONESHOT | IRQF_TRIGGER_FALLING, + dev_name(dev->dev), dev); + if (ret) + goto out; + + return 0; + +out: + lan937x_global_irq_free(dev); + + return ret; +} + +static void lan937x_port_irq_mask(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + unsigned int n = d->hwirq; + + + dev->port_irq.masked |= (1 << n); +} + +static void lan937x_port_irq_unmask(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + unsigned int n = d->hwirq; + + dev->port_irq.masked &= ~(1 << n); +} + +static void lan937x_port_irq_bus_lock(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + + mutex_lock(&dev->lock_irq); +} + +static void lan937x_port_irq_bus_sync_unlock(struct irq_data *d) +{ + struct ksz_device *dev = irq_data_get_irq_chip_data(d); + int ret; + u8 data; + + ksz_pwrite8(dev, 0, REG_PORT_INT_MASK, dev->port_irq.masked); + mutex_unlock(&dev->lock_irq); +} + +static const struct irq_chip lan937x_port_irq_chip = { + .name = "lan937x-port", + .irq_mask = lan937x_port_irq_mask, + .irq_unmask = lan937x_port_irq_unmask, + .irq_bus_lock = lan937x_port_irq_bus_lock, + .irq_bus_sync_unlock = lan937x_port_irq_bus_sync_unlock, +}; + +static int lan937x_port_irq_domain_map(struct irq_domain *d, + unsigned int irq, + irq_hw_number_t hwirq) +{ + struct ksz_device *dev = d->host_data; + + irq_set_chip_data(irq, d->host_data); + irq_set_chip_and_handler(irq, &dev->port_irq.chip, handle_level_irq); + irq_set_noprobe(irq); + + return 0; +} + +static const struct irq_domain_ops lan937x_port_irq_domain_ops = { + .map = lan937x_port_irq_domain_map, + .xlate = irq_domain_xlate_twocell, +}; + +static void lan937x_port_irq_free(struct ksz_device *dev) +{ + int irq, virq; + + for (irq = 0; irq < 8; irq++) { + virq = irq_find_mapping(dev->port_irq.domain, irq); + irq_dispose_mapping(virq); + } + + irq_domain_remove(dev->port_irq.domain); +} + + +static irqreturn_t lan937x_port_irq_thread_fn(int irq, void *dev_id) +{ + struct ksz_device *dev = dev_id; + unsigned int nhandled = 0; + unsigned int sub_irq; + unsigned int n; + u8 data; + int ret; + + /* Read global interrupt status register */ + ksz_pread8(dev, 0, REG_PORT_INT_STATUS, &data); + + for (n = 0; n < 6; ++n) { + if (data & (1 << n)) { + sub_irq = irq_find_mapping(dev- >port_irq.domain, n); + handle_nested_irq(sub_irq); + ++nhandled; + } + } +out: + return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE); +} + +static int lan937x_port_irq_setup(struct ksz_device *dev) +{ + int port_irq; + int ret, irq; + + dev->port_irq.nirqs = 6; + dev->port_irq.domain = irq_domain_add_simple( + dev->dev->of_node, dev->port_irq.nirqs, 0, + &lan937x_port_irq_domain_ops, dev); + if (!dev->port_irq.domain) + return -ENOMEM; + + for (irq = 0; irq < dev->port_irq.nirqs; irq++) + irq_create_mapping(dev->port_irq.domain, irq); + + dev->port_irq.chip = lan937x_port_irq_chip; + dev->port_irq.masked = ~0; + + port_irq = irq_find_mapping(dev->global_irq.domain, + 0); + + ret = request_threaded_irq(port_irq, NULL, + lan937x_port_irq_thread_fn, + IRQF_ONESHOT | IRQF_TRIGGER_FALLING, + "port_irq", dev); + if (ret) + goto out; + + return 0; + +out: + lan937x_port_irq_free(dev); + + return ret; +} + int lan937x_setup(struct dsa_switch *ds) { struct ksz_device *dev = ds->priv; @@ -395,6 +692,10 @@ int lan937x_setup(struct dsa_switch *ds) return ret; } + lan937x_global_irq_setup(dev); + + lan937x_port_irq_setup(dev); + ret = lan937x_mdio_register(dev); if (ret < 0) { dev_err(dev->dev, "failed to register the mdio"); @@ -426,17 +727,6 @@ int lan937x_setup(struct dsa_switch *ds) return 0; }
On Fri, Aug 26, 2022 at 04:21:39PM +0000, Arun.Ramadoss@microchip.com wrote: > Hi Andrew, > Thanks for the reply. > > On Tue, 2022-08-23 at 16:33 +0200, Andrew Lunn wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > know the content is safe > > > > > > > https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts#L93 > > > > Doing it this way is however very verbose. I later discovered a short > > cut: > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1164 > > > > by setting mdiobus->irq[] to the interrupt number, phylib will > > automatically use the correct interrupt without needing an DT. > > > > Andrew > > In LAN937x, register REG_SW_PORT_INT_MASK__4 BIT7:0 used to > enable/disable the interrupts for each port. It is the global interrupt > enable for ports. In turn each port has REG_PORT_INT_MASK register, > which enable/disable interrupts like PTP, PHY (BIT 1). And in turn for > each phy it has different interrupts like link up, link down etc. > > As per your suggestion, I enabled the global_irq domain for each port > and port_irq domain for port 1 alone and the corresponding mdio irq is > updated. The dts is updated with *only one user port*. And it is > working as expected. > > How can I extend the above procedure for remaining ports 2 -8. Do I > need to create the array of port_irq[8] domain. But when I analyzed > code flow, if the port_irq_thread is triggered it has function > parameter as irq only. From this irq how can we derive the parent irq > i.e from which port is triggered. Only if I know the port, then I can > read the corresponding status register and check if the interrupt is > from phy or ptp etc. > > Can you suggest how to enable irq_domain for the each port and find out > the phy interrupt. So that it can be extended further in our ptp > interrupt based implementation. You need an interrupt controller per port. Your top level interrupt controller should be pretty similar to the mv88e6xxx global 1. However, your need to create a second level interrupt controller per port. This is where mv88e6xxx creates its one second level interrupt controller: https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1135 Rather than finding the mapping for a fixed interrupt number, you need to get the mapping for the specific port you are creating the interrupt controller for. In the end you will have a tree of interrupt controllers, where as mv88e6xxx just has a chain. Here is what mv88e6xxx looks like: 47: 0 gpio-vf610 27 Level mv88e6xxx-mdio_mux-0.1:00 51: 0 mv88e6xxx-g1 3 Edge mv88e6xxx-mdio_mux-0.1:00-g1-atu-prob 53: 0 mv88e6xxx-g1 5 Edge mv88e6xxx-mdio_mux-0.1:00-g1-vtu-prob 55: 0 mv88e6xxx-g1 7 Edge mv88e6xxx-mdio_mux-0.1:00-g2 57: 0 mv88e6xxx-g2 0 Edge !mdio-mux!mdio@1!switch@0!mdio:00 58: 0 mv88e6xxx-g2 1 Edge !mdio-mux!mdio@1!switch@0!mdio:01 59: 0 mv88e6xxx-g2 2 Edge !mdio-mux!mdio@1!switch@0!mdio:02 72: 0 mv88e6xxx-g2 15 Edge mv88e6xxx-mdio_mux-0.1:00-watchdog Interrupt 47 is the GPIO line the switch it attached to. Interrupts 51 and 53 are handlers registered to the top level interrupt controller. Interrupt 55 is the chain into the second level interrupt handler. Interrupts 57-59 are PHY interrupts in the second level interrupt controller, and interrupt 72 is also part of the second level. For you, you will have a chain interrupt in the top level for each port, rather than the single 55 interrupt here. Build the tree and it should all work. But make sure you turn on lockdep. I took me a while to get all the locking correct with mv88e6xxx_reg_lock(). Maybe you wont face this problem with the ksz driver, its locking could be different. As to your question about knowing where the interrupt came from, you can see in: https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/global2.c#L1026 that the interrupt handler gets called with a void * dev_id. For mv88e6xxx this points to the chip structure. You can make it point to the port structure. This is the last parameter you pass to request_threaded_irq(). Andrew
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 764ada3a0f42..a84488e6fab6 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -98,6 +98,7 @@ struct ksz_device { struct regmap *regmap[3]; void *priv; + int irq; struct gpio_desc *reset_gpio; /* Optional reset GPIO */ diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c index 05bd089795f8..7ba897b6f950 100644 --- a/drivers/net/dsa/microchip/ksz_spi.c +++ b/drivers/net/dsa/microchip/ksz_spi.c @@ -85,6 +85,8 @@ static int ksz_spi_probe(struct spi_device *spi) if (ret) return ret; + dev->irq = spi->irq; + ret = ksz_switch_register(dev); /* Main DSA driver may not be started yet. */ diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c index daedd2bf20c1..ca786e5edf2c 100644 --- a/drivers/net/dsa/microchip/lan937x_main.c +++ b/drivers/net/dsa/microchip/lan937x_main.c @@ -10,6 +10,7 @@ #include <linux/of_mdio.h> #include <linux/if_bridge.h> #include <linux/if_vlan.h> +#include <linux/irq.h> #include <linux/math.h> #include <net/dsa.h> #include <net/switchdev.h> @@ -23,6 +24,11 @@ static int lan937x_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set) return regmap_update_bits(dev->regmap[0], addr, bits, set ? bits : 0); } +static int lan937x_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set) +{ + return regmap_update_bits(dev->regmap[2], addr, bits, set ? bits : 0); +} + static int lan937x_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits, bool set) { @@ -285,6 +291,16 @@ void lan937x_config_cpu_port(struct dsa_switch *ds) dsa_switch_for_each_user_port(dp, ds) { ksz_port_stp_state_set(ds, dp->index, BR_STATE_DISABLED); + + if (dev->info->internal_phy[dp->index]) { + /* Enable PORT Interrupt - active low */ + lan937x_cfg32(dev, REG_SW_PORT_INT_MASK__4, + BIT(dp->index), false); + + /* Enable PORT_PHY_INT interrupt - active low */ + lan937x_port_cfg(dev, dp->index, REG_PORT_INT_MASK, + PORT_PHY_INT, false); + } } } @@ -383,6 +399,50 @@ void lan937x_setup_rgmii_delay(struct ksz_device *dev, int port) } } +static irqreturn_t lan937x_switch_irq_thread(int irq, void *dev_id) +{ + struct ksz_device *dev = dev_id; + irqreturn_t result = IRQ_NONE; + u32 data; + int ret; + + /* Read global interrupt status register */ + ret = ksz_read32(dev, REG_SW_INT_STATUS__4, &data); + if (ret) + return result; + + if (data & POR_READY_INT) { + ret = ksz_write32(dev, REG_SW_INT_STATUS__4, POR_READY_INT); + if (ret) + return result; + } + + return result; +} + +static int lan937x_register_interrupt(struct ksz_device *dev) +{ + int ret; + + if (dev->irq > 0) { + unsigned long irqflags = + irqd_get_trigger_type(irq_get_irq_data(dev->irq)); + + irqflags |= (IRQF_ONESHOT | IRQF_SHARED); + + ret = devm_request_threaded_irq(dev->dev, dev->irq, NULL, + lan937x_switch_irq_thread, + irqflags, dev_name(dev->dev), + dev); + if (ret) { + dev_err(dev->dev, "failed to request IRQ.\n"); + return ret; + } + } + + return 0; +} + int lan937x_setup(struct dsa_switch *ds) { struct ksz_device *dev = ds->priv; @@ -423,6 +483,10 @@ int lan937x_setup(struct dsa_switch *ds) lan937x_cfg(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, (SW_CLK125_ENB | SW_CLK25_ENB), true); + ret = lan937x_register_interrupt(dev); + if (ret) + return ret; + return 0; } diff --git a/drivers/net/dsa/microchip/lan937x_reg.h b/drivers/net/dsa/microchip/lan937x_reg.h index ba4adaddb3ec..a4b17fc722d2 100644 --- a/drivers/net/dsa/microchip/lan937x_reg.h +++ b/drivers/net/dsa/microchip/lan937x_reg.h @@ -118,6 +118,16 @@ /* Port Registers */ /* 0 - Operation */ +#define REG_PORT_INT_STATUS 0x001B +#define REG_PORT_INT_MASK 0x001F + +#define PORT_TAS_INT BIT(5) +#define PORT_QCI_INT BIT(4) +#define PORT_SGMII_INT BIT(3) +#define PORT_PTP_INT BIT(2) +#define PORT_PHY_INT BIT(1) +#define PORT_ACL_INT BIT(0) + #define REG_PORT_CTRL_0 0x0020 #define PORT_MAC_LOOPBACK BIT(7)
This patch enables the interrupts for internal phy link detection for LAN937x. The interrupt enable bits are active low. It first enables port interrupt and then port phy interrupt. Also patch register the irq thread and in the ISR routine it clears the POR_READY_STS bit. POR_READY_STS bit is write one clear bit and all other bit in the register are read only. Since phy interrupts are handled by the lan937x phy layer, switch interrupt routine does not read the phy layer interrupts. Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com> --- Changes in RFC v2 - fixed the compilation issue drivers/net/dsa/microchip/ksz_common.h | 1 + drivers/net/dsa/microchip/ksz_spi.c | 2 + drivers/net/dsa/microchip/lan937x_main.c | 64 ++++++++++++++++++++++++ drivers/net/dsa/microchip/lan937x_reg.h | 10 ++++ 4 files changed, 77 insertions(+)