[PATCH 1/7] Input: cy8ctmg110_ts - rely on platform code to supply interrupt

Subsystems: input (keyboard, mouse, joystick, touchscreen) drivers, the rest

STALE1900d LANDED

Landed in mainline as 6abee582034c on 2021-06-06.

16 messages, 2 authors, 2021-06-06 · open the first message on its own page

[PATCH 1/7] Input: cy8ctmg110_ts - rely on platform code to supply interrupt

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:38:49

Instead of using platform data to specify GPIO that is used as interrupt
source, rely on the platform and I2C core to set it up properly.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 32 +----------------------
 include/linux/input/cy8ctmg110_pdata.h    |  1 -
 2 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index f465bae618fe..691f35f1bdd7 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -46,7 +46,6 @@ struct cy8ctmg110 {
 	char phys[32];
 	struct i2c_client *client;
 	int reset_pin;
-	int irq_pin;
 };
 
 /*

@@ -191,7 +190,6 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 	ts->client = client;
 	ts->input = input_dev;
 	ts->reset_pin = pdata->reset_pin;
-	ts->irq_pin = pdata->irq_pin;
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -222,38 +220,13 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 	cy8ctmg110_power(ts, true);
 	cy8ctmg110_set_sleepmode(ts, false);
 
-	err = gpio_request(ts->irq_pin, "touch_irq_key");
-	if (err < 0) {
-		dev_err(&client->dev,
-			"Failed to request GPIO %d, error %d\n",
-			ts->irq_pin, err);
-		goto err_shutoff_device;
-	}
-
-	err = gpio_direction_input(ts->irq_pin);
-	if (err < 0) {
-		dev_err(&client->dev,
-			"Failed to configure input direction for GPIO %d, error %d\n",
-			ts->irq_pin, err);
-		goto err_free_irq_gpio;
-	}
-
-	client->irq = gpio_to_irq(ts->irq_pin);
-	if (client->irq < 0) {
-		err = client->irq;
-		dev_err(&client->dev,
-			"Unable to get irq number for GPIO %d, error %d\n",
-			ts->irq_pin, err);
-		goto err_free_irq_gpio;
-	}
-
 	err = request_threaded_irq(client->irq, NULL, cy8ctmg110_irq_thread,
 				   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
 				   "touch_reset_key", ts);
 	if (err < 0) {
 		dev_err(&client->dev,
 			"irq %d busy? error %d\n", client->irq, err);
-		goto err_free_irq_gpio;
+		goto err_shutoff_device;
 	}
 
 	err = input_register_device(input_dev);

@@ -266,8 +239,6 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 
 err_free_irq:
 	free_irq(client->irq, ts);
-err_free_irq_gpio:
-	gpio_free(ts->irq_pin);
 err_shutoff_device:
 	cy8ctmg110_set_sleepmode(ts, true);
 	cy8ctmg110_power(ts, false);

@@ -318,7 +289,6 @@ static int cy8ctmg110_remove(struct i2c_client *client)
 
 	free_irq(client->irq, ts);
 	input_unregister_device(ts->input);
-	gpio_free(ts->irq_pin);
 	if (ts->reset_pin)
 		gpio_free(ts->reset_pin);
 	kfree(ts);

diff --git a/include/linux/input/cy8ctmg110_pdata.h b/include/linux/input/cy8ctmg110_pdata.h
index 77582ae1745a..ee1d44545f30 100644
--- a/include/linux/input/cy8ctmg110_pdata.h
+++ b/include/linux/input/cy8ctmg110_pdata.h

@@ -5,7 +5,6 @@
 struct cy8ctmg110_pdata
 {
 	int reset_pin;		/* Reset pin is wired to this GPIO (optional) */
-	int irq_pin;		/* IRQ pin is wired to this GPIO */
 };
 
 #endif

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 6/7] Input: cy8ctmg110_ts - switch to using managed resources

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:37:41

This simplifies error handling paths and allows to get rid of
cy8ctmg110_remove() method.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 74 +++++++++--------------
 1 file changed, 28 insertions(+), 46 deletions(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index 33c1360a251c..33ccb31cad52 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -161,6 +161,14 @@ static irqreturn_t cy8ctmg110_irq_thread(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void cy8ctmg110_shut_off(void *_ts)
+{
+	struct cy8ctmg110 *ts = _ts;
+
+	cy8ctmg110_set_sleepmode(ts, true);
+	cy8ctmg110_power(ts, false);
+}
+
 static int cy8ctmg110_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {

@@ -179,12 +187,13 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 					I2C_FUNC_SMBUS_READ_WORD_DATA))
 		return -EIO;
 
-	ts = kzalloc(sizeof(struct cy8ctmg110), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
-		err = -ENOMEM;
-		goto err_free_mem;
-	}
+	ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	input_dev = devm_input_allocate_device(&client->dev);
+	if (!input_dev)
+		return -ENOMEM;
 
 	ts->client = client;
 	ts->input = input_dev;

@@ -196,56 +205,46 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 	input_dev->name = CY8CTMG110_DRIVER_NAME " Touchscreen";
 	input_dev->phys = ts->phys;
 	input_dev->id.bustype = BUS_I2C;
-	input_dev->dev.parent = &client->dev;
-
-	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
-	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
+	input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
 	input_set_abs_params(input_dev, ABS_X,
 			CY8CTMG110_X_MIN, CY8CTMG110_X_MAX, 4, 0);
 	input_set_abs_params(input_dev, ABS_Y,
 			CY8CTMG110_Y_MIN, CY8CTMG110_Y_MAX, 4, 0);
 
 	if (ts->reset_pin) {
-		err = gpio_request(ts->reset_pin, NULL);
+		err = devm_gpio_request(&client->dev, ts->reset_pin, NULL);
 		if (err) {
 			dev_err(&client->dev,
 				"Unable to request GPIO pin %d.\n",
 				ts->reset_pin);
-			goto err_free_mem;
+			return err;
 		}
 	}
 
 	cy8ctmg110_power(ts, true);
 	cy8ctmg110_set_sleepmode(ts, false);
 
-	err = request_threaded_irq(client->irq, NULL, cy8ctmg110_irq_thread,
-				   IRQF_ONESHOT, "touch_reset_key", ts);
-	if (err < 0) {
+	err = devm_add_action_or_reset(&client->dev, cy8ctmg110_shut_off, ts);
+	if (err)
+		return err;
+
+	err = devm_request_threaded_irq(&client->dev, client->irq,
+					NULL, cy8ctmg110_irq_thread,
+					IRQF_ONESHOT, "touch_reset_key", ts);
+	if (err) {
 		dev_err(&client->dev,
 			"irq %d busy? error %d\n", client->irq, err);
-		goto err_shutoff_device;
+		return err;
 	}
 
 	err = input_register_device(input_dev);
 	if (err)
-		goto err_free_irq;
+		return err;
 
 	i2c_set_clientdata(client, ts);
 
 	return 0;
-
-err_free_irq:
-	free_irq(client->irq, ts);
-err_shutoff_device:
-	cy8ctmg110_set_sleepmode(ts, true);
-	cy8ctmg110_power(ts, false);
-	if (ts->reset_pin)
-		gpio_free(ts->reset_pin);
-err_free_mem:
-	input_free_device(input_dev);
-	kfree(ts);
-	return err;
 }
 
 static int __maybe_unused cy8ctmg110_suspend(struct device *dev)

@@ -276,22 +275,6 @@ static int __maybe_unused cy8ctmg110_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(cy8ctmg110_pm, cy8ctmg110_suspend, cy8ctmg110_resume);
 
-static int cy8ctmg110_remove(struct i2c_client *client)
-{
-	struct cy8ctmg110 *ts = i2c_get_clientdata(client);
-
-	cy8ctmg110_set_sleepmode(ts, true);
-	cy8ctmg110_power(ts, false);
-
-	free_irq(client->irq, ts);
-	input_unregister_device(ts->input);
-	if (ts->reset_pin)
-		gpio_free(ts->reset_pin);
-	kfree(ts);
-
-	return 0;
-}
-
 static const struct i2c_device_id cy8ctmg110_idtable[] = {
 	{ CY8CTMG110_DRIVER_NAME, 1 },
 	{ }

@@ -306,7 +289,6 @@ static struct i2c_driver cy8ctmg110_driver = {
 	},
 	.id_table	= cy8ctmg110_idtable,
 	.probe		= cy8ctmg110_probe,
-	.remove		= cy8ctmg110_remove,
 };
 
 module_i2c_driver(cy8ctmg110_driver);

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 3/7] Input: cy8ctmg110_ts - do not hardcode as wakeup source

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:37:53

