| Message ID | a9f697906df6599e6b001981e668479da71aa7a0.1676289084.git.zhoubinbin@loongson.cn |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Loongson: irqchip: Add loongson-eiointc DT init support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 13/02/2023 13:15, Binbin Zhou wrote: > Add Loongson Extended I/O Interrupt controller binding with DT schema > format using json-schema. > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > --- > .../loongson,eiointc.yaml | 80 +++++++++++++++++++ > 1 file changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > new file mode 100644 > index 000000000000..88580297f955 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > @@ -0,0 +1,80 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" Drop quotes from bopth. > + > +title: Loongson Extended I/O Interrupt Controller > + > +maintainers: > + - Binbin Zhou <zhoubinbin@loongson.cn> > + > +description: | > + This interrupt controller is found on the Loongson-3 family chips and > + Loongson-2K0500 chip and is used to distribute interrupts directly to > + individual cores without forwarding them through the HT's interrupt line. > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# > + > +properties: > + compatible: > + enum: > + - loongson,eiointc-1.0 Why not using SoC based compatible? It is preferred. > + > + reg: > + minItems: 1 > + maxItems: 3 You need to describe the items. > + > + interrupt-controller: true > + > + interrupts: > + description: > + Interrupt source of the CPU interrupts. You need to describe the items. > + > + interrupt-names: > + description: > + List of names for the parent interrupts. Drop description. > + items: > + - const: int0 > + > + '#interrupt-cells': > + const: 1 > + > + 'loongson,eio-num-vecs': Drop quotes. > + description: > + The number of devices supported by the extended I/O interrupt vector. Why this cannot be inferred from the compatible? Different boards with the same SoC support different devices? > + $ref: "/schemas/types.yaml#/definitions/uint32" Drop quotes. > + minimum: 1 > + maximum: 256 > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-controller > + - '#interrupt-cells' > + - 'loongson,eio-num-vecs' > + > +unevaluatedProperties: false > + > +examples: > + - | > + eiointc: interrupt-controller@1fe11600 { > + compatible = "loongson,eiointc-1.0"; > + reg = <0x1fe11600 0x8 > + 0x1fe11700 0x8 > + 0x1fe11800 0x8>; That's not correct syntax. <>, <>, <> > + > + interrupt-controller; > + #interrupt-cells = <1>; > + > + interrupt-parent = <&cpuintc>; > + interrupts = <3>; > + interrupt-names = "int0"; > + > + loongson,eio-num-vecs = <128>; > + Drop stray blank line. > + }; > + > +... Best regards, Krzysztof
On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 13/02/2023 13:15, Binbin Zhou wrote: > > Add Loongson Extended I/O Interrupt controller binding with DT schema > > format using json-schema. > > > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > > --- > > .../loongson,eiointc.yaml | 80 +++++++++++++++++++ > > 1 file changed, 80 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > new file mode 100644 > > index 000000000000..88580297f955 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > @@ -0,0 +1,80 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > Drop quotes from bopth. > > > + > > +title: Loongson Extended I/O Interrupt Controller > > + > > +maintainers: > > + - Binbin Zhou <zhoubinbin@loongson.cn> > > + > > +description: | > > + This interrupt controller is found on the Loongson-3 family chips and > > + Loongson-2K0500 chip and is used to distribute interrupts directly to > > + individual cores without forwarding them through the HT's interrupt line. > > + > > +allOf: > > + - $ref: /schemas/interrupt-controller.yaml# > > + > > +properties: > > + compatible: > > + enum: > > + - loongson,eiointc-1.0 > > Why not using SoC based compatible? It is preferred. Hi Krzysztof: So far, from the datasheet, I know that only the EXIOINTC of the Loongson-2K0500 is different from the other chips, and that is the "loongson,eio-num-vecs" below, which is 128, while all the others are 256. My original idea was to add this property to make compatible consistent, and also to make it easier to add new chips if they have different eio-num-vecs. > > > + > > + reg: > > + minItems: 1 > > + maxItems: 3 > > You need to describe the items. > > > + > > + interrupt-controller: true > > + > > + interrupts: > > + description: > > + Interrupt source of the CPU interrupts. > > You need to describe the items. Do you mean a more detailed description? > > > + > > + interrupt-names: > > + description: > > + List of names for the parent interrupts. > > Drop description. > > > + items: > > + - const: int0 > > + > > + '#interrupt-cells': > > + const: 1 > > + > > + 'loongson,eio-num-vecs': > > Drop quotes. > > > + description: > > + The number of devices supported by the extended I/O interrupt vector. > > Why this cannot be inferred from the compatible? Different boards with > the same SoC support different devices? See above. Thanks. Binbin > > > + $ref: "/schemas/types.yaml#/definitions/uint32" > > Drop quotes. > > > + minimum: 1 > > + maximum: 256 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - interrupt-controller > > + - '#interrupt-cells' > > + - 'loongson,eio-num-vecs' > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + eiointc: interrupt-controller@1fe11600 { > > + compatible = "loongson,eiointc-1.0"; > > + reg = <0x1fe11600 0x8 > > + 0x1fe11700 0x8 > > + 0x1fe11800 0x8>; > > That's not correct syntax. <>, <>, <> > > > + > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + > > + interrupt-parent = <&cpuintc>; > > + interrupts = <3>; > > + interrupt-names = "int0"; > > + > > + loongson,eio-num-vecs = <128>; > > + > > Drop stray blank line. > > > + }; > > + > > +... > > Best regards, > Krzysztof > >
On 14/02/2023 13:40, Binbin Zhou wrote: > On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 13/02/2023 13:15, Binbin Zhou wrote: >>> Add Loongson Extended I/O Interrupt controller binding with DT schema >>> format using json-schema. >>> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> >>> --- >>> .../loongson,eiointc.yaml | 80 +++++++++++++++++++ >>> 1 file changed, 80 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>> new file mode 100644 >>> index 000000000000..88580297f955 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>> @@ -0,0 +1,80 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" >>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> >> Drop quotes from bopth. >> >>> + >>> +title: Loongson Extended I/O Interrupt Controller >>> + >>> +maintainers: >>> + - Binbin Zhou <zhoubinbin@loongson.cn> >>> + >>> +description: | >>> + This interrupt controller is found on the Loongson-3 family chips and >>> + Loongson-2K0500 chip and is used to distribute interrupts directly to >>> + individual cores without forwarding them through the HT's interrupt line. >>> + >>> +allOf: >>> + - $ref: /schemas/interrupt-controller.yaml# >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - loongson,eiointc-1.0 >> >> Why not using SoC based compatible? It is preferred. > > Hi Krzysztof: > > So far, from the datasheet, I know that only the EXIOINTC of the > Loongson-2K0500 is different from the other chips, and that is the > "loongson,eio-num-vecs" below, which is 128, while all the others are > 256. > My original idea was to add this property to make compatible > consistent, and also to make it easier to add new chips if they have > different eio-num-vecs. We talk about different things. SoC based compatibles are preferred over version ones. This was on the lists expressed many times. Please provide a reason why you deviate from general recommendation. Flexibility and genericness of bindings is not a reason - it's the opposite of the argument, thus this will be a: NAK. :( > >> >>> + >>> + reg: >>> + minItems: 1 >>> + maxItems: 3 >> >> You need to describe the items. >> >>> + >>> + interrupt-controller: true >>> + >>> + interrupts: >>> + description: >>> + Interrupt source of the CPU interrupts. >> >> You need to describe the items. > > Do you mean a more detailed description? I mean constraints on interrupts and/or description if they are not obvious. Best regards, Krzysztof
On Mon, Feb 13, 2023 at 08:15:27PM +0800, Binbin Zhou wrote: > Add Loongson Extended I/O Interrupt controller binding with DT schema > format using json-schema. > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > --- > .../loongson,eiointc.yaml | 80 +++++++++++++++++++ > 1 file changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > new file mode 100644 > index 000000000000..88580297f955 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > @@ -0,0 +1,80 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Loongson Extended I/O Interrupt Controller > + > +maintainers: > + - Binbin Zhou <zhoubinbin@loongson.cn> > + > +description: | > + This interrupt controller is found on the Loongson-3 family chips and > + Loongson-2K0500 chip and is used to distribute interrupts directly to > + individual cores without forwarding them through the HT's interrupt line. > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# > + > +properties: > + compatible: > + enum: > + - loongson,eiointc-1.0 > + > + reg: > + minItems: 1 > + maxItems: 3 > + > + interrupt-controller: true > + > + interrupts: > + description: > + Interrupt source of the CPU interrupts. > + > + interrupt-names: > + description: > + List of names for the parent interrupts. > + items: > + - const: int0 Why do you need this if there is only 1 interrupt? Rob
On 14/02/2023 13:43, Krzysztof Kozlowski wrote: >> >>> >>>> + >>>> + reg: >>>> + minItems: 1 >>>> + maxItems: 3 >>> >>> You need to describe the items.>>> >>>> + >>>> + interrupt-controller: true >>>> + >>>> + interrupts: >>>> + description: >>>> + Interrupt source of the CPU interrupts. >>> >>> You need to describe the items. >> >> Do you mean a more detailed description? > > I mean constraints on interrupts and/or description if they are not obvious. Actually I noticed that for interrupts you provided the name, so it is description and you just need maxItems:1. The reg however need list of items which clearly documents what is there. Best regards, Krzysztof
On Tue, Feb 14, 2023 at 8:43 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 14/02/2023 13:40, Binbin Zhou wrote: > > On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 13/02/2023 13:15, Binbin Zhou wrote: > >>> Add Loongson Extended I/O Interrupt controller binding with DT schema > >>> format using json-schema. > >>> > >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > >>> --- > >>> .../loongson,eiointc.yaml | 80 +++++++++++++++++++ > >>> 1 file changed, 80 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>> new file mode 100644 > >>> index 000000000000..88580297f955 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>> @@ -0,0 +1,80 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" > >>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > >> > >> Drop quotes from bopth. > >> > >>> + > >>> +title: Loongson Extended I/O Interrupt Controller > >>> + > >>> +maintainers: > >>> + - Binbin Zhou <zhoubinbin@loongson.cn> > >>> + > >>> +description: | > >>> + This interrupt controller is found on the Loongson-3 family chips and > >>> + Loongson-2K0500 chip and is used to distribute interrupts directly to > >>> + individual cores without forwarding them through the HT's interrupt line. > >>> + > >>> +allOf: > >>> + - $ref: /schemas/interrupt-controller.yaml# > >>> + > >>> +properties: > >>> + compatible: > >>> + enum: > >>> + - loongson,eiointc-1.0 > >> > >> Why not using SoC based compatible? It is preferred. > > > > Hi Krzysztof: > > > > So far, from the datasheet, I know that only the EXIOINTC of the > > Loongson-2K0500 is different from the other chips, and that is the > > "loongson,eio-num-vecs" below, which is 128, while all the others are > > 256. > > My original idea was to add this property to make compatible > > consistent, and also to make it easier to add new chips if they have > > different eio-num-vecs. > > We talk about different things. SoC based compatibles are preferred over > version ones. This was on the lists expressed many times. Please provide > a reason why you deviate from general recommendation. Flexibility and > genericness of bindings is not a reason - it's the opposite of the > argument, thus this will be a: NAK. :( > > Hi Krzysztof: Allow me to give a brief overview of the current status of eiointc (DT-based): Loongson-3A series supports eiointc; Loongson-2K1000 does not support eiointc now; Loongson-2K0500 supports eiointc, with differences from Loongson-3, e.g. only up to 128 devices are supported; Loongson-2K2000 supports eiointc, similar to Loongson-3. .... As can be seen, there is now a bit of confusion in the chip's design of eiointc. The design of eiointc is probably refined step by step with the chip. The same version of eiointc can be used for multiple chips, and the same chip series may also use different versions of eiointc. Low-end chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, depending on the time it's produced. So in the Loongson-2K series I have defined the current state as eiointc-1.0, using the dts property to indicate the maximum number of devices supported by eiointc that can be used directly in the driver. If there are new changes to the design later on, such as the definition of registers, we can call it eiointc-2.0, which can also cover more than one chip. Thanks. Binbin > > > >> > >>> + > >>> + reg: > >>> + minItems: 1 > >>> + maxItems: 3 > >> > >> You need to describe the items. > >> > >>> + > >>> + interrupt-controller: true > >>> + > >>> + interrupts: > >>> + description: > >>> + Interrupt source of the CPU interrupts. > >> > >> You need to describe the items. > > > > Do you mean a more detailed description? > > I mean constraints on interrupts and/or description if they are not obvious. > > Best regards, > Krzysztof >
On 16/02/2023 02:46, Binbin Zhou wrote: > On Tue, Feb 14, 2023 at 8:43 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 14/02/2023 13:40, Binbin Zhou wrote: >>> On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski >>> <krzysztof.kozlowski@linaro.org> wrote: >>>> >>>> On 13/02/2023 13:15, Binbin Zhou wrote: >>>>> Add Loongson Extended I/O Interrupt controller binding with DT schema >>>>> format using json-schema. >>>>> >>>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> >>>>> --- >>>>> .../loongson,eiointc.yaml | 80 +++++++++++++++++++ >>>>> 1 file changed, 80 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>>>> new file mode 100644 >>>>> index 000000000000..88580297f955 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>>>> @@ -0,0 +1,80 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" >>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>>> >>>> Drop quotes from bopth. >>>> >>>>> + >>>>> +title: Loongson Extended I/O Interrupt Controller >>>>> + >>>>> +maintainers: >>>>> + - Binbin Zhou <zhoubinbin@loongson.cn> >>>>> + >>>>> +description: | >>>>> + This interrupt controller is found on the Loongson-3 family chips and >>>>> + Loongson-2K0500 chip and is used to distribute interrupts directly to >>>>> + individual cores without forwarding them through the HT's interrupt line. >>>>> + >>>>> +allOf: >>>>> + - $ref: /schemas/interrupt-controller.yaml# >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + enum: >>>>> + - loongson,eiointc-1.0 >>>> >>>> Why not using SoC based compatible? It is preferred. >>> >>> Hi Krzysztof: >>> >>> So far, from the datasheet, I know that only the EXIOINTC of the >>> Loongson-2K0500 is different from the other chips, and that is the >>> "loongson,eio-num-vecs" below, which is 128, while all the others are >>> 256. >>> My original idea was to add this property to make compatible >>> consistent, and also to make it easier to add new chips if they have >>> different eio-num-vecs. >> >> We talk about different things. SoC based compatibles are preferred over >> version ones. This was on the lists expressed many times. Please provide >> a reason why you deviate from general recommendation. Flexibility and >> genericness of bindings is not a reason - it's the opposite of the >> argument, thus this will be a: NAK. :( >> >> > Hi Krzysztof: > > Allow me to give a brief overview of the current status of eiointc (DT-based): > Loongson-3A series supports eiointc; > Loongson-2K1000 does not support eiointc now; > Loongson-2K0500 supports eiointc, with differences from > Loongson-3, e.g. only up to 128 devices are supported; > Loongson-2K2000 supports eiointc, similar to Loongson-3. > .... > > As can be seen, there is now a bit of confusion in the chip's design of eiointc. > > The design of eiointc is probably refined step by step with the chip. > The same version of eiointc can be used for multiple chips, and the > same chip series may also use different versions of eiointc. Low-end > chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, > depending on the time it's produced. > > So in the Loongson-2K series I have defined the current state as > eiointc-1.0, using the dts property to indicate the maximum number of > devices supported by eiointc that can be used directly in the driver. > > If there are new changes to the design later on, such as the > definition of registers, we can call it eiointc-2.0, which can also > cover more than one chip. Just go with SoC-based compatibles. If your version is not specific enough, then it is not a good way to represent the hardware. Best regards, Krzysztof
Hi, Krzysztof, On Thu, Feb 16, 2023 at 4:10 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 16/02/2023 02:46, Binbin Zhou wrote: > > On Tue, Feb 14, 2023 at 8:43 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 14/02/2023 13:40, Binbin Zhou wrote: > >>> On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski > >>> <krzysztof.kozlowski@linaro.org> wrote: > >>>> > >>>> On 13/02/2023 13:15, Binbin Zhou wrote: > >>>>> Add Loongson Extended I/O Interrupt controller binding with DT schema > >>>>> format using json-schema. > >>>>> > >>>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > >>>>> --- > >>>>> .../loongson,eiointc.yaml | 80 +++++++++++++++++++ > >>>>> 1 file changed, 80 insertions(+) > >>>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>>>> new file mode 100644 > >>>>> index 000000000000..88580297f955 > >>>>> --- /dev/null > >>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>>>> @@ -0,0 +1,80 @@ > >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>>> +%YAML 1.2 > >>>>> +--- > >>>>> +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" > >>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > >>>> > >>>> Drop quotes from bopth. > >>>> > >>>>> + > >>>>> +title: Loongson Extended I/O Interrupt Controller > >>>>> + > >>>>> +maintainers: > >>>>> + - Binbin Zhou <zhoubinbin@loongson.cn> > >>>>> + > >>>>> +description: | > >>>>> + This interrupt controller is found on the Loongson-3 family chips and > >>>>> + Loongson-2K0500 chip and is used to distribute interrupts directly to > >>>>> + individual cores without forwarding them through the HT's interrupt line. > >>>>> + > >>>>> +allOf: > >>>>> + - $ref: /schemas/interrupt-controller.yaml# > >>>>> + > >>>>> +properties: > >>>>> + compatible: > >>>>> + enum: > >>>>> + - loongson,eiointc-1.0 > >>>> > >>>> Why not using SoC based compatible? It is preferred. > >>> > >>> Hi Krzysztof: > >>> > >>> So far, from the datasheet, I know that only the EXIOINTC of the > >>> Loongson-2K0500 is different from the other chips, and that is the > >>> "loongson,eio-num-vecs" below, which is 128, while all the others are > >>> 256. > >>> My original idea was to add this property to make compatible > >>> consistent, and also to make it easier to add new chips if they have > >>> different eio-num-vecs. > >> > >> We talk about different things. SoC based compatibles are preferred over > >> version ones. This was on the lists expressed many times. Please provide > >> a reason why you deviate from general recommendation. Flexibility and > >> genericness of bindings is not a reason - it's the opposite of the > >> argument, thus this will be a: NAK. :( > >> > >> > > Hi Krzysztof: > > > > Allow me to give a brief overview of the current status of eiointc (DT-based): > > Loongson-3A series supports eiointc; > > Loongson-2K1000 does not support eiointc now; > > Loongson-2K0500 supports eiointc, with differences from > > Loongson-3, e.g. only up to 128 devices are supported; > > Loongson-2K2000 supports eiointc, similar to Loongson-3. > > .... > > > > As can be seen, there is now a bit of confusion in the chip's design of eiointc. > > > > The design of eiointc is probably refined step by step with the chip. > > The same version of eiointc can be used for multiple chips, and the > > same chip series may also use different versions of eiointc. Low-end > > chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, > > depending on the time it's produced. > > > > So in the Loongson-2K series I have defined the current state as > > eiointc-1.0, using the dts property to indicate the maximum number of > > devices supported by eiointc that can be used directly in the driver. > > > > If there are new changes to the design later on, such as the > > definition of registers, we can call it eiointc-2.0, which can also > > cover more than one chip. > > Just go with SoC-based compatibles. If your version is not specific > enough, then it is not a good way to represent the hardware. EIOINTC is a bit like the existing LIOINTC which is already use version to represent hardware. Huacai > > Best regards, > Krzysztof >
On 16/02/2023 10:30, Huacai Chen wrote: > Hi, Krzysztof, > > On Thu, Feb 16, 2023 at 4:10 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 16/02/2023 02:46, Binbin Zhou wrote: >>> On Tue, Feb 14, 2023 at 8:43 PM Krzysztof Kozlowski >>> <krzysztof.kozlowski@linaro.org> wrote: >>>> >>>> On 14/02/2023 13:40, Binbin Zhou wrote: >>>>> On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski >>>>> <krzysztof.kozlowski@linaro.org> wrote: >>>>>> >>>>>> On 13/02/2023 13:15, Binbin Zhou wrote: >>>>>>> Add Loongson Extended I/O Interrupt controller binding with DT schema >>>>>>> format using json-schema. >>>>>>> >>>>>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> >>>>>>> --- >>>>>>> .../loongson,eiointc.yaml | 80 +++++++++++++++++++ >>>>>>> 1 file changed, 80 insertions(+) >>>>>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>>>>>> new file mode 100644 >>>>>>> index 000000000000..88580297f955 >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml >>>>>>> @@ -0,0 +1,80 @@ >>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>>>> +%YAML 1.2 >>>>>>> +--- >>>>>>> +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" >>>>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>>>>> >>>>>> Drop quotes from bopth. >>>>>> >>>>>>> + >>>>>>> +title: Loongson Extended I/O Interrupt Controller >>>>>>> + >>>>>>> +maintainers: >>>>>>> + - Binbin Zhou <zhoubinbin@loongson.cn> >>>>>>> + >>>>>>> +description: | >>>>>>> + This interrupt controller is found on the Loongson-3 family chips and >>>>>>> + Loongson-2K0500 chip and is used to distribute interrupts directly to >>>>>>> + individual cores without forwarding them through the HT's interrupt line. >>>>>>> + >>>>>>> +allOf: >>>>>>> + - $ref: /schemas/interrupt-controller.yaml# >>>>>>> + >>>>>>> +properties: >>>>>>> + compatible: >>>>>>> + enum: >>>>>>> + - loongson,eiointc-1.0 >>>>>> >>>>>> Why not using SoC based compatible? It is preferred. >>>>> >>>>> Hi Krzysztof: >>>>> >>>>> So far, from the datasheet, I know that only the EXIOINTC of the >>>>> Loongson-2K0500 is different from the other chips, and that is the >>>>> "loongson,eio-num-vecs" below, which is 128, while all the others are >>>>> 256. >>>>> My original idea was to add this property to make compatible >>>>> consistent, and also to make it easier to add new chips if they have >>>>> different eio-num-vecs. >>>> >>>> We talk about different things. SoC based compatibles are preferred over >>>> version ones. This was on the lists expressed many times. Please provide >>>> a reason why you deviate from general recommendation. Flexibility and >>>> genericness of bindings is not a reason - it's the opposite of the >>>> argument, thus this will be a: NAK. :( >>>> >>>> >>> Hi Krzysztof: >>> >>> Allow me to give a brief overview of the current status of eiointc (DT-based): >>> Loongson-3A series supports eiointc; >>> Loongson-2K1000 does not support eiointc now; >>> Loongson-2K0500 supports eiointc, with differences from >>> Loongson-3, e.g. only up to 128 devices are supported; >>> Loongson-2K2000 supports eiointc, similar to Loongson-3. >>> .... >>> >>> As can be seen, there is now a bit of confusion in the chip's design of eiointc. >>> >>> The design of eiointc is probably refined step by step with the chip. >>> The same version of eiointc can be used for multiple chips, and the >>> same chip series may also use different versions of eiointc. Low-end >>> chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, >>> depending on the time it's produced. >>> >>> So in the Loongson-2K series I have defined the current state as >>> eiointc-1.0, using the dts property to indicate the maximum number of >>> devices supported by eiointc that can be used directly in the driver. >>> >>> If there are new changes to the design later on, such as the >>> definition of registers, we can call it eiointc-2.0, which can also >>> cover more than one chip. >> >> Just go with SoC-based compatibles. If your version is not specific >> enough, then it is not a good way to represent the hardware. > EIOINTC is a bit like the existing LIOINTC which is already use > version to represent hardware. Heh, so why did you go with version in compatible for liointc if it also does not match it correctly? Best regards, Krzysztof
On Thu, Feb 16, 2023 at 4:10 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 16/02/2023 02:46, Binbin Zhou wrote: > > On Tue, Feb 14, 2023 at 8:43 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 14/02/2023 13:40, Binbin Zhou wrote: > >>> On Tue, Feb 14, 2023 at 5:53 PM Krzysztof Kozlowski > >>> <krzysztof.kozlowski@linaro.org> wrote: > >>>> > >>>> On 13/02/2023 13:15, Binbin Zhou wrote: > >>>>> Add Loongson Extended I/O Interrupt controller binding with DT schema > >>>>> format using json-schema. > >>>>> > >>>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > >>>>> --- > >>>>> .../loongson,eiointc.yaml | 80 +++++++++++++++++++ > >>>>> 1 file changed, 80 insertions(+) > >>>>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>>>> new file mode 100644 > >>>>> index 000000000000..88580297f955 > >>>>> --- /dev/null > >>>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > >>>>> @@ -0,0 +1,80 @@ > >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>>> +%YAML 1.2 > >>>>> +--- > >>>>> +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" > >>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > >>>> > >>>> Drop quotes from bopth. > >>>> > >>>>> + > >>>>> +title: Loongson Extended I/O Interrupt Controller > >>>>> + > >>>>> +maintainers: > >>>>> + - Binbin Zhou <zhoubinbin@loongson.cn> > >>>>> + > >>>>> +description: | > >>>>> + This interrupt controller is found on the Loongson-3 family chips and > >>>>> + Loongson-2K0500 chip and is used to distribute interrupts directly to > >>>>> + individual cores without forwarding them through the HT's interrupt line. > >>>>> + > >>>>> +allOf: > >>>>> + - $ref: /schemas/interrupt-controller.yaml# > >>>>> + > >>>>> +properties: > >>>>> + compatible: > >>>>> + enum: > >>>>> + - loongson,eiointc-1.0 > >>>> > >>>> Why not using SoC based compatible? It is preferred. > >>> > >>> Hi Krzysztof: > >>> > >>> So far, from the datasheet, I know that only the EXIOINTC of the > >>> Loongson-2K0500 is different from the other chips, and that is the > >>> "loongson,eio-num-vecs" below, which is 128, while all the others are > >>> 256. > >>> My original idea was to add this property to make compatible > >>> consistent, and also to make it easier to add new chips if they have > >>> different eio-num-vecs. > >> > >> We talk about different things. SoC based compatibles are preferred over > >> version ones. This was on the lists expressed many times. Please provide > >> a reason why you deviate from general recommendation. Flexibility and > >> genericness of bindings is not a reason - it's the opposite of the > >> argument, thus this will be a: NAK. :( > >> > >> > > Hi Krzysztof: > > > > Allow me to give a brief overview of the current status of eiointc (DT-based): > > Loongson-3A series supports eiointc; > > Loongson-2K1000 does not support eiointc now; > > Loongson-2K0500 supports eiointc, with differences from > > Loongson-3, e.g. only up to 128 devices are supported; > > Loongson-2K2000 supports eiointc, similar to Loongson-3. > > .... > > > > As can be seen, there is now a bit of confusion in the chip's design of eiointc. > > > > The design of eiointc is probably refined step by step with the chip. > > The same version of eiointc can be used for multiple chips, and the > > same chip series may also use different versions of eiointc. Low-end > > chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, > > depending on the time it's produced. > > > > So in the Loongson-2K series I have defined the current state as > > eiointc-1.0, using the dts property to indicate the maximum number of > > devices supported by eiointc that can be used directly in the driver. > > > > If there are new changes to the design later on, such as the > > definition of registers, we can call it eiointc-2.0, which can also > > cover more than one chip. > > Just go with SoC-based compatibles. If your version is not specific > enough, then it is not a good way to represent the hardware. > Hi Krzysztof: I have tried to write the following SoC-based compatibles, is it fine? compatible: enum: - loongson,ls3a-eiointc # For MIPS Loongson-3A if necessary. - loongson,ls2k0500-eiointc - loongson,ls2k200-eiointc .... Also remove the 'loongson,eio-num-vecs' property. Thanks. Binbin > Best regards, > Krzysztof >
On 17/02/2023 07:09, Binbin Zhou wrote: >>> Hi Krzysztof: >>> >>> Allow me to give a brief overview of the current status of eiointc (DT-based): >>> Loongson-3A series supports eiointc; >>> Loongson-2K1000 does not support eiointc now; >>> Loongson-2K0500 supports eiointc, with differences from >>> Loongson-3, e.g. only up to 128 devices are supported; >>> Loongson-2K2000 supports eiointc, similar to Loongson-3. >>> .... >>> >>> As can be seen, there is now a bit of confusion in the chip's design of eiointc. >>> >>> The design of eiointc is probably refined step by step with the chip. >>> The same version of eiointc can be used for multiple chips, and the >>> same chip series may also use different versions of eiointc. Low-end >>> chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, >>> depending on the time it's produced. >>> >>> So in the Loongson-2K series I have defined the current state as >>> eiointc-1.0, using the dts property to indicate the maximum number of >>> devices supported by eiointc that can be used directly in the driver. >>> >>> If there are new changes to the design later on, such as the >>> definition of registers, we can call it eiointc-2.0, which can also >>> cover more than one chip. >> >> Just go with SoC-based compatibles. If your version is not specific >> enough, then it is not a good way to represent the hardware. >> > > Hi Krzysztof: > > I have tried to write the following SoC-based compatibles, is it fine? > > compatible: > enum: > - loongson,ls3a-eiointc # For MIPS Loongson-3A if necessary. > - loongson,ls2k0500-eiointc > - loongson,ls2k200-eiointc Looks good, but didn't you state these are compatible between each other? I have impression there is a common set, so maybe one compatible work on other device with reduced number of devices? Best regards, Krzysztof
On Fri, Feb 17, 2023 at 4:40 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/02/2023 07:09, Binbin Zhou wrote: > > >>> Hi Krzysztof: > >>> > >>> Allow me to give a brief overview of the current status of eiointc (DT-based): > >>> Loongson-3A series supports eiointc; > >>> Loongson-2K1000 does not support eiointc now; > >>> Loongson-2K0500 supports eiointc, with differences from > >>> Loongson-3, e.g. only up to 128 devices are supported; > >>> Loongson-2K2000 supports eiointc, similar to Loongson-3. > >>> .... > >>> > >>> As can be seen, there is now a bit of confusion in the chip's design of eiointc. > >>> > >>> The design of eiointc is probably refined step by step with the chip. > >>> The same version of eiointc can be used for multiple chips, and the > >>> same chip series may also use different versions of eiointc. Low-end > >>> chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, > >>> depending on the time it's produced. > >>> > >>> So in the Loongson-2K series I have defined the current state as > >>> eiointc-1.0, using the dts property to indicate the maximum number of > >>> devices supported by eiointc that can be used directly in the driver. > >>> > >>> If there are new changes to the design later on, such as the > >>> definition of registers, we can call it eiointc-2.0, which can also > >>> cover more than one chip. > >> > >> Just go with SoC-based compatibles. If your version is not specific > >> enough, then it is not a good way to represent the hardware. > >> > > > > Hi Krzysztof: > > > > I have tried to write the following SoC-based compatibles, is it fine? > > > > compatible: > > enum: > > - loongson,ls3a-eiointc # For MIPS Loongson-3A if necessary. > > - loongson,ls2k0500-eiointc > > - loongson,ls2k200-eiointc > > Looks good, but didn't you state these are compatible between each > other? I have impression there is a common set, so maybe one compatible > work on other device with reduced number of devices? > So far, the difference between ls2k SOCs is the number of devices supported by eiointc. Do you mean use unified compatible and reuse loongson,eio-num-vecs? Would this be possible, e.g. compatible: const: loongson,ls2k-eiointc loongson,eio-num-vecs: description: The number of devices supported by the extended I/O interrupt vector. $ref: /schemas/types.yaml#/definitions/uint32 minimum: 1 maximum: 256 Thanks. Binbin > Best regards, > Krzysztof >
On 17/02/2023 11:12, Binbin Zhou wrote: > On Fri, Feb 17, 2023 at 4:40 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 17/02/2023 07:09, Binbin Zhou wrote: >> >>>>> Hi Krzysztof: >>>>> >>>>> Allow me to give a brief overview of the current status of eiointc (DT-based): >>>>> Loongson-3A series supports eiointc; >>>>> Loongson-2K1000 does not support eiointc now; >>>>> Loongson-2K0500 supports eiointc, with differences from >>>>> Loongson-3, e.g. only up to 128 devices are supported; >>>>> Loongson-2K2000 supports eiointc, similar to Loongson-3. >>>>> .... >>>>> >>>>> As can be seen, there is now a bit of confusion in the chip's design of eiointc. >>>>> >>>>> The design of eiointc is probably refined step by step with the chip. >>>>> The same version of eiointc can be used for multiple chips, and the >>>>> same chip series may also use different versions of eiointc. Low-end >>>>> chips may use eiointc-2.0, and high-end chips may use eiointc-1.0, >>>>> depending on the time it's produced. >>>>> >>>>> So in the Loongson-2K series I have defined the current state as >>>>> eiointc-1.0, using the dts property to indicate the maximum number of >>>>> devices supported by eiointc that can be used directly in the driver. >>>>> >>>>> If there are new changes to the design later on, such as the >>>>> definition of registers, we can call it eiointc-2.0, which can also >>>>> cover more than one chip. >>>> >>>> Just go with SoC-based compatibles. If your version is not specific >>>> enough, then it is not a good way to represent the hardware. >>>> >>> >>> Hi Krzysztof: >>> >>> I have tried to write the following SoC-based compatibles, is it fine? >>> >>> compatible: >>> enum: >>> - loongson,ls3a-eiointc # For MIPS Loongson-3A if necessary. >>> - loongson,ls2k0500-eiointc >>> - loongson,ls2k200-eiointc >> >> Looks good, but didn't you state these are compatible between each >> other? I have impression there is a common set, so maybe one compatible >> work on other device with reduced number of devices? >> > > So far, the difference between ls2k SOCs is the number of devices > supported by eiointc. > > Do you mean use unified compatible and reuse loongson,eio-num-vecs? > > Would this be possible, e.g. No. I meant that maybe all these three should have been made compatible. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml new file mode 100644 index 000000000000..88580297f955 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/interrupt-controller/loongson,eiointc.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Loongson Extended I/O Interrupt Controller + +maintainers: + - Binbin Zhou <zhoubinbin@loongson.cn> + +description: | + This interrupt controller is found on the Loongson-3 family chips and + Loongson-2K0500 chip and is used to distribute interrupts directly to + individual cores without forwarding them through the HT's interrupt line. + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + +properties: + compatible: + enum: + - loongson,eiointc-1.0 + + reg: + minItems: 1 + maxItems: 3 + + interrupt-controller: true + + interrupts: + description: + Interrupt source of the CPU interrupts. + + interrupt-names: + description: + List of names for the parent interrupts. + items: + - const: int0 + + '#interrupt-cells': + const: 1 + + 'loongson,eio-num-vecs': + description: + The number of devices supported by the extended I/O interrupt vector. + $ref: "/schemas/types.yaml#/definitions/uint32" + minimum: 1 + maximum: 256 + +required: + - compatible + - reg + - interrupts + - interrupt-controller + - '#interrupt-cells' + - 'loongson,eio-num-vecs' + +unevaluatedProperties: false + +examples: + - | + eiointc: interrupt-controller@1fe11600 { + compatible = "loongson,eiointc-1.0"; + reg = <0x1fe11600 0x8 + 0x1fe11700 0x8 + 0x1fe11800 0x8>; + + interrupt-controller; + #interrupt-cells = <1>; + + interrupt-parent = <&cpuintc>; + interrupts = <3>; + interrupt-names = "int0"; + + loongson,eio-num-vecs = <128>; + + }; + +...
Add Loongson Extended I/O Interrupt controller binding with DT schema format using json-schema. Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> --- .../loongson,eiointc.yaml | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml