[00/10] hwmon: (oxpsensors) Add 2024 OneXPlayer line-up, add charge limiting and turbo LED, fix ABI

Message

Antheas Kapenekakis Dec. 26, 2024, 11:27 a.m. UTC

This three part series updates the oxpsensors module to bring it in line
with its Windows OneXPlayer counterpart. First, it adds support for all
2024 OneXPlayer handhelds and their special variants.

Then, it adds the new charge limiting and bypass features that were first
introduced in the X1 and retrofit to older OneXFly variants and for
controlling the turbo led found in the X1 models. For Bypass, it adds a new
bypass variant BypassS0 that is only active while the device is in the S0
state.

Finally, it performs a minor refactor by moving around switch statements
into their own functions, in order to allow for fixing the pwm1_enable ABI
in the final patch. Currently, pwm1_enable sets the fan to auto with the
value 0 and allows manual control with the value 1. This patch makes it
so 0 sets the fan to full speed, 1 sets the fan to manual control, and
2 sets the fan to auto. This requires both setting enable and the fan
speed when the enable sysfs is written to as 0, hence the refactor.

As this is a breaking ABI change and there is userspace software relying
on this previous behavior, the last patch also changes the /name of the
hwmon endpoint to "oxp_ec" from "oxpec" (mirroring WMI module conventions)
such that userspace software that relied on the previous behavior can be
retrofit to the new kernel while enabling correct functionality on old
and new kernels. Failing that, software that is not updated will just
stop controlling the fans, ensuring no malignant behavior.

As part of this series I also updated my userspace software [1] to work
with this change and verified it works correctly on both 6.12.6 and when
using this module on my X1.

To make testing possible this patch series is rebased on top of the tag
kernel-6.12.6 (hash e4cc9a13) on remote [2]. None of the files changed
in-between master and this tag, but in case this does not apply clean,
you can pull that remote or reference [3].

[1] https://github.com/hhd-dev/adjustor/commit/4b02087ae39bef39288f26431af0ec221da089c4
[2] https://gitlab.com/cki-project/kernel-ark
[3] https://github.com/hhd-dev/patchwork/tree/upstream/oxpsensors

Antheas Kapenekakis (10):
  hwmon: (oxp-sensors) Distinguish the X1 variants
  hwmon: (oxp-sensors) Add all OneXFly variants
  ABI: testing: sysfs-class-power: add BypassS0 charge_type
  hwmon: (oxp-sensors) Add charge threshold and bypass to OneXPlayer
  hwmon: (oxp-sensors) Rename ec group to tt_toggle
  hwmon: (oxp-sensors) Add turbo led support to X1 devices
  hwmon: (oxp-sensors) Move pwm_enable read to its own function
  hwmon: (oxp-sensors) Move pwm value read/write to separate functions
  hwmon: (oxp-sensors) Move fan speed read to separate function
  hwmon: (oxp-sensors) Adhere to sysfs-class-hwmon and enable pwm on 2

 Documentation/ABI/testing/sysfs-class-power |   6 +-
 drivers/hwmon/oxp-sensors.c                 | 647 ++++++++++++++++----
 2 files changed, 523 insertions(+), 130 deletions(-)

Comments

Guenter Roeck Dec. 26, 2024, 9:08 p.m. UTC | #1

On Thu, Dec 26, 2024 at 12:27:30PM +0100, Antheas Kapenekakis wrote:
> This three part series updates the oxpsensors module to bring it in line
> with its Windows OneXPlayer counterpart. First, it adds support for all
> 2024 OneXPlayer handhelds and their special variants.
> 
> Then, it adds the new charge limiting and bypass features that were first
> introduced in the X1 and retrofit to older OneXFly variants and for
> controlling the turbo led found in the X1 models. For Bypass, it adds a new
> bypass variant BypassS0 that is only active while the device is in the S0
> state.
> 

This is a hardware monitoring driver. It is not a charge controller driver,
and it is not a LED controller driver. If such control is wanted/needed for
this system, it should be implemented either as mfd device with client drivers,
or the entire driver should be moved to platform drivers if there is a desire
to keep it as single driver.

Guenter

Derek John Clark Dec. 26, 2024, 9:16 p.m. UTC | #2

On December 26, 2024 1:08:02 PM PST, Guenter Roeck <[e-mail]> wrote:
>On Thu, Dec 26, 2024 at 12:27:30PM +0100, Antheas Kapenekakis wrote:
>> This three part series updates the oxpsensors module to bring it in line
>> with its Windows OneXPlayer counterpart. First, it adds support for all
>> 2024 OneXPlayer handhelds and their special variants.
>> 
>> Then, it adds the new charge limiting and bypass features that were first
>> introduced in the X1 and retrofit to older OneXFly variants and for
>> controlling the turbo led found in the X1 models. For Bypass, it adds a new
>> bypass variant BypassS0 that is only active while the device is in the S0
>> state.
>> 
>
>This is a hardware monitoring driver. It is not a charge controller driver,
>and it is not a LED controller driver. If such control is wanted/needed for
>this system, it should be implemented either as mfd device with client drivers,
>or the entire driver should be moved to platform drivers if there is a desire
>to keep it as single driver.
>
>Guenter

