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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <o7em5dmk6mu3uyd7eqmvshdaj233aacog734gd4wdhk43kuubg@2z726bmyvfcq>
Date: Thu, 6 Mar 2025 10:24:06 +0200
From: Laurentiu Palcu <laurentiu.palcu@....nxp.com>
To: Julien Massot <julien.massot@...labora.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-media@...r.kernel.org
Subject: Re: [PATCH 3/5] dt-bindings: i2c: maxim,max96717: add new properties

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@....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