Let platform specify whether the controller should be a wakeup source
by registering as I2C_CLIENT_WAKE.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index d83257284537..d83bf82f02d4 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -233,7 +233,7 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 		goto err_free_irq;
 
 	i2c_set_clientdata(client, ts);
-	device_init_wakeup(&client->dev, 1);
+
 	return 0;
 
 err_free_irq:

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 4/7] Input: cy8ctmg110_ts - let I2C core configure wake interrupt

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:37:55

I2C core already configures interrupt as wakeup source when device is
registered using I2C_CLIENT_WAKE flag, so let's rely on it instead of
configuring it ourselves.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index d83bf82f02d4..f8d7ab3b6c25 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -254,12 +254,11 @@ static int __maybe_unused cy8ctmg110_suspend(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct cy8ctmg110 *ts = i2c_get_clientdata(client);
 
-	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(client->irq);
-	else {
+	if (!device_may_wakeup(&client->dev)) {
 		cy8ctmg110_set_sleepmode(ts, true);
 		cy8ctmg110_power(ts, false);
 	}
+
 	return 0;
 }

@@ -268,12 +267,11 @@ static int __maybe_unused cy8ctmg110_resume(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct cy8ctmg110 *ts = i2c_get_clientdata(client);
 
-	if (device_may_wakeup(&client->dev))
-		disable_irq_wake(client->irq);
-	else {
+	if (!device_may_wakeup(&client->dev)) {
 		cy8ctmg110_power(ts, true);
 		cy8ctmg110_set_sleepmode(ts, false);
 	}
+
 	return 0;
 }

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 5/7] Input: cy8ctmg110_ts - use endian helpers when converting data on wire

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:37:56

Switch to using be16_to_cpup() instead of shifting and combining data by
hand.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index f8d7ab3b6c25..33c1360a251c 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -16,6 +16,7 @@
 #include <linux/i2c.h>
 #include <linux/gpio.h>
 #include <linux/input/cy8ctmg110_pdata.h>
+#include <asm/byteorder.h>
 
 #define CY8CTMG110_DRIVER_NAME      "cy8ctmg110"

@@ -111,7 +112,6 @@ static int cy8ctmg110_touch_pos(struct cy8ctmg110 *tsc)
 {
 	struct input_dev *input = tsc->input;
 	unsigned char reg_p[CY8CTMG110_REG_MAX];
-	int x, y;
 
 	memset(reg_p, 0, CY8CTMG110_REG_MAX);

@@ -119,16 +119,15 @@ static int cy8ctmg110_touch_pos(struct cy8ctmg110 *tsc)
 	if (cy8ctmg110_read_regs(tsc, reg_p, 9, CY8CTMG110_TOUCH_X1) != 0)
 		return -EIO;
 
-	y = reg_p[2] << 8 | reg_p[3];
-	x = reg_p[0] << 8 | reg_p[1];
-
 	/* Number of touch */
 	if (reg_p[8] == 0) {
 		input_report_key(input, BTN_TOUCH, 0);
 	} else  {
 		input_report_key(input, BTN_TOUCH, 1);
-		input_report_abs(input, ABS_X, x);
-		input_report_abs(input, ABS_Y, y);
+		input_report_abs(input, ABS_X,
+				 be16_to_cpup((__be16 *)(reg_p + 0)));
+		input_report_abs(input, ABS_Y,
+				 be16_to_cpup((__be16 *)(reg_p + 2)));
 	}
 
 	input_sync(input);

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 7/7] Input: cy8ctmg110_ts - switch to using gpiod API

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:38:43

Instead of legacy gpio API let's use newer gpiod API. This also allows us
to get rid of platform data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 41 +++++++++--------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index 33ccb31cad52..3e2d64fb1620 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -7,15 +7,13 @@
  * Some cleanups by Alan Cox <alan@linux.intel.com>
  */
 
-#include <linux/module.h>
-#include <linux/kernel.h>
+#include <linux/i2c.h>
 #include <linux/input.h>
-#include <linux/slab.h>
 #include <linux/interrupt.h>
-#include <linux/io.h>
-#include <linux/i2c.h>
-#include <linux/gpio.h>
-#include <linux/input/cy8ctmg110_pdata.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
 #include <asm/byteorder.h>
 
 #define CY8CTMG110_DRIVER_NAME      "cy8ctmg110"

@@ -46,7 +44,7 @@ struct cy8ctmg110 {
 	struct input_dev *input;
 	char phys[32];
 	struct i2c_client *client;
-	int reset_pin;
+	struct gpio_desc *reset_gpio;
 };
 
 /*

@@ -55,8 +53,8 @@ struct cy8ctmg110 {
  */
 static void cy8ctmg110_power(struct cy8ctmg110 *ts, bool poweron)
 {
-	if (ts->reset_pin)
-		gpio_direction_output(ts->reset_pin, 1 - poweron);
+	if (ts->reset_gpio)
+		gpiod_set_value_cansleep(ts->reset_gpio, !poweron);
 }
 
 static int cy8ctmg110_write_regs(struct cy8ctmg110 *tsc, unsigned char reg,

@@ -172,17 +170,10 @@ static void cy8ctmg110_shut_off(void *_ts)
 static int cy8ctmg110_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
-	const struct cy8ctmg110_pdata *pdata = dev_get_platdata(&client->dev);
 	struct cy8ctmg110 *ts;
 	struct input_dev *input_dev;
 	int err;
 
-	/* No pdata no way forward */
-	if (pdata == NULL) {
-		dev_err(&client->dev, "no pdata\n");
-		return -ENODEV;
-	}
-
 	if (!i2c_check_functionality(client->adapter,
 					I2C_FUNC_SMBUS_READ_WORD_DATA))
 		return -EIO;

@@ -197,7 +188,6 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 
 	ts->client = client;
 	ts->input = input_dev;
-	ts->reset_pin = pdata->reset_pin;
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));

