[PATCH 0/5] media/i2c: max96717: a few changes

[PATCH 0/5] media/i2c: max96717: a few changes

[Laurentiu Palcu]

Hi,

This series adds support for:
 * get_frame_sync(), so that we can pass the VCs and DTs from the
   incoming sensor streams over to the downstream drivers (CSI, ISI);
 * frame synchronization in multi-sensor setups. This one comes with
   some dt-bindings changes in order to allow the user to specify the
   incoming GPIO RX ID and the output pin;
 * operation mode override. This would allow toggling from the pin
   configured mode (for example: pixel mode) to the other one (tunneling
   mode). Also, vice versa is possible as well but the driver only
   supports tunneling mode currently;

On the frame synchronization bindings, I would need some advice on the
property naming. Recently, I sent a RFC adding support for MAX96724
deserializer chip, see [1], and I also added support for FSYNC for that
chip. The max96724 property is also named "maxim,fsync-config" but,
since the chip is a deserializer and it's the one actually sending the
FSYNC signal, that property has an extra item: the fsync mode.

My question is: would it be OK to have the same "maxim,fsync-config"
property name for both or should we have different names?

[1] https://patchwork.linuxtv.org/project/linux-media/list/?series=14427

Thanks,
Laurentiu

Laurentiu Palcu (5):
  media/i2c: max96717: change internal regulator voltage
  media/i2c: max96717: implement the .get_frame_desc() operation
  dt-bindings: i2c: maxim,max96717: add new properties
  media/i2c: max96717: add FSYNC support
  media/i2c: max96717: allow user to override operation mode from DT

 .../bindings/media/i2c/maxim,max96717.yaml    |  28 ++++
 drivers/media/i2c/max96717.c                  | 137 +++++++++++++++++-
 2 files changed, 157 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Laurentiu Palcu]

Add 'maxim,override-mode' property to allow the user to toggle the pin
configured chip operation mode and 'maxim,fsync-config' to configure the
chip for relaying a frame synchronization signal, received from
deserializer, to the attached sensor. The latter is needed for
synchronizing the images in multi-sensor setups.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
index d1e8ba6e368ec..fae578d55fd4d 100644
--- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
@@ -42,10 +42,35 @@ properties:
       number must be in range of [0, 10].
 
   gpio-controller: true
+  gpio-reserved-ranges: true
 
   '#clock-cells':
     const: 0
 
+  maxim,override-mode:
+    description: Toggle the operation mode from the pin configured one.
+    type: boolean
+
+  maxim,fsync-config:
+    description:
+      Frame synchronization (FSYNC) is used to align images sent from multiple
+      sources in surround-view applications and is required for concatenation.
+      In FSYNC mode, the deserializer sends a sync signal to each serializer;
+      the serializers then send the signal to the connected sensor.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - description: FSYNC RX ID, needs to match the TX ID configured in the deserializer.
+        minimum: 0
+        maximum: 31
+        default: 0
+      - description:
+          Output GPIO pin used for sending the FSYNC to the sensor. The pin, however, needs
+          to be excluded from the gpiochip using the gpio-reserved-ranges property since
+          it will be used exclusively for FSYNC generation.
+        minimum: 0
+        maximum: 10
+        default: 0
+
   reg:
     maxItems: 1
 
@@ -113,6 +138,9 @@ examples:
             #gpio-cells = <2>;
             #clock-cells = <0>;
 
