RE: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add i.MX8ULP compatible string

G.N. Zhou (OSS)
2025-11-04T07:13:13Z

Hi Laurent,

Thanks for your review.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Monday, October 27, 2025 8:06 AM
> To: G.N. Zhou (OSS) <guoniu.zhou@oss.nxp.com>
> Cc: Rui Miguel Silva <rmfrfs@gmail.com>; Martin Kepplinger
> <martink@posteo.de>; Purism Kernel Team <kernel@puri.sm>; Mauro
> Carvalho Chehab <mchehab@kernel.org>; Rob Herring <robh@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>; Frank Li <frank.li@nxp.com>; linux-
> media@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; G.N. Zhou
> <guoniu.zhou@nxp.com>; Conor Dooley <conor.dooley@microchip.com>
> Subject: Re: [PATCH v7 1/5] media: dt-bindings: nxp,imx8mq-mipi-csi2: Add
> i.MX8ULP compatible string
>
> Hi Guoniu,
>
> On Thu, Oct 23, 2025 at 05:19:42PM +0800, Guoniu Zhou wrote:
> > From: Guoniu Zhou <guoniu.zhou@nxp.com>
> >
> > The CSI-2 receiver in the i.MX8ULP is almost identical to the version
> > present in the i.MX8QXP/QM, but i.MX8ULP CSI-2 controller needs pclk
> > clock as the input clock for its APB interface of Control and Status
> > register(CSR). So add compatible string fsl,imx8ulp-mipi-csi2 and
> > increase maxItems of Clocks (clock-names) to 4 from 3. And keep the
> > same restriction for existing compatible.
> >
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
> > ---
> > .../bindings/media/nxp,imx8mq-mipi-csi2.yaml | 41
> ++++++++++++++++++++--
> > 1 file changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > index
> >
> 3389bab266a9adbda313c8ad795b998641df12f3..da3978da1cab75292ada3f2
> 48374
> > 43f7f4ab6418 100644
> > ---
> > a/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-
> csi2.yam
> > +++ l
> > @@ -20,6 +20,7 @@ properties:
> > - enum:
> > - fsl,imx8mq-mipi-csi2
> > - fsl,imx8qxp-mipi-csi2
> > + - fsl,imx8ulp-mipi-csi2
> > - items:
> > - const: fsl,imx8qm-mipi-csi2
> > - const: fsl,imx8qxp-mipi-csi2 @@ -39,12 +40,16 @@
> > properties:
> > clock that the RX DPHY receives.
> > - description: ui is the pixel clock (phy_ref up to 333Mhz).
> > See the reference manual for details.
> > + - description: pclk is clock for csr APB interface.
> > + minItems: 3
> >
> > clock-names:
> > items:
> > - const: core
> > - const: esc
> > - const: ui
> > + - const: pclk
> > + minItems: 3
> >
> > power-domains:
> > maxItems: 1
> > @@ -130,19 +135,51 @@ allOf:
> > compatible:
> > contains:
> > enum:
> > - - fsl,imx8qxp-mipi-csi2
> > + - fsl,imx8ulp-mipi-csi2
> > + then:
> > + properties:
> > + reg:
> > + minItems: 2
> > + resets:
> > + minItems: 2
> > + maxItems: 2
> > + clocks:
> > + minItems: 4
> > + clock-names:
> > + minItems: 4
>
> Do we need the clock-names constraint ? The DT schemas will enforce that
> clocks and clock-names always have the same number of elements.

Yes, I prefer to have it since explicit constraint could make our yaml file more clear to show
our design purpose. In addition, it can also help people who are not familiar with the implicit
constraint in DT schema avoid making mistakes.

>
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: fsl,imx8qxp-mipi-csi2
> > then:
> > properties:
> > reg:
> > minItems: 2
> > resets:
> > maxItems: 1
> > - else:
> > + clocks:
> > + maxItems: 3
> > + clock-names:
> > + maxItems: 3
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,imx8mq-mipi-csi2
> > + then:
> > properties:
> > reg:
> > maxItems: 1
> > resets:
> > minItems: 3
> > + clocks:
> > + maxItems: 3
> > + clock-names:
> > + maxItems: 3
> > required:
> > - fsl,mipi-phy-gpr
> >
>
> Could you please sort those conditional blocks by alphabetical order of the
> compatible strings ?

Sure

>
> --
> Regards,
>
> Laurent Pinchart