On Wed, 29 May 2024 12:57:48 +0800 Yasin Lee <yasin.lee.x@xxxxxxxxxxx> wrote: > From: Yasin Lee <yasin.lee.x@xxxxxxxxx> > > A capacitive proximity sensor > > Signed-off-by: Yasin Lee <yasin.lee.x@xxxxxxxxx> Hi Yasin, Some comments inline. A lot of what you have here sounds like it should be under userspace control, not in the dt-binding. Also, look at how we handled optional channels for ADCs and copy that approach here. Jonathan > --- > .../bindings/iio/proximity/tyhx,hx9023s.yaml | 106 ++++++++++++++++++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > 2 files changed, 108 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..ba4d7343bb30 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/proximity/tyhx,hx9023s.yaml > @@ -0,0 +1,106 @@ > +# 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 <yasin.lee.x@xxxxxxxxx> > + > +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: > + description: Main power supply vdd-supply: true is enough. vdd is pretty much always used for the main power supply so the docs are are redundant. > + > + accuracy: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Accuracy level of the sensor No idea what this means! > + > + channel-used-flag: > + description: Bit flag indicating which channels are used > + $ref: /schemas/types.yaml#/definitions/uint32 As below - subnodes not a bunch of arrays. If the subnode is there it should be used. > + > + odr: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Set ODR for all channenls. I assume output data rate? That's something for userspace not the DT binding. > + > + integration-sample: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Set Integration number and Sample number for each channenl. Ok. This one might possibly be hardware related as it depends on the wiring and physical elements of the board. If so give a good description on how this should be set. > + > + osr: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Set number of OSR for each channenl. Expand the acronym. This sounds like oversampling ratio which would be odd to see alongside an average control. Hence more detail needed and consider if it should be controlled by the dt binding. > + > + avg: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Set number of AVG for each channenl. This sounds like oversampling? If so that is normally a userspace problem not a binding thing. Is it related to the physical wiring? If not don't put it in the binding. > + > + lp-alpha: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Set lp-alpha for each channenl. Spell check. Also this needs more description. > + > + cs-position: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: | > + Position of the CS pins. > + Indicates the corresponding bit for each CS pin in the register. I've no idea what this. Add more description. Normally CS is chip select and there is only one of those. Also this not general dt binding material so vendor prefix this stuff. > + > + channel-positive: Use per channel nodes. There are examples in for example adc/adc.yaml on how to do this. Not having a child node is a lot easier to follow than magic values to say something isn't there.
