linux-kernel - Re: [PATCH v3 2/6] dt-bindings: soc: imx93-media-blk-ctrl: Add LDB subnode into schema and example

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: <20250305-dandelion-axolotl-of-excitement-05fa70@krzk-bin>
Date: Wed, 5 Mar 2025 08:13:04 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Alexander Stein <alexander.stein@...tq-group.com>
Cc: Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+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>, 
	Abel Vesa <abelvesa@...nel.org>, Peng Fan <peng.fan@....com>, 
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, 
	Ulf Hansson <ulf.hansson@...aro.org>, devicetree@...r.kernel.org, imx@...ts.linux.dev, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, linux@...tq-group.com, 
	linux-clk@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 2/6] dt-bindings: soc: imx93-media-blk-ctrl: Add LDB
 subnode into schema and example

On Tue, Mar 04, 2025 at 04:49:21PM +0100, Alexander Stein wrote:
> Document the LDB bridge subnode and add the subnode into the example.
> For the subnode to work, the block control must scan its subnodes and

Don't describe drivers, but describe the hardware.


> bind drivers to them, do not misuse either simple-bus or simple-mfd
> here.

I don't understand that simple-bus or simple-mfd statement. There are no
such compatibles here.

> 
> Signed-off-by: Alexander Stein <alexander.stein@...tq-group.com>
> ---
>  .../soc/imx/fsl,imx93-media-blk-ctrl.yaml     | 51 +++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
> index b3554e7f9e76d..cd785111928bf 100644
> --- a/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
> +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx93-media-blk-ctrl.yaml
> @@ -24,6 +24,14 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  ranges: true
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
>    '#power-domain-cells':
>      const: 1
>  
> @@ -46,9 +54,20 @@ properties:
>        - const: csi
>        - const: dsi
>  
> +  bridge@20:

@20 looks wrong. Use 'ranges;' and try again your DTS...

Binding is supposed to be complete. We have several examples when people
added children one-by-one, everytime with different reasoning about
child addressing.

So please confirm: this is complete and no other children will ever be
added here... or you are 100% sure that all future children will be
unit-addressable (will have unit address and appropriate properties).

BTW, I don't quite get why this is both syscon and has translation for
child addresses. Does it mean your child does not use the same MMIO as
parent, thus leading to unsynchronized reg access?

> +    type: object
> +    additionalProperties: true
> +    properties:
> +      compatible:
> +        contains:
> +          const: fsl,imx93-ldb
> +
>  required:
>    - compatible
>    - reg
> +  - ranges
> +  - '#address-cells'
> +  - '#size-cells'
>    - power-domains
>    - clocks
>    - clock-names
> @@ -77,4 +96,36 @@ examples:
>                 clock-names = "apb", "axi", "nic", "disp", "cam",
>                               "pxp", "lcdif", "isi", "csi", "dsi";
>        #power-domain-cells = <1>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      ranges = <0x0 0x4ac10000 0x10000>;
> +
> +      bridge@20 {
> +          compatible = "fsl,imx93-ldb";
> +          reg = <0x20 0x4>, <0x24 0x4>;
> +          reg-names = "ldb", "lvds";
> +          clocks = <&clk IMX93_CLK_LVDS_GATE>;
> +          clock-names = "ldb";
> +
> +          ports {
> +              #address-cells = <1>;
> +              #size-cells = <0>;
> +
> +              port@0 {
> +                  reg = <0>;
> +
> +                  ldb_from_lcdif2: endpoint {
> +                      remote-endpoint = <&lcdif2_to_ldb>;
> +                  };
> +              };
> +
> +              port@1 {
> +                  reg = <1>;
> +
> +                  ldb_lvds: endpoint {
> +                      remote-endpoint = <&ldb_to_panel>;
> +                  };
> +              };
> +          };

Messed indentation.

> +        };

Best regards,
Krzysztof