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

[Conor]

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
>