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@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx> > > > > 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.
Attachment:
signature.asc
Description: PGP signature
