All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Frank Li <Frank.Li@nxp.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rui Miguel Silva <rmfrfs@gmail.com>,
	Martin Kepplinger <martink@posteo.de>,
	Purism Kernel Team <kernel@puri.sm>,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	"Guoniu.zhou" <guoniu.zhou@nxp.com>,
	Robby Cai <robby.cai@nxp.com>,
	Robert Chiras <robert.chiras@nxp.com>,
	Mirela Rabulea <mirela.rabulea@nxp.com>,
	Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Subject: Re: [PATCH 05/14] media: dt-bindings: nxp,imx8-isi: Add i.MX8Q ISI compatible strings
Date: Mon, 3 Feb 2025 16:16:59 -0600	[thread overview]
Message-ID: <20250203221659.GA130749-robh@kernel.org> (raw)
In-Reply-To: <20250131-8qxp_camera-v1-5-319402ab606a@nxp.com>

On Fri, Jan 31, 2025 at 04:33:50PM -0500, Frank Li wrote:
> From: Robert Chiras <robert.chiras@nxp.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@nxp.com>
> Reviewed-by: Robby Cai <robby.cai@nxp.com>
> Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.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

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Frank Li <Frank.Li@nxp.com>
Cc: Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rui Miguel Silva <rmfrfs@gmail.com>,
	Martin Kepplinger <martink@posteo.de>,
	Purism Kernel Team <kernel@puri.sm>,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	"Guoniu.zhou" <guoniu.zhou@nxp.com>,
	Robby Cai <robby.cai@nxp.com>,
	Robert Chiras <robert.chiras@nxp.com>,
	Mirela Rabulea <mirela.rabulea@nxp.com>,
	Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Subject: Re: [PATCH 05/14] media: dt-bindings: nxp,imx8-isi: Add i.MX8Q ISI compatible strings
Date: Mon, 3 Feb 2025 16:16:59 -0600	[thread overview]
Message-ID: <20250203221659.GA130749-robh@kernel.org> (raw)
In-Reply-To: <20250131-8qxp_camera-v1-5-319402ab606a@nxp.com>

On Fri, Jan 31, 2025 at 04:33:50PM -0500, Frank Li wrote:
> From: Robert Chiras <robert.chiras@nxp.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@nxp.com>
> Reviewed-by: Robby Cai <robby.cai@nxp.com>
> Reviewed-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> Reviewed-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2025-02-03 22:17 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-31 21:33 [PATCH 00/14] media: imx8: add camera support Frank Li
2025-01-31 21:33 ` Frank Li
2025-01-31 21:33 ` [PATCH 01/14] dt-bindings: phy: Add MIPI CSI PHY for i.MX8Q Frank Li
2025-01-31 21:33   ` Frank Li
2025-02-03 22:02   ` Rob Herring
2025-02-03 22:02     ` Rob Herring
2025-02-04 15:55     ` Frank Li
2025-02-04 15:55       ` Frank Li
2025-01-31 21:33 ` [PATCH 02/14] phy: freescale: Add MIPI CSI PHY driver " Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 03/14] dt-bindings: reset: Add reset controller for i.MX8QM and i.MX8QXP Frank Li
2025-01-31 21:33   ` Frank Li
2025-02-03 22:06   ` Rob Herring
2025-02-03 22:06     ` Rob Herring
2025-01-31 21:33 ` [PATCH 04/14] reset: imx: Add SCU reset driver for i.MX8QXP and i.MX8QM Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 05/14] media: dt-bindings: nxp,imx8-isi: Add i.MX8Q ISI compatible strings Frank Li
2025-01-31 21:33   ` Frank Li
2025-02-03 22:16   ` Rob Herring [this message]
2025-02-03 22:16     ` Rob Herring
2025-01-31 21:33 ` [PATCH 06/14] media: nxp: imx8-isi: Allow num_sources to be greater than num_sink Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 07/14] media: imx8-isi: Add support for i.MX8QM and i.MX8QXP Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 08/14] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8QM compatible strings Frank Li
2025-01-31 21:33   ` Frank Li
2025-02-03 22:20   ` Rob Herring
2025-02-03 22:20     ` Rob Herring
2025-02-04 15:47     ` Frank Li
2025-02-04 15:47       ` Frank Li
2025-01-31 21:33 ` [PATCH 09/14] media: imx8mq-mipi-csi2: Add imx8mq_plat_data for different " Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 10/14] media: imx8mq-mipi-csi2: Add support for i.MX8QM Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 11/14] arm64: dts: imx8: add capture controller for i.MX8's img subsystem Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 12/14] arm64: dts: imx8qm: add 24MHz clock-xtal24m Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 13/14] arm64: dts: imx8q: add linux,cma node for imx8qm-mek and imx8qxp-mek Frank Li
2025-01-31 21:33   ` Frank Li
2025-01-31 21:33 ` [PATCH 14/14] arm64: dts: imx8q: add camera ov5640 support " Frank Li
2025-01-31 21:33   ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250203221659.GA130749-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=guoniu.zhou@nxp.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=kernel@puri.sm \
    --cc=kishon@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=laurentiu.palcu@oss.nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=martink@posteo.de \
    --cc=mchehab@kernel.org \
    --cc=mirela.rabulea@nxp.com \
    --cc=p.zabel@pengutronix.de \
    --cc=rmfrfs@gmail.com \
    --cc=robby.cai@nxp.com \
    --cc=robert.chiras@nxp.com \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.