@@ -212,14 +202,13 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 	input_set_abs_params(input_dev, ABS_Y,
 			CY8CTMG110_Y_MIN, CY8CTMG110_Y_MAX, 4, 0);
 
-	if (ts->reset_pin) {
-		err = devm_gpio_request(&client->dev, ts->reset_pin, NULL);
-		if (err) {
-			dev_err(&client->dev,
-				"Unable to request GPIO pin %d.\n",
-				ts->reset_pin);
-			return err;
-		}
+	ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(ts->reset_gpio)) {
+		err = PTR_ERR(ts->reset_gpio);
+		dev_err(&client->dev,
+			"Unable to request reset GPIO: %d\n", err);
+		return err;
 	}
 
 	cy8ctmg110_power(ts, true);

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

[PATCH 2/7] Input: cy8ctmg110_ts - do not hard code interrupt trigger

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-03 04:38:51

Rely on the platform to set up interrupt polarity/type properly instead
of hard-coding falling edge.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/cy8ctmg110_ts.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/cy8ctmg110_ts.c b/drivers/input/touchscreen/cy8ctmg110_ts.c
index 691f35f1bdd7..d83257284537 100644
--- a/drivers/input/touchscreen/cy8ctmg110_ts.c
+++ b/drivers/input/touchscreen/cy8ctmg110_ts.c

