| Message ID | 3b9c4f05eaf14bc3b16aebec3ff84c8a2d52c4a5.1681887790.git.zhoubinbin@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | irqchip: loongson-eiointc: Add DT init support | expand |
On 19/04/2023 09:17, 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 | 74 +++++++++++++++++++ > 1 file changed, 74 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..4ab4efb061e1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > @@ -0,0 +1,74 @@ > +# 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-2K series chips 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,ls2k0500-eiointc > + - loongson,ls2k2000-eiointc > + > + reg: > + items: > + - description: Interrupt enable registers > + - description: Interrupt status registers > + - description: Interrupt clear registers > + - description: Interrupt routing configuration registers > + > + reg-names: > + items: > + - const: enable > + - const: status > + - const: clear > + - const: route > + > + interrupts: > + maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + - interrupts > + - interrupt-controller > + - '#interrupt-cells' > + > +unevaluatedProperties: false > + > +examples: > + - | > + eiointc: interrupt-controller@1fe11600 { > + compatible = "loongson,ls2k0500-eiointc"; > + reg = <0x1fe11600 0x10>, > + <0x1fe11700 0x10>, > + <0x1fe11800 0x10>, > + <0x1fe114c0 0x4>; Binding is OK, but are you sure you want to split the address space like this? It looks like two address spaces (enable+clear+status should be one). Are you sure this is correct? Best regards, Krzysztof
On Thu, Apr 20, 2023 at 4:09 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 19/04/2023 09:17, 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 | 74 +++++++++++++++++++ > > 1 file changed, 74 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..4ab4efb061e1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > @@ -0,0 +1,74 @@ > > +# 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-2K series chips 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,ls2k0500-eiointc > > + - loongson,ls2k2000-eiointc > > + > > + reg: > > + items: > > + - description: Interrupt enable registers > > + - description: Interrupt status registers > > + - description: Interrupt clear registers > > + - description: Interrupt routing configuration registers > > + > > + reg-names: > > + items: > > + - const: enable > > + - const: status > > + - const: clear > > + - const: route > > + > > + interrupts: > > + maxItems: 1 > > + > > + interrupt-controller: true > > + > > + '#interrupt-cells': > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - interrupt-controller > > + - '#interrupt-cells' > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + eiointc: interrupt-controller@1fe11600 { > > + compatible = "loongson,ls2k0500-eiointc"; > > + reg = <0x1fe11600 0x10>, > > + <0x1fe11700 0x10>, > > + <0x1fe11800 0x10>, > > + <0x1fe114c0 0x4>; > > Binding is OK, but are you sure you want to split the address space like > this? It looks like two address spaces (enable+clear+status should be > one). Are you sure this is correct? > Hi Krzysztof: These registers are all in the range of chip configuration registers, in the case of LS2K0500, which has a base address of 0x1fe10000. However, the individual register addresses are not contiguous with each other, and most are distributed across modules, so I feel that they should be listed in detail as they are used. Thanks. Binbin > Best regards, > Krzysztof > >
On 20/04/2023 15:00, Binbin Zhou wrote: >>> +examples: >>> + - | >>> + eiointc: interrupt-controller@1fe11600 { >>> + compatible = "loongson,ls2k0500-eiointc"; >>> + reg = <0x1fe11600 0x10>, >>> + <0x1fe11700 0x10>, >>> + <0x1fe11800 0x10>, >>> + <0x1fe114c0 0x4>; >> >> Binding is OK, but are you sure you want to split the address space like >> this? It looks like two address spaces (enable+clear+status should be >> one). Are you sure this is correct? >> > Hi Krzysztof: > > These registers are all in the range of chip configuration registers, > in the case of LS2K0500, which has a base address of 0x1fe10000. > However, the individual register addresses are not contiguous with > each other, and most are distributed across modules, so I feel that > they should be listed in detail as they are used. Do you want to say that: Between 0x1fe11600 and 0x1fe11700 there are EIOINTC registers and other (independent) module registers? Best regards, Krzysztof
On Thu, Apr 20, 2023 at 09:00:42PM +0800, Binbin Zhou wrote: > On Thu, Apr 20, 2023 at 4:09 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 19/04/2023 09:17, 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 | 74 +++++++++++++++++++ > > > 1 file changed, 74 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..4ab4efb061e1 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml > > > @@ -0,0 +1,74 @@ > > > +# 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-2K series chips 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,ls2k0500-eiointc > > > + - loongson,ls2k2000-eiointc > > > + > > > + reg: > > > + items: > > > + - description: Interrupt enable registers > > > + - description: Interrupt status registers > > > + - description: Interrupt clear registers > > > + - description: Interrupt routing configuration registers > > > + > > > + reg-names: > > > + items: > > > + - const: enable > > > + - const: status > > > + - const: clear > > > + - const: route > > > + > > > + interrupts: > > > + maxItems: 1 > > > + > > > + interrupt-controller: true > > > + > > > + '#interrupt-cells': > > > + const: 1 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - interrupts > > > + - interrupt-controller > > > + - '#interrupt-cells' > > > + > > > +unevaluatedProperties: false > > > + > > > +examples: > > > + - | > > > + eiointc: interrupt-controller@1fe11600 { > > > + compatible = "loongson,ls2k0500-eiointc"; > > > + reg = <0x1fe11600 0x10>, > > > + <0x1fe11700 0x10>, > > > + <0x1fe11800 0x10>, > > > + <0x1fe114c0 0x4>; > > > > Binding is OK, but are you sure you want to split the address space like > > this? It looks like two address spaces (enable+clear+status should be > > one). Are you sure this is correct? > > > Hi Krzysztof: > > These registers are all in the range of chip configuration registers, > in the case of LS2K0500, which has a base address of 0x1fe10000. Where is the schema for this? Either it should be the interrupt-controller itself or this binding should be a child node of it. Which way really depends on whether the eiointc is reused on multiple chips with different register offsets or parent block. Can't really give better advice without a complete picture of the 'chip configuration registers'. So please provide that. Rob
On Thu, Apr 20, 2023 at 11:52 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 20/04/2023 15:00, Binbin Zhou wrote: > >>> +examples: > >>> + - | > >>> + eiointc: interrupt-controller@1fe11600 { > >>> + compatible = "loongson,ls2k0500-eiointc"; > >>> + reg = <0x1fe11600 0x10>, > >>> + <0x1fe11700 0x10>, > >>> + <0x1fe11800 0x10>, > >>> + <0x1fe114c0 0x4>; > >> > >> Binding is OK, but are you sure you want to split the address space like > >> this? It looks like two address spaces (enable+clear+status should be > >> one). Are you sure this is correct? > >> > > Hi Krzysztof: > > > > These registers are all in the range of chip configuration registers, > > in the case of LS2K0500, which has a base address of 0x1fe10000. > > However, the individual register addresses are not contiguous with > > each other, and most are distributed across modules, so I feel that > > they should be listed in detail as they are used. > > Do you want to say that: > Between 0x1fe11600 and 0x1fe11700 there are EIOINTC registers and other > (independent) module registers? No, this section is all EIO-related configuration, but there will be undefined space here. Throughout the chip configuration space, there are some relatively common areas, such as the definition of 0x1fe1_14c0. Because our chip supports two interrupt modes, node legacy I/O interrupt and extended I/O interrupt, both modes require interrupt routing registers. Their registers are then defined together: the legacy interrupt I/O start address is 0x1fe1_1400, while the extended I/O interrupt start address is 0x1fe1_14c0. Then I have carefully compared the chip configuration space in LS2K0500 and LS2K2000 and can see that: 1. The chip configuration space base addresses are different, but they both have a size of 64KB; 2. The offset addresses of the EIO related registers are the same, for example the offset of the enable register is 0x1600. Wouldn't it be better to declare the entire configuration space (64KB) directly in the dts and use the offsets to access the corresponding registers? Example: reg = <0x1fe10000 0x10000>. Thanks. Binbin > > Best regards, > Krzysztof
On 23/04/2023 10:30, Binbin Zhou wrote: > On Thu, Apr 20, 2023 at 11:52 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 20/04/2023 15:00, Binbin Zhou wrote: >>>>> +examples: >>>>> + - | >>>>> + eiointc: interrupt-controller@1fe11600 { >>>>> + compatible = "loongson,ls2k0500-eiointc"; >>>>> + reg = <0x1fe11600 0x10>, >>>>> + <0x1fe11700 0x10>, >>>>> + <0x1fe11800 0x10>, >>>>> + <0x1fe114c0 0x4>; >>>> >>>> Binding is OK, but are you sure you want to split the address space like >>>> this? It looks like two address spaces (enable+clear+status should be >>>> one). Are you sure this is correct? >>>> >>> Hi Krzysztof: >>> >>> These registers are all in the range of chip configuration registers, >>> in the case of LS2K0500, which has a base address of 0x1fe10000. >>> However, the individual register addresses are not contiguous with >>> each other, and most are distributed across modules, so I feel that >>> they should be listed in detail as they are used. >> >> Do you want to say that: >> Between 0x1fe11600 and 0x1fe11700 there are EIOINTC registers and other >> (independent) module registers? > > No, this section is all EIO-related configuration, but there will be > undefined space here. > > Throughout the chip configuration space, there are some relatively > common areas, such as the definition of 0x1fe1_14c0. > Because our chip supports two interrupt modes, node legacy I/O > interrupt and extended I/O interrupt, both modes require interrupt > routing registers. > Their registers are then defined together: the legacy interrupt I/O > start address is 0x1fe1_1400, while the extended I/O interrupt start > address is 0x1fe1_14c0. > > Then I have carefully compared the chip configuration space in > LS2K0500 and LS2K2000 and can see that: > > 1. The chip configuration space base addresses are different, but they > both have a size of 64KB; > 2. The offset addresses of the EIO related registers are the same, for > example the offset of the enable register is 0x1600. > > Wouldn't it be better to declare the entire configuration space (64KB) > directly in the dts and use the offsets to access the corresponding > registers? > > Example: > reg = <0x1fe10000 0x10000>. Yes, that's what usually we do. 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..4ab4efb061e1 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml @@ -0,0 +1,74 @@ +# 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-2K series chips 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,ls2k0500-eiointc + - loongson,ls2k2000-eiointc + + reg: + items: + - description: Interrupt enable registers + - description: Interrupt status registers + - description: Interrupt clear registers + - description: Interrupt routing configuration registers + + reg-names: + items: + - const: enable + - const: status + - const: clear + - const: route + + interrupts: + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + const: 1 + +required: + - compatible + - reg + - interrupts + - interrupt-controller + - '#interrupt-cells' + +unevaluatedProperties: false + +examples: + - | + eiointc: interrupt-controller@1fe11600 { + compatible = "loongson,ls2k0500-eiointc"; + reg = <0x1fe11600 0x10>, + <0x1fe11700 0x10>, + <0x1fe11800 0x10>, + <0x1fe114c0 0x4>; + reg-names = "enable", "status", "clear", "route"; + + interrupt-controller; + #interrupt-cells = <1>; + + interrupt-parent = <&cpuintc>; + interrupts = <3>; + }; + +...
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 | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/loongson,eiointc.yaml