linux-kernel - Re: [PATCH v5 15/27] dt-bindings: interrupt-controller: Document the property microchip,nr-irqs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82ca4f3d-fa78-4617-823e-69f16a2c3319@microchip.com>
Date: Tue, 9 Jul 2024 14:06:29 +0000
From: <Nicolas.Ferre@...rochip.com>
To: <Varshini.Rajendran@...rochip.com>, <conor@...nel.org>, <maz@...nel.org>
CC: <tglx@...utronix.de>, <robh@...nel.org>, <krzk+dt@...nel.org>,
	<conor+dt@...nel.org>, <alexandre.belloni@...tlin.com>,
	<claudiu.beznea@...on.dev>, <Dharma.B@...rochip.com>,
	<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v5 15/27] dt-bindings: interrupt-controller: Document the
 property microchip,nr-irqs

On 09/07/2024 at 08:13, Varshini Rajendran - I67070 wrote:
> On 03/07/24 9:11 pm, Conor Dooley wrote:
>> On Wed, Jul 03, 2024 at 03:58:14PM +0530, Varshini Rajendran wrote:
>>> Add the description and conditions to the device tree documentation
>>> for the property microchip,nr-irqs.
>>>
>>> Signed-off-by: Varshini Rajendran<varshini.rajendran@...rochip.com>
>> This needs to be part of patch 14.
>>
>>> ---
>>>    .../bindings/interrupt-controller/atmel,aic.yaml     | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
>>> index 9c5af9dbcb6e..06e5f92e7d53 100644
>>> --- a/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/atmel,aic.yaml
>>> @@ -54,6 +54,10 @@ properties:
>>>        $ref: /schemas/types.yaml#/definitions/uint32-array
>>>        description: u32 array of external irqs.
>>>    
>>> +  microchip,nr-irqs:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    description: u32 array of nr_irqs.
>> This makes no sense, did you just copy from above? Why would the number
>> of irqs be an array? Why can't you determine this from the compatble?
>>
> Sorry for the bad description. I will correct it in the next version.
> 
> For the second part of the question, this change was done as a step to
> resolve having a new compatible while having practically the same IP
> pointed out in the v3 of the series [1]. It is kind of looping back to
> the initial idea now. Even if this is added as a driver data, it
> overrides the expectation from the comment in [1]. Please suggest. I

In your v3 patch, indeed you were extracting the number of IRQs from the 
compatibility string (aka, from device tree...). It's my preferred 
solution as well.

So, come back to v3 [1] and address what Conor said in v4 "...having 
specific $soc_aic5_of_init() functions for each SoC seems silly when 
usually only the number of interrupts changes. The number of IRQs could 
be in the match data and you could use aic5_of_init in your 
IRQCHIP_DECLARE directly"

I think that we can convince Marc/Thomas that it's the best option as it 
prevents introducing another non-standard property to the DT, break the 
ABI (and was used happily for years).

Best regards,
   Nicolas

[1]
https://lore.kernel.org/lkml/87ee1e3c365686bc60e92ba3972dc1a5@kernel.org/


> also read Rob's concerns on having a device tree property for number of
> irqs.
> 
> [1]
> https://lore.kernel.org/lkml/87ee1e3c365686bc60e92ba3972dc1a5@kernel.org/
> 
>> Thanks,
>> Conor.
>>
>>> +
>>>    allOf:
>>>      - $ref: /schemas/interrupt-controller.yaml#
>>>      - if:
>>> @@ -71,6 +75,14 @@ allOf:
>>>            atmel,external-irqs:
>>>              minItems: 1
>>>              maxItems: 1
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: microchip,sam9x7-aic
>>> +    then:
>>> +      required:
>>> +        - microchip,nr-irqs
>>>    
>>>    required:
>>>      - compatible
>>> -- 
>>> 2.25.1
>>>
>