@@ -221,8 +221,7 @@ static int cy8ctmg110_probe(struct i2c_client *client,
 	cy8ctmg110_set_sleepmode(ts, false);
 
 	err = request_threaded_irq(client->irq, NULL, cy8ctmg110_irq_thread,
-				   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
-				   "touch_reset_key", ts);
+				   IRQF_ONESHOT, "touch_reset_key", ts);
 	if (err < 0) {
 		dev_err(&client->dev,
 			"irq %d busy? error %d\n", client->irq, err);

-- 
2.32.0.rc0.204.g9fa02ecfa5-goog

Re: [PATCH 1/7] Input: cy8ctmg110_ts - rely on platform code to supply interrupt

From: Linus Walleij <hidden>
Date: 2021-06-04 07:30:53

Hi Dmitry,

I see you noticed that there is no upstream board defining
any cy8ctmg110_pdata, so I don't see any problem with
fixing this. Outoftree users can adopt.

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

Instead of using platform data to specify GPIO that is used as interrupt
source, rely on the platform and I2C core to set it up properly.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

-       client->irq = gpio_to_irq(ts->irq_pin);

This looks like a violation of the struct anyway....

Yours,
Linus Walleij

Re: [PATCH 2/7] Input: cy8ctmg110_ts - do not hard code interrupt trigger

From: Linus Walleij <hidden>
Date: 2021-06-04 07:32:22

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

Rely on the platform to set up interrupt polarity/type properly instead
of hard-coding falling edge.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

Yours,
Linus Walleij

Re: [PATCH 3/7] Input: cy8ctmg110_ts - do not hardcode as wakeup source

From: Linus Walleij <hidden>
Date: 2021-06-04 07:33:14

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

Let platform specify whether the controller should be a wakeup source
by registering as I2C_CLIENT_WAKE.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

Yours,
Linus Walleij

Re: [PATCH 4/7] Input: cy8ctmg110_ts - let I2C core configure wake interrupt

From: Linus Walleij <hidden>
Date: 2021-06-04 07:33:17

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

I2C core already configures interrupt as wakeup source when device is
registered using I2C_CLIENT_WAKE flag, so let's rely on it instead of
configuring it ourselves.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

I wonder how many bugs of this deep semantic type we have in the kernel :/

Yours,
Linus Walleij

Re: [PATCH 6/7] Input: cy8ctmg110_ts - switch to using managed resources

From: Linus Walleij <hidden>
Date: 2021-06-04 07:35:17

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

This simplifies error handling paths and allows to get rid of
cy8ctmg110_remove() method.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Neat!
Reviewed-by: Linus Walleij <redacted>

Yours,
Linus Walleij

Re: [PATCH 5/7] Input: cy8ctmg110_ts - use endian helpers when converting data on wire

From: Linus Walleij <hidden>
Date: 2021-06-04 07:35:36

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

Switch to using be16_to_cpup() instead of shifting and combining data by
hand.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

Yours,
Linus Walleij

Re: [PATCH 7/7] Input: cy8ctmg110_ts - switch to using gpiod API

From: Linus Walleij <hidden>
Date: 2021-06-04 07:39:18

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

Instead of legacy gpio API let's use newer gpiod API. This also allows us
to get rid of platform data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

 static void cy8ctmg110_power(struct cy8ctmg110 *ts, bool poweron)
 {
-       if (ts->reset_pin)
-               gpio_direction_output(ts->reset_pin, 1 - poweron);
+       if (ts->reset_gpio)
+               gpiod_set_value_cansleep(ts->reset_gpio, !poweron);

I would perhaps drop in a comment that this de-asserts the RESET
line when we power on and asserts it when we power off.

+       ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
+                                                GPIOD_OUT_HIGH);

And here that this will assert RESET.

But that is just nitpicks, the code is fine.

Yours,
Linus Walleij

Re: [PATCH 7/7] Input: cy8ctmg110_ts - switch to using gpiod API

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

On Fri, Jun 04, 2021 at 09:38:04AM +0200, Linus Walleij wrote:

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

quoted

Instead of legacy gpio API let's use newer gpiod API. This also allows us
to get rid of platform data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

quoted

 static void cy8ctmg110_power(struct cy8ctmg110 *ts, bool poweron)
 {
-       if (ts->reset_pin)
-               gpio_direction_output(ts->reset_pin, 1 - poweron);
+       if (ts->reset_gpio)
+               gpiod_set_value_cansleep(ts->reset_gpio, !poweron);

I would perhaps drop in a comment that this de-asserts the RESET
line when we power on and asserts it when we power off.

quoted

+       ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
+                                                GPIOD_OUT_HIGH);

And here that this will assert RESET.

Good idea, added and applied.

Thank you Linus.

-- 
Dmitry

Re: [PATCH 4/7] Input: cy8ctmg110_ts - let I2C core configure wake interrupt

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-06 04:11:42

On Fri, Jun 04, 2021 at 09:32:53AM +0200, Linus Walleij wrote:

On Thu, Jun 3, 2021 at 6:37 AM Dmitry Torokhov
[off-list ref] wrote:

quoted

I2C core already configures interrupt as wakeup source when device is
registered using I2C_CLIENT_WAKE flag, so let's rely on it instead of
configuring it ourselves.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <redacted>

I wonder how many bugs of this deep semantic type we have in the kernel :/

I do not think this is necessarily a bug, it just shows age of the
driver that was written before I2C core was doing this set up for us.

Thanks.

-- 
Dmitry

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help