[PATCH v5 0/3] Input: cap11xx add advanced sensitivity settings

[PATCH v5 0/3] Input: cap11xx add advanced sensitivity settings

[Jiri Valek - 2N]

PATCH 1 - add documentation for new options
PATCH 2 - add support for advanced settings into driver
PATCH 3 - remove unnecessary IRQ parsing

Changes in v2:
  - Removed "sensitivity-base-shift" parameter (not HW propertie) in PATCH 2.
  - Used IRQ from I2C subsystem instead of parsing it again.
  - Fixed some documentation issues in PATCH 1
  
Changes in v3:
  - Remove incorrectly used "Reviewed-by" tag in PATCH 1 and 2

Changes in v4:
  - Remove unused variable in PATCH 2

Changes in v5:
  - Revert IRQ parsing in PATCH 2 and move to separate PATCH 3
  - Fix 'if' condition for properties in PATCH 1

Jiri Valek - 2N (3):
  dt-bindings: input: microchip,cap11xx: add advanced sensitivity
    settings
  Input: cap11xx - add advanced sensitivity settings
  Input: cap11xx - remove unnecessary IRQ parsing

 .../bindings/input/microchip,cap11xx.yaml     |  76 +++++-
 drivers/input/keyboard/cap11xx.c              | 248 ++++++++++++++----
 2 files changed, 269 insertions(+), 55 deletions(-)

-- 
2.25.1

[PATCH v5 1/3] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings

[Jiri Valek - 2N]

Add support for advanced sensitivity settings and signal guard feature.

Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
---
 .../bindings/input/microchip,cap11xx.yaml     | 76 ++++++++++++++++++-
 1 file changed, 73 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
index 5b5d4f7d3482..aa97702c43ef 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,55 @@ 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:
+      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,signal-guard:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 8
+    items:
+      minimum: 0
+      maximum: 1
+    description: |
+      Optional parameter supported only for CAP129x.
+      0 - off
+      1 - on
+      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:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 8
+    items:
+      minimum: 0
+      maximum: 127
+    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:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 8
+    items:
+      minimum: 1
+      maximum: 4
+    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.
+
 patternProperties:
   "^led@[0-7]$":
     type: object
@@ -99,10 +148,29 @@ allOf:
           contains:
             enum:
               - microchip,cap1106
+              - microchip,cap1203
+              - microchip,cap1206
+              - microchip,cap1293
+              - microchip,cap1298
     then:
       patternProperties:
         "^led@[0-7]$": false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,cap1106
+              - microchip,cap1126
+              - microchip,cap1188
+              - microchip,cap1203
+              - microchip,cap1206
+    then:
+      properties:
+        microchip,signal-guard: false
+        microchip,calib-sensitivity: false
+
 required:
   - compatible
   - interrupts
@@ -122,6 +190,8 @@ examples:
         reg = <0x28>;
         autorepeat;
         microchip,sensor-gain = <2>;
+        microchip,sensitivity-delta-sense = <16>;
+        microchip,input-treshold = <21>, <18>, <46>, <46>, <46>, <21>;
 
         linux,keycodes = <103>,	/* KEY_UP */
                          <106>,	/* KEY_RIGHT */
-- 
2.25.1

Re: [PATCH v5 1/3] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings

[Krzysztof Kozlowski]

On 08/11/2023 16:56, Jiri Valek - 2N wrote:
> Add support for advanced sensitivity settings and signal guard feature.
> 
> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
> ---
>  .../bindings/input/microchip,cap11xx.yaml     | 76 ++++++++++++++++++-
>  1 file changed, 73 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> index 5b5d4f7d3482..aa97702c43ef 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,55 @@ 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:
> +      Optional parameter. Controls the sensitivity multiplier of a touch detection.
> +      At the more sensitive settings, touches are detected for a smaller delta

Which values are more sensitive?

> +      capacitance corresponding to a “lighter” touch.

Looks like you use some non-ASCII characters for ".

> +
> +  microchip,signal-guard:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 3
> +    maxItems: 8
> +    items:
> +      minimum: 0
> +      maximum: 1
> +    description: |
> +      Optional parameter supported only for CAP129x.
> +      0 - off
> +      1 - on
> +      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:

typo: threshold

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 3
> +    maxItems: 8
> +    items:
> +      minimum: 0
> +      maximum: 127
> +    description:
> +      Optional parameter. Specifies the delta threshold that is used to

