Message-ID: <2798B14C-C8A4-4976-AF40-22A1E0FFB0FC@gmail.com>
Date: Fri, 21 Mar 2025 07:16:50 -1000
From: "Derek J. Clark" <derekjohn.clark@...il.com>
To: Antheas Kapenekakis <lkml@...heas.dev>, platform-driver-x86@...r.kernel.org
CC: linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-pm@...r.kernel.org, Guenter Roeck <linux@...ck-us.net>,
 Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
 Joaquin Ignacio Aramendia <samsagax@...il.com>,
 Kevin Greenberg <kdgreenberg234@...tonmail.com>,
 Joshua Tam <csinaction@...me>, Parth Menon <parthasarathymenon@...il.com>,
 Eileen <eileen@...-netbook.com>, linux-kernel@...r.kernel.org,
 sre@...nel.org, linux@...ssschuh.net, ilpo.jarvinen@...ux.intel.com,
 hdegoede@...hat.com, mario.limonciello@....com
Subject: Re: [PATCH v7 00/14] hwmon: (oxpsensors) Add devices, features, fix ABI and move to platform/x86



On March 19, 2025 8:10:27 AM HST, Antheas Kapenekakis <lkml@...heas.dev> wrote:
>This four part series updates the oxpsensors module to bring it in line
>with its Windows OneXPlayer counterpart. First, it adds support for all
>2024, 2025 OneXPlayer handhelds and their special variants. Then, it moves
>the module to platform/x86 to allow for including more EC features.
>
>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
>charge_behaviour variant called inhibit-charge-s0.
>
>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 minor ABI break 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.
>
>---
>V6: NA
>V5: https://lore.kernel.org/all/20250317155349.1236188-1-lkml@antheas.dev/
>V4: https://lore.kernel.org/all/20250311165406.331046-1-lkml@antheas.dev/
>V3: https://lore.kernel.org/all/20250309112114.1177361-1-lkml@antheas.dev/
>
>Changes since V6:
>    - Fix compile error due to extra const. My bad on that one, did a
>      double check on a device too.
>

For the remainder of the series without my tag:
Reviewed-by: Derek J. Clark <derekjohn.clark@...il.com>

>Changes since V5:
>    - Separate doc entries with Fixes as by Mario
>    - Add sysfs file name to subject as per Thomas
>    - Make tt_led and tt_turbo const as per Thomas
>    - Align a couple of structs as per Thomas
>    - Remove excess battery check as per Thomas
>    - For Thomas: devices without a BIOS update battery control is a NOOP
>      OXP is a boutique manufacturer for now, so gathering information
>      about old devices to add BIOS checks is not practical unfortunately
>
>Changes since V4:
>    - Fix nits by Hans
>    - change inhibit-charge-s0 to inhibit-charge-awake
>    - use devm_battery_hook_register and power_supply_unregister_extension
>      (based on cros driver)
>    - move charge behavior patches to the end to make the rest of the series
>      easier to merge
>    - CC platform-x86 and power maintainers
>
>Changes since V3:
>    - Fix nits by Derek
>    - Remove the hwmon documentation as it is not required for platform
>      drivers (suggested by Guenter)
>    - Add ACPI_BATTERY and HWMON depends to Kconfig
>      (reported by kernel robot)
>    - Homogenize driver into following reverse xmas convention
>
>Changes since V2:
>    - Add ack by Guenter, move platform move patch to be third (not first
>      to allow for device support backport to lts kernels)
>    - Rework patch text, especially in the refactor patches as per Derek
>    - Change bypass to use charge_behaviour instead of charge_type, as that
>      ABI supports capability detection and is more appropriate
>    - Move battery attach to probe instead of init
>    - Fix bug where reading tt_led would instead use the turbo register
>
>Changes since V1:
>    - Add X1 Pro, F1 Pro variants
>    - Fix minor typo in initial patches
>    - Convert oxp-sensors into a platform driver, as it is no longer
>      considered a hwmon driver.
>    - Add sysfs documentation and myself to the MAINTAINERS file
>    - Update documentation to state that this is the OneXPlayer/AOKZOE
>      platform driver, and that support for Ayaneo/OPI is provided until
>      they gain their own platform driver.
>
>Antheas Kapenekakis (14):
>  hwmon: (oxp-sensors) Distinguish the X1 variants
>  hwmon: (oxp-sensors) Add all OneXFly variants
>  platform/x86: oxpec: Move hwmon/oxp-sensors to platform/x86
>  ABI: testing: sysfs-class-oxp: add missing documentation
>  ABI: testing: sysfs-class-oxp: add tt_led attribute documentation
>  platform/x86: oxpec: Rename ec group to tt_toggle
>  platform/x86: oxpec: Add turbo led support to X1 devices
>  platform/x86: oxpec: Move pwm_enable read to its own function
>  platform/x86: oxpec: Move pwm value read/write to separate functions
>  platform/x86: oxpec: Move fan speed read to separate function
>  platform/x86: oxpec: Adhere to sysfs-class-hwmon and enable pwm on 2
>  platform/x86: oxpec: Follow reverse xmas convention for tt_toggle
>  power: supply: add inhibit-charge-awake to charge_behaviour
>  platform/x86: oxpec: Add charge threshold and behaviour to OneXPlayer
>
> Documentation/ABI/testing/sysfs-class-power   |  11 +-
> Documentation/ABI/testing/sysfs-platform-oxp  |  25 +
> Documentation/hwmon/index.rst                 |   2 +-
> Documentation/hwmon/oxp-sensors.rst           |  89 ---
> MAINTAINERS                                   |   7 +-
> drivers/hwmon/Kconfig                         |  11 -
> drivers/hwmon/Makefile                        |   1 -
> drivers/platform/x86/Kconfig                  |  13 +
> drivers/platform/x86/Makefile                 |   3 +
> .../oxp-sensors.c => platform/x86/oxpec.c}    | 624 ++++++++++++++----
> drivers/power/supply/power_supply_sysfs.c     |   7 +-
> drivers/power/supply/test_power.c             |   1 +
> include/linux/power_supply.h                  |   1 +
> 13 files changed, 540 insertions(+), 255 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-platform-oxp
> delete mode 100644 Documentation/hwmon/oxp-sensors.rst
> rename drivers/{hwmon/oxp-sensors.c => platform/x86/oxpec.c} (52%)
>
>
>base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1

- Derek

Powered by blists - more mailing lists