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: <20240711-molasses-shut-1fc7d9fe6a0e@spud>
Date: Thu, 11 Jul 2024 16:40:58 +0100
From: Conor Dooley <conor@...nel.org>
To: Nicolas Ferre <nicolas.ferre@...rochip.com>
Cc: Varshini.Rajendran@...rochip.com, 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,
	maz@...nel.org
Subject: Re: [PATCH v5 15/27] dt-bindings: interrupt-controller: Document the
 property microchip,nr-irqs

On Thu, Jul 11, 2024 at 02:42:01PM +0200, Nicolas Ferre wrote:
> Answering to myself (again) and to Conor...
> 
> On 09/07/2024 at 16:06, Nicolas.Ferre@...rochip.com wrote:
> > 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"
> 
> Well, after a brief talk with Varshini and a review of the code, I'm not so
> sure it's worth re-writing this part anymore Conor...
> It'll need changing 3-4 files (2 drivers and the "common" .h/.c files,
> because of the type change of ".data"); handling the special case of sama5d2
> (smr_cache thing) and touching lot more code than what is done in v3 of this
> patch series.
> 
> Original design was probably not optimal, but well, it's simple,
> understandable and except if there is a big benefit in moving, I would
> prefer to keep it like this.
> If you agree, we can ask Varshini to re-post a separated IRQ-focused series
> for handling sam9x75 changes.

I dunno, it's up to the folks that care about the driver whether they
want to do restructuring, not me. The nr-irqs property stays NAKed though,
since the information is determinable from the compatible.

Thanks,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)