Re: [PATCH v8 2/2] platform/x86/lenovo: Add Yoga Book 9 keyboard dock detection driver

[Dave Carey <carvsdriver@gmail.com>]

On Wed, 10 Jun 2026 19:59:53 +0200, Armin Wolf wrote:
> how many instances of the LENOVO_FEATURE_STATUS_DATA data block exist?
> You can check this with the lswmi tool (https://pypi.org/project/lswmi/).

One instance:

  E7F300FA-21CD-4003-ADAC-2696135982E6: Data   (Instances: 1)

> Please define a struct like this:
>
> struct lenovo_feature_status {
>     __le32 id;
>     __le32 status;
> } __packed;
>
> You should then use said struct for size calculations using sizeof()
> and for accessing the individual fields using le32_to_cpu().

Understood, will do.

> Please drop the error message, passing on the error code should be enough.

Will do.

> I think the whole 32-bit status field is meant to be used instead of only
> the first 2 bits.
> Can you share the output of "acpidump" on this machine so i can take a
> look myself?
>
> IMHO we should verify here that the ID value is correct.

The DSDT confirms the field layout.  In the EC namespace, BKBD is
declared as a 2-bit field:

  Offset (0x23),
      ,   2,
  PELF,   1,
      ,   1,
  BKBD,   2,     /* bits 4-5 of EC byte 0x23 */
      ,   1,
  U7U9,   1,

WQAF builds an 8-byte buffer and assigns EC0.BKBD to a 32-bit dword:

  Name (LFSD, Buffer (0x08) { 0x00 })
  CreateDWordField (LFSD, Zero, LFID)
  CreateDWordField (LFSD, 0x04, LFST)
  LFID = 0x00060000
  If ((Acquire (EC0.LFCM, 0xA000) == Zero)) {
      LFST = EC0.BKBD
      Release (EC0.LFCM)
  }
  Return (LFSD)

ACPI zero-extends the 2-bit BKBD value to 32 bits, so le32_to_cpu(fs->status)
is always 0-3 and equals the full BKBD encoding.  I'll use the status field
directly and add an ID check (le32_to_cpu(fs->id) == 0x00060000) as you
suggest.

_WED(0xEB) also reads EC0.BKBD directly and returns it as an integer,
which is consistent.

I can provide the full acpidump if it would be useful; let me know.

> Nack, having global data like this is not a good idea.  Please use a
> notifier like uniwill-wmi.c to deliver WMI events to the WMI block
> driver, and put the input device into the private data struct of said
> driver.
>
> This would also allow you to get rid of the mutex.

Understood.  I'll restructure along the lines of uniwill-wmi.c:

  - Event driver (LENOVO_BTKBD_EVENT) owns a BLOCKING_NOTIFIER_HEAD and
    fires it from its notify callback.  It exports
    devm_yb9_kbdock_register_notifier() for consumers.

  - Block driver (LENOVO_FEATURE_STATUS_DATA) owns the input_dev in its
    driver private data, registers a notifier_block at probe time, reads
    initial state via wmidev_query_block() on itself, and handles sysfs
    and PM resume.

No global struct, no mutex.

> Please mark this attribute as const.  The same should be done with
> yb9_kbdock_attrs[].

Will do.

> Please use .notify_new() instead of .notify() to avoid having to deal
> with ACPI objects.

Will do.  notify_new receives a const struct wmi_buffer *, which is
cleaner for extracting the integer.

> I think you should do the DMI check here.  This would allow you to mark
> the DMI whitelist as __initconst.

Good point.  I'll move dmi_check_system() into yb9_kbdock_init() and
mark the table __initconst.

Thanks,
Dave