diff mbox series

[6/8] dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

Message ID 20250608235840.23871-7-laurent.pinchart@ideasonboard.com
State Not Applicable
Headers show
Series media: imx-mipi-csis: Cleanups and debugging improvements | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema success

Commit Message

Laurent Pinchart June 8, 2025, 11:58 p.m. UTC
The CSI-2 receiver can be instantiated with up to four output channels.
This is an integration-specific property, specify the number of
instantiated channels through a new fsl,num-channels property. The
property is optional, and defaults to 1 as only one channel is currently
supported by drivers.

The only known SoC to have more than one channel is the i.MX8MP. As the
binding examples do not cover that SoC, don't update them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Frank Li June 9, 2025, 3:32 p.m. UTC | #1
On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> The CSI-2 receiver can be instantiated with up to four output channels.
> This is an integration-specific property, specify the number of
> instantiated channels through a new fsl,num-channels property. The
> property is optional, and defaults to 1 as only one channel is currently
> supported by drivers.
>
> The only known SoC to have more than one channel is the i.MX8MP. As the
> binding examples do not cover that SoC, don't update them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> index db4889bf881e..41ad5b84eaeb 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> @@ -68,6 +68,13 @@ properties:
>      default: 166000000
>      deprecated: true
>
> +  fsl,num-channels:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of output channels
> +    minimum: 1
> +    maximum: 4
> +    default: 1
> +

Look like it is fixed value for each compabiable string, So it is not
suitable for adding new property. It should be in driver data of each
compatible strings.

I met similar case before. DT team generally don't agree on add such
property, unless there are two instances in the same chip, which have
difference channel number.

Frank

>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
>
> --
> Regards,
>
> Laurent Pinchart
>
Adam Ford June 9, 2025, 5:53 p.m. UTC | #2
On Mon, Jun 9, 2025 at 10:32 AM Frank Li <Frank.li@nxp.com> wrote:
>
> On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > The CSI-2 receiver can be instantiated with up to four output channels.
> > This is an integration-specific property, specify the number of
> > instantiated channels through a new fsl,num-channels property. The
> > property is optional, and defaults to 1 as only one channel is currently
> > supported by drivers.
> >
> > The only known SoC to have more than one channel is the i.MX8MP. As the
> > binding examples do not cover that SoC, don't update them.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > index db4889bf881e..41ad5b84eaeb 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > @@ -68,6 +68,13 @@ properties:
> >      default: 166000000
> >      deprecated: true
> >
> > +  fsl,num-channels:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Number of output channels
> > +    minimum: 1
> > +    maximum: 4
> > +    default: 1
> > +
>
> Look like it is fixed value for each compabiable string, So it is not
> suitable for adding new property. It should be in driver data of each
> compatible strings.
>
> I met similar case before. DT team generally don't agree on add such
> property, unless there are two instances in the same chip, which have
> difference channel number.

If the DT changes are rejected, can the number of channels be added to
the data structure inside mipi_csis_of_match?  We have compatibles for
8mm and imx7.  If we add an imx8mp compatible we could add a reference
to the number of channels.

adam
>
> Frank
>
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
>
Frank Li June 9, 2025, 5:58 p.m. UTC | #3
On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> On Mon, Jun 9, 2025 at 10:32 AM Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > This is an integration-specific property, specify the number of
> > > instantiated channels through a new fsl,num-channels property. The
> > > property is optional, and defaults to 1 as only one channel is currently
> > > supported by drivers.
> > >
> > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > binding examples do not cover that SoC, don't update them.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > index db4889bf881e..41ad5b84eaeb 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > @@ -68,6 +68,13 @@ properties:
> > >      default: 166000000
> > >      deprecated: true
> > >
> > > +  fsl,num-channels:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Number of output channels
> > > +    minimum: 1
> > > +    maximum: 4
> > > +    default: 1
> > > +
> >
> > Look like it is fixed value for each compabiable string, So it is not
> > suitable for adding new property. It should be in driver data of each
> > compatible strings.
> >
> > I met similar case before. DT team generally don't agree on add such
> > property, unless there are two instances in the same chip, which have
> > difference channel number.
>
> If the DT changes are rejected, can the number of channels be added to
> the data structure inside mipi_csis_of_match?  We have compatibles for
> 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> to the number of channels.

Yes, that's prefer method.

Frank

