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

From: Antheas Kapenekakis

Date: Sun Sep 20 2026 - 15:13:10 EST

On Tue, 15 Sept 2026 at 14:46, Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 30 Jul 2026, Antheas Kapenekakis wrote:
>
> > 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;
>
> Why are you doing these manually instead of constructing proper usb id
> table, etc.?
>
> > > + 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.
>
> Yes, definitely.
>
> > 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?
>
> Often platform drivers start with a one feature, and may even be very
> specifically named after that feature, but then more platform stuff gets
> added.
>
> (Even our mailing list now bears "x86" in its name despite the scope of
> platform drivers having expanded also to non-x86 platforms.)

I am waiting for feedback from a user with an X2Mini Pro and I should
be able to test on an X1 this week.

Then I will do another feedback round so we should be able to unblock this.

Antheas

> --
> i.
>
> > 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
> > >
> > >
> >
>
>