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

[Chang Yu <marcus.yu.56@gmail.com>]

On Sun, Sep 13, 2026 at 03:51:32AM +0100, Jonathan Cameron wrote:
> On Sat, 12 Sep 2026 18:35:14 -0700
> Chang Yu <marcus.yu.56@gmail.com> 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@gmail.com> 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.
> 

Agreed. What does this mean for the LED then? For all intents and purposes
it's controlled by only 1 register. It may have some interactions with the
GPIO but since they are completely undocumented so we might as well consider
them non-existent. IMO it probably doesn't warrant a LED subnode and
led-max-microamp is enough. Maybe DT folks can also comment on that.

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