On Thursday 12 February 2015 08:52:35 Dmitry Torokhov wrote:
> On Sun, Feb 08, 2015 at 01:26:32PM +0100, Pali Rohár wrote:
> > On Monday 02 February 2015 15:27:55 Pali Rohár wrote:
> > > On Monday 02 February 2015 11:49:58 Pali Rohár wrote:
> > > > On Monday 02 February 2015 06:49:31 Dmitry Torokhov
wrote:
> > > > > On Sun, Jan 18, 2015 at 10:47:06AM +0100, Pali Rohár
wrote:
> > > > > > On Sunday 18 January 2015 08:22:45 Dmitry Torokhov
wrote:
> > > > > > > On Sat, Jan 17, 2015 at 11:01:56AM +0100, Pali
> > > > > > > Rohár
> >
> > wrote:
> > > > > > > > On Thursday 15 January 2015 22:02:16 Dmitry
> > > > > > > > Torokhov
> > > >
> > > > wrote:
> > > > > > > > > On Thu, Jan 15, 2015 at 09:28:41PM +0100, Pali
> > > > > > > > > Rohár
> > > >
> > > > wrote:
> > > > > > > > > > On Thursday 15 January 2015 20:38:18 Dmitry
> > > > > > > > > > Torokhov
> > > > > >
> > > > > > wrote:
> > > > > > > > > > > On Thu, Jan 15, 2015 at 08:19:59PM +0100,
> > > > > > > > > > > Pali Rohár
> > > > > >
> > > > > > wrote:
> > > > > > > > > > > > On Thursday 15 January 2015 19:18:20
> > > > > > > > > > > > Dmitry Torokhov
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > > > > > > > On Thu, Jan 15, 2015 at 11:49:32AM
> > > > > > > > > > > > > +0100, Pali Rohár
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > > > > > > > > On Wednesday 14 January 2015
> > > > > > > > > > > > > > 23:55:48 Dmitry Torokhov
> > > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > > > > > > > Hi Pali,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > This series try to address the
> > > > > > > > > > > > > > > issue you brought regarding
> > > > > > > > > > > > > > > trackstick initialization on Dell
> > > > > > > > > > > > > > > Latitudes in a different way than
> > > > > > > > > > > > > > > the patches you proposed.
> > > > > > > > > > > > > > > Basically in this series we move
> > > > > > > > > > > > > > > resetting and all detection in
> > > > > > > > > > > > > > > alps_detect() and make sure we
> > > > > > > > > > > > > > > keep the state so alps_init() can
> > > > > > > > > > > > > > > reuse it and not perform the
> > > > > > > > > > > > > > > detection all over again. Doing
> > > > > > > > > > > > > > > this allows us to set up device
> > > > > > > > > > > > > > > characteristics (name, version,
> > > > > > > > > > > > > > > etc) properly from the get go
> > > > > > > > > > > > > > > while still performing reset only
> > > > > > > > > > > > > > > once.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > This is untested as I do not have
> > > > > > > > > > > > > > > any ALPS devices anymore so I'd
> > > > > > > > > > > > > > > appreciate you giving it a spin.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Dmitry,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > on top of which branch/repository
> > > > > > > > > > > > > > should I apply your patches?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Should be applicable to my 'next'
> > > > > > > > > > > > > branch (which I just upreved to
> > > > > > > > > > > > > 3.19-rc4).
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks.
> > > > > > > > > > > >
> > > > > > > > > > > > Not working at top of next (0c3e994).
> > > > > > > > > > > >
> > > > > > > > > > > > Applying: Input: ALPS - renumber
> > > > > > > > > > > > protocol numbers Applying: Input: ALPS
> > > > > > > > > > > > - make Rushmore a separate protocol
> > > > > > > > > > > > error: patch failed:
> > > > > > > > > > > > drivers/input/mouse/alps.c:1275 error:
> > > > > > > > > > > > drivers/input/mouse/alps.c: patch does
> > > > > > > > > > > > not apply Patch failed at 0002 Input:
> > > > > > > > > > > > ALPS - make Rushmore a separate
> > > > > > > > > > > > protocol
> > > > > > > > > > >
> > > > > > > > > > > Hmm.. I created a new alps branch (based
> > > > > > > > > > > on 3.19-rc4), can you try it?
> > > > > > > > > > >
> > > > > > > > > > > Thanks.
> > > > > > > > > >
> > > > > > > > > > Compiled from your new alps branch (with "if
> > > > > > > > > > (!priv)" fix) and modprobing psmouse.ko
> > > > > > > > > > caused laptop freeze :-( Even sysrq not
> > > > > > > > > > responded. So something is not working...
> > > > > > > > >
> > > > > > > > > Hmm, is it on text console or in X? Any chance
> > > > > > > > > you could go through pathes - there are only
> > > > > > > > > 8 of them including 2 of yours that should be
> > > > > > > > > unmodified.
> > > > > > > > >
> > > > > > > > > Thanks.
> > > > > > > >
> > > > > > > > Hi, now I tested patch by patch and kernel crash
> > > > > > > > is caused only by last patch 6/6 and only after
> > > > > > > > I touch touchpad or trackstick.
> > > > > > > >
> > > > > > > > In text console it prints lot of panic messages
> > > > > > > > and because it prints lot of messages I cannot
> > > > > > > > read (or record) more then last.
> > > > > > > >
> > > > > > > > In last call trace I see that
> > > > > > > > alps_register_bare_ps2_mouse() was called and it
> > > > > > > > generated page_fault.
> > > > > > >
> > > > > > > That happens because while you added priv->psmouse
> > > > > > > pointer it looks like you forgot to initialize it
> > > > > > > and I missed that too...
> > > > > >
> > > > > > Right your patch 6/6 does not initialize
> > > > > > priv->psmouse. I looked into my original patch and
> > > > > > it initialize it, so there was some copy-paste
> > > > > > error.
> > > > > >
> > > > > > Look at other emails... can you fix problems and
> > > > > > send new version of your patches for testing?
> > > > >
> > > > > I think I did. Can you please take another look at my
> > > > > alps branch?
> > > > >
> > > > > Thanks!
> > > >
> > > > Now I tried it. It does not crash anymore which is good.
> > > > But it does not working. Driver alsp.c receive only
> > > > bare PS/2 packets, not 6 bites ALPS packets. So ALPS
> > > > touchpad is not properly initialized and so is in
> > > > "legacy" mode when it act as genetic mouse. And events
> > > > are sent via dev3 device (that PS/2 mouse).
> > > >
> > > > So your patches do not initialize my ALPS touchpad
> > > > correctly.
> > >
> > > Hello Dmitry, I found where is problem.
> > >
> > > This chunk is needed:
> > >
> > > @@ -135,7 +135,7 @@ static const struct alps_protocol_info
> > > alps_v3_protocol_data = { };
> > >
> > > static const struct alps_protocol_info
> > > alps_v3_rushmore_data
> > >
> > > = { - ALPS_PROTO_V3, 0x8f, 0x8f, ALPS_DUALPOINT
> > > + ALPS_PROTO_V3_RUSHMORE, 0x8f, 0x8f, ALPS_DUALPOINT
> > >
> > > };
> > >
> > > static const struct alps_protocol_info
> > > alps_v5_protocol_data
> > >
> > > = {
> > >
> > > because rushmore is now separate protocol.
>
> Right you are.
>
> > > And maybe also these two chunks are needed too:
> > >
> > > @@ -663,7 +696,8 @@ static void
> > > alps_process_touchpad_packet_v3_v5(struct psmouse
> > > *psmouse) */
> > >
> > > if (f->is_mp) {
> > >
> > > fingers = f->fingers;
> > >
> > > - if (priv->proto_version == ALPS_PROTO_V3) {
> > > + if (priv->proto_version == ALPS_PROTO_V3 ||
> > > + priv->proto_version ==
ALPS_PROTO_V3_RUSHMORE) {
>
> Yep.
>
> > > if (alps_process_bitmap(priv, f) == 0)
> > >
> > > fingers = 0; /* Use st data */
> > >
> > > @@ -1365,8 +1399,9 @@ static psmouse_ret_t
> > > alps_process_byte(struct psmouse *psmouse) psmouse->pktcnt
> > > - 1,
> > >
> > > psmouse->packet[psmouse->pktcnt - 1]);
> > >
> > > - if (priv->proto_version == ALPS_PROTO_V3_RUSHMORE
&&
> > > - psmouse->pktcnt == psmouse->pktsize) {
> > > + if ((priv->proto_version == ALPS_PROTO_V3 ||
> > > + priv->proto_version == ALPS_PROTO_V3_RUSHMORE
> > > + ) && psmouse->pktcnt == psmouse->pktsize) {
>
> No, I think this particular case we want to leave
> rushmore-only, since that's what we have in those Latitudes.
>
> > > /*
> > >
> > > * Some Dell boxes, such as Latitude E6440 or
E7440
> > > * with closed lid, quite often smash last byte
of
> > >
> > > With all these tree parts, initialization of my ALPS
> > > touchpad is OK and working as expected. I compared PS/2
> > > init commands and are same as on 3.13 kernel.
> >
> > Dmitry, will you prepare v2 series of patches?
>
> Yes, I rebased my alps branch on top of 3.19 and fixed up the
> patches. Can you please try it?
>
> Thanks.
Tested version from commit 48aecc2 and my touchpad and trackstisk
is working fine.
You can add my tested-by line to all patches:
Tested-by: Pali Rohár <pali.rohar@xxxxxxxxx>
Just one small cosmetic/format note:
In alps.c code is this comment:
* XXX - this entry is suspicious. First byte has zero ...
It belongs to line with struct alps_model_info alps_model_data
where is that /* XXX */ comment.
But your patches added another structures between these two
comments and so it is not easy to understand what that XXX means.
Can you move this comment at correct place (maybe immediately
after struct alps_model_info)?
--
Pali Rohár
pali.rohar@xxxxxxxxx
Attachment:
signature.asc
Description: This is a digitally signed message part.
