Re: [PATCH v1 1/4] dt-bindings: iio: light: add apds990x binding
From: Jonathan Cameron <jic23@kernel.org>
Date: 2023-03-11 19:34:29
Also in:
linux-iio, lkml
On Wed, 8 Mar 2023 11:02:16 +0200 Svyatoslav Ryhel [off-list ref] wrote:
Add dt-binding for apds990x ALS/proximity sensor. Signed-off-by: Svyatoslav Ryhel <redacted> --- .../bindings/iio/light/avago,apds990x.yaml | 76 +++++++++++++++++++
I'm not a fan of wild cards. It breaks far too often. Can we name this instead after a particular supported part - same for compatible. I'm not sure what parts are supported by this, but you may want multiple compatibles.
quoted hunk ↗ jump to hunk
1 file changed, 76 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds990x.yamldiff --git a/Documentation/devicetree/bindings/iio/light/avago,apds990x.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds990x.yaml new file mode 100644 index 000000000000..9b47e13f88e3 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/light/avago,apds990x.yaml@@ -0,0 +1,76 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/light/avago,apds990x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Avago APDS990x ALS and proximity sensor + +maintainers: + - Samu Onkalo <samu.p.onkalo@nokia.com> + +description: | + APDS990x is a combined ambient light and proximity sensor. ALS and + proximity functionality are highly connected. ALS measurement path + must be running while the proximity functionality is enabled. + +properties: + compatible: + const: avago,apds990x + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + vdd-supply: true + vled-supply: true + + avago,pdrive: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 32 + description: | + Drive value used in configuring control register.
Is this something where there is a reasonable default? If so I'd prefer it was optional so that the device is easier to use without needing firmware description.
+ + avago,ppcount: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 32 + description: | + Number of pulses used for proximity sensor calibration.
Same for this - if there is a reasonable default it would be good to have that specified.
+ +additionalProperties: false + +required: + - compatible + - reg + - interrupt
It would nice to relax the need for an interrupt if the device is still useable with timeouts etc. Board folk have a habit of deciding they don't need to wire up interrupts. We can relax that a later date though if you prefer not to do it now.
+ - vdd-supply + - vled-supply
Whilst true that the supplies need to be connected, that doesn't mean they need to provided in the device tree binding. If they are always powered up I think we can fallback to stub regulators.
+ - avago,pdrive
+ - avago,ppcount
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ light-sensor@39 {
+ compatible = "avago,apds990x";
+ reg = <0x39>;
+
+ interrupt-parent = <&gpio>;
+ interrupts = <82 IRQ_TYPE_EDGE_RISING>;
+
+ vdd-supply = <&vdd_3v0_proxi>;
+ vled-supply = <&vdd_1v8_sen>;
+
+ avago,pdrive = <0x00>;
+ avago,ppcount = <0x03>;
+ };
+ };
+...