Re: [Letux-kernel] [PATCH v3 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

From: H. Nikolaus Schaller
Date: Fri Sep 30 2016 - 12:38:18 EST


Hi Christ,

> Am 30.09.2016 um 18:23 schrieb Christ van Willegen <cvwillegen@xxxxxxxxx>:
>
> Hi,
>
> I saw this earlier, but didn't think it important to mention, but:
>
>
> On Fri, Sep 23, 2016 at 2:41 PM, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote:
>
>> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
>> index 5d0cd51..9a11509 100644
>> --- a/drivers/input/touchscreen/tsc2007.c
>> +++ b/drivers/input/touchscreen/tsc2007.c
>
>> + /* scale ADC values to desired output range */
>> + sx = (ts->prop.max_x * (tc.x - ts->min_x))
>> + / (ts->max_x - ts->min_x);
>> + sy = (ts->prop.max_y * (tc.y - ts->min_y))
>> + / (ts->max_y - ts->min_y);
>> + rt = (input->absinfo[ABS_PRESSURE].maximum * rt) /
>> + ts->max_rt;
>
> If ts->max_rt is zero, or ts->max_x == ts->min_x, or ts->max_y ==
> ts->min_y, these yield a division by zero error.

Ah, that is a good observation!

>
> Ofcourse, this is under control of the DT-maintainer(s) of the device
> (if I'm not mistaking)

Yes, the DT should be designed properly.

> , but throwing an error on DT parse if (one of)
> these condition(s) is met yields better info to the DT-maintainer than
> a /0 error...

Yes, but I wonder how likely it is to happen at all.

In an case there is an indicative message (either /0 or an explaining one) in
the boot log which goes away as soon as it has been fixed.

This is already better than in many other cases where things simply fail without
any message...

So I would weight the likelihood of happening vs. the additional code
to check every time.

>
> If you (or others) agree, could you add this to the patch?

If maintainers request, I can easily add it. But personally I do not see
it as absolutely necessary.

BR and many thanks,
Nikolaus