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

Krzysztof Kozlowski
2025-03-05T07:13:08Z

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@ew.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