Is this a good first patch for enabling screen rotation?

[Valdis.Kletnieks at vt.edu]

On Fri, 30 Jan 2015 00:45:30 +0800, Brock York said:

(Note, I don't have an Acer, nor am I an HID expert...  so take this
all with a grain of salt...)

> Swap the x and y values and negate the x value converting the
> accelerometers coordinate system into the coordinate system
> userspace udev expects.

This part looks fishy...

1) I wasn't aware that udev had a coordinate system.  I think you
meant that something *else* was using values that udev passed
to userspace.  You probably want to clarify what you meant.

2) There obviously was *some* consumer of the input_report_abs()
value before you added the kobject_uevent() call - convince us
that said consumer is OK with its axes being twiddled like this.

>  	input_report_abs(acer_wmi_accel_dev, ABS_X,
> -		(s16)out_obj->package.elements[0].integer.value);
> +		-(s16)out_obj->package.elements[1].integer.value);
>  	input_report_abs(acer_wmi_accel_dev, ABS_Y,
> -		(s16)out_obj->package.elements[1].integer.value);
> +		(s16)out_obj->package.elements[0].integer.value);

Other issues:

3) "when accelerometer position is updated."

Is that going to generate a flood of events if (for example) you're walking
with the device and it's swinging back and forth a bit?  It's one thing to
have HID devices generating a near-constant stream of updates for mouse
tracking and so on - it's a whole 'nother can of worms to additionally
bash udev with an update stream.

Perhaps udev events should be clamped to only trigger when it's time for
an actual axis swap?

4) This should probably be 2 patches - one to swap the input_report_abs()
values, and a second to add the kobject_uevent() call.

5) I almost have to wonder if the input_report_abs() part is sufficient,
and udev events aren't needed?





-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150129/f1b72986/attachment.bin