Re: [PATCH v7 01/14] dt-bindings: net: mediatek,net: allow irq names

[Frank Wunderlich]

Am 1. Juli 2025 08:44:02 MESZ schrieb Krzysztof Kozlowski <krzk@xxxxxxxxxx>:
>On Sat, Jun 28, 2025 at 06:54:36PM +0200, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
>> 
>> In preparation for MT7988 and RSS/LRO allow the interrupt-names
>
>Why? What preparation, what is the purpose of adding the names, what do
>they solve?

Devicetree handled by the mtk_eth_soc driver have
a wild mix of shared and non-shared irq definitions
accessed by index (shared use index 0,
non-shared
using 1+2). Some soc have only 3 FE irqs (like mt7622).

This makes it unclear which irq is used for what
on which SoC. Adding names for irq cleans this a bit
in device tree and driver.

>> property. Also increase the maximum IRQ count to 8 (4 FE + 4 RSS),
>
>Why? There is no user of 8 items.

MT7988 *with* RSS/LRO (not yet supported by driver
yet,but i add the irqs in devicetree in this series)
use 8 irqs,but RSS is optional and 4 irqs get working
ethernet stack.

I hope this explanation makes things clearer...

>> but set boundaries for all compatibles same as irq count.
>
>Your patch does not do it.

I set Min/max-items for interrupt names below like
interrupts count defined.

>> 
>> Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
>> ---
>> v7: fixed wrong rebase
>> v6: new patch splitted from the mt7988 changes
>> ---
>>  .../devicetree/bindings/net/mediatek,net.yaml | 38 ++++++++++++++++++-
>>  1 file changed, 37 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
>> index 9e02fd80af83..6672db206b38 100644
>> --- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
>> +++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
>> @@ -40,7 +40,19 @@ properties:
>>  
>>    interrupts:
>>      minItems: 1
>> -    maxItems: 4
>> +    maxItems: 8
>> +
>> +  interrupt-names:
>> +    minItems: 1
>> +    items:
>> +      - const: fe0
>> +      - const: fe1
>> +      - const: fe2
>> +      - const: fe3
>> +      - const: pdma0
>> +      - const: pdma1
>> +      - const: pdma2
>> +      - const: pdma3
>>  
>>    power-domains:
>>      maxItems: 1
>> @@ -135,6 +147,10 @@ allOf:
>>            minItems: 3
>>            maxItems: 3
>>  
>> +        interrupt-names:
>> +          minItems: 3
>> +          maxItems: 3
>> +
>>          clocks:
>>            minItems: 4
>>            maxItems: 4
>> @@ -166,6 +182,9 @@ allOf:
>>          interrupts:
>>            maxItems: 1
>>  
>> +        interrupt-namess:
>> +          maxItems: 1
>> +
>>          clocks:
>>            minItems: 2
>>            maxItems: 2
>> @@ -192,6 +211,10 @@ allOf:
>>            minItems: 3
>>            maxItems: 3
>>  
>> +        interrupt-names:
>> +          minItems: 3
>> +          maxItems: 3
>> +
>>          clocks:
>>            minItems: 11
>>            maxItems: 11
>> @@ -232,6 +255,10 @@ allOf:
>>            minItems: 3
>>            maxItems: 3
>>  
>> +        interrupt-names:
>> +          minItems: 3
>> +          maxItems: 3
>> +
>>          clocks:
>>            minItems: 17
>>            maxItems: 17
>> @@ -274,6 +301,9 @@ allOf:
>>          interrupts:
>>            minItems: 4
>>  
>> +        interrupt-names:
>> +          minItems: 4
>> +
>>          clocks:
>>            minItems: 15
>>            maxItems: 15
>> @@ -312,6 +342,9 @@ allOf:
>>          interrupts:
>>            minItems: 4
>>  
>> +        interrupt-names:
>> +          minItems: 4
>
>8 interrupts is now valid?
>
>> +
>>          clocks:
>>            minItems: 15
>>            maxItems: 15
>> @@ -350,6 +383,9 @@ allOf:
>>          interrupts:
>>            minItems: 4
>>  
>> +        interrupt-names:
>> +          minItems: 4
>
>So why sudenly this device gets 8 interrupts? This makes no sense,
>nothing explained in the commit msg.

4 FrameEngine IRQs are required to be defined (currently 2 are used in driver).
The other 4 are optional,but added in the devicetree
to not run into problems supporting old devicetree
when adding RSS/LRO to driver.

>I understand nothing from this patch and I already asked you to clearly
>explain why you are doing things. This patch on its own makes no sense.
>
>Best regards,
>Krzysztof
>


regards Frank