Message-ID: <ZwALfcJYTOuXqPuP@lizhi-Precision-Tower-5810>
Date: Fri, 4 Oct 2024 11:36:29 -0400
From: Frank Li <Frank.li@....com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	"open list:IRQCHIP DRIVERS" <linux-kernel@...r.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
	imx@...ts.linux.dev
Subject: Re: [PATCH v2 1/1] dt-bindings: interrupt-controller: fsl,ls-extirq:
 workaround wrong interrupt-map number

On Fri, Oct 04, 2024 at 08:43:23AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Oct 03, 2024 at 05:43:15PM -0400, Frank Li wrote:
> > The driver(drivers/irqchip/irq-ls-extirq.c) have not use standard DT
> > function to parser interrupt-map. So it doesn't consider '#address-size'
> > in parent interrupt controller, such as GIC.
> >
> > When dt-binding verify interrupt-map, item data matrix is spitted at
> > incorrect position. So cause below warning:
> >
> > arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: interrupt-controller@14:
> > interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], ...
> > is too short
> >
> > Reduce minItems and maxItems to workaround this warning for
> > 'fsl,ls1088a-extirq', 'fsl,ls2080a-extirq' and fsl,lx2160a-extirq.
> > Other keep the same restriction.
> >
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> > Change from v1 to v2
> > - remove duplicate function in commit message
> > - only reduce miniItems for after 1088a chips
> > - maxItems change to 9. Otherwise report too long.
> > ---
> >  .../interrupt-controller/fsl,ls-extirq.yaml   | 27 +++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml
> > index 199b34fdbefc4..1bfced6ed620c 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/fsl,ls-extirq.yaml
> > @@ -82,14 +82,37 @@ allOf:
> >              enum:
> >                - fsl,ls1043a-extirq
> >                - fsl,ls1046a-extirq
> > +    then:
> > +      properties:
> > +        interrupt-map:
> > +          minItems: 12
> > +          maxItems: 12
> > +        interrupt-map-mask:
> > +          items:
> > +            - const: 0xf
> > +            - const: 0
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> >                - fsl,ls1088a-extirq
> >                - fsl,ls2080a-extirq
> >                - fsl,lx2160a-extirq
> > +# The driver(drivers/irqchip/irq-ls-extirq.c) have not use standard DT
> > +# function function to parser interrupt-map. So it doesn't consider
>
> Same issue as last time, double function.
>
> Please run scripts/checkpatch.pl and fix reported warnings. Then please
> run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings.
> Some warnings can be ignored, especially from --strict run, but the code
> here looks like it needs a fix. Feel free to get in touch if the warning
> is not clear.

Thanks, I forget add --strict this time.

>
>
> > +# '#address-size' in parent interrupt controller, such as GIC.
> > +#
> > +# When dt-binding verify interrupt-map, item data matrix is spitted at
> > +# incorrect position. Reduce minItems and maxItems to workaround this
> > +# problem.
> > +
> >      then:
> >        properties:
> >          interrupt-map:
> > -          minItems: 12
> > -          maxItems: 12
> > +          minItems: 8
> > +          maxItems: 9
>
> Are you sure it works? I see 12 items in fsl-ls1088a.dtsi.

interrupt-map =
   <0 0 &gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
   <1 0 &gic GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
   ...
   <11 0 &gic GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;

Total 12*6 = 72 data.

Normal each row should be 6 data.

but when GIC have #address-size, <2>,  dt-schemal split at at (6+2=8).

"interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0]"

So 72/8 = 9, I just realize it can divide to whole number.  so minItems
can be set 9 also.

>
> What's more, I do not see your errors/warnings at all.

I using below command

pip3 install git+https://github.com/devicetree-org/dt-schema.git@main
...
Resolved https://github.com/devicetree-org/dt-schema.git to commit 0934678abc36614cd3c5165ca49ba78b881ee2fa

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 CHECK_DTBS=y  fsl-ls1088a-qds.dtb

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 CHECK_DTBS=y  fsl-ls1088a-qds.dtb
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC [C] arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb
arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: syscon@...0000: interrupt-controller@14:interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too short
	from schema $id: http://devicetree.org/schemas/soc/fsl/fsl,layerscape-scfg.yaml#
arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: interrupt-controller@14: interrupt-map: [[0, 0, 1, 0, 0, 4, 1, 0], [1, 0, 1, 4, 2, 0, 1, 0], [2, 4, 3, 0, 1, 0, 3, 4], [4, 0, 1, 0, 4, 4, 5, 0], [1, 0, 5, 4, 6, 0, 1, 0], [6, 4, 7, 0, 1, 0, 7, 4], [8, 0, 1, 0, 8, 4, 9, 0], [1, 0, 9, 4, 10, 0, 1, 0], [10, 4, 11, 0, 1, 0, 11, 4]] is too short
	from schema $id: http://devicetree.org/schemas/interrupt-controller/fsl,ls-extirq.yaml#
arch/arm64/boot/dts/freescale/fsl-ls1088a-qds.dtb: fsl-mc@...000000: msi-parent:0: [16, 0] is too long
	from schema $id: http://devicetree.org/schemas/misc/fsl,qoriq-mc.yaml#

Frank

>
> Best regards,
> Krzysztof
>

Powered by blists - more mailing lists