+            gpio-reserved-ranges = <0 1>;
+            maxim,fsync-config = <0 0>;
+
             ports {
                 #address-cells = <1>;
                 #size-cells = <0>;
-- 
2.34.1

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

On Fri, Feb 07, 2025 at 01:29:55PM +0200, Laurentiu Palcu wrote:
> Add 'maxim,override-mode' property to allow the user to toggle the pin
> configured chip operation mode and 'maxim,fsync-config' to configure the
> chip for relaying a frame synchronization signal, received from
> deserializer, to the attached sensor. The latter is needed for
> synchronizing the images in multi-sensor setups.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> index d1e8ba6e368ec..fae578d55fd4d 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> @@ -42,10 +42,35 @@ properties:
>        number must be in range of [0, 10].
>  
>    gpio-controller: true
> +  gpio-reserved-ranges: true
>  
>    '#clock-cells':
>      const: 0
>  
> +  maxim,override-mode:
> +    description: Toggle the operation mode from the pin configured one.
> +    type: boolean

type: flag


> +  maxim,fsync-config:
> +    description:
> +      Frame synchronization (FSYNC) is used to align images sent from multiple
> +      sources in surround-view applications and is required for concatenation.
> +      In FSYNC mode, the deserializer sends a sync signal to each serializer;
> +      the serializers then send the signal to the connected sensor.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      - description: FSYNC RX ID, needs to match the TX ID configured in the deserializer.
> +        minimum: 0
> +        maximum: 31
> +        default: 0
> +      - description:
> +          Output GPIO pin used for sending the FSYNC to the sensor. The pin, however, needs
> +          to be excluded from the gpiochip using the gpio-reserved-ranges property since
> +          it will be used exclusively for FSYNC generation.
> +        minimum: 0
> +        maximum: 10
> +        default: 0
> +
>    reg:
>      maxItems: 1
>  
> @@ -113,6 +138,9 @@ examples:
>              #gpio-cells = <2>;
>              #clock-cells = <0>;
>  
> +            gpio-reserved-ranges = <0 1>;
> +            maxim,fsync-config = <0 0>;
> +
>              ports {
>                  #address-cells = <1>;
>                  #size-cells = <0>;
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Rob Herring]

On Tue, Feb 11, 2025 at 06:46:10PM +0000, Conor Dooley wrote:
> On Fri, Feb 07, 2025 at 01:29:55PM +0200, Laurentiu Palcu wrote:
> > Add 'maxim,override-mode' property to allow the user to toggle the pin
> > configured chip operation mode and 'maxim,fsync-config' to configure the
> > chip for relaying a frame synchronization signal, received from
> > deserializer, to the attached sensor. The latter is needed for
> > synchronizing the images in multi-sensor setups.
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > ---
> >  .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > index d1e8ba6e368ec..fae578d55fd4d 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > @@ -42,10 +42,35 @@ properties:
> >        number must be in range of [0, 10].
> >  
> >    gpio-controller: true
> > +  gpio-reserved-ranges: true
> >  
> >    '#clock-cells':
> >      const: 0
> >  
> > +  maxim,override-mode:
> > +    description: Toggle the operation mode from the pin configured one.
> > +    type: boolean
> 
> type: flag

Err, no.

You can do as-is or:

$ref: /schemas/types.yaml#/definitions/flag

I am neutral as to which way. If I wasn't we'd make the meta-schema 
enforce one way or the other.

Rob

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Rob Herring (Arm)]

On Fri, 07 Feb 2025 13:29:55 +0200, Laurentiu Palcu wrote:
> Add 'maxim,override-mode' property to allow the user to toggle the pin
> configured chip operation mode and 'maxim,fsync-config' to configure the
> chip for relaying a frame synchronization signal, received from
> deserializer, to the attached sensor. The latter is needed for
> synchronizing the images in multi-sensor setups.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 

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

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

On Wed, Feb 12, 2025 at 11:42:09AM -0600, Rob Herring wrote:
> On Tue, Feb 11, 2025 at 06:46:10PM +0000, Conor Dooley wrote:
> > On Fri, Feb 07, 2025 at 01:29:55PM +0200, Laurentiu Palcu wrote:
> > > Add 'maxim,override-mode' property to allow the user to toggle the pin
> > > configured chip operation mode and 'maxim,fsync-config' to configure the
> > > chip for relaying a frame synchronization signal, received from
> > > deserializer, to the attached sensor. The latter is needed for
> > > synchronizing the images in multi-sensor setups.
> > > 
> > > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > > ---
> > >  .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > > index d1e8ba6e368ec..fae578d55fd4d 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > > @@ -42,10 +42,35 @@ properties:
> > >        number must be in range of [0, 10].
> > >  
> > >    gpio-controller: true
> > > +  gpio-reserved-ranges: true
> > >  
> > >    '#clock-cells':
> > >      const: 0
> > >  
> > > +  maxim,override-mode:
> > > +    description: Toggle the operation mode from the pin configured one.
> > > +    type: boolean
> > 
> > type: flag
> 
> Err, no.
> 
> You can do as-is or:
> 
> $ref: /schemas/types.yaml#/definitions/flag

Eh, that's sloppy. I must have been rushing or distracted. Sorry.

> I am neutral as to which way. If I wasn't we'd make the meta-schema 
> enforce one way or the other.

I'm biased towards flag, since I've seen confusion about setting the
boolean ones to false to disable them a bunch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Julien Massot]

Hi Laurentiu,

Thanks for your patch

On Fri, 2025-02-07 at 13:29 +0200, Laurentiu Palcu wrote:
> Add 'maxim,override-mode' property to allow the user to toggle the pin
> configured chip operation mode and 'maxim,fsync-config' to configure the
> chip for relaying a frame synchronization signal, received from
> deserializer, to the attached sensor. The latter is needed for
> synchronizing the images in multi-sensor setups.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> index d1e8ba6e368ec..fae578d55fd4d 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> @@ -42,10 +42,35 @@ properties:
>        number must be in range of [0, 10].
>  
>    gpio-controller: true
> +  gpio-reserved-ranges: true
>  
>    '#clock-cells':
>      const: 0
>  
> +  maxim,override-mode:
> +    description: Toggle the operation mode from the pin configured one.
> +    type: boolean
I understand that this property is intended to flip the GMSL link mode between
pixel and tunnel mode.
What about adding a property 'maxim,tunnel-mode' to the GMSL 'port@1'.
Here the MAX96717 only have one GMSL port but other devices, such as MAX96724 can
have 2 GMSL link and may have each link in different mode.

> 
> +
> +  maxim,fsync-config:
> +    description:
> +      Frame synchronization (FSYNC) is used to align images sent from multiple
> +      sources in surround-view applications and is required for concatenation.
> +      In FSYNC mode, the deserializer sends a sync signal to each serializer;
> +      the serializers then send the signal to the connected sensor.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      - description: FSYNC RX ID, needs to match the TX ID configured in the deserializer.
> +        minimum: 0
> +        maximum: 31
> +        default: 0
> +      - description:
> +          Output GPIO pin used for sending the FSYNC to the sensor. The pin, however, needs
> +          to be excluded from the gpiochip using the gpio-reserved-ranges property since
> +          it will be used exclusively for FSYNC generation.
> +        minimum: 0
> +        maximum: 10
> +        default: 0
> +

MAX96717 do not have any knowledge of the frame synchronisation, but this device can forward some
GPIO to/from the deserializer.

GPIO forwarding need some information 
- The local GPIO number
- The forwarding direction Rx, Tx, Bi-directionnal
- The GPIO ID on the GMSL link (RX_ID/TX_ID)

Can we add a maxim,forward-gpio property reflecting that instead ?

>    reg:
>      maxItems: 1
>  
> @@ -113,6 +138,9 @@ examples:
>              #gpio-cells = <2>;
>              #clock-cells = <0>;
>  
> +            gpio-reserved-ranges = <0 1>;
> +            maxim,fsync-config = <0 0>;
> +
>              ports {
>                  #address-cells = <1>;
>                  #size-cells = <0>;

Regards,
-- 
Julien

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Laurentiu Palcu]

Hi,

