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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4497 bytes --]

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.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]