[PATCH RFC 1/6] net: dsa: mv88e6xxx: Implement interrupt support.

[PATCH RFC 1/6] net: dsa: mv88e6xxx: Implement interrupt support.

[Andrew Lunn]

The switch can have up to two interrupt controllers. One of these
contains the interrupts from the integrated PHYs, so is useful to
export. The Marvell PHY driver can then be used in interrupt mode,
rather than polling, speeding up PHY handling and reducing load on the
MDIO bus.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../devicetree/bindings/net/dsa/marvell.txt        |  21 +-
 drivers/net/dsa/mv88e6xxx/chip.c                   | 247 ++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/global2.c                | 140 +++++++++++-
 drivers/net/dsa/mv88e6xxx/global2.h                |  11 +
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  32 ++-
 5 files changed, 438 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index 7629189398aa..2e1e66521882 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -20,16 +20,35 @@ Required properties:
 Optional properties:
 
 - reset-gpios		: Should be a gpio specifier for a reset line
-
+- interrupt-parent	: Parent interrupt controller
+- interrupts		: Interrupt from the switch
+- interrupt-controller	: Indicates the switch is itself an interrupt
+			  controller. This is used for the PHY interrupts.
+#interrupt-cells = <2>	: Controller uses one cell.
+- mdio			: container of PHY and devices on the switches MDIO
+			  bus
 Example:
 
        mdio {
                #address-cells = <1>;
                #size-cells = <0>;
+               interrupt-parent = <&gpio0>;
+               interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+               interrupt-controller;
+               #interrupt-cells = <2>;
 
                switch0: switch@0 {
                        compatible = "marvell,mv88e6085";
                        reg = <0>;
 		       reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
                };
+               mdio {
+                       #address-cells = <1>;
+                       #size-cells = <0>;
+                       switch1phy0: switch1phy0@0 {
+                               reg = <0>;
+                               interrupt-parent = <&switch1>;
+                               interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+                       };
+               };
        };
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index fb24865e9ee4..f0eb49cdca93 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -18,11 +18,15 @@
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
 #include <linux/if_bridge.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/jiffies.h>
 #include <linux/list.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/of_mdio.h>
 #include <linux/netdevice.h>
 #include <linux/gpio/consumer.h>
@@ -322,6 +326,164 @@ static int mv88e6xxx_serdes_write(struct mv88e6xxx_chip *chip, int reg, u16 val)
 					reg, val);
 }
 
+static void mv88e6xxx_g1_irq_mask(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int n = d->hwirq;
+
+	chip->g1_irq.masked |= (1 << n);
+}
+
+static void mv88e6xxx_g1_irq_unmask(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int n = d->hwirq;
+
+	chip->g1_irq.masked &= ~(1 << n);
+}
+
+static irqreturn_t mv88e6xxx_g1_irq_thread_fn(int irq, void *dev_id)
+{
+	struct mv88e6xxx_chip *chip = dev_id;
+	unsigned int nhandled = 0;
+	unsigned int sub_irq;
+	unsigned int n;
+	u16 reg;
+	int err;
+
+	mutex_lock(&chip->reg_lock);
+	err = mv88e6xxx_read(chip, REG_GLOBAL, GLOBAL_STATUS, &reg);
+	mutex_unlock(&chip->reg_lock);
+
+	if (err)
+		goto out;
+
+	for (n = 0; n < chip->g1_irq.nirqs; ++n) {
+		if (reg & (1 << n)) {
+			sub_irq = irq_find_mapping(chip->g1_irq.domain, n);
+			handle_nested_irq(sub_irq);
+			++nhandled;
+		}
+	}
+out:
+	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
+}
+
+static void mv88e6xxx_g1_irq_bus_lock(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+
+	mutex_lock(&chip->reg_lock);
+}
+
+static void mv88e6xxx_g1_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+	u16 mask = GENMASK(chip->g1_irq.nirqs, 0);
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_read(chip, REG_GLOBAL, GLOBAL_CONTROL, &reg);
+	if (err)
+		goto out;
+
+	reg &= ~mask;
+	reg |= (~chip->g1_irq.masked & mask);
+
+	err = mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_CONTROL, reg);
+	if (err)
+		goto out;
+
+out:
+	mutex_unlock(&chip->reg_lock);
+}
+
+static struct irq_chip mv88e6xxx_g1_irq_chip = {
+	.name			= "mv88e6xxx-g1",
+	.irq_mask		= mv88e6xxx_g1_irq_mask,
+	.irq_unmask		= mv88e6xxx_g1_irq_unmask,
+	.irq_bus_lock		= mv88e6xxx_g1_irq_bus_lock,
+	.irq_bus_sync_unlock	= mv88e6xxx_g1_irq_bus_sync_unlock,
+};
+
+static int mv88e6xxx_g1_irq_domain_map(struct irq_domain *d,
+				       unsigned int irq,
+				       irq_hw_number_t hwirq)
+{
+	struct mv88e6xxx_chip *chip = d->host_data;
+
+	irq_set_chip_data(irq, d->host_data);
+	irq_set_chip_and_handler(irq, &chip->g1_irq.chip, handle_level_irq);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops mv88e6xxx_g1_irq_domain_ops = {
+	.map	= mv88e6xxx_g1_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)
+{
+	int irq, virq;
+
+	for (irq = 0; irq < 16; irq++) {
+		virq = irq_find_mapping(chip->g2_irq.domain, irq);
+		irq_dispose_mapping(virq);
+	}
+
+	irq_domain_remove(chip->g2_irq.domain);
+}
+
+static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
+{
+	int err, irq;
+	u16 reg;
+
+	chip->g1_irq.nirqs = chip->info->g1_irqs;
+	chip->g1_irq.domain = irq_domain_add_simple(
+		NULL, chip->g1_irq.nirqs, 0,
+		&mv88e6xxx_g1_irq_domain_ops, chip);
+	if (!chip->g1_irq.domain)
+		return -ENOMEM;
+
+	for (irq = 0; irq < chip->g1_irq.nirqs; irq++)
+		irq_create_mapping(chip->g1_irq.domain, irq);
+
+	chip->g1_irq.chip = mv88e6xxx_g1_irq_chip;
+	chip->g1_irq.masked = ~0;
+
+	err = mv88e6xxx_read(chip, REG_GLOBAL, GLOBAL_CONTROL, &reg);
+	if (err)
+		goto out;
+
+	reg &= ~GENMASK(chip->g1_irq.nirqs, 0);
+
+	err = mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_CONTROL, reg);
+	if (err)
+		goto out;
+
+	/* Reading the interrupt status clears (most of) them */
+	err = mv88e6xxx_read(chip, REG_GLOBAL, GLOBAL_STATUS, &reg);
+	if (err)
+		goto out;
+
+	err = request_threaded_irq(chip->irq, NULL,
+				   mv88e6xxx_g1_irq_thread_fn,
+				   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+				   dev_name(chip->dev), chip);
+	if (err)
+		goto out;
+
+	return 0;
+
+out:
+	mv88e6xxx_g1_irq_free(chip);
+
+	return err;
+}
+
 int mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int addr, int reg, u16 mask)
 {
 	int i;
@@ -2817,7 +2979,11 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
 	/* Enable the PHY Polling Unit if present, don't discard any packets,
 	 * and mask all interrupt sources.
 	 */
-	reg = 0;
+	reg = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_CONTROL);
+	if (reg < 0)
+		return reg;
+
+	reg &= ~GLOBAL_CONTROL_PPU_ENABLE;
 	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU) ||
 	    mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU_ACTIVE))
 		reg |= GLOBAL_CONTROL_PPU_ENABLE;
@@ -2925,10 +3091,6 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 
 	mutex_lock(&chip->reg_lock);
 
-	err = mv88e6xxx_switch_reset(chip);
-	if (err)
-		goto unlock;
-
 	/* Setup Switch Port Registers */
 	for (i = 0; i < chip->info->num_ports; i++) {
 		err = mv88e6xxx_setup_port(chip, i);
@@ -3271,6 +3433,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6097,
+		.g1_irqs = 8,
 	},
 
 	[MV88E6095] = {
@@ -3282,6 +3445,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6095,
+		.g1_irqs = 8,
 	},
 
 	[MV88E6123] = {
@@ -3293,6 +3457,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6131] = {
@@ -3304,6 +3469,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6161] = {
@@ -3315,6 +3481,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6165] = {
@@ -3326,6 +3493,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6171] = {
@@ -3337,6 +3505,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6172] = {
@@ -3348,6 +3517,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6175] = {
@@ -3359,6 +3529,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6176] = {
@@ -3370,6 +3541,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6185] = {
@@ -3381,6 +3553,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
+		.g1_irqs = 8,
 	},
 
 	[MV88E6240] = {
@@ -3392,6 +3565,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6320] = {
@@ -3403,6 +3577,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
+		.g1_irqs = 8,
 	},
 
 	[MV88E6321] = {
@@ -3414,6 +3589,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
+		.g1_irqs = 8,
 	},
 
 	[MV88E6350] = {
@@ -3425,6 +3601,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6351] = {
@@ -3436,6 +3613,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
+		.g1_irqs = 9,
 	},
 
 	[MV88E6352] = {
@@ -3447,6 +3625,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.port_base_addr = 0x10,
 		.age_time_coeff = 15000,
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
+		.g1_irqs = 9,
 	},
 };
 
@@ -3595,6 +3774,12 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
 	if (err)
 		goto free;
 
+	mutex_lock(&chip->reg_lock);
+	err = mv88e6xxx_switch_reset(chip);
+	mutex_unlock(&chip->reg_lock);
+	if (err)
+		goto free;
+
 	mv88e6xxx_phy_init(chip);
 
 	err = mv88e6xxx_mdio_register(chip, NULL);
@@ -3765,17 +3950,55 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	    !of_property_read_u32(np, "eeprom-length", &eeprom_len))
 		chip->eeprom_len = eeprom_len;
 
+	mutex_lock(&chip->reg_lock);
+	err = mv88e6xxx_switch_reset(chip);
+	mutex_unlock(&chip->reg_lock);
+	if (err)
+		goto out;
+
+	chip->irq = of_irq_get(np, 0);
+	if (chip->irq == -EPROBE_DEFER) {
+		err = chip->irq;
+		goto out;
+	}
+
+	if (chip->irq > 0) {
+		/* Has to be performed before the MDIO bus is created,
+		 * because the PHYs will link there interrupts to these
+		 * interrupt controllers
+		 */
+		err = mv88e6xxx_g1_irq_setup(chip);
+		if (err)
+			goto out;
+
+		if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT)) {
+			mutex_unlock(&chip->reg_lock);
+			err = mv88e6xxx_g2_irq_setup(chip);
+			mutex_unlock(&chip->reg_lock);
+			if (err)
+				goto out_g1_irq;
+		}
+	}
+
 	err = mv88e6xxx_mdio_register(chip, np);
 	if (err)
-		return err;
+		goto out_g2_irq;
 
 	err = mv88e6xxx_register_switch(chip, np);
-	if (err) {
-		mv88e6xxx_mdio_unregister(chip);
-		return err;
-	}
+	if (err)
+		goto out_mdio;
 
 	return 0;
+
+out_mdio:
+	mv88e6xxx_mdio_unregister(chip);
+out_g2_irq:
+	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
+		mv88e6xxx_g2_irq_free(chip);
+out_g1_irq:
+	mv88e6xxx_g1_irq_free(chip);
+out:
+	return err;
 }
 
 static void mv88e6xxx_remove(struct mdio_device *mdiodev)
@@ -3786,6 +4009,10 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
 	mv88e6xxx_phy_destroy(chip);
 	mv88e6xxx_unregister_switch(chip);
 	mv88e6xxx_mdio_unregister(chip);
+
+	if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
+		mv88e6xxx_g2_irq_free(chip);
+	mv88e6xxx_g1_irq_free(chip);
 }
 
 static const struct of_device_id mv88e6xxx_of_match[] = {
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 99ed028298ac..e4cbe4560904 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -1,5 +1,6 @@
 /*
- * Marvell 88E6xxx Switch Global 2 Registers support (device address 0x1C)
+ * Marvell 88E6xxx Switch Global 2 Registers support (device address
+ * 0x1C)
  *
  * Copyright (c) 2008 Marvell Semiconductor
  *
@@ -11,6 +12,7 @@
  * (at your option) any later version.
  */
 
+#include <linux/irqdomain.h>
 #include "mv88e6xxx.h"
 #include "global2.h"
 
@@ -395,6 +397,142 @@ int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr, int reg,
 	return mv88e6xxx_g2_smi_phy_cmd(chip, cmd);
 }
 
+static void mv88e6xxx_g2_irq_mask(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int n = d->hwirq;
+
+	chip->g2_irq.masked |= (1 << n);
+}
+
+static void mv88e6xxx_g2_irq_unmask(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+	unsigned int n = d->hwirq;
+
+	chip->g2_irq.masked &= ~(1 << n);
+}
+
+static irqreturn_t mv88e6xxx_g2_irq_thread_fn(int irq, void *dev_id)
+{
+	struct mv88e6xxx_chip *chip = dev_id;
+	unsigned int nhandled = 0;
+	unsigned int sub_irq;
+	unsigned int n;
+	int err;
+	u16 reg;
+
+	mutex_lock(&chip->reg_lock);
+	err = mv88e6xxx_read(chip, REG_GLOBAL2, GLOBAL2_INT_SOURCE, &reg);
+	mutex_unlock(&chip->reg_lock);
+	if (err)
+		goto out;
+
+	for (n = 0; n < 16; ++n) {
+		if (reg & (1 << n)) {
+			sub_irq = irq_find_mapping(chip->g2_irq.domain, n);
+			handle_nested_irq(sub_irq);
+			++nhandled;
+		}
+	}
+out:
+	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
+}
+
+static void mv88e6xxx_g2_irq_bus_lock(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+
+	mutex_lock(&chip->reg_lock);
+}
+
+static void mv88e6xxx_g2_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct mv88e6xxx_chip *chip = irq_data_get_irq_chip_data(d);
+
+	mv88e6xxx_write(chip, REG_GLOBAL2, GLOBAL2_INT_MASK,
+			~chip->g2_irq.masked);
+
+	mutex_unlock(&chip->reg_lock);
+}
+
+static struct irq_chip mv88e6xxx_g2_irq_chip = {
+	.name			= "mv88e6xxx-g2",
+	.irq_mask		= mv88e6xxx_g2_irq_mask,
+	.irq_unmask		= mv88e6xxx_g2_irq_unmask,
+	.irq_bus_lock		= mv88e6xxx_g2_irq_bus_lock,
+	.irq_bus_sync_unlock	= mv88e6xxx_g2_irq_bus_sync_unlock,
+};
+
+static int mv88e6xxx_g2_irq_domain_map(struct irq_domain *d,
+				       unsigned int irq,
+				       irq_hw_number_t hwirq)
+{
+	struct mv88e6xxx_chip *chip = d->host_data;
+
+	irq_set_chip_data(irq, d->host_data);
+	irq_set_chip_and_handler(irq, &chip->g2_irq.chip, handle_level_irq);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops mv88e6xxx_g2_irq_domain_ops = {
+	.map	= mv88e6xxx_g2_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+void mv88e6xxx_g2_irq_free(struct mv88e6xxx_chip *chip)
+{
+	int irq, virq;
+
+	for (irq = 0; irq < 16; irq++) {
+		virq = irq_find_mapping(chip->g2_irq.domain, irq);
+		irq_dispose_mapping(virq);
+	}
+
+	irq_domain_remove(chip->g2_irq.domain);
+}
+
+int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip)
+{
+	int device_irq;
+	int err, irq;
+
+	if (!chip->dev->of_node)
+		return -EINVAL;
+
+	chip->g2_irq.domain = irq_domain_add_simple(
+		chip->dev->of_node, 16, 0, &mv88e6xxx_g2_irq_domain_ops, chip);
+	if (!chip->g2_irq.domain)
+		return -ENOMEM;
+
+	for (irq = 0; irq < 16; irq++)
+		irq_create_mapping(chip->g2_irq.domain, irq);
+
+	chip->g2_irq.chip = mv88e6xxx_g2_irq_chip;
+	chip->g2_irq.masked = ~0;
+
+	device_irq = irq_find_mapping(chip->g1_irq.domain,
+				      GLOBAL_STATUS_IRQ_DEVICE);
+	if (device_irq < 0) {
+		err = device_irq;
+		goto out;
+	}
+
+	err = devm_request_threaded_irq(chip->dev, device_irq, NULL,
+					mv88e6xxx_g2_irq_thread_fn,
+					IRQF_ONESHOT, "mv88e6xxx-g1", chip);
+	if (err)
+		goto out;
+
+	return 0;
+out:
+	mv88e6xxx_g2_irq_free(chip);
+
+	return err;
+}
+
 int mv88e6xxx_g2_setup(struct mv88e6xxx_chip *chip)
 {
 	u16 reg;
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index c4bb9035ee3a..6cbb42b60d80 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -33,6 +33,8 @@ int mv88e6xxx_g2_get_eeprom16(struct mv88e6xxx_chip *chip,
 int mv88e6xxx_g2_set_eeprom16(struct mv88e6xxx_chip *chip,
 			      struct ethtool_eeprom *eeprom, u8 *data);
 int mv88e6xxx_g2_setup(struct mv88e6xxx_chip *chip);
+int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip);
+void mv88e6xxx_g2_irq_free(struct mv88e6xxx_chip *chip);
 
 #else /* !CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
 
@@ -83,6 +85,15 @@ static inline int mv88e6xxx_g2_setup(struct mv88e6xxx_chip *chip)
 	return -EOPNOTSUPP;
 }
 
+static inline int mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void mv88e6xxx_g2_irq_setup(struct mv88e6xxx_chip *chip)
+{
+}
+
 #endif /* CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */
 
 #endif /* _MV88E6XXX_GLOBAL2_H */
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index a365cec615b9..61dec0bc6d59 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -13,6 +13,7 @@
 #define __MV88E6XXX_H
 
 #include <linux/if_vlan.h>
+#include <linux/irq.h>
 #include <linux/gpio/consumer.h>
 
 #ifndef UINT64_MAX
@@ -168,6 +169,15 @@
 #define GLOBAL_STATUS_PPU_INITIALIZING	(0x1 << 14)
 #define GLOBAL_STATUS_PPU_DISABLED	(0x2 << 14)
 #define GLOBAL_STATUS_PPU_POLLING	(0x3 << 14)
+#define GLOBAL_STATUS_IRQ_AVB		8
+#define GLOBAL_STATUS_IRQ_DEVICE	7
+#define GLOBAL_STATUS_IRQ_STATS		6
+#define GLOBAL_STATUS_IRQ_VTU_PROBLEM	5
+#define GLOBAL_STATUS_IRQ_VTU_DONE	4
+#define GLOBAL_STATUS_IRQ_ATU_PROBLEM	3
+#define GLOBAL_STATUS_IRQ_ATU_DONE	2
+#define GLOBAL_STATUS_IRQ_TCAM_DONE	1
+#define GLOBAL_STATUS_IRQ_EEPROM_DONE	0
 #define GLOBAL_MAC_01		0x01
 #define GLOBAL_MAC_23		0x02
 #define GLOBAL_MAC_45		0x03
@@ -414,6 +424,7 @@ enum mv88e6xxx_cap {
 	 * The device contains a second set of global 16-bit registers.
 	 */
 	MV88E6XXX_CAP_GLOBAL2,
+	MV88E6XXX_CAP_G2_INT,		/* (0x00) Interrupt Status */
 	MV88E6XXX_CAP_G2_MGMT_EN_2X,	/* (0x02) MGMT Enable Register 2x */
 	MV88E6XXX_CAP_G2_MGMT_EN_0X,	/* (0x03) MGMT Enable Register 0x */
 	MV88E6XXX_CAP_G2_IRL_CMD,	/* (0x09) Ingress Rate Command */
@@ -463,6 +474,7 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAG_SERDES		BIT_ULL(MV88E6XXX_CAP_SERDES)
 
 #define MV88E6XXX_FLAG_GLOBAL2		BIT_ULL(MV88E6XXX_CAP_GLOBAL2)
+#define MV88E6XXX_FLAG_G2_INT		BIT_ULL(MV88E6XXX_CAP_G2_INT)
 #define MV88E6XXX_FLAG_G2_MGMT_EN_2X	BIT_ULL(MV88E6XXX_CAP_G2_MGMT_EN_2X)
 #define MV88E6XXX_FLAG_G2_MGMT_EN_0X	BIT_ULL(MV88E6XXX_CAP_G2_MGMT_EN_0X)
 #define MV88E6XXX_FLAG_G2_IRL_CMD	BIT_ULL(MV88E6XXX_CAP_G2_IRL_CMD)
@@ -534,6 +546,7 @@ enum mv88e6xxx_cap {
 
 #define MV88E6XXX_FLAGS_FAMILY_6165	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
+	 MV88E6XXX_FLAG_G2_INT |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
@@ -547,6 +560,7 @@ enum mv88e6xxx_cap {
 
 #define MV88E6XXX_FLAGS_FAMILY_6185	\
 	(MV88E6XXX_FLAG_GLOBAL2 |	\
+	 MV88E6XXX_FLAG_G2_INT |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
 	 MV88E6XXX_FLAG_PPU |		\
@@ -573,6 +587,7 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAGS_FAMILY_6351	\
 	(MV88E6XXX_FLAG_EDSA |		\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
+	 MV88E6XXX_FLAG_G2_INT |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
@@ -590,6 +605,7 @@ enum mv88e6xxx_cap {
 	(MV88E6XXX_FLAG_EDSA |		\
 	 MV88E6XXX_FLAG_EEE |		\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
+	 MV88E6XXX_FLAG_G2_INT |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_0X |	\
 	 MV88E6XXX_FLAG_G2_SWITCH_MAC |	\
@@ -615,6 +631,7 @@ struct mv88e6xxx_info {
 	unsigned int port_base_addr;
 	unsigned int age_time_coeff;
 	unsigned long long flags;
+	unsigned int g1_irqs;
 };
 
 struct mv88e6xxx_atu_entry {
@@ -642,6 +659,13 @@ struct mv88e6xxx_priv_port {
 	struct net_device *bridge_dev;
 };
 
+struct mv88e6xxx_irq {
+	u16 masked;
+	struct irq_chip chip;
+	struct irq_domain *domain;
+	unsigned int nirqs;
+};
+
 struct mv88e6xxx_chip {
 	struct dentry *dbgfs;
 
@@ -693,6 +717,13 @@ struct mv88e6xxx_chip {
 
 	/* And the MDIO bus itself */
 	struct mii_bus *mdio_bus;
+
+	/* There can be two interrupt controllers, which are chained
+	 * off a GPIO as interrupt source
+	 */
+	struct mv88e6xxx_irq g1_irq;
+	struct mv88e6xxx_irq g2_irq;
+	int irq;
 };
 
 struct mv88e6xxx_ops {
@@ -724,5 +755,4 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val);
 int mv88e6xxx_update(struct mv88e6xxx_chip *chip, int addr, int reg,
 		     u16 update);
 int mv88e6xxx_wait(struct mv88e6xxx_chip *chip, int addr, int reg, u16 mask);
-
 #endif
-- 
2.9.3

Re: [PATCH RFC 1/6] net: dsa: mv88e6xxx: Implement interrupt support.

[Andrew Lunn]

The cover email got rejected, because i used MV88E6XXX in the subject
line, and it did not like the XXX.

----------------------------------------------------------------------

>From 838e38d8079341ffdeb98c3c1bf043627c24cfc2 Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 28 Sep 2016 03:23:04 -0500
Subject: [PATCH RFC 0/6] Interrupt support for MV88E6XYZ

NOT FOR MERGING

This RFC patchset add interrupt controller support to the MV88E6XXX.
This allows access to the interrupts the internal PHY generate. These
interrupts can then be associated to a PHY device in the device tree
and used by the PHY lib, rather than polling.

An earlier version of these patches were posted many moons ago. They
have been rebased onto net-next and adapted to the new structure of
the driver. However, still outstanding is if this is the correct way
to implement this feature, via an IRQ domain made available in the
device tree, and then using the standard PHY binding. This was
questioned last time, and never really resolved.

Comment welcome.

	Andrew

Andrew Lunn (6):
  net: dsa: mv88e6xxx: Implement interrupt support.
  net: phy: Use threaded IRQ, to allow IRQ from sleeping devices
  net: phy: Threaded interrupts allow some simplification
  net: phy: Use phy name when requesting the interrupt
  net: phy: Trigger state machine on state change and not polling.
  arm: vf610: zii devel b: Add support for switch interrupts

 .../devicetree/bindings/net/dsa/marvell.txt        |  21 +-
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts          |  51 +++++
 drivers/net/dsa/mv88e6xxx/chip.c                   | 247 ++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx/global2.c                | 140 +++++++++++-
 drivers/net/dsa/mv88e6xxx/global2.h                |  11 +
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  32 ++-
 drivers/net/phy/phy.c                              |  66 +++---
 drivers/net/phy/phy_device.c                       |   1 -
 include/linux/phy.h                                |   4 +-
 9 files changed, 529 insertions(+), 44 deletions(-)

-- 
2.9.3

[PATCH RFC 2/6] net: phy: Use threaded IRQ, to allow IRQ from sleeping devices

[Andrew Lunn]

The interrupt lines from PHYs maybe connected to I2C bus expanders, or
from switches on MDIO busses. Such interrupts are sourced from devices
which sleep, so use threaded interrupts. Threaded interrupts require
that the interrupt requester also uses the threaded API. Change the
phylib to use the threaded API, which is backwards compatible with
none-threaded IRQs.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f66832a1a6..5c29ed72f721 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -722,10 +722,9 @@ phy_err:
 int phy_start_interrupts(struct phy_device *phydev)
 {
 	atomic_set(&phydev->irq_disable, 0);
-	if (request_irq(phydev->irq, phy_interrupt,
-				IRQF_SHARED,
-				"phy_interrupt",
-				phydev) < 0) {
+	if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
+				 IRQF_ONESHOT, "phy_interrupt",
+				 phydev) < 0) {
 		pr_warn("%s: Can't get IRQ %d (PHY)\n",
 			phydev->mdio.bus->name, phydev->irq);
 		phydev->irq = PHY_POLL;
-- 
2.9.3

Re: [PATCH RFC 2/6] net: phy: Use threaded IRQ, to allow IRQ from sleeping devices

[Sergei Shtylyov]

Hello.

On 9/28/2016 11:32 AM, Andrew Lunn wrote:

> The interrupt lines from PHYs maybe connected to I2C bus expanders, or
> from switches on MDIO busses. Such interrupts are sourced from devices
> which sleep, so use threaded interrupts. Threaded interrupts require
> that the interrupt requester also uses the threaded API. Change the
> phylib to use the threaded API, which is backwards compatible with
> none-threaded IRQs.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/phy/phy.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index c6f66832a1a6..5c29ed72f721 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -722,10 +722,9 @@ phy_err:
>  int phy_start_interrupts(struct phy_device *phydev)
>  {
>  	atomic_set(&phydev->irq_disable, 0);
> -	if (request_irq(phydev->irq, phy_interrupt,
> -				IRQF_SHARED,
> -				"phy_interrupt",
> -				phydev) < 0) {
> +	if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
> +				 IRQF_ONESHOT, "phy_interrupt",

    What about IRQF_SHARED?

> +				 phydev) < 0) {
>  		pr_warn("%s: Can't get IRQ %d (PHY)\n",
>  			phydev->mdio.bus->name, phydev->irq);
>  		phydev->irq = PHY_POLL;

MBR, Sergei

Re: [PATCH RFC 2/6] net: phy: Use threaded IRQ, to allow IRQ from sleeping devices

[Andrew Lunn]

On Wed, Sep 28, 2016 at 02:38:03PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 9/28/2016 11:32 AM, Andrew Lunn wrote:
> 
> >The interrupt lines from PHYs maybe connected to I2C bus expanders, or
> >from switches on MDIO busses. Such interrupts are sourced from devices
> >which sleep, so use threaded interrupts. Threaded interrupts require
> >that the interrupt requester also uses the threaded API. Change the
> >phylib to use the threaded API, which is backwards compatible with
> >none-threaded IRQs.
> >
> >Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> >---
> > drivers/net/phy/phy.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> >index c6f66832a1a6..5c29ed72f721 100644
> >--- a/drivers/net/phy/phy.c
> >+++ b/drivers/net/phy/phy.c
> >@@ -722,10 +722,9 @@ phy_err:
> > int phy_start_interrupts(struct phy_device *phydev)
> > {
> > 	atomic_set(&phydev->irq_disable, 0);
> >-	if (request_irq(phydev->irq, phy_interrupt,
> >-				IRQF_SHARED,
> >-				"phy_interrupt",
> >-				phydev) < 0) {
> >+	if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
> >+				 IRQF_ONESHOT, "phy_interrupt",
> 
>    What about IRQF_SHARED?

I will check if you can have a shared oneshot.

  Andrew

Re: [PATCH RFC 2/6] net: phy: Use threaded IRQ, to allow IRQ from sleeping devices

[Andrew Lunn]

On Wed, Sep 28, 2016 at 02:38:03PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 9/28/2016 11:32 AM, Andrew Lunn wrote:
> 
> >The interrupt lines from PHYs maybe connected to I2C bus expanders, or
> >from switches on MDIO busses. Such interrupts are sourced from devices
> >which sleep, so use threaded interrupts. Threaded interrupts require
> >that the interrupt requester also uses the threaded API. Change the
> >phylib to use the threaded API, which is backwards compatible with
> >none-threaded IRQs.
> >
> >Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> >---
> > drivers/net/phy/phy.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> >index c6f66832a1a6..5c29ed72f721 100644
> >--- a/drivers/net/phy/phy.c
> >+++ b/drivers/net/phy/phy.c
> >@@ -722,10 +722,9 @@ phy_err:
> > int phy_start_interrupts(struct phy_device *phydev)
> > {
> > 	atomic_set(&phydev->irq_disable, 0);
> >-	if (request_irq(phydev->irq, phy_interrupt,
> >-				IRQF_SHARED,
> >-				"phy_interrupt",
> >-				phydev) < 0) {
> >+	if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
> >+				 IRQF_ONESHOT, "phy_interrupt",
> 
>    What about IRQF_SHARED?

IRQF_ONESHOT | IRQF_SHARED does work.

However, it requires all uses of the interrupt use the same flags.
The commit messages suggests IRQF_SHARED is there because of boards
which have multiple PHYs using one IRQ. That case will work, since all
PHYs will be using this code.

What will be an issue is if the sharing is between a PHY and something
else. That something else will also need to use IRQF_ONESHOT.  Anybody
know if this situation actually exists?

Thanks
     Andrew

[PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Andrew Lunn]

The PHY interrupts are now handled in a threaded interrupt handler,
which can sleep. The work queue is no longer needed, phy_change() can
be called directly. Additionally, none of the callers of
phy_mac_interrupt() did so in interrupt context, so fully remove the
work queue, and document that phy_mac_interrupt() should not be called
in interrupt context.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c        | 37 ++++++++++++++++---------------------
 drivers/net/phy/phy_device.c |  1 -
 include/linux/phy.h          |  4 +---
 3 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 5c29ed72f721..09fa8a950af1 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -647,7 +647,7 @@ static void phy_error(struct phy_device *phydev)
  * @phy_dat: phy_device pointer
  *
  * Description: When a PHY interrupt occurs, the handler disables
- * interrupts, and schedules a work task to clear the interrupt.
+ * interrupts, and uses phy_change to handle the interrupt.
  */
 static irqreturn_t phy_interrupt(int irq, void *phy_dat)
 {
@@ -656,15 +656,10 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
 	if (PHY_HALTED == phydev->state)
 		return IRQ_NONE;		/* It can't be ours.  */
 
-	/* The MDIO bus is not allowed to be written in interrupt
-	 * context, so we need to disable the irq here.  A work
-	 * queue will write the PHY to disable and clear the
-	 * interrupt, and then reenable the irq line.
-	 */
 	disable_irq_nosync(irq);
 	atomic_inc(&phydev->irq_disable);
 
-	queue_work(system_power_efficient_wq, &phydev->phy_queue);
+	phy_change(phydev);
 
 	return IRQ_HANDLED;
 }
@@ -748,12 +743,6 @@ int phy_stop_interrupts(struct phy_device *phydev)
 
 	free_irq(phydev->irq, phydev);
 
-	/* Cannot call flush_scheduled_work() here as desired because
-	 * of rtnl_lock(), but we do not really care about what would
-	 * be done, except from enable_irq(), so cancel any work
-	 * possibly pending and take care of the matter below.
-	 */
-	cancel_work_sync(&phydev->phy_queue);
 	/* If work indeed has been cancelled, disable_irq() will have
 	 * been left unbalanced from phy_interrupt() and enable_irq()
 	 * has to be called so that other devices on the line work.
@@ -766,14 +755,11 @@ int phy_stop_interrupts(struct phy_device *phydev)
 EXPORT_SYMBOL(phy_stop_interrupts);
 
 /**
- * phy_change - Scheduled by the phy_interrupt/timer to handle PHY changes
- * @work: work_struct that describes the work to be done
+ * phy_change - Called by the phy_interrupt to handle PHY changes
+ * @phydev: phy_device struct that interrupted
  */
-void phy_change(struct work_struct *work)
+void phy_change(struct phy_device *phydev)
 {
-	struct phy_device *phydev =
-		container_of(work, struct phy_device, phy_queue);
-
 	if (phy_interrupt_is_valid(phydev)) {
 		if (phydev->drv->did_interrupt &&
 		    !phydev->drv->did_interrupt(phydev))
@@ -1097,12 +1083,21 @@ void phy_state_machine(struct work_struct *work)
 				   PHY_STATE_TIME * HZ);
 }
 
+/**
+ * phy_mac_interrupt - MAC says the link has changed
+ * @phydev: phy_device struct with changed link
+ * @new_link: Link is Up/Down.
+ *
+ * Description: The MAC layer is able indicate there has been a change
+ *   in the PHY link status. Set the new link status, and trigger the
+ *   state machine if needed.
+ *   Cannot be called in Interrupt context.
+ */
 void phy_mac_interrupt(struct phy_device *phydev, int new_link)
 {
 	phydev->link = new_link;
 
-	/* Trigger a state machine change */
-	queue_work(system_power_efficient_wq, &phydev->phy_queue);
+	phy_change(phydev);
 }
 EXPORT_SYMBOL(phy_mac_interrupt);
 
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba931878..bae4452700ab 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -347,7 +347,6 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
-	INIT_WORK(&dev->phy_queue, phy_change);
 
 	/* Request the appropriate module unconditionally; don't
 	 * bother trying to do so only if it isn't already loaded,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e25f1830fbcf..692e3e5a8a7c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -343,7 +343,6 @@ struct phy_c45_device_ids {
  * giving up on the current attempt at acquiring a link
  * irq: IRQ number of the PHY's interrupt (-1 if none)
  * phy_timer: The timer for handling the state machine
- * phy_queue: A work_queue for the interrupt
  * attached_dev: The attached enet driver's device instance ptr
  * adjust_link: Callback for the enet controller to respond to
  * changes in the link state.
@@ -416,7 +415,6 @@ struct phy_device {
 	void *priv;
 
 	/* Interrupt and Polling infrastructure */
-	struct work_struct phy_queue;
 	struct delayed_work state_queue;
 	atomic_t irq_disable;
 
@@ -802,7 +800,7 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
 int phy_drivers_register(struct phy_driver *new_driver, int n,
 			 struct module *owner);
 void phy_state_machine(struct work_struct *work);
-void phy_change(struct work_struct *work);
+void phy_change(struct phy_device *phydev);
 void phy_mac_interrupt(struct phy_device *phydev, int new_link);
 void phy_start_machine(struct phy_device *phydev);
 void phy_stop_machine(struct phy_device *phydev);
-- 
2.9.3

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Sergei Shtylyov]

Hello.

On 9/28/2016 11:32 AM, Andrew Lunn wrote:

> The PHY interrupts are now handled in a threaded interrupt handler,
> which can sleep. The work queue is no longer needed, phy_change() can
> be called directly. Additionally, none of the callers of
> phy_mac_interrupt() did so in interrupt context, so fully remove the

    I did intend to call it from interrupt context (from the ravb driver).

> work queue, and document that phy_mac_interrupt() should not be called
> in interrupt context.

    It was intentionally made callable from the interrupt context, I'd prefer 
if you wouldn't change that.

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
[...]

MBR, Sergei

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Sergei Shtylyov]

On 9/28/2016 2:46 PM, Sergei Shtylyov wrote:

>> The PHY interrupts are now handled in a threaded interrupt handler,
>> which can sleep. The work queue is no longer needed, phy_change() can
>> be called directly. Additionally, none of the callers of
>> phy_mac_interrupt() did so in interrupt context, so fully remove the
>
>    I did intend to call it from interrupt context (from the ravb driver).
>
>> work queue, and document that phy_mac_interrupt() should not be called
>> in interrupt context.
>
>    It was intentionally made callable from the interrupt context, I'd prefer
> if you wouldn't change that.

    OTOH, it's still not very handy to call because of the 'new_link' 
parameter which I'm not sure I can provide...

>> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> [...]

MBR, Sergei

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Andrew Lunn]

On Wed, Sep 28, 2016 at 03:13:46PM +0300, Sergei Shtylyov wrote:
> On 9/28/2016 2:46 PM, Sergei Shtylyov wrote:
> 
> >>The PHY interrupts are now handled in a threaded interrupt handler,
> >>which can sleep. The work queue is no longer needed, phy_change() can
> >>be called directly. Additionally, none of the callers of
> >>phy_mac_interrupt() did so in interrupt context, so fully remove the
> >
> >   I did intend to call it from interrupt context (from the ravb driver).
> >
> >>work queue, and document that phy_mac_interrupt() should not be called
> >>in interrupt context.
> >
> >   It was intentionally made callable from the interrupt context, I'd prefer
> >if you wouldn't change that.
> 
>    OTOH, it's still not very handy to call because of the 'new_link'
> parameter which I'm not sure I can provide...

Hi Sergei

If there is a need for it, i will leave the work queue and keep this
code unchanged.

   Andrew

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Sergei Shtylyov]

On 09/28/2016 03:28 PM, Andrew Lunn wrote:

>>>> The PHY interrupts are now handled in a threaded interrupt handler,
>>>> which can sleep. The work queue is no longer needed, phy_change() can
>>>> be called directly. Additionally, none of the callers of
>>>> phy_mac_interrupt() did so in interrupt context, so fully remove the
>>>
>>>   I did intend to call it from interrupt context (from the ravb driver).
>>>
>>>> work queue, and document that phy_mac_interrupt() should not be called
>>>> in interrupt context.
>>>
>>>   It was intentionally made callable from the interrupt context, I'd prefer
>>> if you wouldn't change that.
>>
>>    OTOH, it's still not very handy to call because of the 'new_link'
>> parameter which I'm not sure I can provide...
>
> Hi Sergei
>
> If there is a need for it, i will leave the work queue and keep this
> code unchanged.

    Let's hear what Florian says...

>    Andrew

MBR, Sergei

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Florian Fainelli]

On 09/28/2016 06:38 AM, Sergei Shtylyov wrote:
> On 09/28/2016 03:28 PM, Andrew Lunn wrote:
> 
>>>>> The PHY interrupts are now handled in a threaded interrupt handler,
>>>>> which can sleep. The work queue is no longer needed, phy_change() can
>>>>> be called directly. Additionally, none of the callers of
>>>>> phy_mac_interrupt() did so in interrupt context, so fully remove the
>>>>
>>>>   I did intend to call it from interrupt context (from the ravb
>>>> driver).
>>>>
>>>>> work queue, and document that phy_mac_interrupt() should not be called
>>>>> in interrupt context.
>>>>
>>>>   It was intentionally made callable from the interrupt context, I'd
>>>> prefer
>>>> if you wouldn't change that.
>>>
>>>    OTOH, it's still not very handy to call because of the 'new_link'
>>> parameter which I'm not sure I can provide...
>>
>> Hi Sergei
>>
>> If there is a need for it, i will leave the work queue and keep this
>> code unchanged.
> 
>    Let's hear what Florian says...

The intent is really to have phy_mac_interrupt() callable from hard IRQ
context, not that this matters really too much because link events
already occur in the slow path, but it's nice to have that property
retained IMHO.
-- 
Florian

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Sergei Shtylyov]

Hello!

On 9/28/2016 8:14 PM, Florian Fainelli wrote:

>>>>>> The PHY interrupts are now handled in a threaded interrupt handler,
>>>>>> which can sleep. The work queue is no longer needed, phy_change() can
>>>>>> be called directly. Additionally, none of the callers of
>>>>>> phy_mac_interrupt() did so in interrupt context, so fully remove the
>>>>>
>>>>>   I did intend to call it from interrupt context (from the ravb
>>>>> driver).
>>>>>
>>>>>> work queue, and document that phy_mac_interrupt() should not be called
>>>>>> in interrupt context.
>>>>>
>>>>>   It was intentionally made callable from the interrupt context, I'd
>>>>> prefer
>>>>> if you wouldn't change that.
>>>>
>>>>    OTOH, it's still not very handy to call because of the 'new_link'
>>>> parameter which I'm not sure I can provide...
>>>
>>> Hi Sergei
>>>
>>> If there is a need for it, i will leave the work queue and keep this
>>> code unchanged.
>>
>>    Let's hear what Florian says...
>
> The intent is really to have phy_mac_interrupt() callable from hard IRQ
> context, not that this matters really too much because link events
> already occur in the slow path, but it's nice to have that property
> retained IMHO.

    Actually, I still don't know how to call phy_mac_interrupt() from the ravb 
driver because of the 'new_link' parameter -- I won't always have that signal 
connected to the MAC...

MBR, Sergei

Re: [PATCH RFC 3/6] net: phy: Threaded interrupts allow some simplification

[Andrew Lunn]

>    Actually, I still don't know how to call phy_mac_interrupt() from
> the ravb driver because of the 'new_link' parameter -- I won't
> always have that signal connected to the MAC...

I'm not sure that parameter is of any use. I really think the
semantics of this call should be, something has changed, go and ask
the PHY driver to find out what. Just the same as if the PHY had
triggered an interrupt.

	  Andrew

[PATCH RFC 4/6] net: phy: Use phy name when requesting the interrupt

[Andrew Lunn]

Using the fixed name "phy_interrupt" is not very informative in
/proc/interrupts when there are a lot of phys, e.g. a device with an
Ethernet switch. So when requesting the interrupt, use the name of the
phy.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 09fa8a950af1..940450c6cd2c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -718,7 +718,7 @@ int phy_start_interrupts(struct phy_device *phydev)
 {
 	atomic_set(&phydev->irq_disable, 0);
 	if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
-				 IRQF_ONESHOT, "phy_interrupt",
+				 IRQF_ONESHOT, phydev_name(phydev),
 				 phydev) < 0) {
 		pr_warn("%s: Can't get IRQ %d (PHY)\n",
 			phydev->mdio.bus->name, phydev->irq);
-- 
2.9.3

Re: [PATCH RFC 4/6] net: phy: Use phy name when requesting the interrupt

[Florian Fainelli]

On 09/28/2016 01:32 AM, Andrew Lunn wrote:
> Using the fixed name "phy_interrupt" is not very informative in
> /proc/interrupts when there are a lot of phys, e.g. a device with an
> Ethernet switch. So when requesting the interrupt, use the name of the
> phy.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

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

[PATCH RFC 5/6] net: phy: Trigger state machine on state change and not polling.

[Andrew Lunn]

The phy_start() is used to indicate the PHY is now ready to do its
work. The state is changed, normally to PHY_UP which means that both
the MAC and the PHY are ready.

If the phy driver is using polling, when the next poll happens, the
state machine notices the PHY is now in PHY_UP, and kicks off
auto-negotiation, if needed.

If however, the PHY is using interrupts, there is no polling. The phy
is stuck in PHY_UP until the next interrupt comes along. And there is
no reason for the PHY to interrupt.

Have phy_start() schedule the state machine to run, which both speeds
up the polling use case, and makes the interrupt use case actually
work.

This problems exists whenever there is a state change which will not
cause an interrupt. Trigger the state machine in these cases,
e.g. phy_error().

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 940450c6cd2c..f446ce04caf3 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -608,6 +608,21 @@ void phy_start_machine(struct phy_device *phydev)
 }
 
 /**
+ * phy_trigger_machine - trigger the state machine to run
+ *
+ * @phydev: the phy_device struct
+ *
+ * Description: There has been a change in state which requires that the
+ *   state machine runs.
+ */
+
+static void phy_trigger_machine(struct phy_device *phydev)
+{
+	cancel_delayed_work_sync(&phydev->state_queue);
+	queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, 0);
+}
+
+/**
  * phy_stop_machine - stop the PHY state machine tracking
  * @phydev: target phy_device struct
  *
@@ -639,6 +654,8 @@ static void phy_error(struct phy_device *phydev)
 	mutex_lock(&phydev->lock);
 	phydev->state = PHY_HALTED;
 	mutex_unlock(&phydev->lock);
+
+	phy_trigger_machine(phydev);
 }
 
 /**
@@ -785,8 +802,7 @@ void phy_change(struct phy_device *phydev)
 	}
 
 	/* reschedule state queue work to run as soon as possible */
-	cancel_delayed_work_sync(&phydev->state_queue);
-	queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, 0);
+	phy_trigger_machine(phydev);
 	return;
 
 ignore:
@@ -875,6 +891,8 @@ void phy_start(struct phy_device *phydev)
 	/* if phy was suspended, bring the physical link up again */
 	if (do_resume)
 		phy_resume(phydev);
+
+	phy_trigger_machine(phydev);
 }
 EXPORT_SYMBOL(phy_start);
 
-- 
2.9.3

Re: [PATCH RFC 5/6] net: phy: Trigger state machine on state change and not polling.

[Florian Fainelli]

On 09/28/2016 01:32 AM, Andrew Lunn wrote:
> The phy_start() is used to indicate the PHY is now ready to do its
> work. The state is changed, normally to PHY_UP which means that both
> the MAC and the PHY are ready.
> 
> If the phy driver is using polling, when the next poll happens, the
> state machine notices the PHY is now in PHY_UP, and kicks off
> auto-negotiation, if needed.
> 
> If however, the PHY is using interrupts, there is no polling. The phy
> is stuck in PHY_UP until the next interrupt comes along. And there is
> no reason for the PHY to interrupt.
> 
> Have phy_start() schedule the state machine to run, which both speeds
> up the polling use case, and makes the interrupt use case actually
> work.
> 
> This problems exists whenever there is a state change which will not
> cause an interrupt. Trigger the state machine in these cases,
> e.g. phy_error().
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

No particular objections, this should also fix this:

http://lists.openwall.net/netdev/2016/05/17/147

> ---
>  drivers/net/phy/phy.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 940450c6cd2c..f446ce04caf3 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -608,6 +608,21 @@ void phy_start_machine(struct phy_device *phydev)
>  }
>  
>  /**
> + * phy_trigger_machine - trigger the state machine to run
> + *
> + * @phydev: the phy_device struct
> + *
> + * Description: There has been a change in state which requires that the
> + *   state machine runs.
> + */
> +
> +static void phy_trigger_machine(struct phy_device *phydev)
> +{
> +	cancel_delayed_work_sync(&phydev->state_queue);
> +	queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, 0);
> +}
> +
> +/**
>   * phy_stop_machine - stop the PHY state machine tracking
>   * @phydev: target phy_device struct
>   *
> @@ -639,6 +654,8 @@ static void phy_error(struct phy_device *phydev)
>  	mutex_lock(&phydev->lock);
>  	phydev->state = PHY_HALTED;
>  	mutex_unlock(&phydev->lock);
> +
> +	phy_trigger_machine(phydev);
>  }
>  
>  /**
> @@ -785,8 +802,7 @@ void phy_change(struct phy_device *phydev)
>  	}
>  
>  	/* reschedule state queue work to run as soon as possible */
> -	cancel_delayed_work_sync(&phydev->state_queue);
> -	queue_delayed_work(system_power_efficient_wq, &phydev->state_queue, 0);
> +	phy_trigger_machine(phydev);
>  	return;
>  
>  ignore:
> @@ -875,6 +891,8 @@ void phy_start(struct phy_device *phydev)
>  	/* if phy was suspended, bring the physical link up again */
>  	if (do_resume)
>  		phy_resume(phydev);
> +
> +	phy_trigger_machine(phydev);
>  }
>  EXPORT_SYMBOL(phy_start);
>  
> 


-- 
Florian

Re: [PATCH RFC 5/6] net: phy: Trigger state machine on state change and not polling.

[Andrew Lunn]

On Wed, Sep 28, 2016 at 02:31:54PM -0700, Florian Fainelli wrote:
> On 09/28/2016 01:32 AM, Andrew Lunn wrote:
> > The phy_start() is used to indicate the PHY is now ready to do its
> > work. The state is changed, normally to PHY_UP which means that both
> > the MAC and the PHY are ready.
> > 
> > If the phy driver is using polling, when the next poll happens, the
> > state machine notices the PHY is now in PHY_UP, and kicks off
> > auto-negotiation, if needed.
> > 
> > If however, the PHY is using interrupts, there is no polling. The phy
> > is stuck in PHY_UP until the next interrupt comes along. And there is
> > no reason for the PHY to interrupt.
> > 
> > Have phy_start() schedule the state machine to run, which both speeds
> > up the polling use case, and makes the interrupt use case actually
> > work.
> > 
> > This problems exists whenever there is a state change which will not
> > cause an interrupt. Trigger the state machine in these cases,
> > e.g. phy_error().
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> No particular objections, this should also fix this:
> 
> http://lists.openwall.net/netdev/2016/05/17/147

Hi Florian

Yes, i was thinking it probably should have a fixes: tag and be a
separate patch. The hard part will be figuring out when this actually
broke, or does it go all the way back to when interrupt support was
added?

	Andrew

[PATCH RFC 6/6] arm: vf610: zii devel b: Add support for switch interrupts

[Andrew Lunn]

The Switches use GPIO lines to indicate interrupts from two of the
switches.

With these interrupts in place, we can make use of the interrupt
controllers within the switch to indicate when the internal PHYs
generate an interrupt. Use standard PHY properties to do this.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 51 +++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index 5c1fcab4a6f7..1a9b4ad138f6 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -88,10 +88,16 @@
 
 			switch0: switch0@0 {
 				compatible = "marvell,mv88e6085";
+				pinctrl-0 = <&pinctrl_gpio_switch0>;
+				pinctrl-names = "default";
 				#address-cells = <1>;
 				#size-cells = <0>;
 				reg = <0>;
 				dsa,member = <0 0>;
+				interrupt-parent = <&gpio0>;
+				interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 
 				ports {
 					#address-cells = <1>;
@@ -99,16 +105,19 @@
 					port@0 {
 						reg = <0>;
 						label = "lan0";
+						phy-handle = <&switch0phy0>;
 					};
 
 					port@1 {
 						reg = <1>;
 						label = "lan1";
+						phy-handle = <&switch0phy1>;
 					};
 
 					port@2 {
 						reg = <2>;
 						label = "lan2";
+						phy-handle = <&switch0phy2>;
 					};
 
 					switch0port5: port@5 {
@@ -133,6 +142,24 @@
 						};
 					};
 				};
+				mdio {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					switch0phy0: switch0phy0@0 {
+						reg = <0>;
+						interrupt-parent = <&switch0>;
+						interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+					};
+					switch0phy1: switch1phy0@1 {
+						reg = <1>;
+						interrupt-parent = <&switch0>;
+						interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;					};
+					switch0phy2: switch1phy0@2 {
+						reg = <2>;
+						interrupt-parent = <&switch0>;
+						interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+					};
+				};
 			};
 		};
 
@@ -143,10 +170,16 @@
 
 			switch1: switch1@0 {
 				compatible = "marvell,mv88e6085";
+				pinctrl-0 = <&pinctrl_gpio_switch1>;
+				pinctrl-names = "default";
 				#address-cells = <1>;
 				#size-cells = <0>;
 				reg = <0>;
 				dsa,member = <0 1>;
+				interrupt-parent = <&gpio0>;
+				interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 
 				ports {
 					#address-cells = <1>;
@@ -196,12 +229,18 @@
 					#size-cells = <0>;
 					switch1phy0: switch1phy0@0 {
 						reg = <0>;
+						interrupt-parent = <&switch1>;
+						interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
 					};
 					switch1phy1: switch1phy0@1 {
 						reg = <1>;
+						interrupt-parent = <&switch1>;
+						interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
 					};
 					switch1phy2: switch1phy0@2 {
 						reg = <2>;
+						interrupt-parent = <&switch1>;
+						interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
 					};
 				};
 			};
@@ -636,6 +675,18 @@
 		>;
 	};
 
+	pinctrl_gpio_switch0: pinctrl-gpio-switch0 {
+		fsl,pins = <
+			VF610_PAD_PTB5__GPIO_27		0x219d
+		>;
+	};
+
+	pinctrl_gpio_switch1: pinctrl-gpio-switch1 {
+		fsl,pins = <
+			VF610_PAD_PTB4__GPIO_26		0x219d
+		>;
+	};
+
 	pinctrl_i2c_mux_reset: pinctrl-i2c-mux-reset {
 		fsl,pins = <
 			 VF610_PAD_PTE14__GPIO_119	0x31c2
-- 
2.9.3