linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
@ 2025-05-05 20:38 David Bauer
  2025-05-05 20:38 ` [PATCH 2/2] Input sx951x: add Semtech SX9512/SX9513 driver David Bauer
  2025-05-06  6:21 ` [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding Krzysztof Kozlowski
  0 siblings, 2 replies; 9+ messages in thread
From: David Bauer @ 2025-05-05 20:38 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-input, devicetree, linux-kernel

Add device-tree binding for the Semtech SX9512/SX9513 family of touch
controllers with integrated LED driver.

Signed-off-by: David Bauer <mail@david-bauer.net>
---
 .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
 1 file changed, 180 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml

diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
new file mode 100644
index 000000000000..e4f938decd86
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
@@ -0,0 +1,180 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Semtech SX9512/SX9513 based capacitive touch sensors
+
+description: |
+  The Semtech SX9512/SX9513 Family of capacitive touch controllers
+  with integrated LED drivers. The device communication is using I2C only.
+
+maintainers:
+  - David Bauer <mail@david-bauer.net>
+
+properties:
+  compatible:
+    enum:
+      - semtech,sx9512
+      - semtech,sx9513
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  poll-interval:
+    default: 100
+    description: |
+      The polling interval for touch events in milliseconds.
+
+patternProperties:
+  "^channel@[0-7]$":
+    $ref: input.yaml#
+    type: object
+    description: |
+      Each node represents a channel of the touch controller.
+      Each channel provides a capacitive touch sensor input and
+      an LED driver output.
+
+    properties:
+      reg:
+        enum: [0, 1, 2, 3, 4, 5, 6, 7]
+
+      linux,keycodes:
+        maxItems: 1
+        description: |
+          Specifies an array of numeric keycode values to
+          be used for the channels. If this property is
+          omitted, the channel is not used as a key.
+
+      semtech,cin-delta:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3
+        default: 3
+        description: |
+          The capacitance delta which is used to detect a touch
+          or release event. The property value is mapped to a
+          farad range between 7pF and 2.3pF internally. The delta
+          becomes smaller the higher the value is.
+
+      semtech,sense-threshold:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 255
+        default: 4
+        description: |
+          The threshold value after which the channel detects a touch.
+          Refer to the datasheet for the internal calculation of the
+          resulting touch sensitivity.
+
+      led:
+        $ref: /schemas/leds/common.yaml#
+        type: object
+        unevaluatedProperties: false
+        description: |
+          Presence of this property indicates the channel
+          is used as an LED driver.
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/leds/common.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touch@2b {
+        compatible = "semtech,sx9512";
+
+        reg = <0x2b>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        poll-interval = <100>;
+
+        channel@1 {
+          reg = <1>;
+
+          semtech,cin-delta = <0x3>;
+          semtech,sense-threshold = <0xff>;
+
+          linux,keycodes = <KEY_A>;
+        };
+
+        channel@2 {
+          reg = <2>;
+
+          semtech,cin-delta = <0x3>;
+          semtech,sense-threshold = <0xff>;
+
+          linux,keycodes = <KEY_B>;
+        };
+
+        channel@3 {
+          reg = <3>;
+
+          semtech,cin-delta = <0x3>;
+          semtech,sense-threshold = <0xff>;
+
+          linux,keycodes = <KEY_WPS_BUTTON>;
+        };
+
+        channel@4 {
+          reg = <4>;
+
+          led {
+            color = <LED_COLOR_ID_RED>;
+            function = LED_FUNCTION_WAN;
+          };
+        };
+
+        channel@5 {
+          reg = <5>;
+
+          led {
+            color = <LED_COLOR_ID_GREEN>;
+            function = LED_FUNCTION_WAN;
+          };
+        };
+
+        channel@6 {
+          reg = <6>;
+
+          led {
+            color = <LED_COLOR_ID_GREEN>;
+            function = LED_FUNCTION_WLAN;
+            linux,default-trigger = "phy1tx";
+          };
+        };
+
+        channel@7 {
+          reg = <7>;
+
+          led {
+            color = <LED_COLOR_ID_GREEN>;
+            function = LED_FUNCTION_WLAN;
+            linux,default-trigger = "phy0tx";
+          };
+        };
+      };
+    };
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] Input sx951x: add Semtech SX9512/SX9513 driver
  2025-05-05 20:38 [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding David Bauer
@ 2025-05-05 20:38 ` David Bauer
  2025-05-06  5:26   ` Dmitry Torokhov
  2025-05-06  6:21 ` [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding Krzysztof Kozlowski
  1 sibling, 1 reply; 9+ messages in thread
From: David Bauer @ 2025-05-05 20:38 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-input

The Semtech SX9512/SX9513 is a family of capacitive touch-keyboard
controllers.

All chips offer 8 channel touch sensitive inputs with one LED driver per
output channel.

The also SX9512 supports proximity detection which is currently not
supported with the driver.

This chip can be found on the Genexis Pulse EX400 repeater platform.

Link: https://www.mouser.com/datasheet/2/761/SEMTS05226_1-2575172.pdf

Signed-off-by: David Bauer <mail@david-bauer.net>
---
 drivers/input/keyboard/Kconfig  |  11 +
 drivers/input/keyboard/Makefile |   1 +
 drivers/input/keyboard/sx951x.c | 490 ++++++++++++++++++++++++++++++++
 3 files changed, 502 insertions(+)
 create mode 100644 drivers/input/keyboard/sx951x.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 1d0c5f4c0f99..6dc397389c64 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -616,6 +616,17 @@ config KEYBOARD_SUNKBD
 	  To compile this driver as a module, choose M here: the
 	  module will be called sunkbd.
 
+config KEYBOARD_SX951X
+	tristate "Semtech SX951X capacitive touch controller"
+	depends on OF && I2C
+	select REGMAP_I2C
+	help
+	  Say Y here to enable the Semtech SX9512/SX9153 capacitive
+	  touch controller driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called sx951x.
+
 config KEYBOARD_SH_KEYSC
 	tristate "SuperH KEYSC keypad support"
 	depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index aecef00c5d09..e59ca83c30ec 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_KEYBOARD_STOWAWAY)		+= stowaway.o
 obj-$(CONFIG_KEYBOARD_ST_KEYSCAN)	+= st-keyscan.o
 obj-$(CONFIG_KEYBOARD_SUN4I_LRADC)	+= sun4i-lradc-keys.o
 obj-$(CONFIG_KEYBOARD_SUNKBD)		+= sunkbd.o
+obj-$(CONFIG_KEYBOARD_SX951X)		+= sx951x.o
 obj-$(CONFIG_KEYBOARD_TC3589X)		+= tc3589x-keypad.o
 obj-$(CONFIG_KEYBOARD_TEGRA)		+= tegra-kbc.o
 obj-$(CONFIG_KEYBOARD_TM2_TOUCHKEY)	+= tm2-touchkey.o
diff --git a/drivers/input/keyboard/sx951x.c b/drivers/input/keyboard/sx951x.c
new file mode 100644
index 000000000000..66355036aa95
--- /dev/null
+++ b/drivers/input/keyboard/sx951x.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Input driver for Semtech SX9512/SX9513 capacitive touch sensors.
+ *
+ * The difference between SX9512 and SX9513 is the presence of proximity
+ * sensing capabilities on the SX9512.
+ *
+ * SX951xB is the identical chip but with a different I2C address.
+ *
+ * (c) 2025 David Bauer <mail@david-bauer.net>
+ */
+
+ #include <linux/kernel.h>
+ #include <linux/module.h>
+ #include <linux/input.h>
+ #include <linux/leds.h>
+ #include <linux/of.h>
+ #include <linux/regmap.h>
+ #include <linux/i2c.h>
+ #include <linux/gpio/consumer.h>
+ #include <linux/bitfield.h>
+
+ /* Generic properties */
+#define SX951X_I2C_ADDRESS		0x2b
+#define SX951XB_I2C_ADDRESS_		0x2d
+#define SX951X_NUM_CHANNELS		8
+#define SX951X_POLL_INTERVAL		100
+
+/* Registers*/
+#define SX951X_REG_IRQ_SRC		0x00
+#define SX951X_REG_TOUCH_STATUS		0x01
+#define SX951X_REG_PROXIMITY_STATUS	0x02
+#define SX951X_REG_COMPENSATION_STATUS	0x03
+#define SX951X_REG_IRQ_NVM_CTRL		0x04
+#define SX951X_REG_SPO2_MODE_CTRL	0x07
+#define SX951X_REG_PWR_KEY_CTRL		0x08
+#define SX951X_REG_IRQ_MASK		0x09
+
+/* LED registers */
+#define SX951X_REG_LED_MAP_ENG1		0x0c
+#define SX951X_REG_LED_MAP_ENG2		0x0d
+#define SX951X_REG_LED_PWM_FREQ		0x0e
+#define SX951X_REG_LED_MODE		0x0f
+#define SX951X_REG_LED_IDLE		0x10
+#define SX951X_REG_LED_OFF_DELAY	0x11
+#define SX951X_REG_LED_ON_ENG1		0x12
+#define SX951X_REG_LED_FADE_ENG1	0x13
+#define SX951X_REG_LED_ON_ENG2		0x14
+#define SX951X_REG_LED_FADE_ENG2	0x15
+#define SX951X_REG_LED_POWER_IDLE	0x16
+#define SX951X_REG_LED_POWER_ON		0x17
+#define SX951X_REG_LED_POWER_OFF	0x18
+#define SX951X_REG_LED_POWER_FADE	0x19
+#define SX951X_REG_LED_POWER_ON_PULSE	0x1a
+#define SX951X_REG_LED_POWER_MODE	0x1b
+
+/* Capacitive touch sensing registers*/
+#define SX951X_REG_CAP_SENSE_ENABLE			0x1e
+
+#define SX951X_REG_CAP_SENSE_RANGE(x)			(0x1f + (x))
+#define SX951X_REG_CAP_SENSE_RANGE_CIN_DELTA_MASK	GENMASK(1, 0)
+
+#define SX951X_REG_CAP_SENSE_THRESH(x)			(0x28 + (x))
+#define SX951X_REG_CAP_SENSE_THRESH_ALL			0x30
+
+#define SX951X_REG_CAP_SENSE_OP				0x31
+#define SX951X_REG_CAP_SENSE_MODE			0x32
+#define SX951X_REG_CAP_SENSE_DEBOUNCE			0x33
+
+/* Reset register*/
+#define SX951X_REG_SOFT_RESET			0xff
+
+/* Default properties (keys)*/
+#define SX951X_KEY_DEFAULT_CIN_DELTA		0x03
+#define SX951X_KEY_DEFAULT_SENSE_THRESHOLD	0x04
+
+struct sx951x_key_data {
+	u32 cin_delta;
+	u32 sense_threshold;
+};
+
+struct sx951x_led {
+#ifdef CONFIG_LEDS_CLASS
+	struct led_classdev cdev;
+	struct sx951x_priv *priv;
+
+	u32 reg;
+	bool registered;
+#endif
+};
+
+struct sx951x_priv {
+	struct regmap *regmap;
+	struct device *dev;
+	struct input_dev *idev;
+	const struct sx951x_hw_data *hw;
+
+	struct sx951x_led leds[SX951X_NUM_CHANNELS];
+
+	/* device-config */
+	u32 poll_interval;
+
+	/* key-config */
+	u32 keycodes[SX951X_NUM_CHANNELS];
+	struct sx951x_key_data key_data[SX951X_NUM_CHANNELS];
+};
+
+struct sx951x_hw_data {
+	bool has_proximity_sensing;
+};
+
+static const struct reg_default sx951x_reg_defaults[] = {
+	{ SX951X_REG_LED_MAP_ENG1,		0x00 },
+	{ SX951X_REG_LED_MAP_ENG2,		0x00 },
+	{ SX951X_REG_LED_PWM_FREQ,		0x10 },
+	{ SX951X_REG_LED_IDLE,			0xff },
+	{ SX951X_REG_LED_ON_ENG1,		0xff },
+	{ SX951X_REG_LED_ON_ENG2,		0xff },
+	{ SX951X_REG_LED_POWER_IDLE,		0xff },
+	{ SX951X_REG_LED_POWER_ON,		0xff },
+	{ SX951X_REG_CAP_SENSE_ENABLE,		0x00 },
+	{ SX951X_REG_CAP_SENSE_RANGE(0),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(1),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(2),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(3),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(4),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(5),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(6),	0x40 },
+	{ SX951X_REG_CAP_SENSE_RANGE(7),	0x40 },
+	{ SX951X_REG_CAP_SENSE_THRESH(0),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(1),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(2),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(3),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(4),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(5),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(6),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH(7),	0x0f },
+	{ SX951X_REG_CAP_SENSE_THRESH_ALL,	0x0f },
+	{ SX951X_REG_CAP_SENSE_OP,		0x14 },
+	{ SX951X_REG_CAP_SENSE_MODE,		0x70 },
+	{ SX951X_REG_CAP_SENSE_DEBOUNCE,	0xff },
+};
+
+static bool sx951x_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case SX951X_REG_TOUCH_STATUS:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config sx951x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = SX951X_REG_SOFT_RESET,
+
+	.reg_defaults = sx951x_reg_defaults,
+	.num_reg_defaults = ARRAY_SIZE(sx951x_reg_defaults),
+
+	.cache_type = REGCACHE_MAPLE,
+	.volatile_reg = sx951x_volatile_reg,
+};
+
+#ifdef CONFIG_LEDS_CLASS
+static int sx951x_led_set(struct led_classdev *cdev, enum led_brightness value)
+{
+	struct sx951x_led *led = container_of(cdev, struct sx951x_led, cdev);
+	struct sx951x_priv *priv = led->priv;
+
+	return regmap_update_bits(priv->regmap,
+				  SX951X_REG_LED_MAP_ENG2,
+				  BIT(led->reg),
+				  value ? BIT(led->reg) : 0);
+}
+
+static int sx951x_led_init(struct sx951x_priv *priv,
+			   struct device_node *channel_node, u32 reg)
+{
+	struct device_node *led_node;
+	struct sx951x_led *led = &priv->leds[reg];
+	struct led_init_data init_data = {};
+	int error;
+
+	if (led->registered) {
+		dev_err(priv->dev,
+			"LED %d already registered\n", reg);
+		return -EINVAL;
+	}
+
+	led_node = of_get_child_by_name(channel_node, "led");
+	if (!led_node) {
+		/* No LED */
+		return 0;
+	}
+
+	led->cdev.flags = 0;
+	led->cdev.brightness_set_blocking = sx951x_led_set;
+	led->cdev.max_brightness = 1;
+	led->cdev.brightness = LED_OFF;
+
+	init_data.default_label = of_get_property(led_node, "label", NULL);
+	init_data.fwnode = of_fwnode_handle(led_node);
+
+	led->reg = reg;
+	led->priv = priv;
+
+	error = devm_led_classdev_register_ext(priv->dev, &led->cdev, &init_data);
+	if (error)
+		return error;
+
+	return 0;
+}
+#endif
+
+static void sx951x_poll(struct input_dev *input)
+{
+	struct sx951x_priv *priv = input_get_drvdata(input);
+	struct device *dev = priv->dev;
+	unsigned int val;
+	int error;
+	int i;
+
+	error = regmap_read(priv->regmap, SX951X_REG_TOUCH_STATUS, &val);
+	if (error) {
+		dev_err(dev, "Failed to read touch status: %d\n", error);
+		return;
+	}
+
+	for (i = 0; i < SX951X_NUM_CHANNELS; i++) {
+		if (priv->keycodes[i] == KEY_RESERVED)
+			continue;
+
+		input_report_key(input, priv->keycodes[i], !!(val & BIT(i)));
+		input_sync(input);
+	}
+}
+
+static int sx951x_channel_init(struct sx951x_priv *priv, struct device_node *of_node,
+			       u32 chan_idx)
+{
+	struct sx951x_key_data *key_data;
+	struct device *dev = priv->dev;
+	int error;
+
+	key_data = &priv->key_data[chan_idx];
+
+	/* Defaults */
+	key_data->cin_delta = SX951X_KEY_DEFAULT_CIN_DELTA;
+	key_data->sense_threshold = SX951X_KEY_DEFAULT_SENSE_THRESHOLD;
+
+	error = of_property_read_u32(of_node, "linux,keycodes",
+				     &priv->keycodes[chan_idx]);
+	if (error) {
+		/* Not configured */
+		return 0;
+	}
+
+	error = of_property_read_u32(of_node, "semtech,cin-delta",
+				     &key_data->cin_delta);
+	if (key_data->cin_delta > 0x03) {
+		dev_err(dev, "Failed to read cin-delta for channel %d: %d\n",
+			chan_idx, error);
+		return error;
+	}
+
+	error = of_property_read_u32(of_node, "semtech,sense-threshold",
+				     &key_data->sense_threshold);
+	if (key_data->sense_threshold > 0xff) {
+		dev_err(dev, "Failed to read sense-threshold for channel %d: %d\n",
+			chan_idx, error);
+		return error;
+	}
+
+	error = regmap_update_bits(priv->regmap,
+				   SX951X_REG_CAP_SENSE_RANGE(chan_idx),
+				   SX951X_REG_CAP_SENSE_RANGE_CIN_DELTA_MASK,
+				   key_data->cin_delta);
+
+	if (error) {
+		dev_err(dev, "Failed to set cin-delta for channel %d: %d\n",
+			chan_idx, error);
+		return error;
+	}
+
+	error = regmap_write(priv->regmap,
+			     SX951X_REG_CAP_SENSE_THRESH(chan_idx),
+			     key_data->sense_threshold);
+	if (error) {
+		dev_err(dev, "Failed to set sense-threshold for channel %d: %d\n",
+			chan_idx, error);
+		return error;
+	}
+
+	return 0;
+}
+
+static int sx951x_channels_init(struct sx951x_priv *priv)
+{
+	struct device *dev = priv->dev;
+	unsigned int channels = 0;
+	int error;
+	u32 reg;
+
+	for_each_child_of_node_scoped(dev->of_node, child) {
+		error = of_property_read_u32(child, "reg", &reg);
+		if (error != 0 || reg >= SX951X_NUM_CHANNELS) {
+			dev_err(dev, "Invalid channel %d\n", reg);
+			return -EINVAL;
+		}
+
+		priv->keycodes[reg] = KEY_RESERVED;
+
+		error = sx951x_channel_init(priv, child, reg);
+		if (error) {
+			dev_err(dev, "Failed to initialize channel %d: %d\n",
+				reg, error);
+			return error;
+		}
+
+		if (priv->keycodes[reg] != KEY_RESERVED)
+			channels |= BIT(reg);
+
+#ifdef CONFIG_LEDS_CLASS
+		error = sx951x_led_init(priv, child, reg);
+		if (error) {
+			dev_err(dev, "Failed to initialize LED %d: %d\n",
+				reg, error);
+			return error;
+		}
+#endif
+	}
+
+	/* Enable sensing on channels with keycode configured */
+	error = regmap_write(priv->regmap,
+			     SX951X_REG_CAP_SENSE_ENABLE,
+			     channels);
+
+	return 0;
+}
+
+static int sx951x_input_init(struct sx951x_priv *priv)
+{
+	struct device *dev = priv->dev;
+	int i, error;
+
+	priv->idev = devm_input_allocate_device(dev);
+	if (!priv->idev)
+		return -ENOMEM;
+
+	priv->idev->name = "SX9512/SX9513 capacitive touch sensor";
+	priv->idev->id.bustype = BUS_I2C;
+	__set_bit(EV_KEY, priv->idev->evbit);
+
+	for (i = 0; i < SX951X_NUM_CHANNELS; i++)
+		__set_bit(priv->keycodes[i], priv->idev->keybit);
+
+	__clear_bit(KEY_RESERVED, priv->idev->keybit);
+
+	priv->idev->keycode = priv->keycodes;
+	priv->idev->keycodesize = sizeof(priv->keycodes[0]);
+	priv->idev->keycodemax = SX951X_NUM_CHANNELS;
+
+	input_set_drvdata(priv->idev, priv);
+
+	error = input_setup_polling(priv->idev, sx951x_poll);
+	if (error) {
+		dev_err(dev, "Unable to set up polling: %d\n", error);
+		return error;
+	}
+
+	input_set_poll_interval(priv->idev, priv->poll_interval);
+
+	error = input_register_device(priv->idev);
+	if (error) {
+		dev_err(dev, "Unable to register polled device: %d\n",
+			error);
+		return error;
+	}
+
+	return 0;
+}
+
+static int sx951x_probe(struct i2c_client *i2c_client)
+{
+	const struct i2c_device_id *id;
+	const struct sx951x_hw_data *hw;
+	struct device *dev = &i2c_client->dev;
+	struct sx951x_priv *priv;
+	int error;
+
+	if (i2c_client->addr != SX951X_I2C_ADDRESS &&
+	    i2c_client->addr != SX951XB_I2C_ADDRESS_) {
+		dev_err(dev, "Invalid I2C address: 0x%02x\n",
+			i2c_client->addr);
+		return -ENODEV;
+	}
+
+	id = i2c_client_get_device_id(i2c_client);
+	hw = i2c_get_match_data(i2c_client);
+	if (!id || !hw) {
+		dev_err(dev, "Invalid device configuration\n");
+		return -EINVAL;
+	}
+
+	priv = devm_kzalloc(dev,
+			    sizeof(struct sx951x_priv),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->hw = hw;
+
+	priv->regmap = devm_regmap_init_i2c(i2c_client, &sx951x_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	/* Parse device configuration */
+	if (of_property_read_u32(dev->of_node, "poll-interval",
+				 &priv->poll_interval))
+		priv->poll_interval = SX951X_POLL_INTERVAL;
+
+	/* Register LED and input channels */
+	error = sx951x_channels_init(priv);
+	if (error) {
+		dev_err(dev, "Failed to initialize channels: %d\n", error);
+		return error;
+	}
+
+	/* Register input device */
+	error = sx951x_input_init(priv);
+	if (error) {
+		dev_err(dev, "Failed to register input device: %d\n", error);
+		return error;
+	}
+
+	return 0;
+}
+
+static void sx951x_remove(struct i2c_client *i2c_client)
+{
+	struct sx951x_priv *priv = i2c_get_clientdata(i2c_client);
+
+	/* Disable sensing */
+	regmap_write(priv->regmap, SX951X_REG_CAP_SENSE_ENABLE, 0x00);
+
+	/* Turn off all LEDs */
+	regmap_write(priv->regmap, SX951X_REG_LED_MAP_ENG2, 0x00);
+}
+
+static const struct sx951x_hw_data sx9512_hw_data = {
+	.has_proximity_sensing = true,
+};
+
+static const struct sx951x_hw_data sx9513_hw_data = {
+	.has_proximity_sensing = false,
+};
+
+static const struct of_device_id sx951x_dt_ids[] = {
+	{ .compatible = "semtech,sx9512", .data = &sx9512_hw_data },
+	{ .compatible = "semtech,sx9513", .data = &sx9513_hw_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sx951x_dt_ids);
+
+static const struct i2c_device_id sx951x_i2c_ids[] = {
+	{ "sx9512", (kernel_ulong_t)&sx9512_hw_data },
+	{ "sx9513", (kernel_ulong_t)&sx9513_hw_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sx951x_i2c_ids);
+
+static struct i2c_driver sx951x_i2c_driver = {
+	.driver = {
+		.name	= "sx951x",
+		.of_match_table = sx951x_dt_ids,
+	},
+	.id_table	= sx951x_i2c_ids,
+	.probe		= sx951x_probe,
+	.remove		= sx951x_remove,
+};
+
+module_i2c_driver(sx951x_i2c_driver);
+
+MODULE_DESCRIPTION("Semtech SX9512/SX9513 driver");
+MODULE_AUTHOR("David Bauer <mail@david-bauer.net>");
+MODULE_LICENSE("GPL");
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] Input sx951x: add Semtech SX9512/SX9513 driver
  2025-05-05 20:38 ` [PATCH 2/2] Input sx951x: add Semtech SX9512/SX9513 driver David Bauer
@ 2025-05-06  5:26   ` Dmitry Torokhov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2025-05-06  5:26 UTC (permalink / raw)
  To: David Bauer; +Cc: linux-kernel, linux-input

Hi David,

On Mon, May 05, 2025 at 10:38:45PM +0200, David Bauer wrote:
> The Semtech SX9512/SX9513 is a family of capacitive touch-keyboard
> controllers.
> 
> All chips offer 8 channel touch sensitive inputs with one LED driver per
> output channel.
> 
> The also SX9512 supports proximity detection which is currently not
> supported with the driver.
> 
> This chip can be found on the Genexis Pulse EX400 repeater platform.
> 
> Link: https://www.mouser.com/datasheet/2/761/SEMTS05226_1-2575172.pdf
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>
> ---
>  drivers/input/keyboard/Kconfig  |  11 +
>  drivers/input/keyboard/Makefile |   1 +
>  drivers/input/keyboard/sx951x.c | 490 ++++++++++++++++++++++++++++++++
>  3 files changed, 502 insertions(+)
>  create mode 100644 drivers/input/keyboard/sx951x.c
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 1d0c5f4c0f99..6dc397389c64 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -616,6 +616,17 @@ config KEYBOARD_SUNKBD
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called sunkbd.
>  
> +config KEYBOARD_SX951X
> +	tristate "Semtech SX951X capacitive touch controller"
> +	depends on OF && I2C

I do not believe it should depend on OF. Please have the driver use
generic device properties instead (device_property_*()).

...

> +
> + #include <linux/kernel.h>
> + #include <linux/module.h>
> + #include <linux/input.h>
> + #include <linux/leds.h>
> + #include <linux/of.h>

You will likely need mod_devicetable.h instead of of.h.

> + #include <linux/regmap.h>
> + #include <linux/i2c.h>
> + #include <linux/gpio/consumer.h>
> + #include <linux/bitfield.h>
> +
> + /* Generic properties */
> +#define SX951X_I2C_ADDRESS		0x2b
> +#define SX951XB_I2C_ADDRESS_		0x2d

Why underscore at the end?

...

> +
> +#ifdef CONFIG_LEDS_CLASS
> +		error = sx951x_led_init(priv, child, reg);
> +		if (error) {
> +			dev_err(dev, "Failed to initialize LED %d: %d\n",
> +				reg, error);
> +			return error;
> +		}
> +#endif

Please provide stub for sx951x_led_init() instead of using #ifdef here. 

...
> +
> +	priv = devm_kzalloc(dev,
> +			    sizeof(struct sx951x_priv),

sizeof(*priv)

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
  2025-05-05 20:38 [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding David Bauer
  2025-05-05 20:38 ` [PATCH 2/2] Input sx951x: add Semtech SX9512/SX9513 driver David Bauer
@ 2025-05-06  6:21 ` Krzysztof Kozlowski
  2025-05-06 10:05   ` David Bauer
  1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-06  6:21 UTC (permalink / raw)
  To: David Bauer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-input, devicetree, linux-kernel

On 05/05/2025 22:38, David Bauer wrote:
> Add device-tree binding for the Semtech SX9512/SX9513 family of touch
> controllers with integrated LED driver.
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>

You CC-ed an address, which suggests you do not work on mainline kernel
or you do not use get_maintainers.pl/b4/patman. Please rebase and always
work on mainline or start using mentioned tools, so correct addresses
will be used.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). 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) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.


> ---
>  .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
>  1 file changed, 180 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> new file mode 100644
> index 000000000000..e4f938decd86
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Semtech SX9512/SX9513 based capacitive touch sensors
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The Semtech SX9512/SX9513 Family of capacitive touch controllers
> +  with integrated LED drivers. The device communication is using I2C only.
> +
> +maintainers:
> +  - David Bauer <mail@david-bauer.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - semtech,sx9512
> +      - semtech,sx9513

Devices are not compatible? What are the differences?

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  poll-interval:
> +    default: 100
> +    description: |

Do not need '|' unless you need to preserve formatting. Same comment
everywhere.

> +      The polling interval for touch events in milliseconds.

Missing -ms property unit suffix... unless you are using existing
property from common schema, but I do not see any reference (and thus
unevaluatedProperties at the end).

> +
> +patternProperties:
> +  "^channel@[0-7]$":
> +    $ref: input.yaml#
> +    type: object
> +    description: |
> +      Each node represents a channel of the touch controller.
> +      Each channel provides a capacitive touch sensor input and
> +      an LED driver output.
> +
> +    properties:
> +      reg:
> +        enum: [0, 1, 2, 3, 4, 5, 6, 7]
> +
> +      linux,keycodes:
> +        maxItems: 1
> +        description: |
> +          Specifies an array of numeric keycode values to
> +          be used for the channels. If this property is
> +          omitted, the channel is not used as a key.
> +
> +      semtech,cin-delta:

