Port keyboard driver changes from DragonFly to allow using keyboard on Chromebook

[grembo]

Summary

This is a port of the essential bits of those two commits from DragonFlyBSD:

http://gitweb.dragonflybsd.org/dragonfly.git/commitdiff/ad8b9749306613f2978808b43fe5e56019aeeb0f
http://gitweb.dragonflybsd.org/dragonfly.git/commitdiff/7f3d8d5546c701eceefe4437a49ffa6f01c7aa09

which allows probing and using the in-built keyboard of the Acer C720 Chromebook (and
potentially other Chromebooks). I made sure to keep comments.

Commit log entries from the original commits (by Matt Dillon):

• Fix a broken conditional that was preventing init_keyboard() from ever being called.
• Reorder the sequence of code in init_keyboard(), placing the soft-reset command first and adding a SETLEDS command later on.
• My chromebook (Acer C720) required that a SETLEDS command be sent to the keyboard before it will send any keystrokes to us, for some reason.
• Chromebooks (Acer C720 at least) appear to be using a BIOS emulator for the i8042 which gets really unhappy if the keyboard port is disabled, even temporarily.
• The original FreeBSD code disables and enables the keyboard port all the time, and even sends discrete disable commands (instead of just trusting that the command register flags will do the job). In addition, the PSM probe code may also disable the keyboard port temporarily while enabling and messing with the AUX port.
• Remove these actions. The keyboard probe will now only disable the keyboard port during the probe and init_keyboard() call, which then proceeds to issue sufficient commands to the keyboard to wake it up again. The PSM probe now no longer touches the keyboard port disable/interrupt bits. At all. Theoetically it should not be necessary to disable the keyboard port while probing the aux port.
• The saved command_mask in the softc is no longer used, remove it.
• It is unclear whether the controller properly prioritizes data returned from controller commands in-front of the data returned from the keyboard or aux ports. However, as it stands now, we cannot safely disable the data from the ports while issuing controller commands and waiting for controller command responses so...

Test Plan

Build, boot, test. I've been using it for a couple of months successfully
on a Chromebook, but there might be ill side-effects on other machines.

[kib]

I did not looked further.

My overall reaction is strong 'no'. This is some random shaking of the code which was tested on all possible keyboard controllers for more that 20 years. I do not believe that it is theoretically possible to provide adequate testing for such change, and I am more that sure that the change breaks existing working controllers.

What other OSes do with this hardware ? Does Linux provide special driver for the machine ? Does Windows boot on it ?

sys/dev/atkbdc/atkbd.c

434 ↗

(On Diff #3691)

I do not understand why this change is even close to valid.
The !KBD_IS_PROBED() block above calls KBD_FOUND_DEVICE() as needed.
Comment claims that condition is reverted without arguing why.

446 ↗

(On Diff #3691)

Again, why this is needed ?

[grembo]

In D1802#5, @kostikbel wrote:

I did not looked further.

My overall reaction is strong 'no'. This is some random shaking of the code which was tested on all possible keyboard controllers for more that 20 years. I do not believe that it is theoretically possible to provide adequate testing for such change, and I am more that sure that the change breaks existing working controllers.

What other OSes do with this hardware ? Does Linux provide special driver for the machine ? Does Windows boot on it ?

Afaik Linux works out of the box (it doesn't seem to reset the keyboard), Windows seems like a no go, not surprising, as the machine had been developed to run Linux.

sys/dev/atkbdc/atkbd.c

434 ↗

(On Diff #3691)

Looking at the logic I can see your point there. Since the original commits were done in two steps, it's possible that this is a workaround that's not required anymore (assuming probe_keyboard actually works). I can do some testing if it's still required.

446 ↗

(On Diff #3691)

See above.

[grembo]

This comment was removed by grembo.

sys/dev/atkbdc/atkbd.c

1304 ↗

(On Diff #3691)

The reason the reset code is moved up here is what's in the comment above (KBDC_DISABLE_KBD_PORT leaves the keyboard sometimes unresponsive).

Linux atkbd has workarounds for this (but not for this specific model).

246 /*
247 * Certain keyboards to not like ATKBD_CMD_RESET_DIS and stop responding
248 * to many commands until full reset (ATKBD_CMD_RESET_BAT) is performed.
249 */
250 static bool atkbd_skip_deactivate;

1393 ↗

(On Diff #3691)

Most changes below here are quite simple and could be made conditional by extending "flags" (and potentially bet set automatically by a quirk).

[grembo]

[grembo]

This comment was removed by grembo.

[grembo]

New approach, reverted to the original version and added changes based
on quirks.

• Moved resetting the keyboard into a function atkbd_reset (so it can be called form different places)
• Instead of the reverse logic workaround, there's a quirk that explicitly states if probing should always report ok
• Quirk for resetting the keyboard early
• Quirk for setting LEDS as part of initializing
• Quirk to set that the controller can't handle disabling properly (changing kbd_set_device_mask to respect that quirk reduced the number of changes in the code).

atkbdc.c contains a quirk table based on smbios. Right now it only
applies the quirks for the exact model. It might make sense to add
coreboot in general, but I don't have a machine to test this myself.

Assuming there are no mistakes in the code, this should behave
exactly like the original code on any machine besides the one listed
in the quirks table.

I would appreciate constructive feedback.

( removed some of my earlier comments for readability)

[kib]

Yes, this is more acceptable, as it seems.

Note that dev/drm2 has very similar quirk system, it probably makes sense to move the code into some common location.

[grembo]

I like the idea of structuring quirks in a more uniform way, but since we have those in many different places (drm2, USB, CAM, ACPI) I think this should be approached in a separate effort.

[jhb]

I think this looks fine now.

[grembo]

Closed by commit rS278787 (authored by @grembo).