Re: [PATCH v2 2/2] platform/x86: oxpec: Report tablet mode on OneXPlayer detachables

[Antheas Kapenekakis]

On Tue, 28 Jul 2026 at 10:35, Alexander Egorov <begeebe@xxxxxxxxx> wrote:
>
> The OneXPlayer X1 family and Super X have detachable pogo-pin
> keyboards. The firmware does not expose a tablet-mode input switch.
>
> The Super X keyboard enumerates as 1a86:1305. The X1 family folio
> keyboard enumerates as 258a:001e.
>
> Register an input device on these boards and report SW_TABLET_MODE from
> the model-specific pogo keyboard USB hotplug state. Other USB and
> Bluetooth keyboards do not affect the switch.
>
> A complete dump of the 256-byte Super X EC register map was compared
> with the keyboard attached, detached, and reattached. No register
> changed predictably with the attachment state.
>
> Tested on a OneXPlayer Super X. Mutter enables accelerometer-driven
> display rotation only while the pogo keyboard is absent.
>
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Alexander Egorov <begeebe@xxxxxxxxx>
> ---
>  drivers/platform/x86/Kconfig |   2 +
>  drivers/platform/x86/oxpec.c | 103 +++++++++++++++++++++++++++++++++++
>  2 files changed, 105 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index b54b521..3bcbd49 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1055,6 +1055,8 @@ config OXP_EC
>         depends on ACPI_EC
>         depends on ACPI_BATTERY
>         depends on HWMON
> +       depends on INPUT
> +       depends on USB
>         depends on X86
>         help
>                 Enables support for the platform EC of OneXPlayer and AOKZOE
> diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c
> index df29e41..5c07a35 100644
> --- a/drivers/platform/x86/oxpec.c
> +++ b/drivers/platform/x86/oxpec.c
> @@ -18,10 +18,12 @@
>  #include <linux/dmi.h>
>  #include <linux/hwmon.h>
>  #include <linux/init.h>
> +#include <linux/input.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/processor.h>
> +#include <linux/usb.h>
>  #include <acpi/battery.h>
>
>  /* Handle ACPI lock mechanism */
> @@ -55,6 +57,8 @@ enum oxp_board {
>
>  static enum oxp_board board;
>  static struct device *oxp_dev;
> +static struct input_dev *oxp_tablet_mode_input;
> +static struct notifier_block oxp_usb_notifier;
>
>  /* Fan reading and PWM */
>  #define OXP_SENSOR_FAN_REG             0x76 /* Fan reading is 2 registers long */
> @@ -293,6 +297,74 @@ static const struct dmi_system_id dmi_table[] = {
>         {},
>  };
>
> +#define OXP_KEYBOARD_VID_QINHENG       0x1a86
> +#define OXP_KEYBOARD_PID_K2445         0x1305
> +#define OXP_KEYBOARD_VID_HAILUCK       0x258a
> +#define OXP_KEYBOARD_PID_HAILUCK       0x001e

These defines need to go above with the other defines.

Can you explain the reasoning behind the name PID_K2445 and PID_HAILUCK?

> +
> +static int oxp_find_keyboard(struct usb_device *udev, void *data)
> +{
> +       bool *keyboard_attached = data;
> +       u16 vid = le16_to_cpu(udev->descriptor.idVendor);
> +       u16 pid = le16_to_cpu(udev->descriptor.idProduct);
> +
> +       switch (board) {
> +       case oxp_x1:
> +               if (vid == OXP_KEYBOARD_VID_HAILUCK &&
> +                   pid == OXP_KEYBOARD_PID_HAILUCK)
> +                       *keyboard_attached = true;
> +               break;
> +       case oxp_super_x:
> +               if (vid == OXP_KEYBOARD_VID_QINHENG &&
> +                   pid == OXP_KEYBOARD_PID_K2445)
> +                       *keyboard_attached = true;
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
> +static int oxp_usb_notify(struct notifier_block *nb,
> +                         unsigned long action, void *data)
> +{
> +       struct usb_device *udev = data;
> +       u16 vid = le16_to_cpu(udev->descriptor.idVendor);
> +       u16 pid = le16_to_cpu(udev->descriptor.idProduct);
> +       bool keyboard = false;
> +
> +       switch (board) {
> +       case oxp_x1:
> +               keyboard = vid == OXP_KEYBOARD_VID_HAILUCK &&
> +                          pid == OXP_KEYBOARD_PID_HAILUCK;
> +               break;
> +       case oxp_super_x:
> +               keyboard = vid == OXP_KEYBOARD_VID_QINHENG &&
> +                          pid == OXP_KEYBOARD_PID_K2445;
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       if (!keyboard)
> +               return NOTIFY_DONE;
> +
> +       switch (action) {
> +       case USB_DEVICE_ADD:
> +               input_report_switch(oxp_tablet_mode_input, SW_TABLET_MODE, false);
> +               break;
> +       case USB_DEVICE_REMOVE:
> +               input_report_switch(oxp_tablet_mode_input, SW_TABLET_MODE, true);
> +               break;
> +       default:
> +               return NOTIFY_DONE;
> +       }
> +
> +       input_sync(oxp_tablet_mode_input);
> +       return NOTIFY_OK;
> +}
> +
>  /* Helper functions to handle EC read/write */
>  static int read_from_ec(u8 reg, int size, long *val)
>  {
> @@ -948,6 +1020,7 @@ static const struct hwmon_chip_info oxp_ec_chip_info = {
>  /* Initialization logic */
>  static int oxp_platform_probe(struct platform_device *pdev)
>  {
> +       bool keyboard_attached = false;
>         struct device *dev = &pdev->dev;
>         struct device *hwdev;
>         int ret;
> @@ -965,6 +1038,33 @@ static int oxp_platform_probe(struct platform_device *pdev)
>                         return ret;
>         }
>
> +       switch (board) {
> +       case oxp_x1:
> +       case oxp_super_x:
> +               oxp_tablet_mode_input = devm_input_allocate_device(dev);
> +               if (!oxp_tablet_mode_input)
> +                       return -ENOMEM;
> +
> +               oxp_tablet_mode_input->name = "OneXPlayer Tablet Mode Switch";
> +               oxp_tablet_mode_input->phys = "oxp-platform/input0";
> +               oxp_tablet_mode_input->id.bustype = BUS_HOST;
> +               input_set_capability(oxp_tablet_mode_input, EV_SW, SW_TABLET_MODE);
> +
> +               ret = input_register_device(oxp_tablet_mode_input);
> +               if (ret)
> +                       return ret;
> +
> +               oxp_usb_notifier.notifier_call = oxp_usb_notify;
> +               usb_register_notify(&oxp_usb_notifier);
> +               usb_for_each_dev(&keyboard_attached, oxp_find_keyboard);
> +               input_report_switch(oxp_tablet_mode_input, SW_TABLET_MODE,
> +                                   !keyboard_attached);
> +               input_sync(oxp_tablet_mode_input);
> +               break;
> +       default:
> +               break;
> +       }
> +
>         return 0;
>  }
>
> @@ -1005,6 +1105,9 @@ static int __init oxp_platform_init(void)
>
>  static void __exit oxp_platform_exit(void)
>  {
> +       if (board == oxp_x1 || board == oxp_super_x)
> +               usb_unregister_notify(&oxp_usb_notifier);

You used a switch statement above. Match the logic and use a switch
statement here too.

Give it a few days before sending a V3 for Ilpo to reply.

@Ilpo do you think this is an acceptable expansion for the EC driver?

The rest of the patch looks fine to me.

Best,
Antheas

> +
>         platform_device_unregister(oxp_platform_device);
>         platform_driver_unregister(&oxp_platform_driver);
>  }
> --
> 2.55.0
>
>