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

Arun.Ramadoss
2022-08-23T13:49:19Z

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