Message-ID: <9AF787EF-A184-4492-A6F1-50B069D780E7@public-files.de>
Date: Tue, 01 Jul 2025 12:51:31 +0200
From: Frank Wunderlich <frank-w@...lic-files.de>
To: Krzysztof Kozlowski <krzk@...nel.org>, Frank Wunderlich <linux@...web.de>
CC: MyungJoo Ham <myungjoo.ham@...sung.com>,
 Kyungmin Park <kyungmin.park@...sung.com>,
 Chanwoo Choi <cw00.choi@...sung.com>, Georgi Djakov <djakov@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Andrew Lunn <andrew@...n.ch>,
 Vladimir Oltean <olteanv@...il.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Matthias Brugger <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Johnson Wang <johnson.wang@...iatek.com>,
 Arınç ÜNAL <arinc.unal@...nc9.com>,
 Landen Chao <Landen.Chao@...iatek.com>, DENG Qingfang <dqfext@...il.com>,
 Sean Wang <sean.wang@...iatek.com>, Daniel Golle <daniel@...rotopia.org>,
 Lorenzo Bianconi <lorenzo@...nel.org>, Felix Fietkau <nbd@....name>,
 linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v7 01/14] dt-bindings: net: mediatek,net: allow irq names

Am 1. Juli 2025 08:44:02 MESZ schrieb Krzysztof Kozlowski <krzk@...nel.org>:
>On Sat, Jun 28, 2025 at 06:54:36PM +0200, Frank Wunderlich wrote:
>> From: Frank Wunderlich <frank-w@...lic-files.de>
>> 
>> 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@...lic-files.de>
>> ---
>> 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

Powered by blists - more mailing lists