Re: [PATCH 3/8] dt-bindings: media: nxp: Add Wave6 video codec device

From: Krzysztof Kozlowski
Date: Thu Feb 20 2025 - 03:32:37 EST


On 20/02/2025 08:35, Nas Chung wrote:
> Hi, Krzysztof.
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
>> Sent: Wednesday, February 19, 2025 9:32 PM
>> To: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
>> Cc: mchehab@xxxxxxxxxx; hverkuil@xxxxxxxxx; sebastian.fricke@xxxxxxxxxxxxx;
>> robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; linux-
>> media@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx; linux-imx@xxxxxxx; linux-arm-
>> kernel@xxxxxxxxxxxxxxxxxxx; jackson.lee <jackson.lee@xxxxxxxxxxxxxxx>;
>> lafley.kim <lafley.kim@xxxxxxxxxxxxxxx>
>> Subject: Re: [PATCH 3/8] dt-bindings: media: nxp: Add Wave6 video codec
>> device
>>
>> 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?
>
> I will update the driver based on the final DT.
>
>>
>>> reg = <0x30000 0x10000>;
>>
>> Where is the rest of resources? You created children only for one
>> resource - address space?
>
> Sorry for the confusion.
> I believe the final example looks like the one below.
>
> 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>;
> 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";
> memory-region = <&vpu_boot>;
> #cooling-cells = <2>;
> sram = <&sram1>;
> };
>
> vpucore0: vpu-core@00000 {
> compatible = "nxp,imx95-vpu-core";
> reg = <0x00000 0x10000>;
> interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&scmi_clk 115>,
> <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;


These are the same resources for every block so for entire device. Why
they are no in top level node?

> clock-names = "vpu", "vpublk_wave";
> power-domains = <&scmi_devpd 21>;

Same here

Everything else also looks duplicated... All my previous comments - from
first email - stand. Don't design DT to match your Linu driver structure.

Best regards,
Krzysztof