Message-ID: <20250203221659.GA130749-robh@kernel.org>
Date: Mon, 3 Feb 2025 16:16:59 -0600
From: Rob Herring <robh@...nel.org>
To: Frank Li <Frank.Li@....com>
Cc: Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...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>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Rui Miguel Silva <rmfrfs@...il.com>,
	Martin Kepplinger <martink@...teo.de>,
	Purism Kernel Team <kernel@...i.sm>, linux-phy@...ts.infradead.org,
	devicetree@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-media@...r.kernel.org, "Guoniu.zhou" <guoniu.zhou@....com>,
	Robby Cai <robby.cai@....com>,
	Robert Chiras <robert.chiras@....com>,
	Mirela Rabulea <mirela.rabulea@....com>,
	Laurentiu Palcu <laurentiu.palcu@....nxp.com>
Subject: Re: [PATCH 05/14] media: dt-bindings: nxp,imx8-isi: Add i.MX8Q ISI
 compatible strings

On Fri, Jan 31, 2025 at 04:33:50PM -0500, Frank Li wrote:
> From: Robert Chiras <robert.chiras@....com>
> 
> Add compatible strings for i.MX8QM and i.MX8QXP platforms.
> 
> Increase the number of max interrupts and clock to 8. i.MX8QM have 8
> channels and i.MX8QXP have 5 channels. Each channel requires one clock
> source and interrupt.
> 
> Remove fsl,blk-ctrl from required list because i.MX8Q needn't it.
> 
> i.MX8QM use port@2 and port@3. i.MX8QXP use port@2 and port@6.
> 
> Keep the same restriction for the other platform.
> 
> Signed-off-by: Robert Chiras <robert.chiras@....com>
> Reviewed-by: Robby Cai <robby.cai@....com>
> Reviewed-by: Mirela Rabulea <mirela.rabulea@....com>
> Reviewed-by: Laurentiu Palcu <laurentiu.palcu@....nxp.com>
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  .../devicetree/bindings/media/nxp,imx8-isi.yaml    | 87 +++++++++++++++++++---
>  1 file changed, 75 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
> index f43b91984f015..b713c8ba79e39 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-isi.yaml
> @@ -21,6 +21,8 @@ properties:
>      enum:
>        - fsl,imx8mn-isi
>        - fsl,imx8mp-isi
> +      - fsl,imx8qm-isi
> +      - fsl,imx8qxp-isi
>        - fsl,imx8ulp-isi
>        - fsl,imx93-isi
>  
> @@ -28,17 +30,12 @@ properties:
>      maxItems: 1
>  
>    clocks:
> -    items:
> -      - description: The AXI clock
> -      - description: The APB clock
> -      # TODO: Check if the per-channel ipg_proc_clk clocks need to be specified
> -      # as well, in case some SoCs have the ability to control them separately.
> -      # This may be the case of the i.MX8[DQ]X(P)
> +    minItems: 1
> +    maxItems: 8

Isn't the minimum still 2?

>  
>    clock-names:
> -    items:
> -      - const: axi
> -      - const: apb
> +    minItems: 1
> +    maxItems: 8
>  
>    fsl,blk-ctrl:
>      $ref: /schemas/types.yaml#/definitions/phandle
> @@ -49,10 +46,11 @@ properties:
>    interrupts:
>      description: Processing pipeline interrupts, one per pipeline
>      minItems: 1
> -    maxItems: 2
> +    maxItems: 8
>  
>    power-domains:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 8
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -66,7 +64,6 @@ required:
>    - interrupts
>    - clocks
>    - clock-names
> -  - fsl,blk-ctrl
>    - ports
>  
>  allOf:
> @@ -79,9 +76,17 @@ allOf:
>                - fsl,imx8ulp-isi
>                - fsl,imx93-isi
>      then:
> +      required:
> +        - fsl,blk-ctrl
>        properties:
>          interrupts:
>            maxItems: 1
> +        clocks:
> +          maxItems: 2
> +        clock-names:
> +          items:
> +            - const: axi
> +            - const: apb
>          ports:
>            properties:
>              port@0:
> @@ -96,9 +101,17 @@ allOf:
>            contains:
>              const: fsl,imx8mp-isi
>      then:
> +      required:
> +        - fsl,blk-ctrl
>        properties:
>          interrupts:
>            maxItems: 2
> +        clocks:
> +          maxItems: 2
> +        clock-names:
> +          items:
> +            - const: axi
> +            - const: apb
>          ports:
>            properties:
>              port@0:
> @@ -109,6 +122,56 @@ allOf:
>              - port@0
>              - port@1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx8qm-isi
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 8
> +        clock-names:
> +          items:
> +            pattern: "^per[0-7]"
> +        interrupts:
> +          minItems: 8
> +        ports:
> +          properties:
> +            port@2:
> +              description: MIPI CSI-2 RX 0
> +            port@3:
> +              description: MIPI CSI-2 RX 1
> +          required:
> +            - port@2
> +            - port@3

This schema is completely missing proper schemas for port nodes. It 
needs to reference the port schema for each port. That should be at the 
top-level.

I think this addition is borderline whether it should be its own schema 
doc. The if/then schemas are larger than the main part. The ports are 
not even the same.

Rob

Powered by blists - more mailing lists