| Message ID | 20250210090725.4580-4-nas.chung@chipsnmedia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for Wave6 video codec driver | expand |
On 10/02/2025 10:07, Nas Chung wrote: > Add documents for the Wave6 video codec on NXP i.MX SoCs. > > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> > --- > .../bindings/media/nxp,wave633c.yaml | 202 ++++++++++++++++++ > MAINTAINERS | 8 + > 2 files changed, 210 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/nxp,wave633c.yaml > > diff --git a/Documentation/devicetree/bindings/media/nxp,wave633c.yaml b/Documentation/devicetree/bindings/media/nxp,wave633c.yaml > new file mode 100644 > index 000000000000..99c3008314c5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/nxp,wave633c.yaml Filename matching compatible. > @@ -0,0 +1,202 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/nxp,wave633c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Chips&Media Wave6 Series multi-standard codec IP on NXP i.MX SoCs. > + > +maintainers: > + - Nas Chung <nas.chung@chipsnmedia.com> > + - Jackson Lee <jackson.lee@chipsnmedia.com> > + > +description: > + The Chips&Media Wave6 codec IP is a multi-standard video encoder/decoder. > + On NXP i.MX SoCs, Wave6 codec IP functionality is split between the VPU control device > + (vpu-ctrl) and the VPU device (vpu). The VPU control device manages shared resources > + such as firmware access and power domains, while the VPU device provides encoding > + and decoding capabilities. The VPU devie cannot operate independently Typo > + without the VPU control device. > + Please wrap code according to the preferred limit expressed in Kernel coding style (checkpatch is not a coding style description, but only a tool). Bindings use strict rule here. > +properties: > + compatible: > + items: > + - enum: > + - nxp,imx95-wave633c-ctrl > + - nxp,imx95-wave633c I don't understand why you duplicated compatibles. You split this for driver? That's a no. There are no two hardwares. These compatibles are anyway weird - why imx95 is in chipmedia product? Is this part of a SoC? > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: VPU clock > + - description: VPU associated block clock > + > + clock-names: > + items: > + - const: vpu > + - const: vpublk_wave > + > + power-domains: > + minItems: 1 > + items: > + - description: Main VPU power domain > + - description: Performance power domain > + > + power-domain-names: > + items: > + - const: vpumix > + - const: vpuperf > + > + cnm,ctrl: What is this prefix about? Is this nxp or something else? > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of the VPU control device node. Required for VPU operation. Explain - required for what. Operation is too generic. If this is phandle to second device, then it's proof your split is really wrong. > + > + boot: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of the boot memory region node for the VPU control device. No clue what is this... if memory region then use existing bindings. Anyway, wrap your code correctly. > + > + sram: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of the SRAM memory region node for the VPU control device. > + > + '#cooling-cells': > + const: 2 > + > + support-follower: > + type: boolean > + description: Indicates whether the VPU domain power always on. You cannot add new common properties in random way. Missing vendor prefix but more important: does not look at all as hardware property but OS policy. > + > +patternProperties: > + "^vpu-ctrl@[0-9a-f]+$": > + type: object > + properties: > + compatible: > + items: > + - enum: > + - nxp,imx95-wave633c-ctrl Really, what? How nxp,imx95-wave633c-ctrl node can have a child with nxp,imx95-wave633c-ctrl compatible? NAK. > + reg: true > + clocks: true > + clock-names: true > + power-domains: > + items: > + - description: Main VPU power domain > + - description: Performance power domain > + power-domain-names: > + items: > + - const: vpumix > + - const: vpuperf > + sram: true > + boot: true > + '#cooling-cells': true > + support-follower: true > + required: > + - compatible > + - reg > + - clocks > + - clock-names > + - power-domains > + - power-domain-names > + - sram > + - boot > + > + additionalProperties: false > + > + "^vpu@[0-9a-f]+$": > + type: object > + properties: > + compatible: > + items: > + - enum: > + - nxp,imx95-wave633c > + reg: true > + interrupts: true > + clocks: true > + clock-names: true > + power-domains: > + maxItems: 1 > + description: Main VPU power domain > + cnm,ctrl: true > + required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - power-domains > + - cnm,ctrl All this is just incorrect. > + > + additionalProperties: false > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/clock/nxp,imx95-clock.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + vpuctrl: vpu-ctrl@4c4c0000 { > + compatible = "nxp,imx95-wave633c-ctrl"; > + reg = <0x0 0x4c4c0000 0x0 0x10000>; > + clocks = <&scmi_clk 115>, > + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > + clock-names = "vpu", "vpublk_wave"; > + power-domains = <&scmi_devpd 21>, <&scmi_perf 10>; > + power-domain-names = "vpumix", "vpuperf"; > + #cooling-cells = <2>; > + boot = <&vpu_boot>; > + sram = <&sram1>; > + }; > + > + vpu0: vpu@4c480000 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "nxp,imx95-wave633c"; > + reg = <0x0 0x4c480000 0x0 0x10000>; > + interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&scmi_clk 115>, > + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > + clock-names = "vpu", "vpublk_wave"; > + power-domains = <&scmi_devpd 21>; > + cnm,ctrl = <&vpuctrl>; > + }; > + > + vpu1: vpu@4c490000 { > + compatible = "nxp,imx95-wave633c"; Drop all duplicated examples. > + reg = <0x0 0x4c490000 0x0 0x10000>; > + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&scmi_clk 115>, > + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > + clock-names = "vpu", "vpublk_wave"; > + power-domains = <&scmi_devpd 21>; > + cnm,ctrl = <&vpuctrl>; > + }; > + > + vpu2: vpu@4c4a0000 { > + compatible = "nxp,imx95-wave633c"; > + reg = <0x0 0x4c4a0000 0x0 0x10000>; > + interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&scmi_clk 115>, > + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > + clock-names = "vpu", "vpublk_wave"; > + power-domains = <&scmi_devpd 21>; > + cnm,ctrl = <&vpuctrl>; > + }; > + > + vpu3: vpu@4c4b0000 { > + compatible = "nxp,imx95-wave633c"; > + reg = <0x0 0x4c4b0000 0x0 0x10000>; > + interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&scmi_clk 115>, > + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > + clock-names = "vpu", "vpublk_wave"; > + power-domains = <&scmi_devpd 21>; > + cnm,ctrl = <&vpuctrl>; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 896a307fa065..5ff5b1f1ced2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -25462,6 +25462,14 @@ S: Maintained > F: Documentation/devicetree/bindings/media/cnm,wave521c.yaml > F: drivers/media/platform/chips-media/wave5/ > > +WAVE6 VPU CODEC DRIVER > +M: Nas Chung <nas.chung@chipsnmedia.com> > +M: Jackson Lee <jackson.lee@chipsnmedia.com> > +L: linux-media@vger.kernel.org > +S: Maintained > +F: Documentation/devicetree/bindings/media/nxp,wave633c.yaml > +F: drivers/media/platform/chips-media/wave6/ There is no such file/directory. Best regards, Krzysztof
Hi, Krzysztof. >-----Original Message----- >From: Krzysztof Kozlowski <krzk@kernel.org> >Sent: Tuesday, February 11, 2025 2:31 AM >To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org; >hverkuil@xs4all.nl; sebastian.fricke@collabora.com; robh@kernel.org; >krzk+dt@kernel.org; conor+dt@kernel.org >Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux- >kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm- >kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>; >lafley.kim <lafley.kim@chipsnmedia.com> >Subject: Re: [PATCH 3/8] dt-bindings: media: nxp: Add Wave6 video codec >device > >On 10/02/2025 10:07, Nas Chung wrote: >> Add documents for the Wave6 video codec on NXP i.MX SoCs. >> >> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> >> --- >> .../bindings/media/nxp,wave633c.yaml | 202 ++++++++++++++++++ >> MAINTAINERS | 8 + >> 2 files changed, 210 insertions(+) >> create mode 100644 >Documentation/devicetree/bindings/media/nxp,wave633c.yaml >> >> diff --git a/Documentation/devicetree/bindings/media/nxp,wave633c.yaml >b/Documentation/devicetree/bindings/media/nxp,wave633c.yaml >> new file mode 100644 >> index 000000000000..99c3008314c5 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/nxp,wave633c.yaml > >Filename matching compatible. > >> @@ -0,0 +1,202 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/nxp,wave633c.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Chips&Media Wave6 Series multi-standard codec IP on NXP i.MX SoCs. >> + >> +maintainers: >> + - Nas Chung <nas.chung@chipsnmedia.com> >> + - Jackson Lee <jackson.lee@chipsnmedia.com> >> + >> +description: >> + The Chips&Media Wave6 codec IP is a multi-standard video >encoder/decoder. >> + On NXP i.MX SoCs, Wave6 codec IP functionality is split between the >VPU control device >> + (vpu-ctrl) and the VPU device (vpu). The VPU control device manages >shared resources >> + such as firmware access and power domains, while the VPU device >provides encoding >> + and decoding capabilities. The VPU devie cannot operate independently > >Typo I'll fix it in v2. > >> + without the VPU control device. >> + > >Please wrap code according to the preferred limit expressed in Kernel >coding style (checkpatch is not a coding style description, but only a >tool). Bindings use strict rule here. I'll address this in v2. > > > >> +properties: >> + compatible: >> + items: >> + - enum: >> + - nxp,imx95-wave633c-ctrl >> + - nxp,imx95-wave633c > >I don't understand why you duplicated compatibles. You split this for >driver? That's a no. There are no two hardwares. Yes, I want to introduce two different devices and drivers, even though there is only one hardware. Wave6 IP has five independent register regions: One register region is dedicated to the control device, which manages shared resources such as firmware loading and power domains. The remaining four register regions are assigned to four independent VPU devices, each accessing its own dedicated region. (to support 4 vms) Would it be reasonable to split the YAML into separate files for the VPU control device and the VPU device ? (like nxp,wave633c-ctrl.yaml) > >These compatibles are anyway weird - why imx95 is in chipmedia product? >Is this part of a SoC? I want to represent that the Wave633 is part of the i.MX95. Chips&Media's Wave633 can also be integrated into SoCs from other vendors. By using the compatible name, the same Wave6 driver can distinguish different implementations. However, I agree that "imx95" is not strictly necessary in current status. So, using "nxp,wave633c" would be a better choice, right ? > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: VPU clock >> + - description: VPU associated block clock >> + >> + clock-names: >> + items: >> + - const: vpu >> + - const: vpublk_wave >> + >> + power-domains: >> + minItems: 1 >> + items: >> + - description: Main VPU power domain >> + - description: Performance power domain >> + >> + power-domain-names: >> + items: >> + - const: vpumix >> + - const: vpuperf >> + >> + cnm,ctrl: > >What is this prefix about? Is this nxp or something else? Yes, using "nxp" as the prefix seems more appropriate. > >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: phandle of the VPU control device node. Required for VPU >operation. > >Explain - required for what. Operation is too generic. phandle of the VPU control device node, which manages shared resources such as firmware access and power domains. > >If this is phandle to second device, then it's proof your split is >really wrong. Are you suggesting that I should separate them into two YAML files, or that I should structure them in a parent-child hierarchy within the same YAML file ? I appreciate any guidance on the best approach to structure this in line with existing bindings. > >> + >> + boot: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: phandle of the boot memory region node for the VPU >control device. > >No clue what is this... if memory region then use existing bindings. Boot is a memory region used for firmware upload. Only the control device can access this region. By "existing bindings," do you mean I should use "memory-region" instead ? > >Anyway, wrap your code correctly. Okay. > >> + >> + sram: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: phandle of the SRAM memory region node for the VPU >control device. >> + >> + '#cooling-cells': >> + const: 2 >> + >> + support-follower: >> + type: boolean >> + description: Indicates whether the VPU domain power always on. > >You cannot add new common properties in random way. Missing vendor >prefix but more important: does not look at all as hardware property but >OS policy. I agree with you. I'll remove it in v2. > >> + >> +patternProperties: >> + "^vpu-ctrl@[0-9a-f]+$": >> + type: object >> + properties: >> + compatible: >> + items: >> + - enum: >> + - nxp,imx95-wave633c-ctrl > >Really, what? How nxp,imx95-wave633c-ctrl node can have a child with >nxp,imx95-wave633c-ctrl compatible? > >NAK. Apologies, I misunderstood the meaning of 'patternProperties'. I'll remove it in v2. > > >> + reg: true >> + clocks: true >> + clock-names: true >> + power-domains: >> + items: >> + - description: Main VPU power domain >> + - description: Performance power domain >> + power-domain-names: >> + items: >> + - const: vpumix >> + - const: vpuperf >> + sram: true >> + boot: true >> + '#cooling-cells': true >> + support-follower: true >> + required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - power-domains >> + - power-domain-names >> + - sram >> + - boot >> + >> + additionalProperties: false >> + >> + "^vpu@[0-9a-f]+$": >> + type: object >> + properties: >> + compatible: >> + items: >> + - enum: >> + - nxp,imx95-wave633c >> + reg: true >> + interrupts: true >> + clocks: true >> + clock-names: true >> + power-domains: >> + maxItems: 1 >> + description: Main VPU power domain >> + cnm,ctrl: true >> + required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + - clock-names >> + - power-domains >> + - cnm,ctrl > >All this is just incorrect. I'll remove it in v2. > >> + >> + additionalProperties: false >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + #include <dt-bindings/clock/nxp,imx95-clock.h> >> + >> + soc { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + vpuctrl: vpu-ctrl@4c4c0000 { >> + compatible = "nxp,imx95-wave633c-ctrl"; >> + reg = <0x0 0x4c4c0000 0x0 0x10000>; >> + clocks = <&scmi_clk 115>, >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> + clock-names = "vpu", "vpublk_wave"; >> + power-domains = <&scmi_devpd 21>, <&scmi_perf 10>; >> + power-domain-names = "vpumix", "vpuperf"; >> + #cooling-cells = <2>; >> + boot = <&vpu_boot>; >> + sram = <&sram1>; >> + }; >> + >> + vpu0: vpu@4c480000 { > >Node names should be generic. See also an explanation and list of >examples (not exhaustive) in DT specification: >https://devicetree-specification.readthedocs.io/en/latest/chapter2- >devicetree-basics.html#generic-names-recommendation Thanks for sharing the link. I'll use "video-codec" as the node name in v2. > > >> + compatible = "nxp,imx95-wave633c"; >> + reg = <0x0 0x4c480000 0x0 0x10000>; >> + interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&scmi_clk 115>, >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> + clock-names = "vpu", "vpublk_wave"; >> + power-domains = <&scmi_devpd 21>; >> + cnm,ctrl = <&vpuctrl>; >> + }; >> + >> + vpu1: vpu@4c490000 { >> + compatible = "nxp,imx95-wave633c"; > >Drop all duplicated examples. Wave6 HW is designed for simultaneous access, as each VPU device has its own separate register space. Therefore, I defined the 4 VPU devices as independent nodes in the example to reflect this. > > >> + reg = <0x0 0x4c490000 0x0 0x10000>; >> + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&scmi_clk 115>, >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> + clock-names = "vpu", "vpublk_wave"; >> + power-domains = <&scmi_devpd 21>; >> + cnm,ctrl = <&vpuctrl>; >> + }; >> + >> + vpu2: vpu@4c4a0000 { >> + compatible = "nxp,imx95-wave633c"; >> + reg = <0x0 0x4c4a0000 0x0 0x10000>; >> + interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&scmi_clk 115>, >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> + clock-names = "vpu", "vpublk_wave"; >> + power-domains = <&scmi_devpd 21>; >> + cnm,ctrl = <&vpuctrl>; >> + }; >> + >> + vpu3: vpu@4c4b0000 { >> + compatible = "nxp,imx95-wave633c"; >> + reg = <0x0 0x4c4b0000 0x0 0x10000>; >> + interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&scmi_clk 115>, >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> + clock-names = "vpu", "vpublk_wave"; >> + power-domains = <&scmi_devpd 21>; >> + cnm,ctrl = <&vpuctrl>; >> + }; >> + }; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 896a307fa065..5ff5b1f1ced2 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -25462,6 +25462,14 @@ S: Maintained >> F: Documentation/devicetree/bindings/media/cnm,wave521c.yaml >> F: drivers/media/platform/chips-media/wave5/ >> >> +WAVE6 VPU CODEC DRIVER >> +M: Nas Chung <nas.chung@chipsnmedia.com> >> +M: Jackson Lee <jackson.lee@chipsnmedia.com> >> +L: linux-media@vger.kernel.org >> +S: Maintained >> +F: Documentation/devicetree/bindings/media/nxp,wave633c.yaml >> +F: drivers/media/platform/chips-media/wave6/ > >There is no such file/directory. Understood. I'll move the MAINTAINERS update to the last patch in v2. > >Best regards, >Krzysztof
On Thu, Feb 13, 2025 at 07:50:53AM +0000, Nas Chung wrote: > >> + items: > >> + - enum: > >> + - nxp,imx95-wave633c-ctrl > >> + - nxp,imx95-wave633c > > > >I don't understand why you duplicated compatibles. You split this for > >driver? That's a no. There are no two hardwares. > > Yes, I want to introduce two different devices and drivers, > even though there is only one hardware. That's a no. Bindings are for hardware, not drivers. Linux driver design is independent of bindings. > > Wave6 IP has five independent register regions: > > One register region is dedicated to the control device, > which manages shared resources such as firmware loading and power domains. > > The remaining four register regions are assigned to > four independent VPU devices, each accessing its own dedicated region. > (to support 4 vms) This could be, but your binding said something completely opposite. Look how other bindings do it, first. > > Would it be reasonable to split the YAML into separate files > for the VPU control device and the VPU device ? > (like nxp,wave633c-ctrl.yaml) No, it changes nothing. > > > > >These compatibles are anyway weird - why imx95 is in chipmedia product? > >Is this part of a SoC? > > I want to represent that the Wave633 is part of the i.MX95. > Chips&Media's Wave633 can also be integrated into SoCs from other vendors. OK > By using the compatible name, the same Wave6 driver can distinguish > different implementations. So you tell DT maintainer how DT works, brilliant... > > However, I agree that "imx95" is not strictly necessary in current status. > So, using "nxp,wave633c" would be a better choice, right ? No, NXP did not create wave633c. SoC components must have SoC prefix, assuming this is a Soc component. > > > > >> + > >> + reg: > >> + maxItems: 1 > >> + > >> + interrupts: > >> + maxItems: 1 > >> + > >> + clocks: > >> + items: > >> + - description: VPU clock > >> + - description: VPU associated block clock > >> + > >> + clock-names: > >> + items: > >> + - const: vpu > >> + - const: vpublk_wave > >> + > >> + power-domains: > >> + minItems: 1 > >> + items: > >> + - description: Main VPU power domain > >> + - description: Performance power domain > >> + > >> + power-domain-names: > >> + items: > >> + - const: vpumix > >> + - const: vpuperf > >> + > >> + cnm,ctrl: > > > >What is this prefix about? Is this nxp or something else? > > Yes, using "nxp" as the prefix seems more appropriate. > > > > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: phandle of the VPU control device node. Required for VPU > >operation. > > > >Explain - required for what. Operation is too generic. > > phandle of the VPU control device node, which manages shared resources > such as firmware access and power domains. Then NAK. Use proper bindings for this, e.g. power-domains. > > > > >If this is phandle to second device, then it's proof your split is > >really wrong. > > Are you suggesting that I should separate them into two YAML files, No. Splitting into separate files would change nothing - you still would have here a phandle, right? > or that I should structure them in a parent-child hierarchy > within the same YAML file ? You did not try hard enough to find similar devices, which there is a ton of. > > I appreciate any guidance on the best approach to structure this in line > with existing bindings. Then look for them. You have one device. If you have sub-blocks with their own distinguishable address space, then they can be children. But you did not write it that way. Just look at your example DTS - no children at all! > > > > >> + > >> + boot: > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: phandle of the boot memory region node for the VPU > >control device. > > > >No clue what is this... if memory region then use existing bindings. > > Boot is a memory region used for firmware upload. > Only the control device can access this region. > By "existing bindings," do you mean I should use "memory-region" instead ? Look how other bindings do this and what property they use. Yes, memory-region. > > > > >Anyway, wrap your code correctly. > > Okay. > > > > >> + > >> + sram: > >> + $ref: /schemas/types.yaml#/definitions/phandle > >> + description: phandle of the SRAM memory region node for the VPU > >control device. > >> + > >> + '#cooling-cells': > >> + const: 2 > >> + > >> + support-follower: > >> + type: boolean > >> + description: Indicates whether the VPU domain power always on. > > > >You cannot add new common properties in random way. Missing vendor > >prefix but more important: does not look at all as hardware property but > >OS policy. > > I agree with you. > I'll remove it in v2. > > > > >> + > >> +patternProperties: > >> + "^vpu-ctrl@[0-9a-f]+$": > >> + type: object > >> + properties: > >> + compatible: > >> + items: > >> + - enum: > >> + - nxp,imx95-wave633c-ctrl > > > >Really, what? How nxp,imx95-wave633c-ctrl node can have a child with > >nxp,imx95-wave633c-ctrl compatible? > > > >NAK. > > Apologies, I misunderstood the meaning of 'patternProperties'. > I'll remove it in v2. > > > > > > >> + reg: true > >> + clocks: true > >> + clock-names: true > >> + power-domains: > >> + items: > >> + - description: Main VPU power domain > >> + - description: Performance power domain > >> + power-domain-names: > >> + items: > >> + - const: vpumix > >> + - const: vpuperf > >> + sram: true > >> + boot: true > >> + '#cooling-cells': true > >> + support-follower: true > >> + required: > >> + - compatible > >> + - reg > >> + - clocks > >> + - clock-names > >> + - power-domains > >> + - power-domain-names > >> + - sram > >> + - boot > >> + > >> + additionalProperties: false > >> + > >> + "^vpu@[0-9a-f]+$": > >> + type: object > >> + properties: > >> + compatible: > >> + items: > >> + - enum: > >> + - nxp,imx95-wave633c > >> + reg: true > >> + interrupts: true > >> + clocks: true > >> + clock-names: true > >> + power-domains: > >> + maxItems: 1 > >> + description: Main VPU power domain > >> + cnm,ctrl: true > >> + required: > >> + - compatible > >> + - reg > >> + - interrupts > >> + - clocks > >> + - clock-names > >> + - power-domains > >> + - cnm,ctrl > > > >All this is just incorrect. > > I'll remove it in v2. > > > > >> + > >> + additionalProperties: false > >> + > >> +additionalProperties: false > >> + > >> +examples: > >> + - | > >> + #include <dt-bindings/interrupt-controller/arm-gic.h> > >> + #include <dt-bindings/clock/nxp,imx95-clock.h> > >> + > >> + soc { > >> + #address-cells = <2>; > >> + #size-cells = <2>; > >> + > >> + vpuctrl: vpu-ctrl@4c4c0000 { So this is the parent device... > >> + compatible = "nxp,imx95-wave633c-ctrl"; > >> + reg = <0x0 0x4c4c0000 0x0 0x10000>; > >> + clocks = <&scmi_clk 115>, > >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > >> + clock-names = "vpu", "vpublk_wave"; > >> + power-domains = <&scmi_devpd 21>, <&scmi_perf 10>; > >> + power-domain-names = "vpumix", "vpuperf"; > >> + #cooling-cells = <2>; > >> + boot = <&vpu_boot>; > >> + sram = <&sram1>; > >> + }; > >> + > >> + vpu0: vpu@4c480000 { And here you have child which is not a child? Your binding and DTS neither match nor make any sense. > > > >Node names should be generic. See also an explanation and list of > >examples (not exhaustive) in DT specification: > >https://devicetree-specification.readthedocs.io/en/latest/chapter2- > >devicetree-basics.html#generic-names-recommendation > > Thanks for sharing the link. > I'll use "video-codec" as the node name in v2. > > > > > > >> + compatible = "nxp,imx95-wave633c"; > >> + reg = <0x0 0x4c480000 0x0 0x10000>; > >> + interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>; > >> + clocks = <&scmi_clk 115>, > >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > >> + clock-names = "vpu", "vpublk_wave"; > >> + power-domains = <&scmi_devpd 21>; > >> + cnm,ctrl = <&vpuctrl>; > >> + }; > >> + > >> + vpu1: vpu@4c490000 { > >> + compatible = "nxp,imx95-wave633c"; > > > >Drop all duplicated examples. > > Wave6 HW is designed for simultaneous access, > as each VPU device has its own separate register space. > Therefore, I defined the 4 VPU devices as independent nodes in the example > to reflect this. They are redundant in this example. Unless you wanted to express something else, but you did not. > > > > > > >> + reg = <0x0 0x4c490000 0x0 0x10000>; > >> + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>; > >> + clocks = <&scmi_clk 115>, > >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > >> + clock-names = "vpu", "vpublk_wave"; > >> + power-domains = <&scmi_devpd 21>; > >> + cnm,ctrl = <&vpuctrl>; > >> + }; > >> + > >> + vpu2: vpu@4c4a0000 { > >> + compatible = "nxp,imx95-wave633c"; > >> + reg = <0x0 0x4c4a0000 0x0 0x10000>; > >> + interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>; > >> + clocks = <&scmi_clk 115>, > >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > >> + clock-names = "vpu", "vpublk_wave"; > >> + power-domains = <&scmi_devpd 21>; > >> + cnm,ctrl = <&vpuctrl>; > >> + }; > >> + > >> + vpu3: vpu@4c4b0000 { > >> + compatible = "nxp,imx95-wave633c"; > >> + reg = <0x0 0x4c4b0000 0x0 0x10000>; > >> + interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>; > >> + clocks = <&scmi_clk 115>, > >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; > >> + clock-names = "vpu", "vpublk_wave"; > >> + power-domains = <&scmi_devpd 21>; > >> + cnm,ctrl = <&vpuctrl>; > >> + }; > >> + }; > >> diff --git a/MAINTAINERS b/MAINTAINERS > >> index 896a307fa065..5ff5b1f1ced2 100644 > >> --- a/MAINTAINERS > >> +++ b/MAINTAINERS > >> @@ -25462,6 +25462,14 @@ S: Maintained > >> F: Documentation/devicetree/bindings/media/cnm,wave521c.yaml > >> F: drivers/media/platform/chips-media/wave5/ > >> > >> +WAVE6 VPU CODEC DRIVER > >> +M: Nas Chung <nas.chung@chipsnmedia.com> > >> +M: Jackson Lee <jackson.lee@chipsnmedia.com> > >> +L: linux-media@vger.kernel.org > >> +S: Maintained > >> +F: Documentation/devicetree/bindings/media/nxp,wave633c.yaml > >> +F: drivers/media/platform/chips-media/wave6/ > > > >There is no such file/directory. > > Understood. I'll move the MAINTAINERS update to the last patch in v2. No, just add entry per entry. Best regards, Krzysztof
Hi, Krzysztof. >-----Original Message----- >From: Krzysztof Kozlowski <krzk@kernel.org> >Sent: Thursday, February 13, 2025 5:50 PM >To: Nas Chung <nas.chung@chipsnmedia.com> >Cc: mchehab@kernel.org; hverkuil@xs4all.nl; sebastian.fricke@collabora.com; >robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; linux- >media@vger.kernel.org; devicetree@vger.kernel.org; linux- >kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm- >kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>; >lafley.kim <lafley.kim@chipsnmedia.com> >Subject: Re: [PATCH 3/8] dt-bindings: media: nxp: Add Wave6 video codec >device > >On Thu, Feb 13, 2025 at 07:50:53AM +0000, Nas Chung wrote: >> >> + items: >> >> + - enum: >> >> + - nxp,imx95-wave633c-ctrl >> >> + - nxp,imx95-wave633c >> > >> >I don't understand why you duplicated compatibles. You split this for >> >driver? That's a no. There are no two hardwares. >> >> Yes, I want to introduce two different devices and drivers, >> even though there is only one hardware. > >That's a no. Bindings are for hardware, not drivers. >Linux driver design is independent of bindings. > >> >> Wave6 IP has five independent register regions: >> >> One register region is dedicated to the control device, >> which manages shared resources such as firmware loading and power domains. >> >> The remaining four register regions are assigned to >> four independent VPU devices, each accessing its own dedicated region. >> (to support 4 vms) > >This could be, but your binding said something completely opposite. Look >how other bindings do it, first. > >> >> Would it be reasonable to split the YAML into separate files >> for the VPU control device and the VPU device ? >> (like nxp,wave633c-ctrl.yaml) > >No, it changes nothing. > >> >> > >> >These compatibles are anyway weird - why imx95 is in chipmedia product? >> >Is this part of a SoC? >> >> I want to represent that the Wave633 is part of the i.MX95. >> Chips&Media's Wave633 can also be integrated into SoCs from other vendors. > >OK > > >> By using the compatible name, the same Wave6 driver can distinguish >> different implementations. > >So you tell DT maintainer how DT works, brilliant... > >> >> However, I agree that "imx95" is not strictly necessary in current status. >> So, using "nxp,wave633c" would be a better choice, right ? > >No, NXP did not create wave633c. SoC components must have SoC prefix, >assuming this is a Soc component. Ah, I had completely misunderstood the purpose and role of DT. My apologies for that. Would it be okay to modify it as follows ? items: - enum: - nxp,imx95-vpu - const: cnm,wave633c > >> >> > >> >> + >> >> + reg: >> >> + maxItems: 1 >> >> + >> >> + interrupts: >> >> + maxItems: 1 >> >> + >> >> + clocks: >> >> + items: >> >> + - description: VPU clock >> >> + - description: VPU associated block clock >> >> + >> >> + clock-names: >> >> + items: >> >> + - const: vpu >> >> + - const: vpublk_wave >> >> + >> >> + power-domains: >> >> + minItems: 1 >> >> + items: >> >> + - description: Main VPU power domain >> >> + - description: Performance power domain >> >> + >> >> + power-domain-names: >> >> + items: >> >> + - const: vpumix >> >> + - const: vpuperf >> >> + >> >> + cnm,ctrl: >> > >> >What is this prefix about? Is this nxp or something else? >> >> Yes, using "nxp" as the prefix seems more appropriate. >> >> > >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: phandle of the VPU control device node. Required for >VPU >> >operation. >> > >> >Explain - required for what. Operation is too generic. >> >> phandle of the VPU control device node, which manages shared resources >> such as firmware access and power domains. > >Then NAK. > >Use proper bindings for this, e.g. power-domains. > > >> >> > >> >If this is phandle to second device, then it's proof your split is >> >really wrong. >> >> Are you suggesting that I should separate them into two YAML files, > >No. Splitting into separate files would change nothing - you still would >have here a phandle, right? > >> or that I should structure them in a parent-child hierarchy >> within the same YAML file ? > >You did not try hard enough to find similar devices, which there is a >ton of. > >> >> I appreciate any guidance on the best approach to structure this in line >> with existing bindings. > >Then look for them. > >You have one device. If you have sub-blocks with their own >distinguishable address space, then they can be children. But you did >not write it that way. Just look at your example DTS - no children at >all! I see that I didn't review similar devices thoroughly enough. Thanks for pointing this out. Okay, I will remove the phandle for second device and handle it in parent node. I have left the modified structure in the example below. > >> >> > >> >> + >> >> + boot: >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: phandle of the boot memory region node for the VPU >> >control device. >> > >> >No clue what is this... if memory region then use existing bindings. >> >> Boot is a memory region used for firmware upload. >> Only the control device can access this region. >> By "existing bindings," do you mean I should use "memory-region" instead ? > >Look how other bindings do this and what property they use. Yes, >memory-region. > >> >> > >> >Anyway, wrap your code correctly. >> >> Okay. >> >> > >> >> + >> >> + sram: >> >> + $ref: /schemas/types.yaml#/definitions/phandle >> >> + description: phandle of the SRAM memory region node for the VPU >> >control device. >> >> + >> >> + '#cooling-cells': >> >> + const: 2 >> >> + >> >> + support-follower: >> >> + type: boolean >> >> + description: Indicates whether the VPU domain power always on. >> > >> >You cannot add new common properties in random way. Missing vendor >> >prefix but more important: does not look at all as hardware property but >> >OS policy. >> >> I agree with you. >> I'll remove it in v2. >> >> > >> >> + >> >> +patternProperties: >> >> + "^vpu-ctrl@[0-9a-f]+$": >> >> + type: object >> >> + properties: >> >> + compatible: >> >> + items: >> >> + - enum: >> >> + - nxp,imx95-wave633c-ctrl >> > >> >Really, what? How nxp,imx95-wave633c-ctrl node can have a child with >> >nxp,imx95-wave633c-ctrl compatible? >> > >> >NAK. >> >> Apologies, I misunderstood the meaning of 'patternProperties'. >> I'll remove it in v2. >> >> > >> > >> >> + reg: true >> >> + clocks: true >> >> + clock-names: true >> >> + power-domains: >> >> + items: >> >> + - description: Main VPU power domain >> >> + - description: Performance power domain >> >> + power-domain-names: >> >> + items: >> >> + - const: vpumix >> >> + - const: vpuperf >> >> + sram: true >> >> + boot: true >> >> + '#cooling-cells': true >> >> + support-follower: true >> >> + required: >> >> + - compatible >> >> + - reg >> >> + - clocks >> >> + - clock-names >> >> + - power-domains >> >> + - power-domain-names >> >> + - sram >> >> + - boot >> >> + >> >> + additionalProperties: false >> >> + >> >> + "^vpu@[0-9a-f]+$": >> >> + type: object >> >> + properties: >> >> + compatible: >> >> + items: >> >> + - enum: >> >> + - nxp,imx95-wave633c >> >> + reg: true >> >> + interrupts: true >> >> + clocks: true >> >> + clock-names: true >> >> + power-domains: >> >> + maxItems: 1 >> >> + description: Main VPU power domain >> >> + cnm,ctrl: true >> >> + required: >> >> + - compatible >> >> + - reg >> >> + - interrupts >> >> + - clocks >> >> + - clock-names >> >> + - power-domains >> >> + - cnm,ctrl >> > >> >All this is just incorrect. >> >> I'll remove it in v2. >> >> > >> >> + >> >> + additionalProperties: false >> >> + >> >> +additionalProperties: false >> >> + >> >> +examples: >> >> + - | >> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> >> + #include <dt-bindings/clock/nxp,imx95-clock.h> >> >> + >> >> + soc { >> >> + #address-cells = <2>; >> >> + #size-cells = <2>; >> >> + >> >> + vpuctrl: vpu-ctrl@4c4c0000 { > >So this is the parent device... > >> >> + compatible = "nxp,imx95-wave633c-ctrl"; >> >> + reg = <0x0 0x4c4c0000 0x0 0x10000>; >> >> + clocks = <&scmi_clk 115>, >> >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> >> + clock-names = "vpu", "vpublk_wave"; >> >> + power-domains = <&scmi_devpd 21>, <&scmi_perf 10>; >> >> + power-domain-names = "vpumix", "vpuperf"; >> >> + #cooling-cells = <2>; >> >> + boot = <&vpu_boot>; >> >> + sram = <&sram1>; >> >> + }; >> >> + >> >> + vpu0: vpu@4c480000 { > > >And here you have child which is not a child? Your binding and DTS >neither match nor make any sense. Would it be reasonable to modify it as follows ? For example: vpu: video-codec@4c480000 { compatible = "nxp,imx95-vpu"; reg = <0x0 0x4c480000 0x0 0x50000>; ranges = <0x0 0x0 0x4c480000 0x50000>; vpuctrl: vpu-ctrl@40000 { compatible = "nxp,imx95-vpu-ctrl"; reg = <0x40000 0x10000>; }; vpucore0: vpu-core@00000 { compatible = "nxp,imx95-vpu-core"; reg = <0x00000 0x10000>; }; vpucore1: vpu-core@10000 { compatible = "nxp,imx95-vpu-core"; reg = <0x10000 0x10000>; }; vpucore2: vpu-core@20000 { compatible = "nxp,imx95-vpu-core"; reg = <0x20000 0x10000>; }; vpucore3: vpu-core@30000 { compatible = "nxp,imx95-vpu-core"; reg = <0x30000 0x10000>; }; }; > >> > >> >Node names should be generic. See also an explanation and list of >> >examples (not exhaustive) in DT specification: >> >https://devicetree-specification.readthedocs.io/en/latest/chapter2- >> >devicetree-basics.html#generic-names-recommendation >> >> Thanks for sharing the link. >> I'll use "video-codec" as the node name in v2. >> >> > >> > >> >> + compatible = "nxp,imx95-wave633c"; >> >> + reg = <0x0 0x4c480000 0x0 0x10000>; >> >> + interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>; >> >> + clocks = <&scmi_clk 115>, >> >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> >> + clock-names = "vpu", "vpublk_wave"; >> >> + power-domains = <&scmi_devpd 21>; >> >> + cnm,ctrl = <&vpuctrl>; >> >> + }; >> >> + >> >> + vpu1: vpu@4c490000 { >> >> + compatible = "nxp,imx95-wave633c"; >> > >> >Drop all duplicated examples. >> >> Wave6 HW is designed for simultaneous access, >> as each VPU device has its own separate register space. >> Therefore, I defined the 4 VPU devices as independent nodes in the >example >> to reflect this. > >They are redundant in this example. Unless you wanted to express >something else, but you did not. Got it. > > >> >> > >> > >> >> + reg = <0x0 0x4c490000 0x0 0x10000>; >> >> + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>; >> >> + clocks = <&scmi_clk 115>, >> >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> >> + clock-names = "vpu", "vpublk_wave"; >> >> + power-domains = <&scmi_devpd 21>; >> >> + cnm,ctrl = <&vpuctrl>; >> >> + }; >> >> + >> >> + vpu2: vpu@4c4a0000 { >> >> + compatible = "nxp,imx95-wave633c"; >> >> + reg = <0x0 0x4c4a0000 0x0 0x10000>; >> >> + interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>; >> >> + clocks = <&scmi_clk 115>, >> >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> >> + clock-names = "vpu", "vpublk_wave"; >> >> + power-domains = <&scmi_devpd 21>; >> >> + cnm,ctrl = <&vpuctrl>; >> >> + }; >> >> + >> >> + vpu3: vpu@4c4b0000 { >> >> + compatible = "nxp,imx95-wave633c"; >> >> + reg = <0x0 0x4c4b0000 0x0 0x10000>; >> >> + interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>; >> >> + clocks = <&scmi_clk 115>, >> >> + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; >> >> + clock-names = "vpu", "vpublk_wave"; >> >> + power-domains = <&scmi_devpd 21>; >> >> + cnm,ctrl = <&vpuctrl>; >> >> + }; >> >> + }; >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> >> index 896a307fa065..5ff5b1f1ced2 100644 >> >> --- a/MAINTAINERS >> >> +++ b/MAINTAINERS >> >> @@ -25462,6 +25462,14 @@ S: Maintained >> >> F: Documentation/devicetree/bindings/media/cnm,wave521c.yaml >> >> F: drivers/media/platform/chips-media/wave5/ >> >> >> >> +WAVE6 VPU CODEC DRIVER >> >> +M: Nas Chung <nas.chung@chipsnmedia.com> >> >> +M: Jackson Lee <jackson.lee@chipsnmedia.com> >> >> +L: linux-media@vger.kernel.org >> >> +S: Maintained >> >> +F: Documentation/devicetree/bindings/media/nxp,wave633c.yaml >> >> +F: drivers/media/platform/chips-media/wave6/ >> > >> >There is no such file/directory. >> >> Understood. I'll move the MAINTAINERS update to the last patch in v2. > >No, just add entry per entry. I'll add each entry separately in v2. Thanks. Nas. > >Best regards, >Krzysztof
On 18/02/2025 10:21, Nas Chung wrote: > For example: > vpu: video-codec@4c480000 { > compatible = "nxp,imx95-vpu"; > reg = <0x0 0x4c480000 0x0 0x50000>; > ranges = <0x0 0x0 0x4c480000 0x50000>; > > vpuctrl: vpu-ctrl@40000 { > compatible = "nxp,imx95-vpu-ctrl"; > reg = <0x40000 0x10000>; > }; > > vpucore0: vpu-core@00000 { > compatible = "nxp,imx95-vpu-core"; > reg = <0x00000 0x10000>; > }; > > vpucore1: vpu-core@10000 { > compatible = "nxp,imx95-vpu-core"; > reg = <0x10000 0x10000>; > }; > > vpucore2: vpu-core@20000 { > compatible = "nxp,imx95-vpu-core"; > reg = <0x20000 0x10000>; > }; > > vpucore3: vpu-core@30000 { > compatible = "nxp,imx95-vpu-core"; Why do you need compatible here? Could it be anything else? > reg = <0x30000 0x10000>; Where is the rest of resources? You created children only for one resource - address space? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/nxp,wave633c.yaml b/Documentation/devicetree/bindings/media/nxp,wave633c.yaml new file mode 100644 index 000000000000..99c3008314c5 --- /dev/null +++ b/Documentation/devicetree/bindings/media/nxp,wave633c.yaml @@ -0,0 +1,202 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/nxp,wave633c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Chips&Media Wave6 Series multi-standard codec IP on NXP i.MX SoCs. + +maintainers: + - Nas Chung <nas.chung@chipsnmedia.com> + - Jackson Lee <jackson.lee@chipsnmedia.com> + +description: + The Chips&Media Wave6 codec IP is a multi-standard video encoder/decoder. + On NXP i.MX SoCs, Wave6 codec IP functionality is split between the VPU control device + (vpu-ctrl) and the VPU device (vpu). The VPU control device manages shared resources + such as firmware access and power domains, while the VPU device provides encoding + and decoding capabilities. The VPU devie cannot operate independently + without the VPU control device. + +properties: + compatible: + items: + - enum: + - nxp,imx95-wave633c-ctrl + - nxp,imx95-wave633c + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: VPU clock + - description: VPU associated block clock + + clock-names: + items: + - const: vpu + - const: vpublk_wave + + power-domains: + minItems: 1 + items: + - description: Main VPU power domain + - description: Performance power domain + + power-domain-names: + items: + - const: vpumix + - const: vpuperf + + cnm,ctrl: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the VPU control device node. Required for VPU operation. + + boot: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the boot memory region node for the VPU control device. + + sram: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the SRAM memory region node for the VPU control device. + + '#cooling-cells': + const: 2 + + support-follower: + type: boolean + description: Indicates whether the VPU domain power always on. + +patternProperties: + "^vpu-ctrl@[0-9a-f]+$": + type: object + properties: + compatible: + items: + - enum: + - nxp,imx95-wave633c-ctrl + reg: true + clocks: true + clock-names: true + power-domains: + items: + - description: Main VPU power domain + - description: Performance power domain + power-domain-names: + items: + - const: vpumix + - const: vpuperf + sram: true + boot: true + '#cooling-cells': true + support-follower: true + required: + - compatible + - reg + - clocks + - clock-names + - power-domains + - power-domain-names + - sram + - boot + + additionalProperties: false + + "^vpu@[0-9a-f]+$": + type: object + properties: + compatible: + items: + - enum: + - nxp,imx95-wave633c + reg: true + interrupts: true + clocks: true + clock-names: true + power-domains: + maxItems: 1 + description: Main VPU power domain + cnm,ctrl: true + required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - power-domains + - cnm,ctrl + + additionalProperties: false + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/nxp,imx95-clock.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + vpuctrl: vpu-ctrl@4c4c0000 { + compatible = "nxp,imx95-wave633c-ctrl"; + reg = <0x0 0x4c4c0000 0x0 0x10000>; + clocks = <&scmi_clk 115>, + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; + clock-names = "vpu", "vpublk_wave"; + power-domains = <&scmi_devpd 21>, <&scmi_perf 10>; + power-domain-names = "vpumix", "vpuperf"; + #cooling-cells = <2>; + boot = <&vpu_boot>; + sram = <&sram1>; + }; + + vpu0: vpu@4c480000 { + compatible = "nxp,imx95-wave633c"; + reg = <0x0 0x4c480000 0x0 0x10000>; + interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&scmi_clk 115>, + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; + clock-names = "vpu", "vpublk_wave"; + power-domains = <&scmi_devpd 21>; + cnm,ctrl = <&vpuctrl>; + }; + + vpu1: vpu@4c490000 { + compatible = "nxp,imx95-wave633c"; + reg = <0x0 0x4c490000 0x0 0x10000>; + interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&scmi_clk 115>, + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; + clock-names = "vpu", "vpublk_wave"; + power-domains = <&scmi_devpd 21>; + cnm,ctrl = <&vpuctrl>; + }; + + vpu2: vpu@4c4a0000 { + compatible = "nxp,imx95-wave633c"; + reg = <0x0 0x4c4a0000 0x0 0x10000>; + interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&scmi_clk 115>, + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; + clock-names = "vpu", "vpublk_wave"; + power-domains = <&scmi_devpd 21>; + cnm,ctrl = <&vpuctrl>; + }; + + vpu3: vpu@4c4b0000 { + compatible = "nxp,imx95-wave633c"; + reg = <0x0 0x4c4b0000 0x0 0x10000>; + interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&scmi_clk 115>, + <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>; + clock-names = "vpu", "vpublk_wave"; + power-domains = <&scmi_devpd 21>; + cnm,ctrl = <&vpuctrl>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 896a307fa065..5ff5b1f1ced2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -25462,6 +25462,14 @@ S: Maintained F: Documentation/devicetree/bindings/media/cnm,wave521c.yaml F: drivers/media/platform/chips-media/wave5/ +WAVE6 VPU CODEC DRIVER +M: Nas Chung <nas.chung@chipsnmedia.com> +M: Jackson Lee <jackson.lee@chipsnmedia.com> +L: linux-media@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/media/nxp,wave633c.yaml +F: drivers/media/platform/chips-media/wave6/ + WHISKEYCOVE PMIC GPIO DRIVER M: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> L: linux-gpio@vger.kernel.org
Add documents for the Wave6 video codec on NXP i.MX SoCs. Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com> --- .../bindings/media/nxp,wave633c.yaml | 202 ++++++++++++++++++ MAINTAINERS | 8 + 2 files changed, 210 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/nxp,wave633c.yaml