Use proper unit suffix and express it in pF.

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 3
> +        default: 3
> +        description: |
> +          The capacitance delta which is used to detect a touch
> +          or release event. The property value is mapped to a
> +          farad range between 7pF and 2.3pF internally. The delta
> +          becomes smaller the higher the value is.
> +
> +      semtech,sense-threshold:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 255
> +        default: 4
> +        description: |
> +          The threshold value after which the channel detects a touch.
> +          Refer to the datasheet for the internal calculation of the
> +          resulting touch sensitivity.
> +
> +      led:

I think subnode is here not needed. This should be part of the channel,
probably.

> +        $ref: /schemas/leds/common.yaml#
> +        type: object
> +        unevaluatedProperties: false
> +        description: |
> +          Presence of this property indicates the channel
> +          is used as an LED driver.
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false

unevaluatedProperties instead

> +
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    #include <dt-bindings/leds/common.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      touch@2b {
> +        compatible = "semtech,sx9512";
> +

Drop blank line

> +        reg = <0x2b>;
> +


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
  2025-05-06  6:21 ` [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding Krzysztof Kozlowski
@ 2025-05-06 10:05   ` David Bauer
  2025-05-12 19:50     ` Rob Herring
  2025-05-20 13:58     ` Krzysztof Kozlowski
  0 siblings, 2 replies; 9+ messages in thread