Drop everywhere the "optional parameter". It's redundant. required:
block tells what is / is not optional.

> +      determine if a touch has been detected.

In what units are the values?

> +      The number of entries must correspond to the number of channels.
> +
> +  microchip,calib-sensitivity:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 3
> +    maxItems: 8
> +    items:
> +      minimum: 1
> +      maximum: 4
> +    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.

Gain is usually in dB, isn't it?

> +      The number of entries must correspond to the number of channels.
> +
>  patternProperties:
>    "^led@[0-7]$":


Best regards,
Krzysztof

Re: [PATCH v5 1/3] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings

[Jiri Valek - 2N]

On 11/10/23 09:22, Krzysztof Kozlowski wrote:
> On 08/11/2023 16:56, Jiri Valek - 2N wrote:
>> Add support for advanced sensitivity settings and signal guard feature.
>>
>> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
>> ---
>>  .../bindings/input/microchip,cap11xx.yaml     | 76 ++++++++++++++++++-
>>  1 file changed, 73 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
>> index 5b5d4f7d3482..aa97702c43ef 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,55 @@ 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:
>> +      Optional parameter. Controls the sensitivity multiplier of a touch detection.
>> +      At the more sensitive settings, touches are detected for a smaller delta
> 
> Which values are more sensitive?

Higher value means more sensitive settings.
I will add this line to description.

> 
>> +      capacitance corresponding to a “lighter” touch.
> 
> Looks like you use some non-ASCII characters for ".

I will fix it.

> 
>> +
>> +  microchip,signal-guard:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 3
>> +    maxItems: 8
>> +    items:
>> +      minimum: 0
>> +      maximum: 1
>> +    description: |
>> +      Optional parameter supported only for CAP129x.
>> +      0 - off
>> +      1 - on
>> +      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:
> 
> typo: threshold

I will fix it in Documentation and also in source code.
Copy paste error...

> 
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 3
>> +    maxItems: 8
>> +    items:
>> +      minimum: 0
>> +      maximum: 127
>> +    description:
>> +      Optional parameter. Specifies the delta threshold that is used to
> 
> Drop everywhere the "optional parameter". It's redundant. required:
> block tells what is / is not optional.

OK will be fixed.

> 
>> +      determine if a touch has been detected.
> 
> In what units are the values?

According to the datasheet it is dimensionless, no more info.
A higher value means a larger difference in capacitance is required for a touch to be registered.

> 
>> +      The number of entries must correspond to the number of channels.
>> +
>> +  microchip,calib-sensitivity:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 3
>> +    maxItems: 8
>> +    items:
>> +      minimum: 1
>> +      maximum: 4
>> +    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.
> 
> Gain is usually in dB, isn't it?

Usually yes, but again there are no units in datasheet.
There is note that gain is based on capacitance touch pad capacitance range
1 - 5-50pF
2 - 0-25pF 
4 - 0-12.5pF
I will add this to description and change items to enum: [1, 2, 4]

> 
>> +      The number of entries must correspond to the number of channels.
>> +
>>  patternProperties:
>>    "^led@[0-7]$":
> 
> 
> Best regards,
> Krzysztof
> 

So are these changes fine for you?
If yes I prepare new patch revision.

Best regards,
Jiri

Re: [PATCH v5 1/3] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings

[Krzysztof Kozlowski]

On 16/11/2023 17:00, Jiri Valek - 2N wrote:

>>> +      The number of entries must correspond to the number of channels.
>>> +
>>>  patternProperties:
>>>    "^led@[0-7]$":
>>
>>
>> Best regards,
>> Krzysztof
>>
> 
> So are these changes fine for you?
> If yes I prepare new patch revision.


Yes

Best regards,
Krzysztof

[PATCH v5 2/3] Input: cap11xx - add advanced sensitivity settings

[Jiri Valek - 2N]

Add support for advanced sensitivity settings that allows more precise
tunig of touch buttons. Input-treshold allows to set the sensitivity for
each channel separately. Also add signal guard feature for CAP129x chips.

Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
---
 drivers/input/keyboard/cap11xx.c | 242 +++++++++++++++++++++++++------
 1 file changed, 196 insertions(+), 46 deletions(-)

diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 1b4937dce672..4711ea985627 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -14,6 +14,7 @@
 #include <linux/regmap.h>
 #include <linux/i2c.h>
 #include <linux/gpio/consumer.h>
+#include <linux/bitfield.h>
 
 #define CAP11XX_REG_MAIN_CONTROL	0x00
 #define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT	(6)
@@ -24,6 +25,7 @@
 #define CAP11XX_REG_NOISE_FLAG_STATUS	0x0a
 #define CAP11XX_REG_SENOR_DELTA(X)	(0x10 + (X))
 #define CAP11XX_REG_SENSITIVITY_CONTROL	0x1f
+#define CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK	0x70
 #define CAP11XX_REG_CONFIG		0x20
 #define CAP11XX_REG_SENSOR_ENABLE	0x21
 #define CAP11XX_REG_SENSOR_CONFIG	0x22
@@ -32,6 +34,7 @@
 #define CAP11XX_REG_CALIBRATION		0x26
 #define CAP11XX_REG_INT_ENABLE		0x27
 #define CAP11XX_REG_REPEAT_RATE		0x28
+#define CAP11XX_REG_SIGNAL_GUARD_ENABLE	0x29
 #define CAP11XX_REG_MT_CONFIG		0x2a
 #define CAP11XX_REG_MT_PATTERN_CONFIG	0x2b
 #define CAP11XX_REG_MT_PATTERN		0x2d
@@ -47,6 +50,8 @@
 #define CAP11XX_REG_SENSOR_BASE_CNT(X)	(0x50 + (X))
 #define CAP11XX_REG_LED_POLARITY	0x73
 #define CAP11XX_REG_LED_OUTPUT_CONTROL	0x74
+#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG	0x80
+#define CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2	0x81
 
 #define CAP11XX_REG_LED_DUTY_CYCLE_1	0x90
 #define CAP11XX_REG_LED_DUTY_CYCLE_2	0x91
