From: Yasin Lee <[email protected]>
A capacitive proximity sensor
Signed-off-by: Yasin Lee <[email protected]>
---
.../bindings/iio/proximity/tyhx,hx9023s.yaml | 103 ++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
2 files changed, 105 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
new file mode 100644
index 000000000000..50bf2849d823
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TYHX HX9023S capacitive proximity sensor
+
+maintainers:
+ - Yasin Lee <[email protected]>
+
+description: |
+ TYHX HX9023S proximity sensor
+
+allOf:
+ - $ref: /schemas/iio/iio.yaml#
+
+properties:
+ compatible:
+ const: tyhx,hx9023s
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description: |
+ Generated by device to announce preceding read request has finished
+ and data is available or that a close/far proximity event has happened.
+ maxItems: 1
+
+ vdd-supply:
+ true
+
+ channel-in-use:
+ description: |
+ Bit flag indicating which channels are used,
+ depends on the hardware circuit design.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+patternProperties:
+ "^channel@[0-9]+$":
+ type: object
+ properties:
+ reg:
+ description: Channel register address
+ $ref: /schemas/types.yaml#/definitions/uint32
+ channel-positive:
+ description: Positive channel assignments
+ $ref: /schemas/types.yaml#/definitions/uint32
+ channel-negative:
+ description: Negative channel assignments
+ $ref: /schemas/types.yaml#/definitions/uint32
+ required:
+ - reg
+ - channel-positive
+ - channel-negative
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ hx9023s@2a {
+ compatible = "tyhx,hx9023s";
+ reg = <0x2a>;
+ interrupt-parent = <&pio>;
+ interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+ vdd-supply = <&pp1800_prox>;
+ channel-in-use = <0x1F>;
+ channel@0 {
+ reg = <0>;
+ channel-positive = <0>;
+ channel-negative = <255>;
+ };
+ channel@1 {
+ reg = <1>;
+ channel-positive = <1>;
+ channel-negative = <255>;
+ };
+ channel@2 {
+ reg = <2>;
+ channel-positive = <2>;
+ channel-negative = <255>;
+ };
+ channel@3 {
+ reg = <3>;
+ channel-positive = <3>;
+ channel-negative = <255>;
+ };
+ channel@4 {
+ reg = <4>;
+ channel-positive = <4>;
+ channel-negative = <255>;
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b97d298b3eb6..e2224eea9ab9 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1507,6 +1507,8 @@ patternProperties:
description: Turing Machines, Inc.
"^tyan,.*":
description: Tyan Computer Corporation
+ "^tyhx,.*":
+ description: NanjingTianyihexin Electronics Ltd.
"^u-blox,.*":
description: u-blox
"^u-boot,.*":
--
2.25.1
On Fri, 7 Jun 2024 19:41:37 +0800
Yasin Lee <[email protected]> wrote:
> From: Yasin Lee <[email protected]>
>
> A capacitive proximity sensor
>
> Signed-off-by: Yasin Lee <[email protected]>
Hi Yasin
Some improvements but seems you missed some of the feedback on v3.
See inline.
Jonathan
> ---
> .../bindings/iio/proximity/tyhx,hx9023s.yaml | 103 ++++++++++++++++++
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> 2 files changed, 105 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> new file mode 100644
> index 000000000000..50bf2849d823
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TYHX HX9023S capacitive proximity sensor
> +
> +maintainers:
> + - Yasin Lee <[email protected]>
> +
> +description: |
> + TYHX HX9023S proximity sensor
> +
> +allOf:
> + - $ref: /schemas/iio/iio.yaml#
> +
> +properties:
> + compatible:
> + const: tyhx,hx9023s
> +
> + reg:
> + maxItems: 1
A device like this needs at least one power supply. Make sure to document
all such supplies and make the ones that are required for functionality part of
your required properties. Note that you should do this even if on your
board they are always turned on.
> +
> + interrupts:
> + description: |
> + Generated by device to announce preceding read request has finished
> + and data is available or that a close/far proximity event has happened.
> + maxItems: 1
> +
> + vdd-supply:
> + true
vdd-supply: true
on single line is commonly done for these.
> +
> + channel-in-use:
> + description: |
> + Bit flag indicating which channels are used,
> + depends on the hardware circuit design.
> + $ref: /schemas/types.yaml#/definitions/uint32
Presence of the channel nodes below should make this clear
without a separate element.
> +
> +patternProperties:
> + "^channel@[0-9]+$":
> + type: object
> + properties:
> + reg:
> + description: Channel register address
> + $ref: /schemas/types.yaml#/definitions/uint32
> + channel-positive:
> + description: Positive channel assignments
> + $ref: /schemas/types.yaml#/definitions/uint32
That size seems implausible. What are the limits. What does
255 mean?
In review of previous version I pointed you at the differential
channel bindings for ADCs. If they cannot be applied here
explain why in your patch description.
> + channel-negative:
> + description: Negative channel assignments
> + $ref: /schemas/types.yaml#/definitions/uint32
> + required:
> + - reg
> + - channel-positive
> + - channel-negative
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + hx9023s@2a {
> + compatible = "tyhx,hx9023s";
> + reg = <0x2a>;
> + interrupt-parent = <&pio>;
> + interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
> + vdd-supply = <&pp1800_prox>;
> + channel-in-use = <0x1F>;
> + channel@0 {
> + reg = <0>;
> + channel-positive = <0>;
> + channel-negative = <255>;
> + };
> + channel@1 {
> + reg = <1>;
> + channel-positive = <1>;
> + channel-negative = <255>;
> + };
> + channel@2 {
> + reg = <2>;
> + channel-positive = <2>;
> + channel-negative = <255>;
> + };
> + channel@3 {
> + reg = <3>;
> + channel-positive = <3>;
> + channel-negative = <255>;
> + };
> + channel@4 {
> + reg = <4>;
> + channel-positive = <4>;
> + channel-negative = <255>;
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index b97d298b3eb6..e2224eea9ab9 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1507,6 +1507,8 @@ patternProperties:
> description: Turing Machines, Inc.
> "^tyan,.*":
> description: Tyan Computer Corporation
> + "^tyhx,.*":
> + description: NanjingTianyihexin Electronics Ltd.
Use a separate patch for the new vendor prefix. Makes it easier for people to cherrypick that
if they are backporting some other tyhx dt binding.
> "^u-blox,.*":
> description: u-blox
> "^u-boot,.*":
On Sat, 8 Jun 2024 17:57:58 +0100
Jonathan Cameron <[email protected]> wrote:
> On Fri, 7 Jun 2024 19:41:37 +0800
> Yasin Lee <[email protected]> wrote:
>
> > From: Yasin Lee <[email protected]>
> >
> > A capacitive proximity sensor
> >
> > Signed-off-by: Yasin Lee <[email protected]>
> Hi Yasin
>
> Some improvements but seems you missed some of the feedback on v3.
>
> See inline.
>
> Jonathan
>
> > ---
> > .../bindings/iio/proximity/tyhx,hx9023s.yaml | 103 ++++++++++++++++++
> > .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> > 2 files changed, 105 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> > new file mode 100644
> > index 000000000000..50bf2849d823
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml
> > @@ -0,0 +1,103 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/proximity/tyhx,hx9023s.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TYHX HX9023S capacitive proximity sensor
> > +
> > +maintainers:
> > + - Yasin Lee <[email protected]>
> > +
> > +description: |
> > + TYHX HX9023S proximity sensor
> > +
> > +allOf:
> > + - $ref: /schemas/iio/iio.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: tyhx,hx9023s
> > +
> > + reg:
> > + maxItems: 1
>
> A device like this needs at least one power supply. Make sure to document
> all such supplies and make the ones that are required for functionality part of
> your required properties. Note that you should do this even if on your
> board they are always turned on.
Ignore this for obvious reasons given you have just below! However should be
required.
>
> > +
> > + interrupts:
> > + description: |
> > + Generated by device to announce preceding read request has finished
> > + and data is available or that a close/far proximity event has happened.
> > + maxItems: 1
> > +
> > + vdd-supply:
> > + true
> vdd-supply: true
>
> on single line is commonly done for these.
>
> > +
> > + channel-in-use:
> > + description: |
> > + Bit flag indicating which channels are used,
> > + depends on the hardware circuit design.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Presence of the channel nodes below should make this clear
> without a separate element.
>
>
> > +
> > +patternProperties:
> > + "^channel@[0-9]+$":
> > + type: object
> > + properties:
> > + reg:
> > + description: Channel register address
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + channel-positive:
> > + description: Positive channel assignments
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> That size seems implausible. What are the limits. What does
> 255 mean?
>
> In review of previous version I pointed you at the differential
> channel bindings for ADCs. If they cannot be applied here
> explain why in your patch description.
>
> > + channel-negative:
> > + description: Negative channel assignments
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + required:
> > + - reg
> > + - channel-positive
> > + - channel-negative
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + hx9023s@2a {
> > + compatible = "tyhx,hx9023s";
> > + reg = <0x2a>;
> > + interrupt-parent = <&pio>;
> > + interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
> > + vdd-supply = <&pp1800_prox>;
> > + channel-in-use = <0x1F>;
> > + channel@0 {
> > + reg = <0>;
> > + channel-positive = <0>;
> > + channel-negative = <255>;
> > + };
> > + channel@1 {
> > + reg = <1>;
> > + channel-positive = <1>;
> > + channel-negative = <255>;
> > + };
> > + channel@2 {
> > + reg = <2>;
> > + channel-positive = <2>;
> > + channel-negative = <255>;
> > + };
> > + channel@3 {
> > + reg = <3>;
> > + channel-positive = <3>;
> > + channel-negative = <255>;
> > + };
> > + channel@4 {
> > + reg = <4>;
> > + channel-positive = <4>;
> > + channel-negative = <255>;
> > + };
> > + };
> > + };
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index b97d298b3eb6..e2224eea9ab9 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -1507,6 +1507,8 @@ patternProperties:
> > description: Turing Machines, Inc.
> > "^tyan,.*":
> > description: Tyan Computer Corporation
> > + "^tyhx,.*":
> > + description: NanjingTianyihexin Electronics Ltd.
>
> Use a separate patch for the new vendor prefix. Makes it easier for people to cherrypick that
> if they are backporting some other tyhx dt binding.
>
> > "^u-blox,.*":
> > description: u-blox
> > "^u-boot,.*":
>
>
On 07/06/2024 13:41, Yasin Lee wrote:
> From: Yasin Lee <[email protected]>
>
> A capacitive proximity sensor
>
> Signed-off-by: Yasin Lee <[email protected]>
Not much improved. You still ignored feedback. Please respond to each
feedback and acknowledge it.
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline), work on fork of kernel
(don't, instead use mainline) or you ignore some maintainers (really
don't). Just use b4 and everything should be fine, although remember
about `b4 prep --auto-to-cc` if you added new patches to the patchset.
You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.
Please kindly resend and include all necessary To/Cc entries.
Best regards,
Krzysztof