Re: [PATCH 1/2] dt-bindings: iio: light: Add AMS TCS3472 and TCS3400

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 16 Oct 2026 16:43:42 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 16/10/2026 10:15, HuskerMinion via B4 Relay wrote:
> > From: HuskerMinion <mocker-mug-unwoven@duck.com>
> > 
> > The tcs3472 driver has no binding yet. It matches DT nodes only through
> > its I2C device ID.
> > 
> > Document the TCS3472 family and the AMS TCS3400 family, which the next
> > patch adds to the driver. Both are I2C color sensors with one threshold
> > interrupt and a VDD supply.
> > 
> > Signed-off-by: HuskerMinion <mocker-mug-unwoven@duck.com>  
> 
> Please read submitting patches how to properly attribute the work with
> your real and known identity
> 
> 
> > ---
> >  .../devicetree/bindings/iio/light/ams,tcs3472.yaml | 57 ++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/light/ams,tcs3472.yaml b/Documentation/devicetree/bindings/iio/light/ams,tcs3472.yaml
> > new file mode 100644
> > index 000000000..cd4a0516e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/light/ams,tcs3472.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/light/ams,tcs3472.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AMS TCS3472 and TCS3400 color light-to-digital converters
> > +
> > +maintainers:
> > +  - HuskerMinion <mocker-mug-unwoven@duck.com>
> > +
> > +description: |
> > +  Color light sensors with 16-bit clear, red, green and blue channels, an
> > +  I2C interface and a threshold interrupt.
> > +
> > +  The TCS3472 family covers the TCS34721, TCS34723, TCS34725 and TCS34727.
> > +  The TCS3400 family covers the TCS34001, TCS34003, TCS34005 and TCS34007.  
> 
> We do not take families, but actual devices. TCS3400 seems to be a real
> device, so I don't get why you call it a family.
> 
> 
> > +  The variants differ in I2C address and interface voltage.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ams,tcs3400
> > +      - ams,tcs3472  
> 
> Your driver cod suggests these are compatible, so it should be expressed.

That's only true because this driver hard rejects a whoami mismatch
which is something we shouldn't really be doing. If we do a look
up in driver and fail to match we should be using the data associated with
the ID that the fallback compatible would have had.

Our old fun of what should we do about fallback compatibles and
devices with ID registers.  Some of which (in this driver) are actually
compatible with each other and some of which are not (in the sense
that the match the WHOAMI and do different things depending on that
matching).

I still prefer that we never claim devices that need different
handling are compatible (even if detectable) because of that prevents
new drop in parts being used (as they normally have different whoami
values even as supposedly drop in replacements).

I don't mind enough to argue much about it however.
Note that before this series all the supported parts were actually
compatible and the only difference was a print on the id match. So
if this patch hadn't turned up an I'd noticed, I have simply
deleted the error return.

Jonathan


> 
> 
> Best regards,
> Krzysztof