On Thu, 3 Sep 2020 15:18:28 -0700 Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > We need to set various bits in the hardware registers for this device to > operate properly depending on how it is installed. Add a handful of DT > properties to configure these things. > > Cc: Daniel Campello <campello@xxxxxxxxxxxx> > Cc: Lars-Peter Clausen <lars@xxxxxxxxxx> > Cc: Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx> > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > Cc: <devicetree@xxxxxxxxxxxxxxx> > Cc: Douglas Anderson <dianders@xxxxxxxxxxxx> > Cc: Gwendal Grignou <gwendal@xxxxxxxxxxxx> > Cc: Evan Green <evgreen@xxxxxxxxxxxx> > Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> > --- > > I haven't written any code to handle these properties yet. I'd rather do > that once the binding patch is reviewed. Patch based on iio.git testing > branch. Makes sense to do docs first for this. Quite a bit feels like it isn't a feature of the device configuration, but rather of the usecase. That stuff should probably be done with a userspace interface, but you may be able to argue me around on some of them! Jonathan > > .../iio/proximity/semtech,sx9310.yaml | 182 ++++++++++++++++++ > 1 file changed, 182 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > index 5739074d3592..e74b81483c14 100644 > --- a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > @@ -40,6 +40,169 @@ properties: > "#io-channel-cells": > const: 1 > > + semtech,cs0-ground: > + description: Indicates the CS0 sensor is connected to ground. > + type: boolean This one is probably fine. I can't think of a similar interface we need to match, but maybe Rob or someone else will have a suggestion. > + > + semtech,combined-sensors: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 1, 2, 3] > + default: 0 > + description: > + Which sensors are combined. 0 for CS3, 1 for CS0+CS1, 2 for CS1+CS2, > + and 3 for all sensors. Make it clear in this description what 'combined' means. Also, I think this would be better as a set of values with an anyOf match to say <3> <0>, <1> <1>, <2> <1>, <2>, <3> Fine to insist they are in numeric order. > + > + semtech,cs0-gain-factor: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [1, 2, 4, 8] > + default: 1 > + description: > + Gain factor for CS0 (and combined if any) sensor. Why is this something that should be in DT as opposed to via a userspace control? We have hardwaregain for this purpose (I think) Also we mostly use child nodes to allow us to specify characteristics of individual channels. > + > + semtech,cs1-gain-factor: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [1, 2, 4, 8] > + default: 1 > + description: > + Gain factor for CS1 sensor. > + > + semtech,cs2-gain-factor: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [1, 2, 4, 8] > + default: 1 > + description: > + Gain factor for CS2 sensor. > + > + semtech,resolution: > + description: > + Capacitance measure resolution. > + enum: > + - coarsest > + - very-coarse > + - coarse > + - medium-coarse > + - medium > + - fine > + - very-fine > + - finest I'd normally be very against cases like this where we have something that feels like it should have a clear definition rather than a random wordy scale but these are all the information I can find in the datasheet. I would suggest adding a specific reference to the datasheet for this one. > + > + semtech,startup-sensor: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 1, 2, 3] > + default: 0 > + description: > + Sensor used for start-up proximity detection. The combined > + sensor is represented by 3. This feels like it should be a userspace control rather than in DT? > + > + semtech,proxraw-strength: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 2, 4, 8] > + default: 2 > + description: > + PROXRAW filter strength. A value of 0 represents off, and other values > + represent 1-1/N. Having looked at the datasheet I have little or now idea of what this filter actually is. However, what is the argument for it being in DT rather than exposing a userspace control of some type. > + > + semtech,compensate-common: > + description: Any sensor triggers compensation of all channels. > + type: boolean Compensation for what? > + > + semtech,avg-pos-strength: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 16, 64, 128, 256, 512, 1024, 4294967295] > + default: 16 > + description: > + Average positive filter strength. A value of 0 represents off and > + UINT_MAX (4294967295) represents infinite. Other values > + represent 1-1/N. I'm not sure about using UINT_MAX to represent infinity. Rob any thoughts on this? Again, why does it make sense to have the filter controls in DT? > + > + semtech,cs0-prox-threshold: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [2, 4, 6, 8, 12, 16, 20, 24, 28, 32, 40, > + 48, 56, 64, 72, 80, 88, 96, 112, 128, 144, > + 160, 192, 224, 256, 320, 384, 512, 640, > + 768, 1024, 1536] > + default: 12 > + description: > + Proximity detection threshold for CS0 (and combined if any) sensor. That is definitely a userspace thing. Why would you put it in DT? Also same comment as above for channels as child nodes > + > + semtech,cs1-prox-threshold: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [2, 4, 6, 8, 12, 16, 20, 24, 28, 32, 40, > + 48, 56, 64, 72, 80, 88, 96, 112, 128, 144, > + 160, 192, 224, 256, 320, 384, 512, 640, > + 768, 1024, 1536] > + default: 12 > + description: > + Proximity detection threshold for CS1 sensor. > + > + semtech,cs2-prox-threshold: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [2, 4, 6, 8, 12, 16, 20, 24, 28, 32, 40, > + 48, 56, 64, 72, 80, 88, 96, 112, 128, 144, > + 160, 192, 224, 256, 320, 384, 512, 640, > + 768, 1024, 1536] > + default: 12 > + description: > + Proximity detection threshold for CS2 sensor. > + > + semtech,cs0-body-threshold: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 300, 600, 900, 1200, 1500, 1800, 30000] > + default: 1800 > + description: > + Body detection threshold for CS0 (and combined if any) sensor. As before, why DT plus child nodes > + > + semtech,cs1-body-threshold: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 300, 600, 900, 1200, 1500, 1800, 30000] > + default: 12 > + description: > + Body detection threshold for CS1 sensor. > + > + semtech,cs2-body-threshold: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 300, 600, 900, 1200, 1500, 1800, 30000] > + default: 12 > + description: > + Body detection threshold for CS2 sensor. > + > + semtech,hysteresis: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 6, 12, 25] > + default: 0 > + description: > + The percentage of hysteresis +/- applied to proximity/body samples. Is this hysteresis on an event? If so we have defined ABI to control that from userspace, though as an absolute value rather than a precentage so some magic will be needed. Hysteresis is usually defined only the 'not event' direction rather than +/- > + > + semtech,close-debounce-samples: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 2, 4, 8] > + default: 0 > + description: > + The number of close samples debounced for proximity/body thresholds. This feels like something that has more to do with the object motion than the sensor setup, so perhaps should be controlled from userspace? > + > + semtech,far-debounce-samples: > + allOf: > + - $ref: /schemas/types.yaml#definitions/uint32 > + - enum: [0, 2, 4, 8] > + default: 0 > + description: > + The number of far samples debounced for proximity/body thresholds. > + > required: > - compatible > - reg > @@ -61,5 +224,24 @@ examples: > vdd-supply = <&pp3300_a>; > svdd-supply = <&pp1800_prox>; > #io-channel-cells = <1>; > + semtech,cs0-ground; > + semtech,combined-sensors = <0>; > + semtech,cs0-gain-factor = <8>; > + semtech,cs1-gain-factor = <8>; > + semtech,cs2-gain-factor = <8>; > + semtech,resolution = "fine"; > + semtech,startup-sensor = <1>; > + semtech,proxraw-strength = <2>; > + semtech,compensate-common; > + semtech,avg-pos-strength = <64>; > + semtech,cs0-prox-threshold = <96>; > + semtech,cs1-prox-threshold = <112>; > + semtech,cs2-prox-threshold = <96>; > + semtech,cs0-body-threshold = <300>; > + semtech,cs1-body-threshold = <300>; > + semtech,cs2-body-threshold = <300>; > + semtech,hysteresis = <0>; > + semtech,close-debounce-samples = <2>; > + semtech,far-debounce-samples = <2>; > }; > }; > > base-commit: 1bebdcb928eba880f3a119bacb8149216206958a
