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

Seth Forshee seth.forshee at canonical.com
Tue Aug 9 13:35:18 UTC 2016


On Tue, Aug 09, 2016 at 03:14:57PM +0800, Phidias Chiang wrote:
> 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!

Sounds good, thanks!

Seth




More information about the kernel-team mailing list