I think moving this to x86 platform makes a lot of sense to ensure two separate drivers can't do async writes to the EC. We probably should have done that when adding the turbo button toggle anyway. I'll coordinate that effort with Tobias and Antheas directly before moving forward.

Thanks Guenter,
- Derek

Antheas Kapenekakis Dec. 26, 2024, 9:59 p.m. UTC | #3

Hi Guenter,
Right now, OneXPlayer features a singular EC with a handful of
features. Where with this patch series we essentially have parity with
windows, apart from a VRAM feature which I will have to ask but I
suspect is WMI. It currently concerns a singular component (the EC)
and the functionality such as turbo button affects the hwmon component
(i.e., if the turbo button is not engaged, the fan control
functionality does not work).

As such it cannot be made into separate drivers. I think, and I
suspect you will agree, that the scope of the driver is _correct_ but
the place is not correct.

Therefore, can you suggest a reasonable path forward that is sparing
to this patch series? Should I add an eleventh patch that grafts this
driver to platform-x86? If appropriate, you can cc the x86 mailing
list on your next email.

Best,
Antheas

On Thu, 26 Dec 2024 at 22:16, Derek J. Clark <[e-mail]> wrote:
>
>
>
> On December 26, 2024 1:08:02 PM PST, Guenter Roeck <[e-mail]> wrote:
> >On Thu, Dec 26, 2024 at 12:27:30PM +0100, Antheas Kapenekakis wrote:
> >> This three part series updates the oxpsensors module to bring it in line
> >> with its Windows OneXPlayer counterpart. First, it adds support for all
> >> 2024 OneXPlayer handhelds and their special variants.
> >>
> >> Then, it adds the new charge limiting and bypass features that were first
> >> introduced in the X1 and retrofit to older OneXFly variants and for
> >> controlling the turbo led found in the X1 models. For Bypass, it adds a new
> >> bypass variant BypassS0 that is only active while the device is in the S0
> >> state.
> >>
> >
> >This is a hardware monitoring driver. It is not a charge controller driver,
> >and it is not a LED controller driver. If such control is wanted/needed for
> >this system, it should be implemented either as mfd device with client drivers,
> >or the entire driver should be moved to platform drivers if there is a desire
> >to keep it as single driver.
> >
> >Guenter
>
> I think moving this to x86 platform makes a lot of sense to ensure two separate drivers can't do async writes to the EC. We probably should have done that when adding the turbo button toggle anyway. I'll coordinate that effort with Tobias and Antheas directly before moving forward.
>
> Thanks Guenter,
> - Derek

Guenter Roeck Dec. 27, 2024, 5:12 p.m. UTC | #4

On Thu, Dec 26, 2024 at 01:16:15PM -0800, Derek J. Clark wrote:
> 
> 
> On December 26, 2024 1:08:02 PM PST, Guenter Roeck <[e-mail]> wrote:
> >On Thu, Dec 26, 2024 at 12:27:30PM +0100, Antheas Kapenekakis wrote:
> >> This three part series updates the oxpsensors module to bring it in line
> >> with its Windows OneXPlayer counterpart. First, it adds support for all
> >> 2024 OneXPlayer handhelds and their special variants.
> >> 
> >> Then, it adds the new charge limiting and bypass features that were first
> >> introduced in the X1 and retrofit to older OneXFly variants and for
> >> controlling the turbo led found in the X1 models. For Bypass, it adds a new
> >> bypass variant BypassS0 that is only active while the device is in the S0
> >> state.
> >> 
> >
> >This is a hardware monitoring driver. It is not a charge controller driver,
> >and it is not a LED controller driver. If such control is wanted/needed for
> >this system, it should be implemented either as mfd device with client drivers,
> >or the entire driver should be moved to platform drivers if there is a desire
> >to keep it as single driver.
> >
> >Guenter
> 
> I think moving this to x86 platform makes a lot of sense to ensure two separate drivers can't do async writes to the EC. We probably should have done that when adding the turbo button toggle anyway. I'll coordinate that effort with Tobias and Antheas directly before moving forward.
> 

Fine with me, but "ensure two drivers can't do asynchronous writes" is not
really an argument. mfd supports that as well for pretty much all mfd
drivers.

Guenter