Meanwhile, I think this patch is not a good idea, at least not in this form.
While it may correctly establish whether there is an auxiliary interface or
not, it may lead to conflicts in cases where a touchpad isn't "meant" to be
run in the legacy PS/2 mode.

Until now, I have thought that the lack of support for the AUXECHO command
is just a flaw of the firmware, but maybe it's the manufacturer's idea of
"phasing out" the legacy interface?

Many modern touchpads have dual connections to the system.  And if they
are recognized and initialized on another bus, then pckbc shouldn't attach
pms in the aux slot.  (Perhaps it would be nice to have some mechanism to
control that, but I don't know what it should look like.)

Since this weekend, I have a Clevo laptop here, and with this commit from
yesterday - which adds support for newer i2c controllers - the touchpad
is recognized and works well with imt(4):
    https://marc.info/?l=openbsd-cvs&m=160131811230169&w=2

My patch isn't necessary in this setup, on the contrary:  The PS/2 interface
of the touchpad is dead, and if we added that code to the kernel now, it
would lead to a void pms and wsmouse node (and to timeouts when attempts are
made to enable or disable pms).


On 9/13/20 13:14, Ulf Brosziewski wrote:
> Not all PS/2-like controllers accept the AUXECHO command, and the test that
> pckbc applies in order to check for the presence of the auxiliary interface
> may yield false negatives, even on newer hardware (see
>     https://marc.info/?l=openbsd-misc&m=158413132831425&w=2
> ).
> 
> This patch adds an alternative test.  It is applied if AUXECHO fails, and
> attempts to toggle and reset the KC8_MDISABLE flag by issuing an AUXDISABLE
> and AUXENABLE command, which should only succeed if there's an AUX interface.
> 
> (The test has a branch which makes it work regardless of the initial state
> of the flag; that's unnecessary at present but might be useful in a future
> version.)
> 
> OK?
> 
> 
> Index: pckbc.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/pckbc.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 pckbc.c
> --- pckbc.c   30 Nov 2019 18:18:34 -0000      1.53
> +++ pckbc.c   13 Sep 2020 09:27:10 -0000
> @@ -295,6 +295,32 @@ pckbc_attach_slot(struct pckbc_softc *sc
>       return (found);
>  }
> 
> +int
> +pckbc_auxcheck(struct pckbc_internal *t)
> +{
> +     bus_space_tag_t iot = t->t_iot;
> +     bus_space_handle_t ioh_c = t->t_ioh_c;
> +     int res = -1;
> +
> +     if ((t->t_cmdbyte & KC8_MDISABLE) == 0) {
> +             if (pckbc_send_cmd(iot, ioh_c, KBC_AUXDISABLE)
> +                 && pckbc_get8042cmd(t))
> +                     res = (t->t_cmdbyte & KC8_MDISABLE);
> +             pckbc_send_cmd(iot, ioh_c, KBC_AUXENABLE);
> +     } else {
> +             if (pckbc_send_cmd(iot, ioh_c, KBC_AUXENABLE)
> +                 && pckbc_get8042cmd(t))
> +                     res = (~t->t_cmdbyte & KC8_MDISABLE);
> +             pckbc_send_cmd(iot, ioh_c, KBC_AUXDISABLE);
> +     }
> +     if (res == -1)
> +             printf("kbc: auxcheck: error\n");
> +     else
> +             printf("kbc: auxcheck: %d\n", (res > 0));
> +     pckbc_get8042cmd(t);
> +     return (res > 0);
> +}
> +
>  void
>  pckbc_attach(struct pckbc_softc *sc, int flags)
>  {
> @@ -410,13 +436,17 @@ pckbc_attach(struct pckbc_softc *sc, int
>                */
>               DPRINTF("kbc: aux echo: %x\n", res);
>               t->t_haveaux = 1;
> -             if (pckbc_attach_slot(sc, PCKBC_AUX_SLOT, 0))
> -                     cmdbits |= KC8_MENABLE;
>       }
>  #ifdef PCKBCDEBUG
>       else
>               printf("kbc: aux echo test failed\n");
>  #endif
> +
> +     if (res == -1)
> +             t->t_haveaux = pckbc_auxcheck(t);
> +
> +     if (t->t_haveaux && pckbc_attach_slot(sc, PCKBC_AUX_SLOT, 0))
> +             cmdbits |= KC8_MENABLE;
> 
>  #if defined(__i386__) || defined(__amd64__)
>       if (haskbd == 0 && !ISSET(flags, PCKBCF_FORCE_KEYBOARD_PRESENT)) {
> 

Reply via email to