* Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size
2021-03-02 10:08 [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size Dmitry Osipenko
@ 2021-03-03 23:19 ` Michał Mirosław
2021-03-26 13:24 ` Dmitry Osipenko
2021-03-28 4:44 ` Dmitry Torokhov
2 siblings, 0 replies; 5+ messages in thread
From: Michał Mirosław @ 2021-03-03 23:19 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Dmitry Torokhov, Johnny Chuang, Jasper Korten, Svyatoslav Ryhel,
linux-input, linux-tegra, linux-kernel
On Tue, Mar 02, 2021 at 01:08:24PM +0300, Dmitry Osipenko wrote:
> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
> size. Hence check whether the size is zero and don't set the resolution in
> this case.
>
> Reported-by: Jasper Korten <jja2000@gmail.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Rewieved-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Please note that ASUS TF700T isn't yet supported by upstream kernel,
> hence this is not a critical fix.
>
> drivers/input/touchscreen/elants_i2c.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 4c2b579f6c8b..a2e1cc4192b0 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client *client,
>
> touchscreen_parse_properties(ts->input, true, &ts->prop);
>
> - if (ts->chip_id == EKTF3624) {
> + if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
> /* calculate resolution from size */
> ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
> ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
> }
>
> - input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> - input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> + if (ts->x_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> + if (ts->y_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
I would guess that you can tie X and Y in one if, as they are unlikely
to work independently.
Best Regards
Michal Mirosław
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size
2021-03-02 10:08 [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size Dmitry Osipenko
2021-03-03 23:19 ` Michał Mirosław
@ 2021-03-26 13:24 ` Dmitry Osipenko
2021-03-28 4:44 ` Dmitry Torokhov
2 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2021-03-26 13:24 UTC (permalink / raw)
To: Dmitry Torokhov, Michał Mirosław, Johnny Chuang,
Jasper Korten, Svyatoslav Ryhel
Cc: linux-input, linux-tegra, linux-kernel
02.03.2021 13:08, Dmitry Osipenko пишет:
> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
> size. Hence check whether the size is zero and don't set the resolution in
> this case.
>
> Reported-by: Jasper Korten <jja2000@gmail.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>
> Please note that ASUS TF700T isn't yet supported by upstream kernel,
> hence this is not a critical fix.
>
> drivers/input/touchscreen/elants_i2c.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 4c2b579f6c8b..a2e1cc4192b0 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client *client,
>
> touchscreen_parse_properties(ts->input, true, &ts->prop);
>
> - if (ts->chip_id == EKTF3624) {
> + if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
> /* calculate resolution from size */
> ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
> ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
> }
>
> - input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> - input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> + if (ts->x_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> + if (ts->y_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> if (ts->major_res > 0)
> input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res);
>
>
Hi,
This is a very minor fix, but still will be nice if we could get it into
5.13 in order to have one less patch to care about. Thanks in advance!
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size
2021-03-02 10:08 [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size Dmitry Osipenko
2021-03-03 23:19 ` Michał Mirosław
2021-03-26 13:24 ` Dmitry Osipenko
@ 2021-03-28 4:44 ` Dmitry Torokhov
2021-03-28 17:21 ` Dmitry Osipenko
2 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2021-03-28 4:44 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Michał Mirosław, Johnny Chuang, Jasper Korten,
Svyatoslav Ryhel, linux-input, linux-tegra, linux-kernel
Hi Dmitry,
On Tue, Mar 02, 2021 at 01:08:24PM +0300, Dmitry Osipenko wrote:
> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
> size. Hence check whether the size is zero and don't set the resolution in
> this case.
>
> Reported-by: Jasper Korten <jja2000@gmail.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>
> Please note that ASUS TF700T isn't yet supported by upstream kernel,
> hence this is not a critical fix.
>
> drivers/input/touchscreen/elants_i2c.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 4c2b579f6c8b..a2e1cc4192b0 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client *client,
>
> touchscreen_parse_properties(ts->input, true, &ts->prop);
>
> - if (ts->chip_id == EKTF3624) {
> + if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
> /* calculate resolution from size */
> ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
> ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
> }
>
> - input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
> - input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> + if (ts->x_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
There is absolutely no difference between setting respluton to 0 vs not
setting it at all, so I dropped the conditionals and applied.
> + if (ts->y_res > 0)
> + input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
> if (ts->major_res > 0)
We could drop this condition as well.
> input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res);
>
> --
> 2.29.2
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] Input: elants_i2c - fix division by zero if firmware reports zero phys size
2021-03-28 4:44 ` Dmitry Torokhov
@ 2021-03-28 17:21 ` Dmitry Osipenko
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2021-03-28 17:21 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michał Mirosław, Johnny Chuang, Jasper Korten,
Svyatoslav Ryhel, linux-input, linux-tegra, linux-kernel
28.03.2021 07:44, Dmitry Torokhov пишет:
> Hi Dmitry,
>
> On Tue, Mar 02, 2021 at 01:08:24PM +0300, Dmitry Osipenko wrote:
>> Touchscreen firmware of ASUS Transformer TF700T reports zeros for the phys
>> size. Hence check whether the size is zero and don't set the resolution in
>> this case.
>>
>> Reported-by: Jasper Korten <jja2000@gmail.com>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>
>> Please note that ASUS TF700T isn't yet supported by upstream kernel,
>> hence this is not a critical fix.
>>
>> drivers/input/touchscreen/elants_i2c.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
>> index 4c2b579f6c8b..a2e1cc4192b0 100644
>> --- a/drivers/input/touchscreen/elants_i2c.c
>> +++ b/drivers/input/touchscreen/elants_i2c.c
>> @@ -1441,14 +1441,16 @@ static int elants_i2c_probe(struct i2c_client *client,
>>
>> touchscreen_parse_properties(ts->input, true, &ts->prop);
>>
>> - if (ts->chip_id == EKTF3624) {
>> + if (ts->chip_id == EKTF3624 && ts->phy_x && ts->phy_y) {
>> /* calculate resolution from size */
>> ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, ts->phy_x);
>> ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, ts->phy_y);
>> }
>>
>> - input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
>> - input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
>> + if (ts->x_res > 0)
>> + input_abs_set_res(ts->input, ABS_MT_POSITION_X, ts->x_res);
>
> There is absolutely no difference between setting respluton to 0 vs not
> setting it at all, so I dropped the conditionals and applied.
>
>> + if (ts->y_res > 0)
>> + input_abs_set_res(ts->input, ABS_MT_POSITION_Y, ts->y_res);
>> if (ts->major_res > 0)
>
> We could drop this condition as well.
>
>> input_abs_set_res(ts->input, ABS_MT_TOUCH_MAJOR, ts->major_res);
I'll make a follow up patch to clean up this condition, if you haven't
done it yet. Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread