[PATCH v3 0/2] Input: cypress-sf - Add support for Cypress Streetfighter touchkeys

STALE1800d

Revision v3 of 4 in this series.

5 messages, 2 authors, 2021-09-07 · open the first message on its own page

[PATCH v3 0/2] Input: cypress-sf - Add support for Cypress Streetfighter touchkeys

From: Yassine Oudjana <y.oudjana@protonmail.com>
Date: 2021-09-01 16:53:10

This patchset adds support for Cypress StreetFighter touchkeys.

Due to lack of documentation, this driver is entirely based on information
gathered from a driver written for an old Android kernel fork[1][2].

[1] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/drivers/input/touchscreen/cyttsp_button.c
[2] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/arch/arm/boot/dts/qcom/a4-msm8996-mtp.dtsi#L291-L314

Changes since v2:
 - Code style fixes.
 - Added copyright statement.
Changes since v1:
 - Changed version variables in probe to int to allow storing error codes.
 - Fixed some issues in dt binding.
 
Yassine Oudjana (2):
  Input: cypress-sf - Add Cypress StreetFighter touchkey driver
  dt-bindings: input: Add binding for cypress-sf

 .../devicetree/bindings/input/cypress-sf.yaml |  61 +++++
 drivers/input/keyboard/Kconfig                |  10 +
 drivers/input/keyboard/Makefile               |   1 +
 drivers/input/keyboard/cypress-sf.c           | 223 ++++++++++++++++++
 4 files changed, 295 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cypress-sf.yaml
 create mode 100644 drivers/input/keyboard/cypress-sf.c

-- 
2.33.0

[PATCH v3 2/2] dt-bindings: input: Add binding for cypress-sf

From: Yassine Oudjana <y.oudjana@protonmail.com>
Date: 2021-09-01 16:53:48

Add a device tree binding for Cypress StreetFighter.

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes since v1:
 - Changed version variables in probe to int to allow storing error codes.
 .../devicetree/bindings/input/cypress-sf.yaml | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cypress-sf.yaml
diff --git a/Documentation/devicetree/bindings/input/cypress-sf.yaml b/Documentation/devicetree/bindings/input/cypress-sf.yaml
new file mode 100644
index 000000000000..14689daf6567
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cypress-sf.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cypress-sf.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cypress StreetFighter touchkey controller
+
+maintainers:
+  - Yassine Oudjana <y.oudjana@protonmail.com>
+
+allOf:
+  - $ref: input.yaml#
+
+properties:
+  compatible:
+    const: cypress,sf3155
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  avdd-supply:
+    description: Regulator for AVDD analog voltage
+
+  vdd-supply:
+    description: Regulator for VDD digital voltage
+
+  linux,keycodes:
+    minItems: 1
+    maxItems: 8
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - avdd-supply
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        touchkey@28 {
+                compatible = "cypress,sf3155";
+                reg = <0x28>;
+                interrupt-parent = <&msmgpio>;
+                interrupts = <77 IRQ_TYPE_EDGE_FALLING>;
+                avdd-supply = <&vreg_l6a_1p8>;
+                vdd-supply = <&vdd_3v2_tp>;
+                linux,keycodes = <KEY_BACK KEY_MENU>;
+        };
+    };
-- 
2.33.0

[PATCH v3 1/2] Input: cypress-sf - Add Cypress StreetFighter touchkey driver

From: Yassine Oudjana <y.oudjana@protonmail.com>
Date: 2021-09-01 16:55:18

This adds support for Cypress StreetFighter touchkey controllers such as sf3155.
This driver supports managing regulators and generating input events.

Due to lack of documentation, this driver is entirely based on information
gathered from a driver written for an old Android kernel fork[1][2].

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

[1] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/drivers/input/touchscreen/cyttsp_button.c
[2] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/arch/arm/boot/dts/qcom/a4-msm8996-mtp.dtsi#L291-L314
---
Changes since v2:
 - Code style fixes.
 - Added copyright statement.
Changes since v1:
 - Changed version variables in probe to int to allow storing error codes.

 drivers/input/keyboard/Kconfig      |  10 ++
 drivers/input/keyboard/Makefile     |   1 +
 drivers/input/keyboard/cypress-sf.c | 223 ++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/input/keyboard/cypress-sf.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 40a070a2e7f5..6f3fbea8b803 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -788,4 +788,14 @@ config KEYBOARD_MTK_PMIC
 	  To compile this driver as a module, choose M here: the
 	  module will be called pmic-keys.
 
+config KEYBOARD_CYPRESS_SF
+	tristate "Cypress StreetFighter touchkey support"
+	depends on I2C
+	help
+	  Say Y here if you want to enable support for Cypress StreetFighter
+	  touchkeys.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cypress-sf.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d689fdd5c00..e3c8648f834e 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_KEYBOARD_BCM)		+= bcm-keypad.o
 obj-$(CONFIG_KEYBOARD_CAP11XX)		+= cap11xx.o
 obj-$(CONFIG_KEYBOARD_CLPS711X)		+= clps711x-keypad.o
 obj-$(CONFIG_KEYBOARD_CROS_EC)		+= cros_ec_keyb.o
+obj-$(CONFIG_KEYBOARD_CYPRESS_SF)	+= cypress-sf.o
 obj-$(CONFIG_KEYBOARD_DAVINCI)		+= davinci_keyscan.o
 obj-$(CONFIG_KEYBOARD_DLINK_DIR685)	+= dlink-dir685-touchkeys.o
 obj-$(CONFIG_KEYBOARD_EP93XX)		+= ep93xx_keypad.o
diff --git a/drivers/input/keyboard/cypress-sf.c b/drivers/input/keyboard/cypress-sf.c
new file mode 100644
index 000000000000..f2862547f633
--- /dev/null
+++ b/drivers/input/keyboard/cypress-sf.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cypress StreetFighter Touchkey Driver
+ *
+ * Copyright (c) 2021 Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+
+#define CYPRESS_SF_DEV_NAME "cypress-sf"
+
+#define CYPRESS_SF_REG_FW_VERSION	0x46
+#define CYPRESS_SF_REG_HW_VERSION	0x48
+#define CYPRESS_SF_REG_BUTTON_STATUS	0x4a
+
+struct cypress_sf_data {
+	struct i2c_client *client;
+	struct input_dev *input_dev;
+	struct regulator_bulk_data regulators[2];
+	u32 *keycodes;
+	unsigned long keystates;
+	int num_keys;
+};
+
+static irqreturn_t cypress_sf_irq_handler(int irq, void *devid)
+{
+	struct cypress_sf_data *touchkey = devid;
+	unsigned long keystates;
+	bool curr_state, new_state;
+	int key;
+
+	keystates = i2c_smbus_read_byte_data(touchkey->client,
+					CYPRESS_SF_REG_BUTTON_STATUS);
+	if (keystates < 0) {
+		dev_err(&touchkey->client->dev, "Failed to read button status");
+		return IRQ_NONE;
+	}
+
+	for(key = 0; key < touchkey->num_keys; ++key) {
+		curr_state = test_bit(key, &touchkey->keystates);
+		new_state = test_bit(key, &keystates);
+
+		if(curr_state ^ new_state) {
+			dev_dbg(&touchkey->client->dev,\
+				"Key %d changed to %d", key, new_state);
+			input_report_key(touchkey->input_dev,
+					touchkey->keycodes[key],
+					new_state);
+		}
+	}
+	input_sync(touchkey->input_dev);
+	touchkey->keystates = keystates;
+
+	return IRQ_HANDLED;
+}
+
+static int cypress_sf_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	struct cypress_sf_data *touchkey;
+	int hw_version, fw_version;
+	int i, ret;
+
+	touchkey = devm_kzalloc(&client->dev, sizeof(*touchkey), GFP_KERNEL);
+	if(!touchkey)
+		return -ENOMEM;
+
+	touchkey->client = client;
+	i2c_set_clientdata(client, touchkey);
+
+	touchkey->regulators[0].supply = "vdd";
+	touchkey->regulators[1].supply = "avdd";
+
+	ret = devm_regulator_bulk_get(&client->dev,
+					ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(&client->dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
+	touchkey->num_keys = of_property_count_elems_of_size(client->dev.of_node,
+							"linux,keycodes",
+							sizeof(u32));
+	if(touchkey->num_keys < 0)
+		/* Default key count */
+		touchkey->num_keys = 2;
+
+	touchkey->keycodes = devm_kzalloc(&client->dev,
+				sizeof(u32) * touchkey->num_keys, GFP_KERNEL);
+	if(!touchkey->keycodes)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(client->dev.of_node, "linux,keycodes",
+						touchkey->keycodes, touchkey->num_keys);
+
+	if(touchkey->num_keys < 0) {
+		/* Default keycodes */
+		touchkey->keycodes[0] = KEY_BACK;
+		touchkey->keycodes[1] = KEY_MENU;
+	}
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(&client->dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	touchkey->input_dev = devm_input_allocate_device(&client->dev);
+	if(!touchkey->input_dev) {
+		dev_err(&client->dev, "Failed to allocate input device\n");
+		return -ENOMEM;
+	}
+
+	touchkey->input_dev->name = CYPRESS_SF_DEV_NAME;
+	touchkey->input_dev->id.bustype = BUS_I2C;
+
+	hw_version = i2c_smbus_read_byte_data(touchkey->client,
+				CYPRESS_SF_REG_HW_VERSION);
+	fw_version = i2c_smbus_read_word_data(touchkey->client,
+				CYPRESS_SF_REG_FW_VERSION);
+	if(hw_version < 0 || fw_version < 0)
+		dev_warn(&client->dev, "Failed to read versions\n");
+	else
+		dev_info(&client->dev, "HW version %d, FW version %d\n",
+				hw_version, fw_version);
+
+	for(i = 0; i < touchkey->num_keys; ++i)
+		input_set_capability(touchkey->input_dev, EV_KEY,
+					touchkey->keycodes[i]);
+
+	ret = input_register_device(touchkey->input_dev);
+	if(ret) {
+		dev_err(&client->dev,
+			"Failed to register input device: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&client->dev, client->irq,
+					NULL, cypress_sf_irq_handler,
+					IRQF_ONESHOT,
+					CYPRESS_SF_DEV_NAME, touchkey);
+	if(ret) {
+		dev_err(&client->dev,
+			"Failed to register threaded irq: %d", ret);
+		return ret;
+	}
+
+	return 0;
+};
+
+static int __maybe_unused cypress_sf_suspend(struct device *dev) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct cypress_sf_data *touchkey = i2c_get_clientdata(client);
+	int ret;
+
+	disable_irq(client->irq);
+	ret = regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(dev, "Failed to disable regulators: %d", ret);
+		enable_irq(client->irq);
+		return ret;
+	}
+	dev_dbg(dev, "Suspended device");
+
+	return 0;
+}
+
+static int __maybe_unused cypress_sf_resume(struct device *dev) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct cypress_sf_data *touchkey = i2c_get_clientdata(client);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(dev, "Failed to enable regulators: %d", ret);
+		return ret;
+	}
+	enable_irq(client->irq);
+	dev_dbg(dev, "Resumed device");
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(cypress_sf_pm_ops,
+			 cypress_sf_suspend, cypress_sf_resume);
+
+static struct i2c_device_id cypress_sf_id_table[] = {
+	{ CYPRESS_SF_DEV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, cypress_sf_id_table);
+
+static const struct of_device_id cypress_sf_of_match[] = {
+	{ .compatible = "cypress,sf3155", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, cypress_sf_of_match);
+
+static struct i2c_driver cypress_sf_driver = {
+	.driver = {
+		.name = CYPRESS_SF_DEV_NAME,
+		.pm = &cypress_sf_pm_ops,
+		.of_match_table = of_match_ptr(cypress_sf_of_match),
+	},
+	.id_table = cypress_sf_id_table,
+	.probe = cypress_sf_probe,
+};
+module_i2c_driver(cypress_sf_driver);
+
+MODULE_AUTHOR("Yassine Oudjana <y.oudjana@protonmail.com>");
+MODULE_DESCRIPTION("Cypress StreetFighter Touchkey Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.33.0

Re: [PATCH v3 1/2] Input: cypress-sf - Add Cypress StreetFighter touchkey driver

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-09-06 06:31:53

Hi Yassine,

On Wed, Sep 01, 2021 at 04:52:58PM +0000, Yassine Oudjana wrote:
This adds support for Cypress StreetFighter touchkey controllers such as sf3155.
This driver supports managing regulators and generating input events.

Due to lack of documentation, this driver is entirely based on information
gathered from a driver written for an old Android kernel fork[1][2].

Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
Thank you for the patch. Please run your patches through checkpatch.pl.
See additional comments below.
quoted hunk
[1] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/drivers/input/touchscreen/cyttsp_button.c
[2] https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/arch/arm/boot/dts/qcom/a4-msm8996-mtp.dtsi#L291-L314
---
Changes since v2:
 - Code style fixes.
 - Added copyright statement.
Changes since v1:
 - Changed version variables in probe to int to allow storing error codes.

 drivers/input/keyboard/Kconfig      |  10 ++
 drivers/input/keyboard/Makefile     |   1 +
 drivers/input/keyboard/cypress-sf.c | 223 ++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/input/keyboard/cypress-sf.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 40a070a2e7f5..6f3fbea8b803 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -788,4 +788,14 @@ config KEYBOARD_MTK_PMIC
 	  To compile this driver as a module, choose M here: the
 	  module will be called pmic-keys.
 
+config KEYBOARD_CYPRESS_SF
+	tristate "Cypress StreetFighter touchkey support"
+	depends on I2C
+	help
+	  Say Y here if you want to enable support for Cypress StreetFighter
+	  touchkeys.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cypress-sf.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d689fdd5c00..e3c8648f834e 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_KEYBOARD_BCM)		+= bcm-keypad.o
 obj-$(CONFIG_KEYBOARD_CAP11XX)		+= cap11xx.o
 obj-$(CONFIG_KEYBOARD_CLPS711X)		+= clps711x-keypad.o
 obj-$(CONFIG_KEYBOARD_CROS_EC)		+= cros_ec_keyb.o
+obj-$(CONFIG_KEYBOARD_CYPRESS_SF)	+= cypress-sf.o
 obj-$(CONFIG_KEYBOARD_DAVINCI)		+= davinci_keyscan.o
 obj-$(CONFIG_KEYBOARD_DLINK_DIR685)	+= dlink-dir685-touchkeys.o
 obj-$(CONFIG_KEYBOARD_EP93XX)		+= ep93xx_keypad.o
diff --git a/drivers/input/keyboard/cypress-sf.c b/drivers/input/keyboard/cypress-sf.c
new file mode 100644
index 000000000000..f2862547f633
--- /dev/null
+++ b/drivers/input/keyboard/cypress-sf.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cypress StreetFighter Touchkey Driver
+ *
+ * Copyright (c) 2021 Yassine Oudjana <y.oudjana@protonmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+
+#define CYPRESS_SF_DEV_NAME "cypress-sf"
+
+#define CYPRESS_SF_REG_FW_VERSION	0x46
+#define CYPRESS_SF_REG_HW_VERSION	0x48
+#define CYPRESS_SF_REG_BUTTON_STATUS	0x4a
+
+struct cypress_sf_data {
+	struct i2c_client *client;
+	struct input_dev *input_dev;
+	struct regulator_bulk_data regulators[2];
+	u32 *keycodes;
+	unsigned long keystates;
+	int num_keys;
+};
+
+static irqreturn_t cypress_sf_irq_handler(int irq, void *devid)
+{
+	struct cypress_sf_data *touchkey = devid;
+	unsigned long keystates;
+	bool curr_state, new_state;
+	int key;
+
+	keystates = i2c_smbus_read_byte_data(touchkey->client,
+					CYPRESS_SF_REG_BUTTON_STATUS);
+	if (keystates < 0) {
keystates is declared as unsigned, so this condition will never trigger.
+		dev_err(&touchkey->client->dev, "Failed to read button status");
+		return IRQ_NONE;
+	}
+
+	for(key = 0; key < touchkey->num_keys; ++key) {
+		curr_state = test_bit(key, &touchkey->keystates);
+		new_state = test_bit(key, &keystates);
+
+		if(curr_state ^ new_state) {
Instead of this please do

	bitmap_xor(&changed, &keystates, &touchkey->keystates,
		   touchkey->num_keys);
	for_each_set_bit(key, &changed, touchkey->num_keys) {
		...
	}
+			dev_dbg(&touchkey->client->dev,\
+				"Key %d changed to %d", key, new_state);
+			input_report_key(touchkey->input_dev,
+					touchkey->keycodes[key],
+					new_state);
+		}
+	}
+	input_sync(touchkey->input_dev);
+	touchkey->keystates = keystates;
+
+	return IRQ_HANDLED;
+}
+
+static int cypress_sf_probe(struct i2c_client *client,
+				const struct i2c_device_id *id)
+{
+	struct cypress_sf_data *touchkey;
+	int hw_version, fw_version;
+	int i, ret;
+
+	touchkey = devm_kzalloc(&client->dev, sizeof(*touchkey), GFP_KERNEL);
+	if(!touchkey)
+		return -ENOMEM;
+
+	touchkey->client = client;
+	i2c_set_clientdata(client, touchkey);
+
+	touchkey->regulators[0].supply = "vdd";
+	touchkey->regulators[1].supply = "avdd";
+
+	ret = devm_regulator_bulk_get(&client->dev,
+					ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(&client->dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
+	touchkey->num_keys = of_property_count_elems_of_size(client->dev.of_node,
+							"linux,keycodes",
+							sizeof(u32));
Please use device_property_* API instead of of_property_*.
+	if(touchkey->num_keys < 0)
+		/* Default key count */
+		touchkey->num_keys = 2;
+
+	touchkey->keycodes = devm_kzalloc(&client->dev,
+				sizeof(u32) * touchkey->num_keys, GFP_KERNEL);
+	if(!touchkey->keycodes)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(client->dev.of_node, "linux,keycodes",
+						touchkey->keycodes, touchkey->num_keys);
+
+	if(touchkey->num_keys < 0) {
+		/* Default keycodes */
+		touchkey->keycodes[0] = KEY_BACK;
+		touchkey->keycodes[1] = KEY_MENU;
+	}
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
Please call variables that carry error core or 0 for success 'error'.
+	if(ret) {
+		dev_err(&client->dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	touchkey->input_dev = devm_input_allocate_device(&client->dev);
+	if(!touchkey->input_dev) {
+		dev_err(&client->dev, "Failed to allocate input device\n");
+		return -ENOMEM;
+	}
+
+	touchkey->input_dev->name = CYPRESS_SF_DEV_NAME;
+	touchkey->input_dev->id.bustype = BUS_I2C;
+
+	hw_version = i2c_smbus_read_byte_data(touchkey->client,
+				CYPRESS_SF_REG_HW_VERSION);
+	fw_version = i2c_smbus_read_word_data(touchkey->client,
+				CYPRESS_SF_REG_FW_VERSION);
+	if(hw_version < 0 || fw_version < 0)
+		dev_warn(&client->dev, "Failed to read versions\n");
+	else
+		dev_info(&client->dev, "HW version %d, FW version %d\n",
+				hw_version, fw_version);
Why is this needed?
+
+	for(i = 0; i < touchkey->num_keys; ++i)
+		input_set_capability(touchkey->input_dev, EV_KEY,
+					touchkey->keycodes[i]);
+
+	ret = input_register_device(touchkey->input_dev);
+	if(ret) {
+		dev_err(&client->dev,
+			"Failed to register input device: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_request_threaded_irq(&client->dev, client->irq,
+					NULL, cypress_sf_irq_handler,
+					IRQF_ONESHOT,
+					CYPRESS_SF_DEV_NAME, touchkey);
+	if(ret) {
+		dev_err(&client->dev,
+			"Failed to register threaded irq: %d", ret);
+		return ret;
+	}
+
+	return 0;
+};
+
+static int __maybe_unused cypress_sf_suspend(struct device *dev) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct cypress_sf_data *touchkey = i2c_get_clientdata(client);
+	int ret;
+
+	disable_irq(client->irq);
+	ret = regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(dev, "Failed to disable regulators: %d", ret);
+		enable_irq(client->irq);
+		return ret;
+	}
+	dev_dbg(dev, "Suspended device");
+
+	return 0;
+}
+
+static int __maybe_unused cypress_sf_resume(struct device *dev) {
+	struct i2c_client *client = to_i2c_client(dev);
+	struct cypress_sf_data *touchkey = i2c_get_clientdata(client);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
+					touchkey->regulators);
+	if(ret) {
+		dev_err(dev, "Failed to enable regulators: %d", ret);
+		return ret;
+	}
+	enable_irq(client->irq);
+	dev_dbg(dev, "Resumed device");
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(cypress_sf_pm_ops,
+			 cypress_sf_suspend, cypress_sf_resume);
+
+static struct i2c_device_id cypress_sf_id_table[] = {
+	{ CYPRESS_SF_DEV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, cypress_sf_id_table);
+
+static const struct of_device_id cypress_sf_of_match[] = {
+	{ .compatible = "cypress,sf3155", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, cypress_sf_of_match);
+
+static struct i2c_driver cypress_sf_driver = {
+	.driver = {
+		.name = CYPRESS_SF_DEV_NAME,
+		.pm = &cypress_sf_pm_ops,
+		.of_match_table = of_match_ptr(cypress_sf_of_match),
+	},
+	.id_table = cypress_sf_id_table,
+	.probe = cypress_sf_probe,
+};
+module_i2c_driver(cypress_sf_driver);
+
+MODULE_AUTHOR("Yassine Oudjana [off-list ref]");
+MODULE_DESCRIPTION("Cypress StreetFighter Touchkey Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.33.0
Thanks.

-- 
Dmitry

Re: [PATCH v3 1/2] Input: cypress-sf - Add Cypress StreetFighter touchkey driver

From: Yassine Oudjana <y.oudjana@protonmail.com>
Date: 2021-09-07 09:02:52


On Mon, Sep 6 2021 at 10:31:42 +0400, Dmitry Torokhov 
[off-list ref] wrote:
Hi Yassine,

On Wed, Sep 01, 2021 at 04:52:58PM +0000, Yassine Oudjana wrote:
quoted
 This adds support for Cypress StreetFighter touchkey controllers 
such as sf3155.
 This driver supports managing regulators and generating input 
events.

 Due to lack of documentation, this driver is entirely based on 
information
 gathered from a driver written for an old Android kernel fork[1][2].

 Signed-off-by: Yassine Oudjana [off-list ref]
Thank you for the patch. Please run your patches through 
checkpatch.pl.
Sure.
See additional comments below.

quoted
 [1] 
https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/drivers/input/touchscreen/cyttsp_button.c
 [2] 
https://github.com/LineageOS/android_kernel_xiaomi_msm8996/blob/lineage-18.1/arch/arm/boot/dts/qcom/a4-msm8996-mtp.dtsi#L291-L314
 ---
 Changes since v2:
  - Code style fixes.
  - Added copyright statement.
 Changes since v1:
  - Changed version variables in probe to int to allow storing error 
codes.

  drivers/input/keyboard/Kconfig      |  10 ++
  drivers/input/keyboard/Makefile     |   1 +
  drivers/input/keyboard/cypress-sf.c | 223 
++++++++++++++++++++++++++++
  3 files changed, 234 insertions(+)
  create mode 100644 drivers/input/keyboard/cypress-sf.c

 diff --git a/drivers/input/keyboard/Kconfig 
b/drivers/input/keyboard/Kconfig
 index 40a070a2e7f5..6f3fbea8b803 100644
 --- a/drivers/input/keyboard/Kconfig
 +++ b/drivers/input/keyboard/Kconfig
 @@ -788,4 +788,14 @@ config KEYBOARD_MTK_PMIC
  	  To compile this driver as a module, choose M here: the
  	  module will be called pmic-keys.

 +config KEYBOARD_CYPRESS_SF
 +	tristate "Cypress StreetFighter touchkey support"
 +	depends on I2C
 +	help
 +	  Say Y here if you want to enable support for Cypress 
StreetFighter
 +	  touchkeys.
 +
 +	  To compile this driver as a module, choose M here: the
 +	  module will be called cypress-sf.
 +
  endif
 diff --git a/drivers/input/keyboard/Makefile 
b/drivers/input/keyboard/Makefile
 index 1d689fdd5c00..e3c8648f834e 100644
 --- a/drivers/input/keyboard/Makefile
 +++ b/drivers/input/keyboard/Makefile
 @@ -17,6 +17,7 @@ obj-$(CONFIG_KEYBOARD_BCM)		+= bcm-keypad.o
  obj-$(CONFIG_KEYBOARD_CAP11XX)		+= cap11xx.o
  obj-$(CONFIG_KEYBOARD_CLPS711X)		+= clps711x-keypad.o
  obj-$(CONFIG_KEYBOARD_CROS_EC)		+= cros_ec_keyb.o
 +obj-$(CONFIG_KEYBOARD_CYPRESS_SF)	+= cypress-sf.o
  obj-$(CONFIG_KEYBOARD_DAVINCI)		+= davinci_keyscan.o
  obj-$(CONFIG_KEYBOARD_DLINK_DIR685)	+= dlink-dir685-touchkeys.o
  obj-$(CONFIG_KEYBOARD_EP93XX)		+= ep93xx_keypad.o
 diff --git a/drivers/input/keyboard/cypress-sf.c 
b/drivers/input/keyboard/cypress-sf.c
 new file mode 100644
 index 000000000000..f2862547f633
 --- /dev/null
 +++ b/drivers/input/keyboard/cypress-sf.c
 @@ -0,0 +1,223 @@
 +// SPDX-License-Identifier: GPL-2.0-only
 +/*
 + * Cypress StreetFighter Touchkey Driver
 + *
 + * Copyright (c) 2021 Yassine Oudjana [off-list ref]
 + */
 +
 +#include <linux/bitops.h>
 +#include <linux/device.h>
 +#include <linux/i2c.h>
 +#include <linux/input.h>
 +#include <linux/interrupt.h>
 +#include <linux/module.h>
 +#include <linux/pm.h>
 +#include <linux/regulator/consumer.h>
 +
 +#define CYPRESS_SF_DEV_NAME "cypress-sf"
 +
 +#define CYPRESS_SF_REG_FW_VERSION	0x46
 +#define CYPRESS_SF_REG_HW_VERSION	0x48
 +#define CYPRESS_SF_REG_BUTTON_STATUS	0x4a
 +
 +struct cypress_sf_data {
 +	struct i2c_client *client;
 +	struct input_dev *input_dev;
 +	struct regulator_bulk_data regulators[2];
 +	u32 *keycodes;
 +	unsigned long keystates;
 +	int num_keys;
 +};
 +
 +static irqreturn_t cypress_sf_irq_handler(int irq, void *devid)
 +{
 +	struct cypress_sf_data *touchkey = devid;
 +	unsigned long keystates;
 +	bool curr_state, new_state;
 +	int key;
 +
 +	keystates = i2c_smbus_read_byte_data(touchkey->client,
 +					CYPRESS_SF_REG_BUTTON_STATUS);
 +	if (keystates < 0) {
keystates is declared as unsigned, so this condition will never 
trigger.
Right. I'll change it.
quoted
 +		dev_err(&touchkey->client->dev, "Failed to read button status");
 +		return IRQ_NONE;
 +	}
 +
 +	for(key = 0; key < touchkey->num_keys; ++key) {
 +		curr_state = test_bit(key, &touchkey->keystates);
 +		new_state = test_bit(key, &keystates);
 +
 +		if(curr_state ^ new_state) {
Instead of this please do

	bitmap_xor(&changed, &keystates, &touchkey->keystates,
		   touchkey->num_keys);
	for_each_set_bit(key, &changed, touchkey->num_keys) {
		...
	}
Noted.
quoted
 +			dev_dbg(&touchkey->client->dev,\
 +				"Key %d changed to %d", key, new_state);
 +			input_report_key(touchkey->input_dev,
 +					touchkey->keycodes[key],
 +					new_state);
 +		}
 +	}
 +	input_sync(touchkey->input_dev);
 +	touchkey->keystates = keystates;
 +
 +	return IRQ_HANDLED;
 +}
 +
 +static int cypress_sf_probe(struct i2c_client *client,
 +				const struct i2c_device_id *id)
 +{
 +	struct cypress_sf_data *touchkey;
 +	int hw_version, fw_version;
 +	int i, ret;
 +
 +	touchkey = devm_kzalloc(&client->dev, sizeof(*touchkey), 
GFP_KERNEL);
 +	if(!touchkey)
 +		return -ENOMEM;
 +
 +	touchkey->client = client;
 +	i2c_set_clientdata(client, touchkey);
 +
 +	touchkey->regulators[0].supply = "vdd";
 +	touchkey->regulators[1].supply = "avdd";
 +
 +	ret = devm_regulator_bulk_get(&client->dev,
 +					ARRAY_SIZE(touchkey->regulators),
 +					touchkey->regulators);
 +	if(ret) {
 +		dev_err(&client->dev, "Failed to get regulators: %d\n", ret);
 +		return ret;
 +	}
 +
 +	touchkey->num_keys = 
of_property_count_elems_of_size(client->dev.of_node,
 +							"linux,keycodes",
 +							sizeof(u32));
Please use device_property_* API instead of of_property_*.
Noted.
quoted
 +	if(touchkey->num_keys < 0)
 +		/* Default key count */
 +		touchkey->num_keys = 2;
 +
 +	touchkey->keycodes = devm_kzalloc(&client->dev,
 +				sizeof(u32) * touchkey->num_keys, GFP_KERNEL);
 +	if(!touchkey->keycodes)
 +		return -ENOMEM;
 +
 +	ret = of_property_read_u32_array(client->dev.of_node, 
"linux,keycodes",
 +						touchkey->keycodes, touchkey->num_keys);
 +
 +	if(touchkey->num_keys < 0) {
 +		/* Default keycodes */
 +		touchkey->keycodes[0] = KEY_BACK;
 +		touchkey->keycodes[1] = KEY_MENU;
 +	}
 +
 +	ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
 +					touchkey->regulators);
Please call variables that carry error core or 0 for success 'error'.
I'll rename them.
quoted
 +	if(ret) {
 +		dev_err(&client->dev, "Failed to enable regulators: %d\n", ret);
 +		return ret;
 +	}
 +
 +	touchkey->input_dev = devm_input_allocate_device(&client->dev);
 +	if(!touchkey->input_dev) {
 +		dev_err(&client->dev, "Failed to allocate input device\n");
 +		return -ENOMEM;
 +	}
 +
 +	touchkey->input_dev->name = CYPRESS_SF_DEV_NAME;
 +	touchkey->input_dev->id.bustype = BUS_I2C;
 +
 +	hw_version = i2c_smbus_read_byte_data(touchkey->client,
 +				CYPRESS_SF_REG_HW_VERSION);
 +	fw_version = i2c_smbus_read_word_data(touchkey->client,
 +				CYPRESS_SF_REG_FW_VERSION);
 +	if(hw_version < 0 || fw_version < 0)
 +		dev_warn(&client->dev, "Failed to read versions\n");
 +	else
 +		dev_info(&client->dev, "HW version %d, FW version %d\n",
 +				hw_version, fw_version);
Why is this needed?
I was using it as an indicator for success finding the chip, but it 
isn't
strictly needed. I'll remove it.
quoted
 +
 +	for(i = 0; i < touchkey->num_keys; ++i)
 +		input_set_capability(touchkey->input_dev, EV_KEY,
 +					touchkey->keycodes[i]);
 +
 +	ret = input_register_device(touchkey->input_dev);
 +	if(ret) {
 +		dev_err(&client->dev,
 +			"Failed to register input device: %d\n", ret);
 +		return ret;
 +	}
 +
 +	ret = devm_request_threaded_irq(&client->dev, client->irq,
 +					NULL, cypress_sf_irq_handler,
 +					IRQF_ONESHOT,
 +					CYPRESS_SF_DEV_NAME, touchkey);
 +	if(ret) {
 +		dev_err(&client->dev,
 +			"Failed to register threaded irq: %d", ret);
 +		return ret;
 +	}
 +
 +	return 0;
 +};
 +
 +static int __maybe_unused cypress_sf_suspend(struct device *dev) {
 +	struct i2c_client *client = to_i2c_client(dev);
 +	struct cypress_sf_data *touchkey = i2c_get_clientdata(client);
 +	int ret;
 +
 +	disable_irq(client->irq);
 +	ret = regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators),
 +					touchkey->regulators);
 +	if(ret) {
 +		dev_err(dev, "Failed to disable regulators: %d", ret);
 +		enable_irq(client->irq);
 +		return ret;
 +	}
 +	dev_dbg(dev, "Suspended device");
 +
 +	return 0;
 +}
 +
 +static int __maybe_unused cypress_sf_resume(struct device *dev) {
 +	struct i2c_client *client = to_i2c_client(dev);
 +	struct cypress_sf_data *touchkey = i2c_get_clientdata(client);
 +	int ret;
 +
 +	ret = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
 +					touchkey->regulators);
 +	if(ret) {
 +		dev_err(dev, "Failed to enable regulators: %d", ret);
 +		return ret;
 +	}
 +	enable_irq(client->irq);
 +	dev_dbg(dev, "Resumed device");
 +
 +	return 0;
 +}
 +
 +static SIMPLE_DEV_PM_OPS(cypress_sf_pm_ops,
 +			 cypress_sf_suspend, cypress_sf_resume);
 +
 +static struct i2c_device_id cypress_sf_id_table[] = {
 +	{ CYPRESS_SF_DEV_NAME, 0 },
 +	{ }
 +};
 +MODULE_DEVICE_TABLE(i2c, cypress_sf_id_table);
 +
 +static const struct of_device_id cypress_sf_of_match[] = {
 +	{ .compatible = "cypress,sf3155", },
 +	{ },
 +};
 +MODULE_DEVICE_TABLE(of, cypress_sf_of_match);
 +
 +static struct i2c_driver cypress_sf_driver = {
 +	.driver = {
 +		.name = CYPRESS_SF_DEV_NAME,
 +		.pm = &cypress_sf_pm_ops,
 +		.of_match_table = of_match_ptr(cypress_sf_of_match),
 +	},
 +	.id_table = cypress_sf_id_table,
 +	.probe = cypress_sf_probe,
 +};
 +module_i2c_driver(cypress_sf_driver);
 +
 +MODULE_AUTHOR("Yassine Oudjana [off-list ref]");
 +MODULE_DESCRIPTION("Cypress StreetFighter Touchkey Driver");
 +MODULE_LICENSE("GPL v2");
 --
 2.33.0
Thanks.

--
Dmitry
Thanks for the review!
Yassine


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help