Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection

Arun.Ramadoss
2022-08-26T16:22:03Z

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;
}