RE: [PATCH net-next 3/4] net: dsa: microchip: handle most interrupts in KSZ9477/KSZ9893 switch families

[Tristram.Ha]

> > +static irqreturn_t ksz9477_handle_port_irq(struct ksz_device *dev, u8 port,
> > +                                        u8 *data)
> > +{
> > +     struct dsa_switch *ds = dev->ds;
> > +     struct phy_device *phydev;
> > +     int cnt = 0;
> > +
> > +     phydev = mdiobus_get_phy(ds->user_mii_bus, port);
> > +     if (*data & PORT_PHY_INT) {
> > +             /* Handle the interrupt if there is no PHY device or its
> > +              * interrupt is not registered yet.
> > +              */
> > +             if (!phydev || phydev->interrupts != PHY_INTERRUPT_ENABLED) {
> > +                     u8 phy_status;
> > +
> > +                     ksz_pread8(dev, port, REG_PORT_PHY_INT_STATUS,
> > +                                &phy_status);
> > +                     if (phydev)
> > +                             phy_trigger_machine(phydev);
> > +                     ++cnt;
> > +                     *data &= ~PORT_PHY_INT;
> > +             }
> > +     }
> 
> This looks like a layering violation. Why is this needed? An interrupt
> controller generally has no idea what the individual interrupt is
> about. It just calls into the interrupt core to get the handler
> called, and then clears the interrupt. Why does that not work here?
> 
> What other DSA drivers do if they need to handle some of the
> interrupts is just request the interrupt like any other driver:
> 
> https://elixir.bootlin.com/linux/v6.10.3/source/drivers/net/dsa/mv88e6xxx/pcs-
> 639x.c#L95

The PHY and ACL interrupt handling can be removed, but the SGMII
interrupt handling cannot as the SGMII port is simulated as having an
internal PHY but the regular PHY interrupt processing will not clear the
interrupt.

Furthermore, there will be a situation where the SGMII interrupt is
triggered before the PHY interrupt handling function is registered.