[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PSAPR06MB4949CF57BD69B2F7C1D141CD89052@PSAPR06MB4949.apcprd06.prod.outlook.com>
Date: Wed, 18 Dec 2024 03:04:32 +0000
From: Kevin Chen <kevin_chen@...eedtech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "robh@...nel.org"
<robh@...nel.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>, "joel@....id.au"
<joel@....id.au>, "andrew@...econstruct.com.au"
<andrew@...econstruct.com.au>, "tglx@...utronix.de" <tglx@...utronix.de>,
"catalin.marinas@....com" <catalin.marinas@....com>, "will@...nel.org"
<will@...nel.org>, "arnd@...db.de" <arnd@...db.de>, "olof@...om.net"
<olof@...om.net>, "quic_bjorande@...cinc.com" <quic_bjorande@...cinc.com>,
"geert+renesas@...der.be" <geert+renesas@...der.be>,
"dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
"konradybcio@...nel.org" <konradybcio@...nel.org>,
"neil.armstrong@...aro.org" <neil.armstrong@...aro.org>,
"johan+linaro@...nel.org" <johan+linaro@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
<linux-aspeed@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "soc@...ts.linux.dev" <soc@...ts.linux.dev>
Subject: RE: [PATCH v3 1/6] dt-bindings: interrupt-controller: Refine
size/interrupt-cell usage.
Hi Krzk,
> > 1. Because size-cells is no need to use 2, modify to 1 for use.
>
> ???
So, is it OK that I change the size-cells back to 2 include the aspeed,ast2700-intc.yaml examples and aspeed-g7.dtsi?
>
> > 2. Add minItems to 1 for interrupts for intc1.
>
> ???
For variable interrupt numbers, I need to fix the below warnings by minItems.
DTC [C] arch/arm64/boot/dts/aspeed/ast2700-evb.dtb
/home/kevin/linux-mainline/arch/arm64/boot/dts/aspeed/ast2700-evb.dtb: interrupt-controller@100: interrupts-extended: [[3, 0, 3844]] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
/home/kevin/linux-mainline/arch/arm64/boot/dts/aspeed/ast2700-evb.dtb: interrupt-controller@110: interrupts-extended: [[3, 1, 3844]] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
/home/kevin/linux-mainline/arch/arm64/boot/dts/aspeed/ast2700-evb.dtb: interrupt-controller@120: interrupts-extended: [[3, 2, 3844]] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
/home/kevin/linux-mainline/arch/arm64/boot/dts/aspeed/ast2700-evb.dtb: interrupt-controller@130: interrupts-extended: [[3, 3, 3844]] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
/home/kevin/linux-mainline/arch/arm64/boot/dts/aspeed/ast2700-evb.dtb: interrupt-controller@140: interrupts-extended: [[3, 4, 3844]] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
/home/kevin/linux-mainline/arch/arm64/boot/dts/aspeed/ast2700-evb.dtb: interrupt-controller@150: interrupts-extended: [[3, 5, 3844]] is too short
from schema $id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml#
>
> > 3. Add 1 interrupt of intc1 example into yaml file.
>
> > 4. Add intc1 sub-module of uart12 as example using the intc0 and intc1.
>
> What is all this?
>
> BTW, there was no such patch in previous version and your changelog is silent
> about it.
Agree, I will restore the previous version.
>
> Subject: drop all full stops. Subject never ends with full stop.
>
> > ---
> > .../aspeed,ast2700-intc.yaml | 60
> +++++++++++++++----
> > 1 file changed, 47 insertions(+), 13 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml
> > b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml index 55636d06a674..eadfbc45326b 100644
> > ---
> > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270
> > 0-intc.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,as
> > +++ t2700-intc.yaml
> > @@ -31,6 +31,7 @@ properties:
> > type as defined in interrupt.txt in this directory.
> >
> > interrupts:
> > + minItems: 1
>
> Nope, not explained, not constrained. Your schema is supposed to be
> constrained.
>
>
> > maxItems: 6
> > description: |
> > Depend to which INTC0 or INTC1 used.
> > @@ -68,19 +69,52 @@ examples:
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> >
> > bus {
> > + #address-cells = <2>;
> > + #size-cells = <1>;
> > +
> > + intc0: interrupt-controller@...00000 {
> > + compatible = "simple-mfd";
> > + reg = <0 0x12100000 0x4000>;
> > + ranges = <0x0 0x0 0x0 0x12100000 0x4000>;
> > #address-cells = <2>;
> > - #size-cells = <2>;
> > -
> > - interrupt-controller@...01b00 {
> > - compatible = "aspeed,ast2700-intc-ic";
> > - reg = <0 0x12101b00 0 0x10>;
> > - #interrupt-cells = <2>;
> > - interrupt-controller;
> > - interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> > - <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
>
>
> I don't understand what is all this.
>
> > + #size-cells = <1>;
> > +
> > + intc0_11: interrupt-controller@...0 {
> > + compatible = "aspeed,ast2700-intc-ic";
> > + reg = <0 0x12101b00 0x10>;
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + interrupts = <GIC_SPI 192 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 193 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 194 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 195 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 196 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
> > + <GIC_SPI 197 (GIC_CPU_MASK_SIMPLE(4) |
> > + IRQ_TYPE_LEVEL_HIGH)>;
> > };
> > + };
> > +
> > + intc1: interrupt-controller@...18000 {
> > + compatible = "simple-mfd";
> > + reg = <0 0x14c18000 0x400>;
> > + ranges = <0x0 0x0 0x0 0x14c18000 0x400>;
> > + #address-cells = <2>;
> > + #size-cells = <1>;
> > +
> > + intc1_4: interrupt-controller@140 {
> > + compatible = "aspeed,ast2700-intc-ic";
> > + reg = <0x0 0x140 0x10>;
> > + #interrupt-cells = <2>;
> > + interrupt-controller;
> > + interrupts-extended = <&intc0_11 4
> (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> > + };
> > + };
> > +
> > + uart12: serial@...33b00 {
> > + compatible = "ns16550a";
> > + reg = <0x0 0x14c33b00 0x100>;
> > + interrupts-extended = <&intc1_4 18 (GIC_CPU_MASK_SIMPLE(4)
> | IRQ_TYPE_LEVEL_HIGH)>;
> > + reg-shift = <2>;
> > + reg-io-width = <4>;
> > + no-loopback-test;
> > + };
>
> And above is not related at all. Don't add entirely unrelated changes. Drop.
>
>
> Best regards,
> Krzysztof
Best Regards,
Kevin. Chen
Powered by blists - more mailing lists