>
> adam
> >
> > Frank
> >
> > >    ports:
> > >      $ref: /schemas/graph.yaml#/properties/ports
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > >
> >
Laurent Pinchart June 9, 2025, 6:20 p.m. UTC | #4
On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > This is an integration-specific property, specify the number of
> > > instantiated channels through a new fsl,num-channels property. The
> > > property is optional, and defaults to 1 as only one channel is currently
> > > supported by drivers.
> > >
> > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > binding examples do not cover that SoC, don't update them.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > index db4889bf881e..41ad5b84eaeb 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > @@ -68,6 +68,13 @@ properties:
> > >      default: 166000000
> > >      deprecated: true
> > >
> > > +  fsl,num-channels:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Number of output channels
> > > +    minimum: 1
> > > +    maximum: 4
> > > +    default: 1
> > > +
> >
> > Look like it is fixed value for each compabiable string, So it is not
> > suitable for adding new property. It should be in driver data of each
> > compatible strings.
> >
> > I met similar case before. DT team generally don't agree on add such
> > property, unless there are two instances in the same chip, which have
> > difference channel number.
> 
> If the DT changes are rejected, can the number of channels be added to
> the data structure inside mipi_csis_of_match?  We have compatibles for
> 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> to the number of channels.

I thought about it, and decided to add a new property because the number
of channels is really a synthesis time configuration parameter, and
could differ between different CSIS instances in the same SoC.

> > >    ports:
> > >      $ref: /schemas/graph.yaml#/properties/ports
> > >
Frank Li June 9, 2025, 7:08 p.m. UTC | #5
On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > This is an integration-specific property, specify the number of
> > > > instantiated channels through a new fsl,num-channels property. The
> > > > property is optional, and defaults to 1 as only one channel is currently
> > > > supported by drivers.
> > > >
> > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > binding examples do not cover that SoC, don't update them.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > @@ -68,6 +68,13 @@ properties:
> > > >      default: 166000000
> > > >      deprecated: true
> > > >
> > > > +  fsl,num-channels:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: Number of output channels
> > > > +    minimum: 1
> > > > +    maximum: 4
> > > > +    default: 1
> > > > +
> > >
> > > Look like it is fixed value for each compabiable string, So it is not
> > > suitable for adding new property. It should be in driver data of each
> > > compatible strings.
> > >
> > > I met similar case before. DT team generally don't agree on add such
> > > property, unless there are two instances in the same chip, which have
> > > difference channel number.
> >
> > If the DT changes are rejected, can the number of channels be added to
> > the data structure inside mipi_csis_of_match?  We have compatibles for
> > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > to the number of channels.
>
> I thought about it, and decided to add a new property because the number
> of channels is really a synthesis time configuration parameter, and
> could differ between different CSIS instances in the same SoC.

Need add such information at binding doc's commit message, ideally provide
an example for it.

Frank
>
> > > >    ports:
> > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > >
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart June 10, 2025, 8:18 a.m. UTC | #6
Hi Frank,

On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > This is an integration-specific property, specify the number of
> > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > supported by drivers.
> > > > >
> > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > binding examples do not cover that SoC, don't update them.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > ---
> > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > >  1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > @@ -68,6 +68,13 @@ properties:
> > > > >      default: 166000000
> > > > >      deprecated: true
> > > > >
> > > > > +  fsl,num-channels:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +    description: Number of output channels
> > > > > +    minimum: 1
> > > > > +    maximum: 4
> > > > > +    default: 1
> > > > > +
> > > >
> > > > Look like it is fixed value for each compabiable string, So it is not
> > > > suitable for adding new property. It should be in driver data of each
> > > > compatible strings.
> > > >
> > > > I met similar case before. DT team generally don't agree on add such
> > > > property, unless there are two instances in the same chip, which have
> > > > difference channel number.
> > >
> > > If the DT changes are rejected, can the number of channels be added to
> > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > to the number of channels.
> >
> > I thought about it, and decided to add a new property because the number
> > of channels is really a synthesis time configuration parameter, and
> > could differ between different CSIS instances in the same SoC.
> 
> Need add such information at binding doc's commit message,

I'll update the commit message.

> ideally provide an example for it.

That I can't provide because the few SoCs I'm working with do not
integrate multiple CSIS instances with different parameters.

> > > > >    ports:
> > > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > > >
Laurent Pinchart June 19, 2025, 9:02 p.m. UTC | #7
On Tue, Jun 10, 2025 at 11:18:29AM +0300, Laurent Pinchart wrote:
> On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> > On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > > This is an integration-specific property, specify the number of
> > > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > > supported by drivers.
> > > > > >
> > > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > > binding examples do not cover that SoC, don't update them.
> > > > > >
> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > > >  1 file changed, 7 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > @@ -68,6 +68,13 @@ properties:
> > > > > >      default: 166000000
> > > > > >      deprecated: true
> > > > > >
> > > > > > +  fsl,num-channels:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    description: Number of output channels
> > > > > > +    minimum: 1
> > > > > > +    maximum: 4
> > > > > > +    default: 1
> > > > > > +
> > > > >
> > > > > Look like it is fixed value for each compabiable string, So it is not
> > > > > suitable for adding new property. It should be in driver data of each
> > > > > compatible strings.
> > > > >
> > > > > I met similar case before. DT team generally don't agree on add such
> > > > > property, unless there are two instances in the same chip, which have
> > > > > difference channel number.
> > > >
> > > > If the DT changes are rejected, can the number of channels be added to
> > > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > > to the number of channels.
> > >
> > > I thought about it, and decided to add a new property because the number
> > > of channels is really a synthesis time configuration parameter, and
> > > could differ between different CSIS instances in the same SoC.
> > 
> > Need add such information at binding doc's commit message,
> 
> I'll update the commit message.

