| 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
| ||
|
Message-ID:
<SL2P216MB1246002B8EFD5CBE69E447ACFB96A@SL2P216MB1246.KORP216.PROD.OUTLOOK.COM>
Date: Tue, 13 May 2025 07:39:04 +0000
From: Nas Chung <nas.chung@...psnmedia.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: "mchehab@...nel.org" <mchehab@...nel.org>, "hverkuil@...all.nl"
<hverkuil@...all.nl>, "sebastian.fricke@...labora.com"
<sebastian.fricke@...labora.com>, "robh@...nel.org" <robh@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
<conor+dt@...nel.org>, "linux-media@...r.kernel.org"
<linux-media@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-imx@....com" <linux-imx@....com>,
"marex@...x.de" <marex@...x.de>, jackson.lee <jackson.lee@...psnmedia.com>,
lafley.kim <lafley.kim@...psnmedia.com>
Subject: RE: [PATCH v2 2/8] dt-bindings: media: nxp: Add Wave6 video codec
device
Hi, Krzysztof.
>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@...nel.org>
>Sent: Friday, May 9, 2025 7:12 PM
>To: Nas Chung <nas.chung@...psnmedia.com>
>Cc: mchehab@...nel.org; hverkuil@...all.nl; sebastian.fricke@...labora.com;
>robh@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org; linux-
>media@...r.kernel.org; devicetree@...r.kernel.org; linux-
>kernel@...r.kernel.org; linux-imx@....com; marex@...x.de; jackson.lee
><jackson.lee@...psnmedia.com>; lafley.kim <lafley.kim@...psnmedia.com>
>Subject: Re: [PATCH v2 2/8] dt-bindings: media: nxp: Add Wave6 video codec
>device
>
>On 09/05/2025 11:59, Nas Chung wrote:
>>>> +examples:
>>>> + - |
>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> + #include <dt-bindings/clock/nxp,imx95-clock.h>
>>>> +
>>>> + soc {
>>>> + #address-cells = <2>;
>>>> + #size-cells = <2>;
>>>> +
>>>> + vpu: video-codec {
>>>
>>> Why this device does not have MMIO? Sorry, but makes little sense and if
>>> you posted and tested your entire DTS you would see why.
>>
>> I initially thought that if the reg property is declared in the child,
>> it would not need to be declared in the parent node.
>> I based this approach on the mediatek,mt8195-jpegenc.yaml binding,
>> where the parent node does not include MMIO.
>
>Do you have access to mt8195 datasheet? What is the memory/register
>layout there?
>
>If you do not have access, why do you think these are similar devices?
I'm sorry, I made an incorrect reference without fully understanding
the mt8195 architecture.
>
>>
>> But, if this structure is problematic, I will address it in patch v3.
>>
>>>
>>> Can we see the entire DTS?
>>
>> Sure !
>> Below is the cnm.wave633c.example.dts file created from dt_binding_check.
>
>This is not the entire DTS.
I have found errors in dtbs_check.
I will make the necessary corrections and add the DTS patch to the patch series
>
>>
>> /dts-v1/;
>> /plugin/; // silence any missing phandle references
>
>This is bindings example. I want to see entire DTS or DTSI of the SoC.
>Once you see entire DTS, you will notice that your current split is just
>not correct - it should be pretty obvious.
>
>And that's why we should keep rejecting such works which do not bring
>any DTS user, because the no one - neither we nor the contributors - see
>big picture and if someone saw the big picture then immediately would
>notice - it's just bollocks.
I understand the meaning. I will test the entire DTS file
to properly use the VPU and check for errors in dtbs_check.
>
>What you claim is:
>
>soc@0 {
> // MMIO bus
>
> video-codec {
> // which is a non-MMIO device and clearly a no-go.
>
>Just look how DTS is organized and learn from it.
>
Thank you for pointing this out.
I now understand that placing a non-MMIO device on the MMIO bus is
fundamentally incorrect.
To address this, I will update the video-codec device to define
the reg properties covering the entire reg region.
>>
>> /{
>> compatible = "foo";
>> model = "foo";
>> #address-cells = <1>;
>> #size-cells = <1>;
>>
>>
>> example-0 {
>> #address-cells = <1>;
>> #size-cells = <1>;
>>
>>
>> interrupt-parent = <&fake_intc0>;
>> fake_intc0: fake-interrupt-controller {
>> interrupt-controller;
>> #interrupt-cells = < 3 >;
>> };
>>
>
>All of above are wrong for the SoC...
>
>>
>> #include <dt-bindings/interrupt-controller/arm-gic.h>
>> #include <dt-bindings/clock/nxp,imx95-clock.h>
>>
>> soc {
>> #address-cells = <2>;
>> #size-cells = <2>;
>>
>> vpu: video-codec {
>> compatible = "nxp,imx95-vpu", "cnm,wave633c";
>
>What does this device represent? It is not "ctrl", because you made ctrl
>separate device node. Your binding description suggests that is the VPU
>control region.
My intention was to represent the MMIO VPU device, which includes
both the core and control nodes.
>
>> clocks = <&scmi_clk 115>,
>> <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
>
>For which sub devices these clocks are valid? For all of them?
Yes, these clocks are valid for all of sub devices.
Thanks.
Nas.
>
>> clock-names = "vpu", "vpublk_wave";
>> power-domains = <&scmi_devpd 21>;
>> #address-cells = <2>;
>> #size-cells = <2>;
>> ranges;
>>
>> vpucore0: video-core@...80000 {
>> compatible = "nxp,imx95-vpu-core";
>> reg = <0x0 0x4c480000 0x0 0x10000>;
>> interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
>> };
>>
>> vpucore1: video-core@...90000 {
>> compatible = "nxp,imx95-vpu-core";
>> reg = <0x0 0x4c490000 0x0 0x10000>;
>> interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
>> };
>>
>> vpucore2: video-core@...a0000 {
>> compatible = "nxp,imx95-vpu-core";
>> reg = <0x0 0x4c4a0000 0x0 0x10000>;
>> interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>;
>> };
>>
>> vpucore3: video-core@...b0000 {
>> compatible = "nxp,imx95-vpu-core";
>> reg = <0x0 0x4c4b0000 0x0 0x10000>;
>> interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
>> };
>>
>> vpuctrl: video-controller@...c0000 {
>> compatible = "nxp,imx95-vpu-ctrl";
>> reg = <0x0 0x4c4c0000 0x0 0x10000>;
>> memory-region = <&vpu_boot>;
>> power-domains = <&scmi_perf 10>;
>> #cooling-cells = <2>;
>> sram = <&sram1>;
>> };
>> };
>> };
>>
>> };
>> };
>>
>>>
>>> Best regards,
>>> Krzysztof
>>
>
>
>Best regards,
>Krzysztof
Powered by blists - more mailing lists