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

[Jonathan Cameron]

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

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

Agreed, given software is controlling the current we need that and there
isn't a lot more we can do.  I guess you have the Adafruit board. Are the
pins wired out so you can perhaps see what happens in practice?

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