linux-kernel - Re: [PATCH v1 17/18] dt-bindings: media: nxp,imx8mq-vpu: Update bindings

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]
Message-ID: <0a12de91-e6c9-642a-9c43-207041996c22@collabora.com>
Date:   Thu, 18 Feb 2021 15:48:26 +0100
From:   Benjamin Gaignard <benjamin.gaignard@...labora.com>
To:     Rob Herring <robh@...nel.org>
Cc:     ezequiel@...labora.com, p.zabel@...gutronix.de, mchehab@...nel.org,
        shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
        festevam@...il.com, linux-imx@....com, gregkh@...uxfoundation.org,
        mripard@...nel.org, paul.kocialkowski@...tlin.com, wens@...e.org,
        jernej.skrabec@...l.net, krzk@...nel.org, shengjiu.wang@....com,
        adrian.ratiu@...labora.com, aisheng.dong@....com, peng.fan@....com,
        Anson.Huang@....com, hverkuil-cisco@...all.nl,
        linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
        kernel@...labora.com
Subject: Re: [PATCH v1 17/18] dt-bindings: media: nxp,imx8mq-vpu: Update
 bindings


Le 17/02/2021 à 23:48, Rob Herring a écrit :
> On Wed, Feb 17, 2021 at 09:03:05AM +0100, Benjamin Gaignard wrote:
>> The introduction on HEVC decoder lead to update the bindings
>> to support it.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel@...labora.com>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@...labora.com>
>> ---
>>   .../bindings/media/nxp,imx8mq-vpu.yaml        | 54 ++++++++++++-------
>>   1 file changed, 36 insertions(+), 18 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>> index 762be3f96ce9..468435c70eef 100644
>> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>> @@ -15,24 +15,25 @@ description:
>>   
>>   properties:
>>     compatible:
>> -    const: nxp,imx8mq-vpu
>> +    enum:
>> +      - nxp,imx8mq-vpu
>> +      - nxp,imx8mq-vpu-g2
>>   
>>     reg:
>> -    maxItems: 3
>> +    maxItems: 1
>>   
>>     reg-names:
>> -    items:
>> -      - const: g1
>> -      - const: g2
>> -      - const: ctrl
>> +    enum:
>> +      - g1
>> +      - g2
> This isn't a compatible change. You need to state why that's okay if it
> is okay.

I will change the commit message to this in the next version:
The current bindings seem to make the assumption that the
two VPUs hardware blocks (G1 and G2) are only one set of
registers.
After implementing the VPU reset driver and G2 decoder driver
it shows that all the VPUs are independent and don't need to
know about the registers of the other blocks.
Remove from the bindings the need to set all blocks register
but keep reg-names property because removing it from the driver
may affect other variants.

Benjamin

>
>>   
>>     interrupts:
>> -    maxItems: 2
>> +    maxItems: 1
>>   
>>     interrupt-names:
>> -    items:
>> -      - const: g1
>> -      - const: g2
>> +    enum:
>> +      - g1
>> +      - g2
>>   
>>     clocks:
>>       maxItems: 3
>> @@ -46,6 +47,9 @@ properties:
>>     power-domains:
>>       maxItems: 1
>>   
>> +  resets:
>> +    maxItems: 1
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -54,6 +58,7 @@ required:
>>     - interrupt-names
>>     - clocks
>>     - clock-names
>> +  - resets
>>   
>>   additionalProperties: false
>>   
>> @@ -61,19 +66,32 @@ examples:
>>     - |
>>           #include <dt-bindings/clock/imx8mq-clock.h>
>>           #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +        #include <dt-bindings/reset/imx8mq-vpu-reset.h>
>>   
>> -        vpu: video-codec@...00000 {
>> +        vpu_g1: video-codec@...00000 {
>>                   compatible = "nxp,imx8mq-vpu";
>> -                reg = <0x38300000 0x10000>,
>> -                      <0x38310000 0x10000>,
>> -                      <0x38320000 0x10000>;
>> -                reg-names = "g1", "g2", "ctrl";
>> -                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
>> -                             <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
>> -                interrupt-names = "g1", "g2";
>> +                reg = <0x38300000 0x10000>;
>> +                reg-names = "g1";
>> +                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
>> +                interrupt-names = "g1";
>> +                clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
>> +                         <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
>> +                         <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
>> +                clock-names = "g1", "g2", "bus";
>> +                power-domains = <&pgc_vpu>;
>> +                resets = <&vpu_reset IMX8MQ_RESET_VPU_RESET_G1>;
>> +        };
>> +
>> +        vpu_g2: video-codec@...10000 {
>> +                compatible = "nxp,imx8mq-vpu-g2";
>> +                reg = <0x38310000 0x10000>;
>> +                reg-names = "g2";
>> +                interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
>> +                interrupt-names = "g2";
>>                   clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
>>                            <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
>>                            <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
>>                   clock-names = "g1", "g2", "bus";
>>                   power-domains = <&pgc_vpu>;
>> +                resets = <&vpu_reset IMX8MQ_RESET_VPU_RESET_G2>;
>>           };
>> -- 
>> 2.25.1
>>