From: David Bauer @ 2025-05-06 10:05 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-input, devicetree, linux-kernel

Hi Krzysztof,

thanks for the review.

On 5/6/25 08:21, Krzysztof Kozlowski wrote:
> On 05/05/2025 22:38, David Bauer wrote:
>> Add device-tree binding for the Semtech SX9512/SX9513 family of touch
>> controllers with integrated LED driver.
>>
>> Signed-off-by: David Bauer <mail@david-bauer.net>
> 
> You CC-ed an address, which suggests you do not work on mainline kernel
> or you do not use get_maintainers.pl/b4/patman. Please rebase and always
> work on mainline or start using mentioned tools, so correct addresses
> will be used.
I'm a bit unsure what you are referring to - maybe I've set the options
for get_maintainer.pl wrong, but i use

get_maintainer.pl --nogit --nogit-fallback --norolestats --nol

to determine TO recipients and

get_maintainer.pl --nogit --nogit-fallback --norolestats --nom

for CC destinations.

Granted, my tree was a bit out of date but it was from mainline
and after rebase both commands returned consistent results.

Hope you can provide me with some guidance there.

> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument, so you will
> not CC people just because they made one commit years ago). 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) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
> 
> 
>> ---
>>   .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
>>   1 file changed, 180 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>> new file mode 100644
>> index 000000000000..e4f938decd86
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>> @@ -0,0 +1,180 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Semtech SX9512/SX9513 based capacitive touch sensors
>> +
>> +description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
>> +  The Semtech SX9512/SX9513 Family of capacitive touch controllers
>> +  with integrated LED drivers. The device communication is using I2C only.
>> +
>> +maintainers:
>> +  - David Bauer <mail@david-bauer.net>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - semtech,sx9512
>> +      - semtech,sx9513
> 
> Devices are not compatible? What are the differences?