On Tue, Feb 18, 2025 at 02:54:07PM +0100, Julien Massot wrote:
> Hi Laurentiu,
> 
> Thanks for your patch
> 
> On Fri, 2025-02-07 at 13:29 +0200, Laurentiu Palcu wrote:
> > Add 'maxim,override-mode' property to allow the user to toggle the pin
> > configured chip operation mode and 'maxim,fsync-config' to configure the
> > chip for relaying a frame synchronization signal, received from
> > deserializer, to the attached sensor. The latter is needed for
> > synchronizing the images in multi-sensor setups.
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > ---
> >  .../bindings/media/i2c/maxim,max96717.yaml    | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > index d1e8ba6e368ec..fae578d55fd4d 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> > @@ -42,10 +42,35 @@ properties:
> >        number must be in range of [0, 10].
> >  
> >    gpio-controller: true
> > +  gpio-reserved-ranges: true
> >  
> >    '#clock-cells':
> >      const: 0
> >  
> > +  maxim,override-mode:
> > +    description: Toggle the operation mode from the pin configured one.
> > +    type: boolean
> I understand that this property is intended to flip the GMSL link mode between
> pixel and tunnel mode.
> What about adding a property 'maxim,tunnel-mode' to the GMSL 'port@1'.
> Here the MAX96717 only have one GMSL port but other devices, such as MAX96724 can
> have 2 GMSL link and may have each link in different mode.

I'm OK with moving the property inside "port@1". But I have some
concerns about the logic. So. 'maxim,tunnel-mode' presence would
indicate that we want to force the functioning mode to "tunnel". But
what if it's absent? Do we use the pin configuration? What if the pin
configuration is "tunnel" and the user wants to override the mode to
"pixel"? In this case 'maxim,tunnel-mode' doesn't really work...
Am I missing something here?

> 
> > 
> > +
> > +  maxim,fsync-config:
> > +    description:
> > +      Frame synchronization (FSYNC) is used to align images sent from multiple
> > +      sources in surround-view applications and is required for concatenation.
> > +      In FSYNC mode, the deserializer sends a sync signal to each serializer;
> > +      the serializers then send the signal to the connected sensor.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    items:
> > +      - description: FSYNC RX ID, needs to match the TX ID configured in the deserializer.
> > +        minimum: 0
> > +        maximum: 31
> > +        default: 0
> > +      - description:
> > +          Output GPIO pin used for sending the FSYNC to the sensor. The pin, however, needs
> > +          to be excluded from the gpiochip using the gpio-reserved-ranges property since
> > +          it will be used exclusively for FSYNC generation.
> > +        minimum: 0
> > +        maximum: 10
> > +        default: 0
> > +
> 
> MAX96717 do not have any knowledge of the frame synchronisation, but this device can forward some
> GPIO to/from the deserializer.

Good point.

> 
> GPIO forwarding need some information 
> - The local GPIO number
> - The forwarding direction Rx, Tx, Bi-directionnal
> - The GPIO ID on the GMSL link (RX_ID/TX_ID)
> 
> Can we add a maxim,forward-gpio property reflecting that instead ?

Agreed.

Thanks,
Laurentiu

> 
> >    reg:
> >      maxItems: 1
> >  
> > @@ -113,6 +138,9 @@ examples:
> >              #gpio-cells = <2>;
> >              #clock-cells = <0>;
> >  
> > +            gpio-reserved-ranges = <0 1>;
> > +            maxim,fsync-config = <0 0>;
> > +
> >              ports {
> >                  #address-cells = <1>;
> >                  #size-cells = <0>;
> 
> Regards,
> -- 
> Julien

Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

[Julien Massot]

Hi Laurentiu,

> > >  
> > > +  maxim,override-mode:
> > > +    description: Toggle the operation mode from the pin configured one.
> > > +    type: boolean
> > I understand that this property is intended to flip the GMSL link mode between
> > pixel and tunnel mode.
> > What about adding a property 'maxim,tunnel-mode' to the GMSL 'port@1'.
> > Here the MAX96717 only have one GMSL port but other devices, such as MAX96724 can
> > have 2 GMSL link and may have each link in different mode.
> 
> I'm OK with moving the property inside "port@1". But I have some
> concerns about the logic. So. 'maxim,tunnel-mode' presence would
> indicate that we want to force the functioning mode to "tunnel". But
> what if it's absent? Do we use the pin configuration? What if the pin
> configuration is "tunnel" and the user wants to override the mode to
> "pixel"? In this case 'maxim,tunnel-mode' doesn't really work...
> Am I missing something here?
> 
> 
> What about maxim,gmsl2-mode that could be either TUNNEL or PIXEL, if the
property is missing then just run with the pin configured mode.

Regards,
-- 
Julien