* [PATCH v1 2/3] Input: ili210x - add resolution to chip operations structure
2019-11-12 16:44 [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sven Van Asbroeck
@ 2019-11-12 16:44 ` Sven Van Asbroeck
2019-11-12 19:42 ` Dmitry Torokhov
2019-11-12 16:44 ` [PATCH v1 3/3] Input: ili210x - optionally hide calibrate sysfs attribute Sven Van Asbroeck
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Sven Van Asbroeck @ 2019-11-12 16:44 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marek Vasut, Adam Ford, linux-kernel, linux-input
Optionally allow the touch screen resolution to be set by adding
it to the chip operations structure. If it is omitted (left zero),
the resolution defaults to 64K. Which is the previously hard-coded
value.
Set the ili2117 resolution to 2048, as indicated in its datasheet.
Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
Cc: Marek Vasut <marex@denx.de>
Cc: Adam Ford <aford173@gmail.com>
Cc: <linux-kernel@vger.kernel.org>
Cc: linux-input@vger.kernel.org
Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
drivers/input/touchscreen/ili210x.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index a6feae5ce887..4321f0d676cc 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -31,6 +31,7 @@ struct ili2xxx_chip {
unsigned int *x, unsigned int *y);
bool (*continue_polling)(const u8 *data, bool touch);
unsigned int max_touches;
+ unsigned int resolution;
};
struct ili210x {
@@ -160,6 +161,7 @@ static const struct ili2xxx_chip ili211x_chip = {
.parse_touch_data = ili211x_touchdata_to_coords,
.continue_polling = ili211x_decline_polling,
.max_touches = 10,
+ .resolution = 2048,
};
static int ili251x_read_reg(struct i2c_client *client,
@@ -336,6 +338,7 @@ static int ili210x_i2c_probe(struct i2c_client *client,
struct gpio_desc *reset_gpio;
struct input_dev *input;
int error;
+ unsigned int max_xy;
dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");
@@ -386,8 +389,12 @@ static int ili210x_i2c_probe(struct i2c_client *client,
input->id.bustype = BUS_I2C;
/* Multi touch */
- input_set_abs_params(input, ABS_MT_POSITION_X, 0, 0xffff, 0, 0);
- input_set_abs_params(input, ABS_MT_POSITION_Y, 0, 0xffff, 0, 0);
+ if (chip->resolution)
+ max_xy = chip->resolution - 1;
+ else
+ max_xy = 0xffff;
+ input_set_abs_params(input, ABS_MT_POSITION_X, 0, max_xy, 0, 0);
+ input_set_abs_params(input, ABS_MT_POSITION_Y, 0, max_xy, 0, 0);
touchscreen_parse_properties(input, true, &priv->prop);
error = input_mt_init_slots(input, priv->chip->max_touches,
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v1 2/3] Input: ili210x - add resolution to chip operations structure
2019-11-12 16:44 ` [PATCH v1 2/3] Input: ili210x - add resolution to chip operations structure Sven Van Asbroeck
@ 2019-11-12 19:42 ` Dmitry Torokhov
0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2019-11-12 19:42 UTC (permalink / raw)
To: Sven Van Asbroeck; +Cc: Marek Vasut, Adam Ford, linux-kernel, linux-input
On Tue, Nov 12, 2019 at 11:44:28AM -0500, Sven Van Asbroeck wrote:
> Optionally allow the touch screen resolution to be set by adding
> it to the chip operations structure. If it is omitted (left zero),
> the resolution defaults to 64K. Which is the previously hard-coded
> value.
>
> Set the ili2117 resolution to 2048, as indicated in its datasheet.
>
> Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
> Cc: Marek Vasut <marex@denx.de>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: <linux-kernel@vger.kernel.org>
> Cc: linux-input@vger.kernel.org
> Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
> drivers/input/touchscreen/ili210x.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index a6feae5ce887..4321f0d676cc 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -31,6 +31,7 @@ struct ili2xxx_chip {
> unsigned int *x, unsigned int *y);
> bool (*continue_polling)(const u8 *data, bool touch);
> unsigned int max_touches;
> + unsigned int resolution;
> };
>
> struct ili210x {
> @@ -160,6 +161,7 @@ static const struct ili2xxx_chip ili211x_chip = {
> .parse_touch_data = ili211x_touchdata_to_coords,
> .continue_polling = ili211x_decline_polling,
> .max_touches = 10,
> + .resolution = 2048,
> };
>
> static int ili251x_read_reg(struct i2c_client *client,
> @@ -336,6 +338,7 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> struct gpio_desc *reset_gpio;
> struct input_dev *input;
> int error;
> + unsigned int max_xy;
>
> dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");
>
> @@ -386,8 +389,12 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> input->id.bustype = BUS_I2C;
>
> /* Multi touch */
> - input_set_abs_params(input, ABS_MT_POSITION_X, 0, 0xffff, 0, 0);
> - input_set_abs_params(input, ABS_MT_POSITION_Y, 0, 0xffff, 0, 0);
> + if (chip->resolution)
> + max_xy = chip->resolution - 1;
> + else
> + max_xy = 0xffff;
max_xy = (chip->resolution ?: 65536) - 1;
> + input_set_abs_params(input, ABS_MT_POSITION_X, 0, max_xy, 0, 0);
> + input_set_abs_params(input, ABS_MT_POSITION_Y, 0, max_xy, 0, 0);
> touchscreen_parse_properties(input, true, &priv->prop);
>
> error = input_mt_init_slots(input, priv->chip->max_touches,
> --
> 2.17.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 3/3] Input: ili210x - optionally hide calibrate sysfs attribute
2019-11-12 16:44 [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sven Van Asbroeck
2019-11-12 16:44 ` [PATCH v1 2/3] Input: ili210x - add resolution to chip operations structure Sven Van Asbroeck
@ 2019-11-12 16:44 ` Sven Van Asbroeck
2019-11-12 19:38 ` Dmitry Torokhov
2019-11-12 23:34 ` [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sebastian Reichel
2019-11-12 23:53 ` Dmitry Torokhov
3 siblings, 1 reply; 8+ messages in thread
From: Sven Van Asbroeck @ 2019-11-12 16:44 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marek Vasut, Adam Ford, linux-kernel, linux-input
Only show the 'calibrate' sysfs attribute on chip flavours
which support calibration by writing to a calibration register.
Do this by adding a flag to the chip operations structure.
Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
Cc: Marek Vasut <marex@denx.de>
Cc: Adam Ford <aford173@gmail.com>
Cc: <linux-kernel@vger.kernel.org>
Cc: linux-input@vger.kernel.org
Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
drivers/input/touchscreen/ili210x.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 4321f0d676cc..810770ad02e2 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -32,6 +32,7 @@ struct ili2xxx_chip {
bool (*continue_polling)(const u8 *data, bool touch);
unsigned int max_touches;
unsigned int resolution;
+ bool no_calibrate_reg;
};
struct ili210x {
@@ -162,6 +163,7 @@ static const struct ili2xxx_chip ili211x_chip = {
.continue_polling = ili211x_decline_polling,
.max_touches = 10,
.resolution = 2048,
+ .no_calibrate_reg = true,
};
static int ili251x_read_reg(struct i2c_client *client,
@@ -310,8 +312,19 @@ static struct attribute *ili210x_attributes[] = {
NULL,
};
+static umode_t ili210x_calibrate_visible(struct kobject *kobj,
+ struct attribute *attr, int index)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct ili210x *priv = i2c_get_clientdata(client);
+
+ return !priv->chip->no_calibrate_reg;
+}
+
static const struct attribute_group ili210x_attr_group = {
.attrs = ili210x_attributes,
+ .is_visible = ili210x_calibrate_visible,
};
static void ili210x_power_down(void *data)
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v1 3/3] Input: ili210x - optionally hide calibrate sysfs attribute
2019-11-12 16:44 ` [PATCH v1 3/3] Input: ili210x - optionally hide calibrate sysfs attribute Sven Van Asbroeck
@ 2019-11-12 19:38 ` Dmitry Torokhov
0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2019-11-12 19:38 UTC (permalink / raw)
To: Sven Van Asbroeck; +Cc: Marek Vasut, Adam Ford, linux-kernel, linux-input
On Tue, Nov 12, 2019 at 11:44:29AM -0500, Sven Van Asbroeck wrote:
> Only show the 'calibrate' sysfs attribute on chip flavours
> which support calibration by writing to a calibration register.
>
> Do this by adding a flag to the chip operations structure.
>
> Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
> Cc: Marek Vasut <marex@denx.de>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: <linux-kernel@vger.kernel.org>
> Cc: linux-input@vger.kernel.org
> Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
> drivers/input/touchscreen/ili210x.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 4321f0d676cc..810770ad02e2 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -32,6 +32,7 @@ struct ili2xxx_chip {
> bool (*continue_polling)(const u8 *data, bool touch);
> unsigned int max_touches;
> unsigned int resolution;
> + bool no_calibrate_reg;
Please use positive logic and have chips opt-in into the calibration.
> };
>
> struct ili210x {
> @@ -162,6 +163,7 @@ static const struct ili2xxx_chip ili211x_chip = {
> .continue_polling = ili211x_decline_polling,
> .max_touches = 10,
> .resolution = 2048,
> + .no_calibrate_reg = true,
> };
>
> static int ili251x_read_reg(struct i2c_client *client,
> @@ -310,8 +312,19 @@ static struct attribute *ili210x_attributes[] = {
> NULL,
> };
>
> +static umode_t ili210x_calibrate_visible(struct kobject *kobj,
> + struct attribute *attr, int index)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct i2c_client *client = to_i2c_client(dev);
> + struct ili210x *priv = i2c_get_clientdata(client);
> +
> + return !priv->chip->no_calibrate_reg;
> +}
> +
> static const struct attribute_group ili210x_attr_group = {
> .attrs = ili210x_attributes,
> + .is_visible = ili210x_calibrate_visible,
> };
>
> static void ili210x_power_down(void *data)
> --
> 2.17.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version
2019-11-12 16:44 [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sven Van Asbroeck
2019-11-12 16:44 ` [PATCH v1 2/3] Input: ili210x - add resolution to chip operations structure Sven Van Asbroeck
2019-11-12 16:44 ` [PATCH v1 3/3] Input: ili210x - optionally hide calibrate sysfs attribute Sven Van Asbroeck
@ 2019-11-12 23:34 ` Sebastian Reichel
2019-11-13 0:06 ` Dmitry Torokhov
2019-11-12 23:53 ` Dmitry Torokhov
3 siblings, 1 reply; 8+ messages in thread
From: Sebastian Reichel @ 2019-11-12 23:34 UTC (permalink / raw)
To: Sven Van Asbroeck
Cc: Dmitry Torokhov, Marek Vasut, Adam Ford, linux-kernel, linux-input
[-- Attachment #1: Type: text/plain, Size: 3050 bytes --]
Hi,
On Tue, Nov 12, 2019 at 11:44:27AM -0500, Sven Van Asbroeck wrote:
> The driver's method to retrieve the firmware version on ili2117/
> ili2118 chip flavours is incorrect. The firmware version register
> address and layout are wrong.
>
> The firmware version is not actually used anywhere inside or
> outside this driver. There is a dev_dbg() print, but that is
> only visible when the developer explicitly compiles in debug
> support.
>
> Don't make the code more complicated to preserve a feature that
> no-one is using. Remove all code associated with chip firmware
> version.
>
> Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
> Cc: Marek Vasut <marex@denx.de>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: <linux-kernel@vger.kernel.org>
> Cc: linux-input@vger.kernel.org
> Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
The firmware version check does one relevant thing: It tests
the I2C communication, which tends to be useful for board
bringup and development boards (which often allow to disconnect
(touch-)screens).
-- Sebastian
> drivers/input/touchscreen/ili210x.c | 21 ---------------------
> 1 file changed, 21 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 0ed6014af6d7..a6feae5ce887 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -21,15 +21,8 @@
> /* Touchscreen commands */
> #define REG_TOUCHDATA 0x10
> #define REG_PANEL_INFO 0x20
> -#define REG_FIRMWARE_VERSION 0x40
> #define REG_CALIBRATE 0xcc
>
> -struct firmware_version {
> - u8 id;
> - u8 major;
> - u8 minor;
> -} __packed;
> -
> struct ili2xxx_chip {
> int (*read_reg)(struct i2c_client *client, u8 reg,
> void *buf, size_t len);
> @@ -342,7 +335,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> struct ili210x *priv;
> struct gpio_desc *reset_gpio;
> struct input_dev *input;
> - struct firmware_version firmware;
> int error;
>
> dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");
> @@ -389,15 +381,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> priv->chip = chip;
> i2c_set_clientdata(client, priv);
>
> - /* Get firmware version */
> - error = chip->read_reg(client, REG_FIRMWARE_VERSION,
> - &firmware, sizeof(firmware));
> - if (error) {
> - dev_err(dev, "Failed to get firmware version, err: %d\n",
> - error);
> - return error;
> - }
> -
> /* Setup input device */
> input->name = "ILI210x Touchscreen";
> input->id.bustype = BUS_I2C;
> @@ -439,10 +422,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> return error;
> }
>
> - dev_dbg(dev,
> - "ILI210x initialized (IRQ: %d), firmware version %d.%d.%d",
> - client->irq, firmware.id, firmware.major, firmware.minor);
> -
> return 0;
> }
>
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version
2019-11-12 23:34 ` [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sebastian Reichel
@ 2019-11-13 0:06 ` Dmitry Torokhov
0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2019-11-13 0:06 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sven Van Asbroeck, Marek Vasut, Adam Ford, linux-kernel, linux-input
On Wed, Nov 13, 2019 at 12:34:21AM +0100, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Nov 12, 2019 at 11:44:27AM -0500, Sven Van Asbroeck wrote:
> > The driver's method to retrieve the firmware version on ili2117/
> > ili2118 chip flavours is incorrect. The firmware version register
> > address and layout are wrong.
> >
> > The firmware version is not actually used anywhere inside or
> > outside this driver. There is a dev_dbg() print, but that is
> > only visible when the developer explicitly compiles in debug
> > support.
> >
> > Don't make the code more complicated to preserve a feature that
> > no-one is using. Remove all code associated with chip firmware
> > version.
> >
> > Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Adam Ford <aford173@gmail.com>
> > Cc: <linux-kernel@vger.kernel.org>
> > Cc: linux-input@vger.kernel.org
> > Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
> > Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> > ---
>
> The firmware version check does one relevant thing: It tests
> the I2C communication, which tends to be useful for board
> bringup and development boards (which often allow to disconnect
> (touch-)screens).
If/when this is needed I propose we add a separate "lite" xfer check,
similar to what elants_i2c and many other drivers are doing.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version
2019-11-12 16:44 [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sven Van Asbroeck
` (2 preceding siblings ...)
2019-11-12 23:34 ` [PATCH v1 1/3] Input: ili210x - do not retrieve/print chip firmware version Sebastian Reichel
@ 2019-11-12 23:53 ` Dmitry Torokhov
3 siblings, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2019-11-12 23:53 UTC (permalink / raw)
To: Sven Van Asbroeck; +Cc: Marek Vasut, Adam Ford, linux-kernel, linux-input
On Tue, Nov 12, 2019 at 11:44:27AM -0500, Sven Van Asbroeck wrote:
> The driver's method to retrieve the firmware version on ili2117/
> ili2118 chip flavours is incorrect. The firmware version register
> address and layout are wrong.
>
> The firmware version is not actually used anywhere inside or
> outside this driver. There is a dev_dbg() print, but that is
> only visible when the developer explicitly compiles in debug
> support.
>
> Don't make the code more complicated to preserve a feature that
> no-one is using. Remove all code associated with chip firmware
> version.
>
> Link: https://lore.kernel.org/lkml/20191111181657.GA57214@dtor-ws/
> Cc: Marek Vasut <marex@denx.de>
> Cc: Adam Ford <aford173@gmail.com>
> Cc: <linux-kernel@vger.kernel.org>
> Cc: linux-input@vger.kernel.org
> Tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=next
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/ili210x.c | 21 ---------------------
> 1 file changed, 21 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 0ed6014af6d7..a6feae5ce887 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -21,15 +21,8 @@
> /* Touchscreen commands */
> #define REG_TOUCHDATA 0x10
> #define REG_PANEL_INFO 0x20
> -#define REG_FIRMWARE_VERSION 0x40
> #define REG_CALIBRATE 0xcc
>
> -struct firmware_version {
> - u8 id;
> - u8 major;
> - u8 minor;
> -} __packed;
> -
> struct ili2xxx_chip {
> int (*read_reg)(struct i2c_client *client, u8 reg,
> void *buf, size_t len);
> @@ -342,7 +335,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> struct ili210x *priv;
> struct gpio_desc *reset_gpio;
> struct input_dev *input;
> - struct firmware_version firmware;
> int error;
>
> dev_dbg(dev, "Probing for ILI210X I2C Touschreen driver");
> @@ -389,15 +381,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> priv->chip = chip;
> i2c_set_clientdata(client, priv);
>
> - /* Get firmware version */
> - error = chip->read_reg(client, REG_FIRMWARE_VERSION,
> - &firmware, sizeof(firmware));
> - if (error) {
> - dev_err(dev, "Failed to get firmware version, err: %d\n",
> - error);
> - return error;
> - }
> -
> /* Setup input device */
> input->name = "ILI210x Touchscreen";
> input->id.bustype = BUS_I2C;
> @@ -439,10 +422,6 @@ static int ili210x_i2c_probe(struct i2c_client *client,
> return error;
> }
>
> - dev_dbg(dev,
> - "ILI210x initialized (IRQ: %d), firmware version %d.%d.%d",
> - client->irq, firmware.id, firmware.major, firmware.minor);
> -
> return 0;
> }
>
> --
> 2.17.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread