linux-kernel - Re: [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: Support split G1 and G2 nodes with vpu-blk-ctrl

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:   Thu, 9 Dec 2021 05:36:04 -0600
From:   Adam Ford <aford173@...il.com>
To:     Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
Cc:     linux-media <linux-media@...r.kernel.org>,
        Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        Chris Healy <cphealy@...il.com>,
        Adam Ford-BE <aford@...conembedded.com>,
        Nicolas Dufresne <nicolas@...fresne.ca>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Lucas Stach <l.stach@...gutronix.de>,
        "open list:HANTRO VPU CODEC DRIVER" 
        <linux-rockchip@...ts.infradead.org>,
        devicetree <devicetree@...r.kernel.org>,
        arm-soc <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:STAGING SUBSYSTEM" <linux-staging@...ts.linux.dev>
Subject: Re: [PATCH 04/10] dt-bindings: media: nxp,imx8mq-vpu: Support split
 G1 and G2 nodes with vpu-blk-ctrl

On Thu, Dec 9, 2021 at 4:26 AM Ezequiel Garcia
<ezequiel@...guardiasur.com.ar> wrote:
>
> Hi,
>
> Thanks for the patch.
>
> On Wed, Dec 08, 2021 at 04:50:23PM -0600, Adam Ford wrote:
> > The G1 and G2 are separate decoder blocks that are enabled by the
> > vpu-blk-ctrl power-domain controller, which now has a proper driver.
> > Update the bindings to support separate nodes for the G1 and G2
> > decoders using the proper driver or the older unified node with
> > the legacy controls.
> >
> > To be compatible with older DT the driver, mark certain items as
> > deprecated and retain the backwards compatible example.
> >
> > Signed-off-by: Adam Ford <aford173@...il.com>
> > ---
> >  .../bindings/media/nxp,imx8mq-vpu.yaml        | 83 ++++++++++++++-----
> >  1 file changed, 64 insertions(+), 19 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > index 762be3f96ce9..eeb7bd6281f9 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > @@ -15,29 +15,39 @@ description:
> >
> >  properties:
> >    compatible:
> > -    const: nxp,imx8mq-vpu
> > +    oneOf:
> > +      - const: nxp,imx8mq-vpu
> > +        deprecated: true
> > +      - const: nxp,imx8mq-vpu-g1
> > +      - const: nxp,imx8mq-vpu-g2
> >
> >    reg:
> > +    minItems: 1
> >      maxItems: 3
>
> Is it really useful to keep the deprecated binding nxp,imx8mq-vpu
> as something supported by the binding file?

Since I was told that the driver needed to be backwards compatible, i
wanted to make sure that any attempts to build the old device tree
would not fail
>
> In other words, can we drop the deprecated binding from this file,
> while keeping the support in the driver for legacy device-trees?

I was trying to represent both the old driver binding and the new one
at the same time.  I thought that's what I was told to do.
>
> [..]
> > +
> > +  # VPU G1 with vpu-blk-ctrl
> > +  - |
> > +    #include <dt-bindings/clock/imx8mq-clock.h>
> > +    #include <dt-bindings/power/imx8mq-power.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    vpu_g1: video-codec@...00000 {
> > +        compatible = "nxp,imx8mq-vpu-g1";
> > +        reg = <0x38300000 0x10000>;
> > +        reg-names "g1";
> > +        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > +        interrupt-names = "g1";
> > +        clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>;
> > +        clock-names = "g1";
>
> reg-names, interrupt-names and clock-names should be removed
> given for this device there's only one of each.
>

I attempted to remove the reg-names, but it failed to enumerate for me
when I did that.

> This will make the binding actually quite easier, but it also
> means you need to make some changes to struct hantro_variant imx8mq_vpu_g1_variant
> to make it work properly.
>
> See Rob's feedback on the SAMA5 VPU binding:
>
> https://yhbt.net/lore/all/20210324151715.GA3070006@robh.at.kernel.org/
>
> Also, take a look at drivers/staging/media/hantro/sama5d4_vdec_hw.c
> for reference.

I can try again using this as an example.

>
> > +        power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>;
> > +    };
> > +
> > +  # VPU G2 with vpu-blk-ctrl
> > +  - |
> > +    #include <dt-bindings/clock/imx8mq-clock.h>
> > +    #include <dt-bindings/power/imx8mq-power.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    vpu_g2: video-codec@...10000 {
> > +        compatible = "nxp,imx8mq-vpu-g2";
> > +        reg = <0x38310000 0x10000>;
> > +        reg-names "g2";
>
> And same here.
>
> Thanks!
> Ezequiel