The SX9513 is a cost-reduced version which does not
support proximity sensing. With the current support
of the driver they work identical. Should i add this
information as a comment?

> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +  poll-interval:
>> +    default: 100
>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting. Same comment
> everywhere.
> 
>> +      The polling interval for touch events in milliseconds.
> 
> Missing -ms property unit suffix... unless you are using existing
> property from common schema, but I do not see any reference (and thus
> unevaluatedProperties at the end).
> 
>> +
>> +patternProperties:
>> +  "^channel@[0-7]$":
>> +    $ref: input.yaml#
>> +    type: object
>> +    description: |
>> +      Each node represents a channel of the touch controller.
>> +      Each channel provides a capacitive touch sensor input and
>> +      an LED driver output.
>> +
>> +    properties:
>> +      reg:
>> +        enum: [0, 1, 2, 3, 4, 5, 6, 7]
>> +
>> +      linux,keycodes:
>> +        maxItems: 1
>> +        description: |
>> +          Specifies an array of numeric keycode values to
>> +          be used for the channels. If this property is
>> +          omitted, the channel is not used as a key.
>> +
>> +      semtech,cin-delta:
> 
> Use proper unit suffix and express it in pF.

To represent 2.3 and 3.8 pF, would it be better to represent in
femtofarad?

> 
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 3
>> +        default: 3
>> +        description: |
>> +          The capacitance delta which is used to detect a touch
>> +          or release event. The property value is mapped to a
>> +          farad range between 7pF and 2.3pF internally. The delta
>> +          becomes smaller the higher the value is.
>> +
>> +      semtech,sense-threshold:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 255
>> +        default: 4
>> +        description: |
>> +          The threshold value after which the channel detects a touch.
>> +          Refer to the datasheet for the internal calculation of the
>> +          resulting touch sensitivity.
>> +
>> +      led:
> 
> I think subnode is here not needed. This should be part of the channel,
> probably.

Just to be sure - you mean to have a property "led" upon which instructs
the channel to be used to drive an LED and include the LED specific
properties in the node of the channel?

> 
>> +        $ref: /schemas/leds/common.yaml#
>> +        type: object
>> +        unevaluatedProperties: false
>> +        description: |
>> +          Presence of this property indicates the channel
>> +          is used as an LED driver.
>> +
>> +    required:
>> +      - reg
>> +
>> +    additionalProperties: false
> 
> unevaluatedProperties instead
> 
>> +
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/input/input.h>
>> +    #include <dt-bindings/leds/common.h>
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      touch@2b {
>> +        compatible = "semtech,sx9512";
>> +
> 
> Drop blank line
> 
>> +        reg = <0x2b>;
>> +
> 
> 
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
  2025-05-06 10:05   ` David Bauer
@ 2025-05-12 19:50     ` Rob Herring
  2025-05-20 13:58     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 9+ messages in thread
From: Rob Herring @ 2025-05-12 19:50 UTC (permalink / raw)
  To: David Bauer
  Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, linux-input,
	devicetree, linux-kernel

On Tue, May 06, 2025 at 12:05:43PM +0200, David Bauer wrote:
> Hi Krzysztof,
> 
> thanks for the review.
> 
> On 5/6/25 08:21, Krzysztof Kozlowski wrote:
> > On 05/05/2025 22:38, David Bauer wrote:
> > > Add device-tree binding for the Semtech SX9512/SX9513 family of touch
> > > controllers with integrated LED driver.
> > > 
> > > Signed-off-by: David Bauer <mail@david-bauer.net>
> > 
> > You CC-ed an address, which suggests you do not work on mainline kernel
> > or you do not use get_maintainers.pl/b4/patman. Please rebase and always
> > work on mainline or start using mentioned tools, so correct addresses
> > will be used.
> I'm a bit unsure what you are referring to - maybe I've set the options
> for get_maintainer.pl wrong, but i use
> 
> get_maintainer.pl --nogit --nogit-fallback --norolestats --nol
> 
> to determine TO recipients and
> 
> get_maintainer.pl --nogit --nogit-fallback --norolestats --nom
> 
> for CC destinations.
> 
> Granted, my tree was a bit out of date but it was from mainline
> and after rebase both commands returned consistent results.
> 
> Hope you can provide me with some guidance there.

Probably that I don't use 'robh+dt' for a while now. Just 'robh'.

> > 
> > Please use scripts/get_maintainers.pl to get a list of necessary people
> > and lists to CC (and consider --no-git-fallback argument, so you will
> > not CC people just because they made one commit years ago). 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) or work on fork of kernel
> > (don't, instead use mainline). Just use b4 and everything should be
> > fine, although remember about `b4 prep --auto-to-cc` if you added new
> > patches to the patchset.
> > 
> > 
> > > ---
> > >   .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
> > >   1 file changed, 180 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> > > new file mode 100644
> > > index 000000000000..e4f938decd86
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> > > @@ -0,0 +1,180 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Semtech SX9512/SX9513 based capacitive touch sensors
> > > +
> > > +description: |
> > 
> > Do not need '|' unless you need to preserve formatting.
> > 
> > > +  The Semtech SX9512/SX9513 Family of capacitive touch controllers
> > > +  with integrated LED drivers. The device communication is using I2C only.
> > > +
> > > +maintainers:
> > > +  - David Bauer <mail@david-bauer.net>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - semtech,sx9512
> > > +      - semtech,sx9513
> > 
> > Devices are not compatible? What are the differences?
> 
> The SX9513 is a cost-reduced version which does not
> support proximity sensing. With the current support
> of the driver they work identical. Should i add this
> information as a comment?

In the 'description' above, but not the driver support part.

> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  '#address-cells':
> > > +    const: 1
> > > +
> > > +  '#size-cells':
> > > +    const: 0
> > > +
> > > +  poll-interval:
> > > +    default: 100
> > > +    description: |
> > 
> > Do not need '|' unless you need to preserve formatting. Same comment
> > everywhere.
> > 
> > > +      The polling interval for touch events in milliseconds.
> > 
> > Missing -ms property unit suffix... unless you are using existing
> > property from common schema, but I do not see any reference (and thus
> > unevaluatedProperties at the end).
> > 
> > > +
> > > +patternProperties:
> > > +  "^channel@[0-7]$":
> > > +    $ref: input.yaml#
> > > +    type: object
> > > +    description: |
> > > +      Each node represents a channel of the touch controller.
> > > +      Each channel provides a capacitive touch sensor input and
> > > +      an LED driver output.
> > > +
> > > +    properties:
> > > +      reg:
> > > +        enum: [0, 1, 2, 3, 4, 5, 6, 7]

maximum: 7

> > > +
> > > +      linux,keycodes:
> > > +        maxItems: 1
> > > +        description: |
> > > +          Specifies an array of numeric keycode values to
> > > +          be used for the channels. If this property is
> > > +          omitted, the channel is not used as a key.
> > > +
> > > +      semtech,cin-delta:
> > 
> > Use proper unit suffix and express it in pF.
> 
> To represent 2.3 and 3.8 pF, would it be better to represent in
> femtofarad?

Yes.


> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        minimum: 0
> > > +        maximum: 3
> > > +        default: 3
> > > +        description: |
> > > +          The capacitance delta which is used to detect a touch
> > > +          or release event. The property value is mapped to a
> > > +          farad range between 7pF and 2.3pF internally. The delta
> > > +          becomes smaller the higher the value is.
> > > +
> > > +      semtech,sense-threshold:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        minimum: 0
> > > +        maximum: 255
> > > +        default: 4
> > > +        description: |
> > > +          The threshold value after which the channel detects a touch.
> > > +          Refer to the datasheet for the internal calculation of the
> > > +          resulting touch sensitivity.
> > > +
> > > +      led:
> > 
> > I think subnode is here not needed. This should be part of the channel,
> > probably.
> 
> Just to be sure - you mean to have a property "led" upon which instructs
> the channel to be used to drive an LED and include the LED specific
> properties in the node of the channel?

Actually, I think it is fine as-is if the LED driver works 
simultaneously with the touch input and isn't mutually exclusive. The 
'led' node is for the LED. The parent node is the driver/controller.

If they are mutually exclusive, then I'd say you want channel@[0-7] and 
led@[0-7] nodes at the same level.

Rob


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
  2025-05-06 10:05   ` David Bauer
  2025-05-12 19:50     ` Rob Herring
@ 2025-05-20 13:58     ` Krzysztof Kozlowski
  2025-05-20 14:03       ` Krzysztof Kozlowski
  2025-05-20 19:40       ` David Bauer
  1 sibling, 2 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-20 13:58 UTC (permalink / raw)
  To: David Bauer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-input, devicetree, linux-kernel

On 06/05/2025 12:05, David Bauer wrote:
> Hi Krzysztof,
> 
> thanks for the review.
> 
> On 5/6/25 08:21, Krzysztof Kozlowski wrote:
>> On 05/05/2025 22:38, David Bauer wrote:
>>> Add device-tree binding for the Semtech SX9512/SX9513 family of touch
>>> controllers with integrated LED driver.
>>>
>>> Signed-off-by: David Bauer <mail@david-bauer.net>
>>
>> You CC-ed an address, which suggests you do not work on mainline kernel
>> or you do not use get_maintainers.pl/b4/patman. Please rebase and always
>> work on mainline or start using mentioned tools, so correct addresses
>> will be used.
> I'm a bit unsure what you are referring to - maybe I've set the options
> for get_maintainer.pl wrong, but i use
> 
> get_maintainer.pl --nogit --nogit-fallback --norolestats --nol
> 
> to determine TO recipients and
> 
> get_maintainer.pl --nogit --nogit-fallback --norolestats --nom
> 
> for CC destinations.
> 
> Granted, my tree was a bit out of date but it was from mainline

Mainline means latest RC or maintainer tree or linux next. v5.0 is not
mainline anymoer.

> and after rebase both commands returned consistent results.
> 
> Hope you can provide me with some guidance there.

Well, read full reply. It is impossible to get such email address from
above commands. Such email address does not exist since long time and it
easy to prove - just git grep for it. No results, so how could it be
printed by get_maintainers.pl?

If you disagree then please paste full output of:

$ git describe
$ git status
$ scripts/get_maintainer.pl 0*

I provided you extensive guideline exactly to avoid further trivial
discussions about that triviality, so I would really appreciate if you
followed it.

> 
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC (and consider --no-git-fallback argument, so you will
>> not CC people just because they made one commit years ago). 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) or work on fork of kernel
>> (don't, instead use mainline). Just use b4 and everything should be
>> fine, although remember about `b4 prep --auto-to-cc` if you added new
>> patches to the patchset.
>>
>>
>>> ---
>>>   .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
>>>   1 file changed, 180 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>> new file mode 100644
>>> index 000000000000..e4f938decd86
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>> @@ -0,0 +1,180 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Semtech SX9512/SX9513 based capacitive touch sensors
>>> +
>>> +description: |
>>
>> Do not need '|' unless you need to preserve formatting.
>>
>>> +  The Semtech SX9512/SX9513 Family of capacitive touch controllers
>>> +  with integrated LED drivers. The device communication is using I2C only.
>>> +
>>> +maintainers:
>>> +  - David Bauer <mail@david-bauer.net>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - semtech,sx9512
>>> +      - semtech,sx9513
>>
>> Devices are not compatible? What are the differences?
> 
> The SX9513 is a cost-reduced version which does not
> support proximity sensing. With the current support
> of the driver they work identical. Should i add this
> information as a comment?

So they are compatible and this should be expressed via fallback.


> 
>>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  '#address-cells':
>>> +    const: 1
>>> +
>>> +  '#size-cells':
>>> +    const: 0
>>> +
>>> +  poll-interval:
>>> +    default: 100
>>> +    description: |
>>
>> Do not need '|' unless you need to preserve formatting. Same comment
>> everywhere.
>>
>>> +      The polling interval for touch events in milliseconds.
>>
>> Missing -ms property unit suffix... unless you are using existing
>> property from common schema, but I do not see any reference (and thus
>> unevaluatedProperties at the end).
>>
>>> +
>>> +patternProperties:
>>> +  "^channel@[0-7]$":
>>> +    $ref: input.yaml#
>>> +    type: object
>>> +    description: |
>>> +      Each node represents a channel of the touch controller.
>>> +      Each channel provides a capacitive touch sensor input and
>>> +      an LED driver output.
>>> +
>>> +    properties:
>>> +      reg:
>>> +        enum: [0, 1, 2, 3, 4, 5, 6, 7]
>>> +
>>> +      linux,keycodes:
>>> +        maxItems: 1
>>> +        description: |
>>> +          Specifies an array of numeric keycode values to
>>> +          be used for the channels. If this property is
>>> +          omitted, the channel is not used as a key.
>>> +
>>> +      semtech,cin-delta:
>>
>> Use proper unit suffix and express it in pF.
> 
> To represent 2.3 and 3.8 pF, would it be better to represent in
> femtofarad?
> 
>>
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0
>>> +        maximum: 3
>>> +        default: 3
>>> +        description: |
>>> +          The capacitance delta which is used to detect a touch
>>> +          or release event. The property value is mapped to a
>>> +          farad range between 7pF and 2.3pF internally. The delta
>>> +          becomes smaller the higher the value is.
>>> +
>>> +      semtech,sense-threshold:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0
>>> +        maximum: 255
>>> +        default: 4
>>> +        description: |
>>> +          The threshold value after which the channel detects a touch.
>>> +          Refer to the datasheet for the internal calculation of the
>>> +          resulting touch sensitivity.
>>> +
>>> +      led:
>>
>> I think subnode is here not needed. This should be part of the channel,
>> probably.
> 
> Just to be sure - you mean to have a property "led" upon which instructs
> the channel to be used to drive an LED and include the LED specific
> properties in the node of the channel?
No, I do not mean a property led. I mean that child node should be
folded into parent.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
  2025-05-20 13:58     ` Krzysztof Kozlowski
@ 2025-05-20 14:03       ` Krzysztof Kozlowski
  2025-05-20 19:40       ` David Bauer
  1 sibling, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-05-20 14:03 UTC (permalink / raw)
  To: David Bauer, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-input, devicetree, linux-kernel

On 20/05/2025 15:58, Krzysztof Kozlowski wrote:
> On 06/05/2025 12:05, David Bauer wrote:
>> Hi Krzysztof,
>>
>> thanks for the review.
>>
>> On 5/6/25 08:21, Krzysztof Kozlowski wrote:
>>> On 05/05/2025 22:38, David Bauer wrote:
>>>> Add device-tree binding for the Semtech SX9512/SX9513 family of touch
>>>> controllers with integrated LED driver.
>>>>
>>>> Signed-off-by: David Bauer <mail@david-bauer.net>
>>>
>>> You CC-ed an address, which suggests you do not work on mainline kernel
>>> or you do not use get_maintainers.pl/b4/patman. Please rebase and always
>>> work on mainline or start using mentioned tools, so correct addresses
>>> will be used.
>> I'm a bit unsure what you are referring to - maybe I've set the options
>> for get_maintainer.pl wrong, but i use
>>
>> get_maintainer.pl --nogit --nogit-fallback --norolestats --nol
>>
>> to determine TO recipients and
>>
>> get_maintainer.pl --nogit --nogit-fallback --norolestats --nom
>>
>> for CC destinations.
>>
>> Granted, my tree was a bit out of date but it was from mainline
> 
> Mainline means latest RC or maintainer tree or linux next. v5.0 is not
> mainline anymoer.
> 
>> and after rebase both commands returned consistent results.
>>
>> Hope you can provide me with some guidance there.
> 
> Well, read full reply. It is impossible to get such email address from
> above commands. Such email address does not exist since long time and it
> easy to prove - just git grep for it. No results, so how could it be
> printed by get_maintainers.pl?
> 
> If you disagree then please paste full output of:
> 
> $ git describe
> $ git status
> $ scripts/get_maintainer.pl 0*
> 
> I provided you extensive guideline exactly to avoid further trivial
> discussions about that triviality, so I would really appreciate if you
> followed it.

Huh, I noticed I responded after two weeks so pretty late... Huh,
exactly because of the reason why I asked to use up2date addresses - the
mailbox used in this patchset is not being used/checked/accessed since
long time and kernel since long time has correct address.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding
  2025-05-20 13:58     ` Krzysztof Kozlowski
  2025-05-20 14:03       ` Krzysztof Kozlowski
@ 2025-05-20 19:40       ` David Bauer
  1 sibling, 0 replies; 9+ messages in thread
From: David Bauer @ 2025-05-20 19:40 UTC (permalink / raw)
  To: Dmitry Torokhov, Conor Dooley, Krzysztof Kozlowski, Rob Herring
  Cc: linux-input, devicetree, linux-kernel

Hi Krzysztof,

On 5/20/25 15:58, Krzysztof Kozlowski wrote:
> On 06/05/2025 12:05, David Bauer wrote:
>> Hi Krzysztof,
>>
>> thanks for the review.
>>
>> On 5/6/25 08:21, Krzysztof Kozlowski wrote:
>>> On 05/05/2025 22:38, David Bauer wrote:
>>>> Add device-tree binding for the Semtech SX9512/SX9513 family of touch
>>>> controllers with integrated LED driver.
>>>>
>>>> Signed-off-by: David Bauer <mail@david-bauer.net>
>>>
>>> You CC-ed an address, which suggests you do not work on mainline kernel
>>> or you do not use get_maintainers.pl/b4/patman. Please rebase and always
>>> work on mainline or start using mentioned tools, so correct addresses
>>> will be used.
>> I'm a bit unsure what you are referring to - maybe I've set the options
>> for get_maintainer.pl wrong, but i use
>>
>> get_maintainer.pl --nogit --nogit-fallback --norolestats --nol
>>
>> to determine TO recipients and
>>
>> get_maintainer.pl --nogit --nogit-fallback --norolestats --nom
>>
>> for CC destinations.
>>
>> Granted, my tree was a bit out of date but it was from mainline
> 
> Mainline means latest RC or maintainer tree or linux next. v5.0 is not
> mainline anymoer.
> 
>> and after rebase both commands returned consistent results.
>>
>> Hope you can provide me with some guidance there.
> 
> Well, read full reply. It is impossible to get such email address from
> above commands. Such email address does not exist since long time and it
> easy to prove - just git grep for it. No results, so how could it be
> printed by get_maintainers.pl?
> 
> If you disagree then please paste full output of:
> 
> $ git describe
> $ git status
> $ scripts/get_maintainer.pl 0*
> 
> I provided you extensive guideline exactly to avoid further trivial
> discussions about that triviality, so I would really appreciate if you
> followed it.

I did a complete new clone of my repository without changing the base-branch
and now the script outputs the correct set of addresses. Be assured v2
will reach the correct inbox(es).

Sorry for the confusion here. Still appreciate you did point this
out to me.

> 
>>
>>>
>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>> and lists to CC (and consider --no-git-fallback argument, so you will
>>> not CC people just because they made one commit years ago). 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) or work on fork of kernel
>>> (don't, instead use mainline). Just use b4 and everything should be
>>> fine, although remember about `b4 prep --auto-to-cc` if you added new
>>> patches to the patchset.
>>>
>>>
>>>> ---
>>>>    .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
>>>>    1 file changed, 180 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>>> new file mode 100644
>>>> index 000000000000..e4f938decd86
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
>>>> @@ -0,0 +1,180 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Semtech SX9512/SX9513 based capacitive touch sensors
>>>> +
>>>> +description: |
>>>
>>> Do not need '|' unless you need to preserve formatting.
>>>
>>>> +  The Semtech SX9512/SX9513 Family of capacitive touch controllers
>>>> +  with integrated LED drivers. The device communication is using I2C only.
>>>> +
>>>> +maintainers:
>>>> +  - David Bauer <mail@david-bauer.net>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - semtech,sx9512
>>>> +      - semtech,sx9513
>>>
>>> Devices are not compatible? What are the differences?
>>
>> The SX9513 is a cost-reduced version which does not
>> support proximity sensing. With the current support
>> of the driver they work identical. Should i add this
>> information as a comment?
> 
> So they are compatible and this should be expressed via fallback.
> 
> 
>>
>>>
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  '#address-cells':
>>>> +    const: 1
>>>> +
>>>> +  '#size-cells':
>>>> +    const: 0
>>>> +
>>>> +  poll-interval:
>>>> +    default: 100
>>>> +    description: |
>>>
>>> Do not need '|' unless you need to preserve formatting. Same comment
>>> everywhere.
>>>
>>>> +      The polling interval for touch events in milliseconds.
>>>
>>> Missing -ms property unit suffix... unless you are using existing
>>> property from common schema, but I do not see any reference (and thus
>>> unevaluatedProperties at the end).
>>>
>>>> +
>>>> +patternProperties:
>>>> +  "^channel@[0-7]$":
>>>> +    $ref: input.yaml#
>>>> +    type: object
>>>> +    description: |
>>>> +      Each node represents a channel of the touch controller.
>>>> +      Each channel provides a capacitive touch sensor input and
>>>> +      an LED driver output.
>>>> +
>>>> +    properties:
>>>> +      reg:
>>>> +        enum: [0, 1, 2, 3, 4, 5, 6, 7]
>>>> +
>>>> +      linux,keycodes:
>>>> +        maxItems: 1
>>>> +        description: |
>>>> +          Specifies an array of numeric keycode values to
>>>> +          be used for the channels. If this property is
>>>> +          omitted, the channel is not used as a key.
>>>> +
>>>> +      semtech,cin-delta:
>>>
>>> Use proper unit suffix and express it in pF.
>>
>> To represent 2.3 and 3.8 pF, would it be better to represent in
>> femtofarad?
>>
>>>
>>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>>> +        minimum: 0
>>>> +        maximum: 3
>>>> +        default: 3
>>>> +        description: |
>>>> +          The capacitance delta which is used to detect a touch
>>>> +          or release event. The property value is mapped to a
>>>> +          farad range between 7pF and 2.3pF internally. The delta
>>>> +          becomes smaller the higher the value is.
>>>> +
>>>> +      semtech,sense-threshold:
>>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>>> +        minimum: 0
>>>> +        maximum: 255
>>>> +        default: 4
>>>> +        description: |
>>>> +          The threshold value after which the channel detects a touch.
>>>> +          Refer to the datasheet for the internal calculation of the
>>>> +          resulting touch sensitivity.
>>>> +
>>>> +      led:
>>>
>>> I think subnode is here not needed. This should be part of the channel,
>>> probably.
>>
>> Just to be sure - you mean to have a property "led" upon which instructs
>> the channel to be used to drive an LED and include the LED specific
>> properties in the node of the channel?
> No, I do not mean a property led. I mean that child node should be
> folded into parent.

Okay, now i get it. The hardware supports the exclusive as well as combined
use of channels as LED-driver and touch input.

In case the channel is not wired up to work as an input, the linux,keycodes
property can be omitted, not creating the input channel.

In turn if the channel is not wired up to an LED, omitting the led subnode
prevents an led device from being created for this channel.

If I were to fold the led subnode into the channel node I would still need
a way to indicate if an LED is actually present in hardware, correct? This
was what i was aiming for with the "led" property. However if I've had to
choose between these two approaches, I prefer the subnode.

If you have a different idea how to handle this apart from Rob's suggestion,
It could also be the case I'm creating more problems than I solve with this
approach :)

Best
David


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-05-20 19:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-05 20:38 [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding David Bauer
2025-05-05 20:38 ` [PATCH 2/2] Input sx951x: add Semtech SX9512/SX9513 driver David Bauer
2025-05-06  5:26   ` Dmitry Torokhov
2025-05-06  6:21 ` [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding Krzysztof Kozlowski
2025-05-06 10:05   ` David Bauer
2025-05-12 19:50     ` Rob Herring
2025-05-20 13:58     ` Krzysztof Kozlowski
2025-05-20 14:03       ` Krzysztof Kozlowski
2025-05-20 19:40       ` David Bauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).