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

[Phidias Chiang]

On Tue, Aug 9, 2016 at 5:02 AM, Seth Forshee <seth.forshee at canonical.com> wrote:
>
> 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?
>

No I didn't, because from the driver they compare a series of very specific
register values to determine which protocol/functions it should use, and I
don't think older touchpad will go to the newly added code path.

> 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?
>

They actually sent 2 other patch sets, and the latest one fixes the issues
addressed by upstream but still under reviewing[1].

> 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.
>

This is also fixed in the latest patch set.

> Thanks,
> Seth

I'll drop this patch set. And since I've already missed the window on last
Friday (which I really shouldn't rush for it), I'll do more test to make sure
it really doesn't regress on older alps touchpad, meanwhile to see if the
patch set get merged.

Cheers, and thanks for reviewing!

[1]: https://patchwork.kernel.org/patch/9190015/

-- 
phid at irc.canonical.com
HWE Team
Meng-Hsiu Chiang