The commit message in v2 will state

    dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property

    The CSI-2 receiver can be instantiated with up to four output channels.
    This is an integration-specific property, specify the number of
    instantiated channels through a new fsl,num-channels property. The
    property is optional, and defaults to 1 as only one channel is currently
    supported by drivers.

    Using the compatible string to infer the number of channels has been
    considered, but multiple instances of the same CSIS in the same SoC
    could conceptually be synthesized with a different number of channels.
    An explicit property is therefore more appropriate.

    The only known SoC to have more than one channel is the i.MX8MP. As the
    binding examples do not cover that SoC, don't update them.

Rob, Krzysztof, Conor, are you fine with adding this property ?

> > ideally provide an example for it.
> 
> That I can't provide because the few SoCs I'm working with do not
> integrate multiple CSIS instances with different parameters.
> 
> > > > > >    ports:
> > > > > >      $ref: /schemas/graph.yaml#/properties/ports
> > > > > >
Rob Herring (Arm) June 25, 2025, 7:27 p.m. UTC | #8
On Fri, Jun 20, 2025 at 12:02:37AM +0300, Laurent Pinchart wrote:
> On Tue, Jun 10, 2025 at 11:18:29AM +0300, Laurent Pinchart wrote:
> > On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> > > On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > > > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > > > This is an integration-specific property, specify the number of
> > > > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > > > supported by drivers.
> > > > > > >
> > > > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > > > binding examples do not cover that SoC, don't update them.
> > > > > > >
> > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > > > >  1 file changed, 7 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > @@ -68,6 +68,13 @@ properties:
> > > > > > >      default: 166000000
> > > > > > >      deprecated: true
> > > > > > >
> > > > > > > +  fsl,num-channels:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > +    description: Number of output channels
> > > > > > > +    minimum: 1
> > > > > > > +    maximum: 4
> > > > > > > +    default: 1
> > > > > > > +
> > > > > >
> > > > > > Look like it is fixed value for each compabiable string, So it is not
> > > > > > suitable for adding new property. It should be in driver data of each
> > > > > > compatible strings.
> > > > > >
> > > > > > I met similar case before. DT team generally don't agree on add such
> > > > > > property, unless there are two instances in the same chip, which have
> > > > > > difference channel number.
> > > > >
> > > > > If the DT changes are rejected, can the number of channels be added to
> > > > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > > > to the number of channels.
> > > >
> > > > I thought about it, and decided to add a new property because the number
> > > > of channels is really a synthesis time configuration parameter, and
> > > > could differ between different CSIS instances in the same SoC.
> > > 
> > > Need add such information at binding doc's commit message,
> > 
> > I'll update the commit message.
> 
> The commit message in v2 will state
> 
>     dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property
> 
>     The CSI-2 receiver can be instantiated with up to four output channels.
>     This is an integration-specific property, specify the number of
>     instantiated channels through a new fsl,num-channels property. The
>     property is optional, and defaults to 1 as only one channel is currently
>     supported by drivers.
> 
>     Using the compatible string to infer the number of channels has been
>     considered, but multiple instances of the same CSIS in the same SoC
>     could conceptually be synthesized with a different number of channels.
>     An explicit property is therefore more appropriate.
> 
>     The only known SoC to have more than one channel is the i.MX8MP. As the
>     binding examples do not cover that SoC, don't update them.

So how many channels does i.MX8MP have in a DT without this property? 
It's either 1 or the driver overrides it to 2.

> Rob, Krzysztof, Conor, are you fine with adding this property ?

Yes, but seems a little late.

