NAK: [Xenial][SAUCE][PATCH 0/3] New Alps touchpad support

[Seth Forshee]

On Fri, Aug 05, 2016 at 02:35:30PM +0800, Phidias Chiang wrote:
> [Impact]
> 
> Some newer Alps touchpad isn't supported by current kernel therefore
> recognized as PS/2 mouse and lost some key functions.
> 
> From debug message we saw:
> alps_rpt_cmd: psmouse serio1: alps: E6 report: 00 00 64
> alps_rpt_cmd: psmouse serio1: alps: E7 report: 73 03 28
> alps_rpt_cmd: psmouse serio1: alps: EC report: 73 01 13
> alps_identify: psmouse serio1: alps: Likely not an ALPS touchpad: E7=73 03 28, EC=73 01 13
> 
> [Fix]
> 
> The fix has been submitted to linux-input[1] for a few month but haven't
> got merged. Subsystem maintainers has some concerns[2] for data structure
> it uses but it won't affect the stability (coding style mainly).
> 
> [Test]
> 
> We've tested the patch on affected platforms and it works fine. Also we've 
> seen users testing the patch and shows positive results. This driver reads
> the register and check for a specific value to decide if it's gonna be used
> therefore no regression is expected on platforms don't use this touchpad
> 
> [1]: https://patchwork.kernel.org/patch/9136835/
> [2]: http://www.spinics.net/lists/linux-input/msg44795.html
> 
> Ben Gamari (3):
>   UBUNTU: SAUCE: input/alps: Split up ALPS_BUTTONPAD behavior change
>   UBUNTU: SAUCE: input/alps: Add touchstick support for V8 protocol
>     devices
>   UBUNTU: SAUCE: input/alps: Add device description for Dell Latitude
>     E7470
> 
>  drivers/input/mouse/alps.c | 77 +++++++++++++++++++++++++++++++++++++---------
>  drivers/input/mouse/alps.h |  2 ++
>  2 files changed, 65 insertions(+), 14 deletions(-)

Generally this doesn't look too terrible. The code does look like it
restricts the behavior changes to specific hardware. Was regression
testing done with other Alps touchpads?

But there are a couple of concerns that are causing me to nack this.

The first is that your description above leads me to believe that the
upstreaming efforts have been abandoned. Is that true, or is someone
actively working on addressing the concerns raised in the email thread?

The main issue I have though is this statement in the email you linked
to:

> Above alps_model_data list looks like hooks for special touchpads
> which needs more flags to work correctly...

Which makes it sound like there's a risk that the code may not work
correctly. I'd like some reassurance that you understand what the issue
is, and that it is either not really an issue or that it has been fixed.

Thanks,
Seth