linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
@ 2022-08-22  9:20 Arun Ramadoss
  2022-08-22 12:56 ` Andrew Lunn
  2022-08-22 13:12 ` Andrew Lunn
  0 siblings, 2 replies; 7+ messages in thread
From: Arun Ramadoss @ 2022-08-22  9:20 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Russell King, Tristram Ha

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(+)

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)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
  2022-08-22  9:20 [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection Arun Ramadoss
@ 2022-08-22 12:56 ` Andrew Lunn
  2022-08-22 13:12 ` Andrew Lunn
  1 sibling, 0 replies; 7+ messages in thread
From: Andrew Lunn @ 2022-08-22 12:56 UTC (permalink / raw)
  To: Arun Ramadoss
  Cc: linux-kernel, netdev, Woojung Huh, UNGLinuxDriver,
	Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Russell King, Tristram Ha

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
  2022-08-22  9:20 [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection Arun Ramadoss
  2022-08-22 12:56 ` Andrew Lunn
@ 2022-08-22 13:12 ` Andrew Lunn
  2022-08-23 10:45   ` Arun.Ramadoss
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2022-08-22 13:12 UTC (permalink / raw)
  To: Arun Ramadoss
  Cc: linux-kernel, netdev, Woojung Huh, UNGLinuxDriver,
	Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Russell King, Tristram Ha

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
  2022-08-22 13:12 ` Andrew Lunn
@ 2022-08-23 10:45   ` Arun.Ramadoss
  2022-08-23 14:33     ` Andrew Lunn
  0 siblings, 1 reply; 7+ messages in thread
From: Arun.Ramadoss @ 2022-08-23 10:45 UTC (permalink / raw)
  To: andrew
  Cc: olteanv, linux-kernel, UNGLinuxDriver, vivien.didelot, linux,
	Tristram.Ha, f.fainelli, kuba, edumazet, pabeni, netdev,
	Woojung.Huh, davem

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
  2022-08-23 10:45   ` Arun.Ramadoss
@ 2022-08-23 14:33     ` Andrew Lunn
  2022-08-26 16:21       ` Arun.Ramadoss
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2022-08-23 14:33 UTC (permalink / raw)
  To: Arun.Ramadoss
  Cc: olteanv, linux-kernel, UNGLinuxDriver, vivien.didelot, linux,
	Tristram.Ha, f.fainelli, kuba, edumazet, pabeni, netdev,
	Woojung.Huh, davem

> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
  2022-08-23 14:33     ` Andrew Lunn
@ 2022-08-26 16:21       ` Arun.Ramadoss
  2022-08-26 19:11         ` Andrew Lunn
  0 siblings, 1 reply; 7+ messages in thread
From: Arun.Ramadoss @ 2022-08-26 16:21 UTC (permalink / raw)
  To: andrew
  Cc: olteanv, linux-kernel, UNGLinuxDriver, vivien.didelot, linux,
	Tristram.Ha, f.fainelli, kuba, edumazet, pabeni, netdev,
	Woojung.Huh, davem

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection
  2022-08-26 16:21       ` Arun.Ramadoss
@ 2022-08-26 19:11         ` Andrew Lunn
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Lunn @ 2022-08-26 19:11 UTC (permalink / raw)
  To: Arun.Ramadoss
  Cc: olteanv, linux-kernel, UNGLinuxDriver, vivien.didelot, linux,
	Tristram.Ha, f.fainelli, kuba, edumazet, pabeni, netdev,
	Woojung.Huh, davem

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-26 19:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-22  9:20 [RFC Patch net-next v2] net: dsa: microchip: lan937x: enable interrupt for internal phy link detection Arun Ramadoss
2022-08-22 12:56 ` Andrew Lunn
2022-08-22 13:12 ` Andrew Lunn
2022-08-23 10:45   ` Arun.Ramadoss
2022-08-23 14:33     ` Andrew Lunn
2022-08-26 16:21       ` Arun.Ramadoss
2022-08-26 19:11         ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).