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

[Conor]

On 21 September 2026 05:51:44 GMT+02:00, Chang Yu <marcus.yu.56@xxxxxxxxx> wrote:
>On Sun, Sep 20, 2026 at 03:14:30PM +0200, Conor wrote:
>> 
>> 
>> On 20 September 2026 02:04:48 GMT+02:00, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>> >On Fri, 18 Sep 2026 18:55:00 -0700
>> >Chang Yu <marcus.yu.56@xxxxxxxxx> wrote:
>> >
>> >> On Thu, Sep 17, 2026 at 03:48:23AM +0100, Jonathan Cameron wrote:
>> >> > On Tue, 15 Sep 2026 18:11:17 +0100
>> >> > Conor Dooley <conor@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxx>
>> >> > > > > > ---
>> >> > > > > > 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@xxxxxxxxx>
>> >> > > > > > +
>> >> > > > > > +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.  
>> >> >   
>> >> A quick heads up, adding a ref produces the following error when running
>> >> dt_binding_check:
>> >> 
>> >> properties:led-max-microamp: '$ref' should not be valid under {'const': '$ref'}
>> >> 	hint: Standard unit suffix properties don't need a type $ref
>> >> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml
>> >> 
>> >> So it looks like we can just put led-max-microamp, and not add the $ref.
>> >
>> >Ah. That's a bit irritating as it is assuming the ref is to get the
>> >type rather than to make the connection to the standard definition
>> >of what it is.  Conor, what is the best way around this? 
>> >Maybe a comment to say where it is documented instead of a $ref?
>> 
>> From the error, it looks like this was done incorrectly, but impossible to tell for sure without the snippet.
>> The ref I was talking about would be at the  top level
>> 
>> >
>> >J
>> >
>I was doing something like:
>    led-max-microamp:
>      $ref: /schemas/leds/common.yaml#/properties/led-max-microamp
>      minimum: 4000
>which gave me the error.
>
>IIUC, Conor was suggesting something like:
>    allOf:
>      - $ref: /schemas/leds/common.yaml#
>
>From my limited understanding of dt bindings, the above should not be
>used together with additionalProperties: false ? So do we want something

It can be used with it, you just need to list the property in the schema you're writing - which you have to do anyway because you're setting constraints on it.

Cheers,
Conor 

>like:
>
>    allOf:
>      - $ref: /schemas/leds/common.yaml#
>
>    unevaluatedProperties: false
>
>Or just stick with what's in v5?
>