Re: [PATCH v4 1/2] dt-bindings: iio: light: add as7343

[Jonathan Cameron]

On Sat, 12 Sep 2026 18:35:14 -0700
Chang Yu <marcus.yu.56@xxxxxxxxx> wrote:

> Hi Jonathan,
> 
> A quick comment inline.
> 
> On Sun, Sep 13, 2026 at 01:56:52AM +0100, Jonathan Cameron wrote:
> > On Fri, 11 Sep 2026 18:39:11 -0700
> > Chang Yu <marcus.yu.56@xxxxxxxxx> wrote:
> >   
> > > +    description:
> > > +      The driver current for the external LED connected to the LDR pin.
> > > +    minimum: 4000
> > > +    maximum: 258000
> > > +    multipleOf: 2000
> > > +    default: 12000
> > > +
> > > +  sync-gpios:
> > > +    description: |
> > > +      Optional GPIO pin that can be used either as a synchronization input to
> > > +      start/stop measurement or as an external start/stop signal for
> > > +      synchronizing with the connected LED.  
> > 
> > So the datasheet starts referring to using this for some level of sync
> > but then only actually treats it as as a gpio (maybe I'm missing something).
> > Any idea what that stuff about start/stop is talking about?
> > 
> > The only thing I can find about this is the device diagram in figure 13 that has the GPIO
> > wired to the led driver and the automatic measurement engine.
> >   
> That's what I got from the datasheet as well. They don't describe how to
> use the GPIO pin to perform synchronizations at all.
> 
> The adafruit arduino driver
> (https://github.com/adafruit/Adafruit_AS7343/blob/main/Adafruit_AS7343.cpp)
> is slightly more helpful - the GPIO in output mode signals when the
> spectral measurement starts. Unfortunately no information about input
> mode.

There is no separate control?  If so we can't use it as a GPIO as whilst
there is a control to set the state it will randomly get overwritten if
we do such a read. So to me sounds like that is there for testing the
connection and similar usecases only and we should not expose this as
a GPIO. Maybe input is possible but that may also have side effects.
So I'd be tempted to drop it from the binding - with some description
of why it is not there in the patch description.

> 
> > > +    maxItems: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - vdd-supply
> > > +
> > > +additionalProperties: false  
> > 
> >   
>