* [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description @ 2023-08-15 8:47 Binbin Zhou 2023-08-15 14:19 ` Conor Dooley 0 siblings, 1 reply; 5+ messages in thread From: Binbin Zhou @ 2023-08-15 8:47 UTC (permalink / raw) To: Binbin Zhou, Huacai Chen, Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Huacai Chen, loongson-kernel, devicetree, Jiaxun Yang, diasyzhang, linux-kernel, Binbin Zhou As we know, some Loongson-2K CPUs are single-core, e.g. Loongson-2K0500, and the "isr1" means routing interrupts to core1, which should be optional. So add maxItems/minItems limits to reg/reg-names. Also, The interrupt-names attribute represents a list of parent interrupt names that should change with interrupts. Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> --- .../interrupt-controller/loongson,liointc.yaml | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml index 00b570c82903..adb428211a72 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml @@ -11,7 +11,7 @@ maintainers: description: | This interrupt controller is found in the Loongson-3 family of chips and - Loongson-2K1000 chip, as the primary package interrupt controller which + Loongson-2K series chips, as the primary package interrupt controller which can route local I/O interrupt to interrupt lines of cores. allOf: @@ -33,6 +33,7 @@ properties: - const: main - const: isr0 - const: isr1 + minItems: 2 interrupt-controller: true @@ -45,11 +46,9 @@ properties: interrupt-names: description: List of names for the parent interrupts. items: - - const: int0 - - const: int1 - - const: int2 - - const: int3 + pattern: int[0-3] minItems: 1 + maxItems: 4 '#interrupt-cells': const: 2 @@ -73,7 +72,6 @@ required: - '#interrupt-cells' - loongson,parent_int_map - unevaluatedProperties: false if: @@ -86,7 +84,8 @@ if: then: properties: reg: - minItems: 3 + minItems: 2 + maxItems: 3 required: - reg-names @@ -113,7 +112,6 @@ examples: <0x0f000000>, /* int1 */ <0x00000000>, /* int2 */ <0x00000000>; /* int3 */ - }; ... -- 2.39.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description 2023-08-15 8:47 [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description Binbin Zhou @ 2023-08-15 14:19 ` Conor Dooley 2023-08-17 2:56 ` Binbin Zhou 0 siblings, 1 reply; 5+ messages in thread From: Conor Dooley @ 2023-08-15 14:19 UTC (permalink / raw) To: Binbin Zhou Cc: Binbin Zhou, Huacai Chen, Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel, devicetree, Jiaxun Yang, diasyzhang, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2900 bytes --] Hey, On Tue, Aug 15, 2023 at 04:47:13PM +0800, Binbin Zhou wrote: > As we know, some Loongson-2K CPUs are single-core, e.g. Loongson-2K0500, > and the "isr1" means routing interrupts to core1, which should be > optional. So add maxItems/minItems limits to reg/reg-names. > Also, The interrupt-names attribute represents a list of parent > interrupt names that should change with interrupts. This should have been with the other series that introduces the users probably so that things make more sense to the reader. > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > --- > .../interrupt-controller/loongson,liointc.yaml | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > index 00b570c82903..adb428211a72 100644 > --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > @@ -11,7 +11,7 @@ maintainers: > > description: | > This interrupt controller is found in the Loongson-3 family of chips and > - Loongson-2K1000 chip, as the primary package interrupt controller which > + Loongson-2K series chips, as the primary package interrupt controller which > can route local I/O interrupt to interrupt lines of cores. > > allOf: > @@ -33,6 +33,7 @@ properties: > - const: main > - const: isr0 > - const: isr1 > + minItems: 2 > > interrupt-controller: true > > @@ -45,11 +46,9 @@ properties: > interrupt-names: > description: List of names for the parent interrupts. > items: > - - const: int0 > - - const: int1 > - - const: int2 > - - const: int3 > + pattern: int[0-3] From a quick look at the new devicetrees, I don't understand the ordering relaxation. Do you actually have a system that only has, for example, int3? Also, as the interrupt-names are not required, changing the ordering here is not ABI compatible AFAICT. Does that have any fallout? Thanks, Conor. > minItems: 1 > + maxItems: 4 > > '#interrupt-cells': > const: 2 > @@ -73,7 +72,6 @@ required: > - '#interrupt-cells' > - loongson,parent_int_map > > - > unevaluatedProperties: false > > if: > @@ -86,7 +84,8 @@ if: > then: > properties: > reg: > - minItems: 3 > + minItems: 2 > + maxItems: 3 > > required: > - reg-names > @@ -113,7 +112,6 @@ examples: > <0x0f000000>, /* int1 */ > <0x00000000>, /* int2 */ > <0x00000000>; /* int3 */ > - > }; > > ... > -- > 2.39.3 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description 2023-08-15 14:19 ` Conor Dooley @ 2023-08-17 2:56 ` Binbin Zhou 2023-08-17 8:51 ` Conor Dooley 0 siblings, 1 reply; 5+ messages in thread From: Binbin Zhou @ 2023-08-17 2:56 UTC (permalink / raw) To: Conor Dooley Cc: Binbin Zhou, Huacai Chen, Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel, devicetree, Jiaxun Yang, diasyzhang, linux-kernel Hi Conor: Thanks for your reply. On Tue, Aug 15, 2023 at 10:20 PM Conor Dooley <conor@kernel.org> wrote: > > Hey, > > On Tue, Aug 15, 2023 at 04:47:13PM +0800, Binbin Zhou wrote: > > As we know, some Loongson-2K CPUs are single-core, e.g. Loongson-2K0500, > > and the "isr1" means routing interrupts to core1, which should be > > optional. So add maxItems/minItems limits to reg/reg-names. > > Also, The interrupt-names attribute represents a list of parent > > interrupt names that should change with interrupts. > > This should have been with the other series that introduces the users > probably so that things make more sense to the reader. I was under the impression that the mips Loongson-2K1000 was also required for this patch, so I committed it separately. Maybe my commit should still be described in more detail. > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > > --- > > .../interrupt-controller/loongson,liointc.yaml | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > index 00b570c82903..adb428211a72 100644 > > --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > @@ -11,7 +11,7 @@ maintainers: > > > > description: | > > This interrupt controller is found in the Loongson-3 family of chips and > > - Loongson-2K1000 chip, as the primary package interrupt controller which > > + Loongson-2K series chips, as the primary package interrupt controller which > > can route local I/O interrupt to interrupt lines of cores. > > > > allOf: > > @@ -33,6 +33,7 @@ properties: > > - const: main > > - const: isr0 > > - const: isr1 > > + minItems: 2 > > > > interrupt-controller: true > > > > @@ -45,11 +46,9 @@ properties: > > interrupt-names: > > description: List of names for the parent interrupts. > > items: > > - - const: int0 > > - - const: int1 > > - - const: int2 > > - - const: int3 > > + pattern: int[0-3] > > From a quick look at the new devicetrees, I don't understand the > ordering relaxation. Do you actually have a system that only has, for > example, int3? For a better understanding, allow me to first explain the composition of the interrupt routing register: It is an 8 bit register that is divided into two parts: 0-3 : The processor core vector number of the route, this part is handled in the code. 4-7 : The processor core interrupt pin vector number for routing, i.e. int0-int3. Each intx can handle 32 interrupt sources. For example, in Loongson-2K1000/Loongson-2K0500, there are a total of 64 interrupt sources, and we need to route them to two intx. We don't mandate which interrupt vector number must be used, in our practice the tendency is to start with int0. It is worth noting that we must follow the following correspondence: interrupt->interrupt-names 2->int0 3->int1 4->int2 5->int3 > > Also, as the interrupt-names are not required, changing the ordering > here is not ABI compatible AFAICT. Does that have any fallout? Oh, this should be another point that needs to be modified, the interrupt-names should be required. because in the driver code the parent interrupts are fetched through of_irq_get_byname(). https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/irqchip/irq-loongson-liointc.c?h=v6.5-rc6#n345 for (i = 0; i < LIOINTC_NUM_PARENT; i++) { parent_irq[i] = of_irq_get_byname(node, parent_names[i]); if (parent_irq[i] > 0) have_parent = TRUE; } if (!have_parent) return -ENODEV; Also the way liointc-2.0 is written in the dts does not match the dt-binding. The dts using loongson,liointc-2.0 are: arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi (mips Loongson-2K1000) arch/loongarch/boot/dts/loongson-2k0500.dtsi arch/loongarch/boot/dts/loongson-2k1000.dtsi arch/loongarch/boot/dts/loongson-2k2000.dtsi liointc0: interrupt-controller@1fe01400 { ...... interrupts = <2>; interrupt-names = "int0"; loongson,parent_int_map = <0xffffffff>, /* int0 */ <0x00000000>, /* int1 */ <0x00000000>, /* int2 */ <0x00000000>; /* int3 */ }; liointc1: interrupt-controller@1fe01440 { .... interrupts = <3>; interrupt-names = "int1"; loongson,parent_int_map = <0x00000000>, /* int0 */ <0xffffffff>, /* int1 */ <0x00000000>, /* int2 */ <0x00000000>; /* int3 */ }; We split the two intx into two nodes because of register definitions etc. There is the following WARNING at liointc1: arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: interrupt-controller@1fe01440: interrupt-names:0: 'int0' was expected From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: interrupt-controller@1fe01440: Unevaluated properties are not allowed ('interrupt-names' was unexpected) From schema: Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml But actually, in liointc1, we only need int1. Thanks. Binbin > > Thanks, > Conor. > > > minItems: 1 > > + maxItems: 4 > > > > '#interrupt-cells': > > const: 2 > > @@ -73,7 +72,6 @@ required: > > - '#interrupt-cells' > > - loongson,parent_int_map > > > > - > > unevaluatedProperties: false > > > > if: > > @@ -86,7 +84,8 @@ if: > > then: > > properties: > > reg: > > - minItems: 3 > > + minItems: 2 > > + maxItems: 3 > > > > required: > > - reg-names > > @@ -113,7 +112,6 @@ examples: > > <0x0f000000>, /* int1 */ > > <0x00000000>, /* int2 */ > > <0x00000000>; /* int3 */ > > - > > }; > > > > ... > > -- > > 2.39.3 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description 2023-08-17 2:56 ` Binbin Zhou @ 2023-08-17 8:51 ` Conor Dooley 2023-08-17 11:26 ` Binbin Zhou 0 siblings, 1 reply; 5+ messages in thread From: Conor Dooley @ 2023-08-17 8:51 UTC (permalink / raw) To: Binbin Zhou Cc: Binbin Zhou, Huacai Chen, Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel, devicetree, Jiaxun Yang, diasyzhang, linux-kernel [-- Attachment #1: Type: text/plain, Size: 7122 bytes --] On Thu, Aug 17, 2023 at 10:56:37AM +0800, Binbin Zhou wrote: > Hi Conor: > > Thanks for your reply. > > On Tue, Aug 15, 2023 at 10:20 PM Conor Dooley <conor@kernel.org> wrote: > > > > Hey, > > > > On Tue, Aug 15, 2023 at 04:47:13PM +0800, Binbin Zhou wrote: > > > As we know, some Loongson-2K CPUs are single-core, e.g. Loongson-2K0500, > > > and the "isr1" means routing interrupts to core1, which should be > > > optional. So add maxItems/minItems limits to reg/reg-names. > > > Also, The interrupt-names attribute represents a list of parent > > > interrupt names that should change with interrupts. > > > > This should have been with the other series that introduces the users > > probably so that things make more sense to the reader. > > I was under the impression that the mips Loongson-2K1000 was also > required for this patch, so I committed it separately. > Maybe my commit should still be described in more detail. Ah, I just assumed, given the timing, that it was for the loongson stuff only. > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > > > --- > > > .../interrupt-controller/loongson,liointc.yaml | 14 ++++++-------- > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > index 00b570c82903..adb428211a72 100644 > > > --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > @@ -11,7 +11,7 @@ maintainers: > > > > > > description: | > > > This interrupt controller is found in the Loongson-3 family of chips and > > > - Loongson-2K1000 chip, as the primary package interrupt controller which > > > + Loongson-2K series chips, as the primary package interrupt controller which > > > can route local I/O interrupt to interrupt lines of cores. > > > > > > allOf: > > > @@ -33,6 +33,7 @@ properties: > > > - const: main > > > - const: isr0 > > > - const: isr1 > > > + minItems: 2 > > > > > > interrupt-controller: true > > > > > > @@ -45,11 +46,9 @@ properties: > > > interrupt-names: > > > description: List of names for the parent interrupts. > > > items: > > > - - const: int0 > > > - - const: int1 > > > - - const: int2 > > > - - const: int3 > > > + pattern: int[0-3] > > > > From a quick look at the new devicetrees, I don't understand the > > ordering relaxation. Do you actually have a system that only has, for > > example, int3? > > For a better understanding, allow me to first explain the composition > of the interrupt routing register: > It is an 8 bit register that is divided into two parts: > 0-3 : The processor core vector number of the route, this part is > handled in the code. > 4-7 : The processor core interrupt pin vector number for routing, i.e. > int0-int3. > Each intx can handle 32 interrupt sources. > > For example, in Loongson-2K1000/Loongson-2K0500, there are a total of > 64 interrupt sources, and we need to route them to two intx. > > We don't mandate which interrupt vector number must be used, in our > practice the tendency is to start with int0. > It is worth noting that we must follow the following correspondence: > interrupt->interrupt-names > 2->int0 > 3->int1 > 4->int2 > 5->int3 > > > > > Also, as the interrupt-names are not required, changing the ordering > > here is not ABI compatible AFAICT. Does that have any fallout? > > Oh, this should be another point that needs to be modified, the > interrupt-names should be required. because in the driver code the > parent interrupts are fetched through of_irq_get_byname(). Yeah, that should probably be made required so. > Also the way liointc-2.0 is written in the dts does not match the dt-binding. > The dts using loongson,liointc-2.0 are: > arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi (mips Loongson-2K1000) > arch/loongarch/boot/dts/loongson-2k0500.dtsi > arch/loongarch/boot/dts/loongson-2k1000.dtsi > arch/loongarch/boot/dts/loongson-2k2000.dtsi > > liointc0: interrupt-controller@1fe01400 { > ...... > interrupts = <2>; > interrupt-names = "int0"; > loongson,parent_int_map = <0xffffffff>, /* int0 */ > <0x00000000>, /* int1 */ > <0x00000000>, /* int2 */ > <0x00000000>; /* int3 */ > }; > > liointc1: interrupt-controller@1fe01440 { > .... > interrupts = <3>; > interrupt-names = "int1"; > loongson,parent_int_map = <0x00000000>, /* int0 */ > <0xffffffff>, /* int1 */ > <0x00000000>, /* int2 */ > <0x00000000>; /* int3 */ > }; > > We split the two intx into two nodes because of register definitions > etc. There is the following WARNING at liointc1: Did you split it in two because of register definitions, or because there are physically two controllers on the SoC? Your comments earlier sound like there are physically two interrupt controllers, which would be a valid reason to split the nodes. Thanks, Conor. > arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: > interrupt-controller@1fe01440: interrupt-names:0: 'int0' was expected > From schema: > Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: > interrupt-controller@1fe01440: Unevaluated properties are not allowed > ('interrupt-names' was unexpected) > From schema: > Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > But actually, in liointc1, we only need int1. > > Thanks. > Binbin > > > > > Thanks, > > Conor. > > > > > minItems: 1 > > > + maxItems: 4 > > > > > > '#interrupt-cells': > > > const: 2 > > > @@ -73,7 +72,6 @@ required: > > > - '#interrupt-cells' > > > - loongson,parent_int_map > > > > > > - > > > unevaluatedProperties: false > > > > > > if: > > > @@ -86,7 +84,8 @@ if: > > > then: > > > properties: > > > reg: > > > - minItems: 3 > > > + minItems: 2 > > > + maxItems: 3 > > > > > > required: > > > - reg-names > > > @@ -113,7 +112,6 @@ examples: > > > <0x0f000000>, /* int1 */ > > > <0x00000000>, /* int2 */ > > > <0x00000000>; /* int3 */ > > > - > > > }; > > > > > > ... > > > -- > > > 2.39.3 > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description 2023-08-17 8:51 ` Conor Dooley @ 2023-08-17 11:26 ` Binbin Zhou 0 siblings, 0 replies; 5+ messages in thread From: Binbin Zhou @ 2023-08-17 11:26 UTC (permalink / raw) To: Conor Dooley Cc: Binbin Zhou, Huacai Chen, Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Huacai Chen, loongson-kernel, devicetree, Jiaxun Yang, diasyzhang, linux-kernel On Thu, Aug 17, 2023 at 4:51 PM Conor Dooley <conor@kernel.org> wrote: > > On Thu, Aug 17, 2023 at 10:56:37AM +0800, Binbin Zhou wrote: > > Hi Conor: > > > > Thanks for your reply. > > > > On Tue, Aug 15, 2023 at 10:20 PM Conor Dooley <conor@kernel.org> wrote: > > > > > > Hey, > > > > > > On Tue, Aug 15, 2023 at 04:47:13PM +0800, Binbin Zhou wrote: > > > > As we know, some Loongson-2K CPUs are single-core, e.g. Loongson-2K0500, > > > > and the "isr1" means routing interrupts to core1, which should be > > > > optional. So add maxItems/minItems limits to reg/reg-names. > > > > Also, The interrupt-names attribute represents a list of parent > > > > interrupt names that should change with interrupts. > > > > > > This should have been with the other series that introduces the users > > > probably so that things make more sense to the reader. > > > > I was under the impression that the mips Loongson-2K1000 was also > > required for this patch, so I committed it separately. > > Maybe my commit should still be described in more detail. > > Ah, I just assumed, given the timing, that it was for the loongson > stuff only. > > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > > > > --- > > > > .../interrupt-controller/loongson,liointc.yaml | 14 ++++++-------- > > > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > > index 00b570c82903..adb428211a72 100644 > > > > --- a/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > > @@ -11,7 +11,7 @@ maintainers: > > > > > > > > description: | > > > > This interrupt controller is found in the Loongson-3 family of chips and > > > > - Loongson-2K1000 chip, as the primary package interrupt controller which > > > > + Loongson-2K series chips, as the primary package interrupt controller which > > > > can route local I/O interrupt to interrupt lines of cores. > > > > > > > > allOf: > > > > @@ -33,6 +33,7 @@ properties: > > > > - const: main > > > > - const: isr0 > > > > - const: isr1 > > > > + minItems: 2 > > > > > > > > interrupt-controller: true > > > > > > > > @@ -45,11 +46,9 @@ properties: > > > > interrupt-names: > > > > description: List of names for the parent interrupts. > > > > items: > > > > - - const: int0 > > > > - - const: int1 > > > > - - const: int2 > > > > - - const: int3 > > > > + pattern: int[0-3] > > > > > > From a quick look at the new devicetrees, I don't understand the > > > ordering relaxation. Do you actually have a system that only has, for > > > example, int3? > > > > For a better understanding, allow me to first explain the composition > > of the interrupt routing register: > > It is an 8 bit register that is divided into two parts: > > 0-3 : The processor core vector number of the route, this part is > > handled in the code. > > 4-7 : The processor core interrupt pin vector number for routing, i.e. > > int0-int3. > > Each intx can handle 32 interrupt sources. > > > > For example, in Loongson-2K1000/Loongson-2K0500, there are a total of > > 64 interrupt sources, and we need to route them to two intx. > > > > We don't mandate which interrupt vector number must be used, in our > > practice the tendency is to start with int0. > > It is worth noting that we must follow the following correspondence: > > interrupt->interrupt-names > > 2->int0 > > 3->int1 > > 4->int2 > > 5->int3 > > > > > > > > Also, as the interrupt-names are not required, changing the ordering > > > here is not ABI compatible AFAICT. Does that have any fallout? > > > > Oh, this should be another point that needs to be modified, the > > interrupt-names should be required. because in the driver code the > > parent interrupts are fetched through of_irq_get_byname(). > > Yeah, that should probably be made required so. > > > Also the way liointc-2.0 is written in the dts does not match the dt-binding. > > The dts using loongson,liointc-2.0 are: > > > > arch/mips/boot/dts/loongson/loongson64-2k1000.dtsi (mips Loongson-2K1000) > > arch/loongarch/boot/dts/loongson-2k0500.dtsi > > arch/loongarch/boot/dts/loongson-2k1000.dtsi > > arch/loongarch/boot/dts/loongson-2k2000.dtsi > > > > liointc0: interrupt-controller@1fe01400 { > > ...... > > interrupts = <2>; > > interrupt-names = "int0"; > > loongson,parent_int_map = <0xffffffff>, /* int0 */ > > <0x00000000>, /* int1 */ > > <0x00000000>, /* int2 */ > > <0x00000000>; /* int3 */ > > }; > > > > liointc1: interrupt-controller@1fe01440 { > > .... > > interrupts = <3>; > > interrupt-names = "int1"; > > loongson,parent_int_map = <0x00000000>, /* int0 */ > > <0xffffffff>, /* int1 */ > > <0x00000000>, /* int2 */ > > <0x00000000>; /* int3 */ > > }; > > > > We split the two intx into two nodes because of register definitions > > etc. There is the following WARNING at liointc1: > > Did you split it in two because of register definitions, or because > there are physically two controllers on the SoC? Your comments earlier > sound like there are physically two interrupt controllers, which would > be a valid reason to split the nodes. Hi, Conor: Whoops, it looks like my description is still not clear enough. First of all, a chip has only one physical liointc. Unlike other chips, Loongson-2K1000/2K0500 has 64 interrupt sources, which requires two sets of registers to set the routing of all the interrupts, and correspondingly two nodes to describe them in dts. Concretely, in Loongson-2K1000/2K0500: liointc0: indicates the routing of interrupt sources 0-31; liointc1: indicates the routing of interrupt sources 32-63. Others, such as Loongson-3A, have only 32 interrupt sources and need only one liointc node. For better understanding, I'll add a more detailed description, which should especially point out the 2K1000/2K0500 differences. For example, for reg-names description: main: interrupt routing registers isr0: low/high 32-bit interrupt status routed to CORE0. isr1: low/high 32-bit interrupt status routed to CORE1. Thanks. Binbin > > Thanks, > Conor. > > > arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: > > interrupt-controller@1fe01440: interrupt-names:0: 'int0' was expected > > From schema: > > Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > arch/loongarch/boot/dts/loongson-2k1000-ref.dtb: > > interrupt-controller@1fe01440: Unevaluated properties are not allowed > > ('interrupt-names' was unexpected) > > From schema: > > Documentation/devicetree/bindings/interrupt-controller/loongson,liointc.yaml > > > > But actually, in liointc1, we only need int1. > > > > Thanks. > > Binbin > > > > > > > > Thanks, > > > Conor. > > > > > > > minItems: 1 > > > > + maxItems: 4 > > > > > > > > '#interrupt-cells': > > > > const: 2 > > > > @@ -73,7 +72,6 @@ required: > > > > - '#interrupt-cells' > > > > - loongson,parent_int_map > > > > > > > > - > > > > unevaluatedProperties: false > > > > > > > > if: > > > > @@ -86,7 +84,8 @@ if: > > > > then: > > > > properties: > > > > reg: > > > > - minItems: 3 > > > > + minItems: 2 > > > > + maxItems: 3 > > > > > > > > required: > > > > - reg-names > > > > @@ -113,7 +112,6 @@ examples: > > > > <0x0f000000>, /* int1 */ > > > > <0x00000000>, /* int2 */ > > > > <0x00000000>; /* int3 */ > > > > - > > > > }; > > > > > > > > ... > > > > -- > > > > 2.39.3 > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-17 11:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-15 8:47 [PATCH] dt-bindings: interrupt-controller: loongson,liointc: Fix warnings about reg and interrupt description Binbin Zhou 2023-08-15 14:19 ` Conor Dooley 2023-08-17 2:56 ` Binbin Zhou 2023-08-17 8:51 ` Conor Dooley 2023-08-17 11:26 ` Binbin Zhou
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).