On 28/04/2023 19:09, Jiri Valek - 2N wrote: > Hi Krzysztof, > and thanks for the review > > On 4/15/23 11:10, Krzysztof Kozlowski wrote: >> On 15/04/2023 01:38, Jiri Valek - 2N wrote: >>> Add support for advanced sensitivity settings and signal guard feature. >>> >>> Signed-off-by: Jiri Valek - 2N <jiriv@xxxxxxxx> >>> --- >>> .../bindings/input/microchip,cap11xx.yaml | 64 ++++++++++++++++++- >>> 1 file changed, 61 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml >>> index 5fa625b5c5fb..08e28226a164 100644 >>> --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml >>> +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml >>> @@ -45,13 +45,13 @@ properties: >>> Enables the Linux input system's autorepeat feature on the input device. >>> >>> linux,keycodes: >>> - minItems: 6 >>> - maxItems: 6 >>> + minItems: 3 >>> + maxItems: 8 >>> description: | >>> Specifies an array of numeric keycode values to >>> be used for the channels. If this property is >>> omitted, KEY_A, KEY_B, etc are used as defaults. >>> - The array must have exactly six entries. >>> + The number of entries must correspond to the number of channels. >>> >>> microchip,sensor-gain: >>> $ref: /schemas/types.yaml#/definitions/uint32 >>> @@ -70,6 +70,58 @@ properties: >>> open drain. This property allows using the active >>> high push-pull output. >>> >>> + microchip,sensitivity-delta-sense: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + default: 32 >>> + enum: [1, 2, 4, 8, 16, 32, 64, 128] >>> + description: | >> >> Do not need '|' unless you need to preserve formatting. > > OK. Will remove them. > >> >>> + Optional parameter. Controls the sensitivity multiplier of a touch detection. >>> + At the more sensitive settings, touches are detected for a smaller delta >>> + capacitance corresponding to a “lighter” touch. >>> + >>> + microchip,sensitivity-base-shift: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + default: 256 >>> + enum: [1, 2, 4, 8, 16, 32, 64, 128, 256] >>> + description: | >>> + Optional parameter. Controls data scaling factor. >>> + The higher the value of these bits, the larger the range and the lower >>> + the resolution of the data presented. These settings will not affect >>> + touch detection or sensitivity. >>> + >>> + microchip,signal-guard: >>> + minItems: 3 >>> + maxItems: 8 >>> + enum: [0, 1] >>> + default: 0 >> >> This was not really tested. Missing ref, mixing scalar and array >> properties. You want items with enum. And drop default. > > Ack. I will fix it. > >> >> >>> + description: | >>> + Optional parameter supported only for CAP129x. >> >> Then disallow it for others (allOf:if:then: ... >> microchip,signal-guard:false) > > Ack. I will fix it. > >>> + The signal guard isolates the signal from virtual grounds. >>> + If enabled then the behavior of the channel is changed to signal guard. >>> + The number of entries must correspond to the number of channels. >>> + >>> + microchip,input-treshold: >>> + minItems: 3 >>> + maxItems: 8 >>> + minimum: 0 >>> + maximum: 127 >>> + default: 64 >>> + description: | >>> + Optional parameter. Specifies the delta threshold that is used to >>> + determine if a touch has been detected. >>> + The number of entries must correspond to the number of channels. >>> + >>> + microchip,calib-sensitivity: >>> + minItems: 3 >>> + maxItems: 8 >>> + enum: [1, 2, 4] >>> + default: 1 >>> + description: | >>> + Optional parameter supported only for CAP129x. Specifies an array of >>> + numeric values that controls the gain used by the calibration routine to >>> + enable sensor inputs to be more sensitive for proximity detection. >>> + The number of entries must correspond to the number of channels. >> >> Most of these properties do not look like hardware properties. Policies >> and runtime configuration should not be put into DT. Explain please why >> these are board-specific thus suitable for DT. > > All these parameters are intended to set HW properties of touch buttons. I know, but some HW properties are software policies. Consider the simplest example - audio volume of a speaker. It's a hardware property, but it is not for DT. Software should choose audio volume based on user's decisions. > Each button can have different PCB layout and when you start without > setting these parameters in DT, then touches won't be detected or you > will get false positive readings. > E.g. 'signal-guard' change property of analog input from button to some > type of shield. > I made all of them optional for backward compatibility. > Maybe 'sensitivity-base-shift' is really not necessary to have in DT. > I will remove it if you agree. Keep only ones which are not policies but depend on physical/electrical characteristic of boards. Best regards, Krzysztof