Rob
Laurent Pinchart June 25, 2025, 7:34 p.m. UTC | #9
On Wed, Jun 25, 2025 at 02:27:28PM -0500, Rob Herring wrote:
> On Fri, Jun 20, 2025 at 12:02:37AM +0300, Laurent Pinchart wrote:
> > On Tue, Jun 10, 2025 at 11:18:29AM +0300, Laurent Pinchart wrote:
> > > On Mon, Jun 09, 2025 at 03:08:39PM -0400, Frank Li wrote:
> > > > On Mon, Jun 09, 2025 at 09:20:33PM +0300, Laurent Pinchart wrote:
> > > > > On Mon, Jun 09, 2025 at 12:53:48PM -0500, Adam Ford wrote:
> > > > > > On Mon, Jun 9, 2025 at 10:32 AM Frank Li wrote:
> > > > > > > On Mon, Jun 09, 2025 at 02:58:38AM +0300, Laurent Pinchart wrote:
> > > > > > > > The CSI-2 receiver can be instantiated with up to four output channels.
> > > > > > > > This is an integration-specific property, specify the number of
> > > > > > > > instantiated channels through a new fsl,num-channels property. The
> > > > > > > > property is optional, and defaults to 1 as only one channel is currently
> > > > > > > > supported by drivers.
> > > > > > > >
> > > > > > > > The only known SoC to have more than one channel is the i.MX8MP. As the
> > > > > > > > binding examples do not cover that SoC, don't update them.
> > > > > > > >
> > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > > > > ---
> > > > > > > >  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
> > > > > > > >  1 file changed, 7 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > > index db4889bf881e..41ad5b84eaeb 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > > +++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
> > > > > > > > @@ -68,6 +68,13 @@ properties:
> > > > > > > >      default: 166000000
> > > > > > > >      deprecated: true
> > > > > > > >
> > > > > > > > +  fsl,num-channels:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > +    description: Number of output channels
> > > > > > > > +    minimum: 1
> > > > > > > > +    maximum: 4
> > > > > > > > +    default: 1
> > > > > > > > +
> > > > > > >
> > > > > > > Look like it is fixed value for each compabiable string, So it is not
> > > > > > > suitable for adding new property. It should be in driver data of each
> > > > > > > compatible strings.
> > > > > > >
> > > > > > > I met similar case before. DT team generally don't agree on add such
> > > > > > > property, unless there are two instances in the same chip, which have
> > > > > > > difference channel number.
> > > > > >
> > > > > > If the DT changes are rejected, can the number of channels be added to
> > > > > > the data structure inside mipi_csis_of_match?  We have compatibles for
> > > > > > 8mm and imx7.  If we add an imx8mp compatible we could add a reference
> > > > > > to the number of channels.
> > > > >
> > > > > I thought about it, and decided to add a new property because the number
> > > > > of channels is really a synthesis time configuration parameter, and
> > > > > could differ between different CSIS instances in the same SoC.
> > > > 
> > > > Need add such information at binding doc's commit message,
> > > 
> > > I'll update the commit message.
> > 
> > The commit message in v2 will state
> > 
> >     dt-bindings: media: nxp,imx-mipi-csi2: Add fsl,num-channels property
> > 
> >     The CSI-2 receiver can be instantiated with up to four output channels.
> >     This is an integration-specific property, specify the number of
> >     instantiated channels through a new fsl,num-channels property. The
> >     property is optional, and defaults to 1 as only one channel is currently
> >     supported by drivers.
> > 
> >     Using the compatible string to infer the number of channels has been
> >     considered, but multiple instances of the same CSIS in the same SoC
> >     could conceptually be synthesized with a different number of channels.
> >     An explicit property is therefore more appropriate.
> > 
> >     The only known SoC to have more than one channel is the i.MX8MP. As the
> >     binding examples do not cover that SoC, don't update them.
> 
> So how many channels does i.MX8MP have in a DT without this property? 
> It's either 1 or the driver overrides it to 2.

Only 1. I only realized a few weeks ago that the CSIS has multiple
output channels on i.MX8MP. This wasn't documented anywhere, and
understanding how all of this works took lots of trial-and-error.

> > Rob, Krzysztof, Conor, are you fine with adding this property ?
> 
> Yes, but seems a little late.

It is, I wish I had known about this hardware feature years ago. Now I
need to figure out how to properly support it in the driver and expose
it to userspace through V4L2 without breaking backward compatibility
with existing applications. The DT binding is the easy part :-)
Rob Herring (Arm) June 26, 2025, 11:22 p.m. UTC | #10
On Mon, 09 Jun 2025 02:58:38 +0300, Laurent Pinchart wrote:
> The CSI-2 receiver can be instantiated with up to four output channels.
> This is an integration-specific property, specify the number of
> instantiated channels through a new fsl,num-channels property. The
> property is optional, and defaults to 1 as only one channel is currently
> supported by drivers.
> 
> The only known SoC to have more than one channel is the i.MX8MP. As the
> binding examples do not cover that SoC, don't update them.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx-mipi-csi2.yaml       | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
index db4889bf881e..41ad5b84eaeb 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
@@ -68,6 +68,13 @@  properties:
     default: 166000000
     deprecated: true
 
+  fsl,num-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of output channels
+    minimum: 1
+    maximum: 4
+    default: 1
+
   ports:
     $ref: /schemas/graph.yaml#/properties/ports