Re: [PATCH net v2 2/2] dt-bindings: net: updated interrupt type to be active low, level triggered

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The DT binding correctly updates the interrupt type to active low, level-triggered (`IRQ_TYPE_LEVEL_LOW`), but the `oa_tc6` driver hardcodes `IRQF_TRIGGER_FALLING` in its IRQ request. This mismatch causes dropped interrupts, and attempting to fix the driver to use level-triggered interrupts would currently result in an interrupt storm.
--

commit 50f6d43fc72305690796fcff8249a6220cbe4714
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

dt-bindings: net: updated interrupt type to be active low, level triggered

This commit updates the LAN8650 device tree binding to specify an active
low, level-triggered interrupt, aligning with the hardware specification.

> diff --git a/Documentation/devicetree/bindings/net/microchip,lan8650.yaml b/Documentation/devicetree/bindings/net/microchip,lan8650.yaml
> --- a/Documentation/devicetree/bindings/net/microchip,lan8650.yaml
> +++ b/Documentation/devicetree/bindings/net/microchip,lan8650.yaml
> @@ -67,7 +67,7 @@ examples:
>          pinctrl-names = "default";
>          pinctrl-0 = <&eth0_pins>;
>          interrupt-parent = <&gpio>;
> -        interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> +        interrupts = <6 IRQ_TYPE_LEVEL_LOW>;

[Severity: High]
This is a pre-existing issue, but this DT binding update highlights a
mismatch with the driver implementation. The hardware uses an active-low
level-triggered interrupt, but the driver currently hardcodes an
edge-triggered configuration:

drivers/net/ethernet/oa_tc6.c:oa_tc6_init() {
    ...
    ret = devm_request_irq(&tc6->spi->dev, tc6->spi->irq, oa_tc6_macphy_isr,
                           IRQF_TRIGGER_FALLING, dev_name(&tc6->spi->dev),
                           tc6);
    ...
}

Does the driver need to be updated to respect the device tree's interrupt
trigger configuration instead of hardcoding IRQF_TRIGGER_FALLING?

If the driver is updated to use level-triggered interrupts, could this
cause an interrupt storm? Looking at the current hardirq handler:

drivers/net/ethernet/oa_tc6.c:oa_tc6_macphy_isr() {
    struct oa_tc6 *tc6 = data;

    tc6->int_flag = true;
    /* Wake spi kthread to perform spi transfer */
    wake_up_interruptible(&tc6->spi_wq);

    return IRQ_HANDLED;
}

Since oa_tc6_macphy_isr() returns IRQ_HANDLED without clearing the hardware
interrupt condition (which requires a slow SPI transaction), wouldn't the
IRQ core immediately fire the interrupt again if it were configured as
level-triggered?

Would it be safer to migrate this driver to use devm_request_threaded_irq()
with IRQF_ONESHOT so the interrupt remains masked until the threaded handler
can clear it, properly supporting the level-triggered configuration defined
in this binding?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CY8PR02MB92497E67D5AB4C72E7A4C12683162@CY8PR02MB9249.namprd02.prod.outlook.com?part=2