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

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 15 Sep 2026 18:11:17 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Sun, Sep 13, 2026 at 01:42:03AM +0100, Jonathan Cameron wrote:
> > On Tue, 8 Sep 2026 19:13:55 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> >   
> > > On Mon, Sep 07, 2026 at 02:00:41PM -0700, Chang Yu wrote:  
> > > > Add binding for AMS AS7343 which is a 14-channel multi-spectral sensor
> > > > with i2c address of 0x39.
> > > > 
> > > > The GPIO pin is described as a generic GPIO for now. Binding design for the
> > > > more advanced measurement/LED synchronization use cases are deferred to
> > > > future patches.    
> > > 
> > > Unfortunately, you can't change what you document, so picking something
> > > correct now is needed - even if the driver doesn't use it yet.  
> > 
> > Definitely needs an outline of how it would be backwards compatible and
> > an explanation of why not now.  Sometimes a portion of the binding is
> > so uncertain that we do kick it back from initial version but we 'must'
> > be sure we can extend the binding to new configurations.  Normally this
> > is one of those we are fairly sure, but not entirely sure cases - or
> > picking between two options where consensus isn't being reached.
> > 
> > Chang Yu: This sort of things needs discussion and is one of the reasons to
> > go slowly.  
> 
> Ye, I note that there are 2 more versions of this since I left this
> comment, but you seem to be on top of that.
> 
> > > > Datasheet: https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> > > > Signed-off-by: Chang Yu <marcus.yu.56@gmail.com>
> > > > ---
> > > > Changes in v2:
> > > >   - Add the LDR, the interrupt pin, and the GPIO pin to the bindings.
> > > >   - Fix node name and unit address mismatch.
> > > >   - Include MAINTAINERS changes.
> > > > 
> > > >  .../bindings/iio/light/ams,as7343.yaml        | 69 +++++++++++++++++++
> > > >  MAINTAINERS                                   |  6 ++
> > > >  2 files changed, 75 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> > > > new file mode 100644
> > > > index 000000000000..b06d445b92b3
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> > > > @@ -0,0 +1,69 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/iio/light/ams,as7343.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: AMS AS7343 14-Channel Multi-Spectral Sensor
> > > > +
> > > > +maintainers:
> > > > +  - Chang Yu <marcus.yu.56@gmail.com>
> > > > +
> > > > +description: |
> > > > +  The AMS AS7343 is a 14-channel multi-spectral sensor with i2c address of 0x39.
> > > > +  https://look.ams-osram.com/m/5f2d27fff9a874d2/original/AS7343-14-Channel-Multi-Spectral-Sensor.pdf
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - ams,as7343
> > > > +
> > > > +  reg:
> > > > +    description:
> > > > +      I2C address of the device (0x39).
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    description:
> > > > +      Open drain output active low interrupt pin.
> > > > +    maxItems: 1
> > > > +
> > > > +  vdd-supply: true
> > > > +
> > > > +  ams,led-current-microamp:
> > > > +    description:
> > > > +      The driver current for the external LED connected to the LDR pin.
> > > > +    minimum: 4000
> > > > +    maximum: 258000
> > > > +    multipleOf: 2000
> > > > +    default: 12000    
> > > 
> > > Rather than a custom property, the tsl2772 uses led-max-microamp:
> > > tsl2772.yaml
> > > 46:  led-max-microamp:
> > > 81:                led-max-microamp = <100000>;
> > > 
> > > I wonder if the same should be done here, or if there should be an leds
> > > subnode? Perhaps the IIO folks can comment on that.  
> > 
> > I don't think we've ever bothered with a subnode as there only tends
> > to be one of them.  Given the enabling etc is all hardware controlled
> > I'm not sure a more generic LED binding makes sense. I don't know that
> > much about the led bindings though so maybe it is worth doing a subnode
> > just to use the leds/common.yaml definition of led-max-microamp?  
> 
> Could always just put a ref in to leds/common.yaml and not bother with a
> child node. I'd stick with the additionalProperties: false, since
> there's lots of leds properties that probably don't apply to a device
> like this.
> 
Sounds good to me.

> Cheers,
> Conor.