@@ -78,12 +83,20 @@ struct cap11xx_led {
 
 struct cap11xx_priv {
 	struct regmap *regmap;
+	struct device *dev;
 	struct input_dev *idev;
+	const struct cap11xx_hw_model *model;
+	u8 id;
 
 	struct cap11xx_led *leds;
 	int num_leds;
 
 	/* config */
+	u8 analog_gain;
+	u8 sensitivity_delta_sense;
+	u8 signal_guard_inputs_mask;
+	u32 thresholds[8];
+	u32 calib_sensitivities[8];
 	u32 keycodes[];
 };
 
@@ -181,6 +194,178 @@ static const struct regmap_config cap11xx_regmap_config = {
 	.volatile_reg = cap11xx_volatile_reg,
 };
 
+static int
+cap11xx_write_calib_sens_config_1(struct cap11xx_priv *priv)
+{
+	return regmap_write(priv->regmap,
+			CAP11XX_REG_CALIB_SENSITIVITY_CONFIG,
+			(priv->calib_sensitivities[3] << 6) |
+			(priv->calib_sensitivities[2] << 4) |
+			(priv->calib_sensitivities[1] << 2) |
+			priv->calib_sensitivities[0]);
+}
+
+static int
+cap11xx_write_calib_sens_config_2(struct cap11xx_priv *priv)
+{
+	return regmap_write(priv->regmap,
+			CAP11XX_REG_CALIB_SENSITIVITY_CONFIG2,
+			(priv->calib_sensitivities[7] << 6) |
+			(priv->calib_sensitivities[6] << 4) |
+			(priv->calib_sensitivities[5] << 2) |
+			priv->calib_sensitivities[4]);
+}
+
+static int
+cap11xx_init_keys(struct cap11xx_priv *priv)
+{
+	struct device_node *node = priv->dev->of_node;
+	struct device *dev = priv->dev;
+	int i, error;
+	u32 u32_val;
+
+	if (!node) {
+		dev_err(dev, "Corresponding DT entry is not available\n");
+		return -ENODEV;
+	}
+
+	if (!of_property_read_u32(node, "microchip,sensor-gain", &u32_val)) {
+		if (priv->model->no_gain) {
+			dev_warn(dev,
+				 "This model doesn't support 'sensor-gain'\n");
+		} else if (is_power_of_2(u32_val) && u32_val <= 8) {
+			priv->analog_gain = (u8)ilog2(u32_val);
+
+			error = regmap_update_bits(priv->regmap,
+				CAP11XX_REG_MAIN_CONTROL,
+				CAP11XX_REG_MAIN_CONTROL_GAIN_MASK,
+				priv->analog_gain << CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT);
+			if (error)
+				return error;
+		} else {
+			dev_err(dev, "Invalid sensor-gain value %u\n", u32_val);
+			return -EINVAL;
+		}
+	}
+
+	if (of_property_read_bool(node, "microchip,irq-active-high")) {
+		if (priv->id == CAP1106 ||
+		    priv->id == CAP1126 ||
+		    priv->id == CAP1188) {
+			error = regmap_update_bits(priv->regmap,
+						   CAP11XX_REG_CONFIG2,
+						   CAP11XX_REG_CONFIG2_ALT_POL,
+						   0);
+			if (error)
+				return error;
+		} else {
+			dev_warn(dev,
+				 "This model doesn't support 'irq-active-high'\n");
+		}
+	}
+
+	if (!of_property_read_u32(node,
+				  "microchip,sensitivity-delta-sense", &u32_val)) {
+		if (!is_power_of_2(u32_val) || u32_val > 128) {
+			dev_err(dev, "Invalid sensitivity-delta-sense value %u\n", u32_val);
+			return -EINVAL;
+		}
+
+		priv->sensitivity_delta_sense = (u8)ilog2(u32_val);
+		u32_val = ~(FIELD_PREP(CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK,
+					priv->sensitivity_delta_sense));
+
+		error = regmap_update_bits(priv->regmap,
+					   CAP11XX_REG_SENSITIVITY_CONTROL,
+					   CAP11XX_REG_SENSITIVITY_CONTROL_DELTA_SENSE_MASK,
+					   u32_val);
+		if (error)
+			return error;
+	}
+
+	if (!of_property_read_u32_array(node, "microchip,input-treshold",
+					priv->thresholds, priv->model->num_channels)) {
+		for (i = 0; i < priv->model->num_channels; i++) {
+			if (priv->thresholds[i] > 127) {
+				dev_err(dev, "Invalid input-treshold value %u\n",
+					priv->thresholds[i]);
+				return -EINVAL;
+			}
+
+			error = regmap_write(priv->regmap,
+					     CAP11XX_REG_SENSOR_THRESH(i),
+					     priv->thresholds[i]);
+			if (error)
+				return error;
+		}
+	}
+
+	if (!of_property_read_u32_array(node, "microchip,calib-sensitivity",
+					priv->calib_sensitivities, priv->model->num_channels)) {
+		if (priv->id == CAP1293 || priv->id == CAP1298) {
+			for (i = 0; i < priv->model->num_channels; i++) {
+				if (!is_power_of_2(priv->calib_sensitivities[i]) ||
+				    priv->calib_sensitivities[i] > 4) {
+					dev_err(dev, "Invalid calib-sensitivity value %u\n",
+						priv->calib_sensitivities[i]);
+					return -EINVAL;
+				}
+				priv->calib_sensitivities[i] = ilog2(priv->calib_sensitivities[i]);
+			}
+
+			error = cap11xx_write_calib_sens_config_1(priv);
+			if (error)
+				return error;
+
+			if (priv->id == CAP1298) {
+				error = cap11xx_write_calib_sens_config_2(priv);
+				if (error)
+					return error;
+			}
+		} else {
+			dev_warn(dev,
+				 "This model doesn't support 'calib-sensitivity'\n");
+		}
+	}
+
+	for (i = 0; i < priv->model->num_channels; i++) {
+		if (!of_property_read_u32_index(node, "microchip,signal-guard",
+						i, &u32_val)) {
+			if (u32_val > 1)
+				return -EINVAL;
+			if (u32_val)
+				priv->signal_guard_inputs_mask |= 0x01 << i;
+		}
+	}
+
+	if (priv->signal_guard_inputs_mask) {
+		if (priv->id == CAP1293 || priv->id == CAP1298) {
+			error = regmap_write(priv->regmap,
+					     CAP11XX_REG_SIGNAL_GUARD_ENABLE,
+					     priv->signal_guard_inputs_mask);
+			if (error)
+				return error;
+		} else {
+			dev_warn(dev,
+				 "This model doesn't support 'signal-guard'\n");
+		}
+	}
+
+	/* Provide some useful defaults */
+	for (i = 0; i < priv->model->num_channels; i++)
+		priv->keycodes[i] = KEY_A + i;
+
+	of_property_read_u32_array(node, "linux,keycodes",
+				   priv->keycodes, priv->model->num_channels);
+
+	/* Disable autorepeat. The Linux input system has its own handling. */
+	error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+	if (error)
+		return error;
+
+	return 0;
+}
+
 static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
 {
 	struct cap11xx_priv *priv = data;
@@ -332,11 +517,9 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	const struct i2c_device_id *id = i2c_client_get_device_id(i2c_client);
 	struct device *dev = &i2c_client->dev;
 	struct cap11xx_priv *priv;
-	struct device_node *node;
 	const struct cap11xx_hw_model *cap;
-	int i, error, irq, gain = 0;
+	int i, error, irq;
 	unsigned int val, rev;
-	u32 gain32;
 
 	if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
 		dev_err(dev, "Invalid device ID %lu\n", id->driver_data);
@@ -355,6 +538,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->dev = dev;
+
 	priv->regmap = devm_regmap_init_i2c(i2c_client, &cap11xx_regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
@@ -384,50 +569,15 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 		return error;
 
 	dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
-		 id->name, rev);
-	node = dev->of_node;
-
-	if (!of_property_read_u32(node, "microchip,sensor-gain", &gain32)) {
-		if (cap->no_gain)
-			dev_warn(dev,
-				 "This version doesn't support sensor gain\n");
-		else if (is_power_of_2(gain32) && gain32 <= 8)
-			gain = ilog2(gain32);
-		else
-			dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
-	}
+			 id->name, rev);
 
-	if (id->driver_data == CAP1106 ||
-	    id->driver_data == CAP1126 ||
-	    id->driver_data == CAP1188) {
-		if (of_property_read_bool(node, "microchip,irq-active-high")) {
-			error = regmap_update_bits(priv->regmap,
-						   CAP11XX_REG_CONFIG2,
-						   CAP11XX_REG_CONFIG2_ALT_POL,
-						   0);
-			if (error)
-				return error;
-		}
-	}
+	priv->model = cap;
+	priv->id = id->driver_data;
 
-	/* Provide some useful defaults */
-	for (i = 0; i < cap->num_channels; i++)
-		priv->keycodes[i] = KEY_A + i;
-
-	of_property_read_u32_array(node, "linux,keycodes",
-				   priv->keycodes, cap->num_channels);
-
-	if (!cap->no_gain) {
-		error = regmap_update_bits(priv->regmap,
-				CAP11XX_REG_MAIN_CONTROL,
-				CAP11XX_REG_MAIN_CONTROL_GAIN_MASK,
-				gain << CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT);
-		if (error)
-			return error;
-	}
+	dev_info(dev, "CAP11XX device detected, model %s, revision 0x%02x\n",
+		 id->name, rev);
 
-	/* Disable autorepeat. The Linux input system has its own handling. */
-	error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+	error = cap11xx_init_keys(priv);
 	if (error)
 		return error;
 
@@ -439,7 +589,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	priv->idev->id.bustype = BUS_I2C;
 	priv->idev->evbit[0] = BIT_MASK(EV_KEY);
 
-	if (of_property_read_bool(node, "autorepeat"))
+	if (of_property_read_bool(dev->of_node, "autorepeat"))
 		__set_bit(EV_REP, priv->idev->evbit);
 
 	for (i = 0; i < cap->num_channels; i++)
@@ -474,7 +624,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	if (error)
 		return error;
 
-	irq = irq_of_parse_and_map(node, 0);
+	irq = irq_of_parse_and_map(dev->of_node, 0);
 	if (!irq) {
 		dev_err(dev, "Unable to parse or map IRQ\n");
 		return -ENXIO;
-- 
2.25.1

[PATCH v5 3/3] Input: cap11xx - remove unnecessary IRQ parsing

[Jiri Valek - 2N]

Separate IRQ parsing is not necessary, I2C core do the job.

Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
---
 drivers/input/keyboard/cap11xx.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 4711ea985627..ccca9936ef25 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -518,7 +518,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	struct device *dev = &i2c_client->dev;
 	struct cap11xx_priv *priv;
 	const struct cap11xx_hw_model *cap;
-	int i, error, irq;
+	int i, error;
 	unsigned int val, rev;
 
 	if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
@@ -624,13 +624,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
 	if (error)
 		return error;
 
-	irq = irq_of_parse_and_map(dev->of_node, 0);
-	if (!irq) {
-		dev_err(dev, "Unable to parse or map IRQ\n");
-		return -ENXIO;
-	}
-
-	error = devm_request_threaded_irq(dev, irq, NULL, cap11xx_thread_func,
+	error = devm_request_threaded_irq(dev, i2c_client->irq, NULL, cap11xx_thread_func,
 					  IRQF_ONESHOT, dev_name(dev), priv);
 	if (error)
 		return error;
-- 
2.25.1

Re: [PATCH v5 3/3] Input: cap11xx - remove unnecessary IRQ parsing

[Rob Herring]

On Wed, Nov 8, 2023 at 9:57 AM Jiri Valek - 2N <jiriv@axis.com> wrote:
>
> Separate IRQ parsing is not necessary, I2C core do the job.
>
> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
> ---
>  drivers/input/keyboard/cap11xx.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 4711ea985627..ccca9936ef25 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c
> @@ -518,7 +518,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>         struct device *dev = &i2c_client->dev;
>         struct cap11xx_priv *priv;
>         const struct cap11xx_hw_model *cap;
> -       int i, error, irq;
> +       int i, error;
>         unsigned int val, rev;
>
>         if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
> @@ -624,13 +624,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>         if (error)
>                 return error;
>
> -       irq = irq_of_parse_and_map(dev->of_node, 0);

Probably can drop the include of of_irq.h as well.

Re: [PATCH v5 3/3] Input: cap11xx - remove unnecessary IRQ parsing

[Jiri Valek - 2N]

On 11/8/23 20:53, Rob Herring wrote:
> On Wed, Nov 8, 2023 at 9:57 AM Jiri Valek - 2N <jiriv@axis.com> wrote:
>>
>> Separate IRQ parsing is not necessary, I2C core do the job.
>>
>> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
>> ---
>>  drivers/input/keyboard/cap11xx.c | 10 ++--------
>>  1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
>> index 4711ea985627..ccca9936ef25 100644
>> --- a/drivers/input/keyboard/cap11xx.c
>> +++ b/drivers/input/keyboard/cap11xx.c
>> @@ -518,7 +518,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>>         struct device *dev = &i2c_client->dev;
>>         struct cap11xx_priv *priv;
>>         const struct cap11xx_hw_model *cap;
>> -       int i, error, irq;
>> +       int i, error;
>>         unsigned int val, rev;
>>
>>         if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
>> @@ -624,13 +624,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>>         if (error)
>>                 return error;
>>
>> -       irq = irq_of_parse_and_map(dev->of_node, 0);
> 
> Probably can drop the include of of_irq.h as well.
Ack. Thanks for notice!

Re: [PATCH v5 3/3] Input: cap11xx - remove unnecessary IRQ parsing

[Dmitry Torokhov]

On Thu, Nov 16, 2023 at 08:43:00AM +0100, Jiri Valek - 2N wrote:
> On 11/8/23 20:53, Rob Herring wrote:
> > On Wed, Nov 8, 2023 at 9:57 AM Jiri Valek - 2N <jiriv@axis.com> wrote:
> >>
> >> Separate IRQ parsing is not necessary, I2C core do the job.
> >>
> >> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
> >> ---
> >>  drivers/input/keyboard/cap11xx.c | 10 ++--------
> >>  1 file changed, 2 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> >> index 4711ea985627..ccca9936ef25 100644
> >> --- a/drivers/input/keyboard/cap11xx.c
> >> +++ b/drivers/input/keyboard/cap11xx.c
> >> @@ -518,7 +518,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
> >>         struct device *dev = &i2c_client->dev;
> >>         struct cap11xx_priv *priv;
> >>         const struct cap11xx_hw_model *cap;
> >> -       int i, error, irq;
> >> +       int i, error;
> >>         unsigned int val, rev;
> >>
> >>         if (id->driver_data >= ARRAY_SIZE(cap11xx_devices)) {
> >> @@ -624,13 +624,7 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
> >>         if (error)
> >>                 return error;
> >>
> >> -       irq = irq_of_parse_and_map(dev->of_node, 0);
> > 
> > Probably can drop the include of of_irq.h as well.
> Ack. Thanks for notice!

I replaced it with #include <of.h> and applied, thank you.

-- 
Dmitry