> +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
> +irqreturn_t ksz9477_handle_irq(struct ksz_device *dev, u8 port, u8 *data)
> +{
> + irqreturn_t ret = IRQ_NONE;
> + u32 data32;
> +
> + if (port > 0)
> + return ksz9477_handle_port_irq(dev, port - 1, data);
> +
> + ksz_read32(dev, REG_SW_INT_STATUS__4, &data32);
> + if (data32 & LUE_INT) {
> + u8 lue;
> +
> + ksz_read8(dev, REG_SW_LUE_INT_STATUS, &lue);
> + ksz_write8(dev, REG_SW_LUE_INT_STATUS, lue);
> + if (lue & LEARN_FAIL_INT)
> + dev_info_ratelimited(dev->dev, "lue learn fail\n");
> + if (lue & WRITE_FAIL_INT)
> + dev_info_ratelimited(dev->dev, "lue write fail\n");
> + ret = IRQ_HANDLED;
> + }
https://elixir.bootlin.com/linux/v6.10.3/source/drivers/net/dsa/mv88e6xxx/global1_atu.c#L474
Andrew
