linux-kernel - Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: fsl,ls-extirq: convert to YAML

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Apr 2022 08:53:18 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Michael Walle <michael@...le.cc>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Li Yang <leoyang.li@....com>, Shawn Guo <shawnguo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>, linuxppc-dev@...ts.ozlabs.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: interrupt-controller: fsl,ls-extirq:
 convert to YAML

On 25/04/2022 23:58, Michael Walle wrote:
>>> +  reg:
>>> +    maxItems: 1
>>> +    description:
>>> +      Specifies the Interrupt Polarity Control Register (INTPCR) in 
>>> the
>>> +      SCFG or the External Interrupt Control Register (IRQCR) in the 
>>> ISC.
>>> +
>>> +  interrupt-map:
> 
> btw.
> 
> minItems: 12
> maxItems: 12
> 
> Isn't working here, is that expected? The validator seem to get the 
> count
> of the elements of one tuple wrong.
> 
> I.e.
> arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.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


Works for me (in different schema)... maybe update your dtschema?

> 
>>> +    description: Specifies the mapping from external interrupts to 
>>> GIC interrupts.
>>> +
>>> +  interrupt-map-mask:
>>> +    items:
>>> +      - const: 0xffffffff
>>
>> This looks highly permissive mask and should be instead defined per
>> variant, for example (quickly looking at DTS):
>> 0x7 for ls1021
>> 0xf for ls1043a and ls1088a
> 
> Just that I understand it correctly, the result of the AND with that
> mask is then looked up in the interrupt-map (the first entry there)?

Yes, the child (first) interrupt specifier. Since address-cells are 0,
this will be bit-AND with [0-5].

>> You might need to correct the DTS. Some confirmation from someone with
>> datasheet would be good.
> 
> According to their datasheets they have the following number of external
> IRQs:
> - ls1021a has 6,
> - ls1043a has 12,
> - ls1046a has 12,
> - ls1088a has 12,
> - ls2080a has 12,
> - lx2160a has 12.
> 
> That is what I need to confirm, right?

Yes.

> 
> Is there a better way than the following snippet:
> 
> properties:
>    interrupt-map-mask: true
> 
> allOf:
>    - if:
>        properties:
>          compatible:
>            contains:
>              enum:
>                - fsl,ls1021a-extirq
>      then:
>        properties:
>          interrupt-map-mask:
>            items:
>              - const: 0x7
>              - const: 0
>    - if:
>        properties:
>          compatible:
>            contains:
>              enum:
>                - fsl,ls1043a-extirq
>                - fsl,ls1046a-extirq
>                - fsl,ls1088a-extirq
>                - fsl,ls2080a-extirq
>                - fsl,lx2160a-extirq
>      then:
>        properties:
>          interrupt-map-mask:
>            items:
>              - const: 0xf
>              - const: 0
> 


Exactly like this, looks good. Thank you.


Best regards,
Krzysztof