All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Chang Yu <marcus.yu.56@gmail.com>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"Joshua Crofts" <joshua.crofts1@gmail.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, "Shi Hao" <i.shihao.999@gmail.com>,
	"Jose A. Perez de Azpillaga" <azpijr@gmail.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: light: add as7343
Date: Tue, 8 Sep 2026 19:13:55 +0100	[thread overview]
Message-ID: <20260908-dinner-shelve-1d61e19ee3b7@spud> (raw)
In-Reply-To: <20260907210042.32552-2-marcus.yu.56@gmail.com>

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

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.
> 
> 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.

> +
> +  gpios:
> +    description:
> +      Optional GPIO pin for general I/O.

Please expand on "general IO". Docs appear to suggest this is used for
synchronisation.

Additionally, I think this name will make the gpio somewhat difficult to
request. Calling it "sync-gpios" or w/e will permit the usual gpiod
functions being used.

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        multispectral-sensor@39 {
> +            compatible = "ams,as7343";
> +            reg = <0x39>;
> +            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +            vdd-supply = <&vdd_regulator>;
> +            ams,led-current-microamp = <14000>;
> +            gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;

Valid Sashiko comment here I believe.
pw-bot: changes-requested

Thanks,
Conor.

> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 04fa5322d9f7..8cf4e1635053 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1430,6 +1430,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/light/ams,as73211.yaml
>  F:	drivers/iio/light/as73211.c
>  
> +AMS AS7343 DRIVER
> +M:	Chang Yu <marcus.yu.56@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/light/ams,as7343.yaml
> +
>  AMT (Automatic Multicast Tunneling)
>  M:	Taehee Yoo <ap420073@gmail.com>
>  L:	netdev@vger.kernel.org
> -- 
> 2.55.0
> 

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

  parent reply	other threads:[~2026-09-08 18:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 21:00 [PATCH v2 0/2] Add support for AS7343 multi-spectral sensor Chang Yu
2026-09-07 21:00 ` [PATCH v2 1/2] dt-bindings: iio: light: add as7343 Chang Yu
2026-09-07 21:06   ` sashiko-bot
2026-09-08 18:13   ` Conor Dooley [this message]
2026-09-13  0:42     ` Jonathan Cameron
2026-09-15 17:11       ` Conor Dooley
2026-09-17  2:48         ` Jonathan Cameron
2026-09-19  1:55           ` Chang Yu
2026-09-20  0:04             ` Jonathan Cameron
2026-09-20 13:14               ` Conor
2026-09-21  3:51                 ` Chang Yu
2026-09-21 11:59                   ` Conor
2026-09-07 21:00 ` [PATCH v2 2/2] iio: light: add AS7343 multi-spectral sensor driver Chang Yu
2026-09-07 21:10   ` sashiko-bot
2026-09-08  8:01   ` Joshua Crofts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908-dinner-shelve-1d61e19ee3b7@spud \
    --to=conor@kernel.org \
    --cc=andy@kernel.org \
    --cc=azpijr@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=i.shihao.999@gmail.com \
    --cc=jic23@kernel.org \
    --cc=joshua.crofts1@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcus.yu.56@gmail.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.