Re: [PATCH 2/2 v3] elantech: Report multitouch with proper ABS_MT messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Éric Piel wrote:
> Op 10-05-10 22:53, Henrik Rydberg schreef:
>> Hi again Éric,
>>
>> thanks for your changes, the first patch looks better now. However, both patches
>> still have style problems. Also, the second patch now makes unrelated changes to
>> the ABS_X etc, so it is difficult to even know what was tested, and whether the
>> first patch really works as claimed.
> What are the style problems of the first patch? I tried to fix all of them.

WARNING: line over 80 characters
#212: FILE: drivers/input/mouse/elantech.c:483:
+		input_set_abs_params(dev, ABS_TOOL_WIDTH, ETP_WMIN_V2, ETP_WMAX_V2, 0, 0);

WARNING: line over 80 characters
#119: FILE: drivers/input/mouse/elantech.c:498:
+		input_set_abs_params(dev, ABS_MT_POSITION_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);

WARNING: line over 80 characters
#120: FILE: drivers/input/mouse/elantech.c:499:
+		input_set_abs_params(dev, ABS_MT_POSITION_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);

> 
> Concerning the second patch, it's necessary to change the lines with
> ABS_X, because otherwise the computation would be duplicated for the MT
> part and it would make everything look more complicated. Honestly, I
> think this change is still very limited and can easily be reviewed for
> not changing the semantic.

Yes, you are right about x1 and y1. The patch patch touches the width variable
for no apparent reason, and I stopped reading there.

If this process seems trivial and tedious to you, consider how it must appear to
those who deal with patches every day. Most people will not even look at a patch
with style problems, so the real review _starts_ when those issues are fixed.

Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux