| Message ID | 20250309112114.1177361-7-lkml@antheas.dev (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | hwmon: (oxpsensors) Add devices, features, fix ABI and move to platform/x86 | expand |
On Sun, Mar 9, 2025 at 4:21 AM Antheas Kapenekakis <lkml@antheas.dev> wrote: > > With the X1 (AMD), OneXPlayer added a charge limit and charge bypass to > their devices. Charge limit allows for choosing an arbitrary battery > charge setpoint in percentages. Charge bypass allows to instruct the > device to stop charging either when it is in s0 or always. > > This feature was then extended for the F1Pro as well. OneXPlayer also > released BIOS updates for the X1 Mini, X1 (Intel), and F1 devices that > add support for this feature. Therefore, enable it for all F1 and > X1 devices. > As noted in your previous patch, I think checking for BIOS support is a wise move here. > Add both of these under the standard sysfs battery endpoints for them, > by looking for the battery. OneXPlayer devices have a single battery. > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> > --- > drivers/platform/x86/oxpec.c | 217 +++++++++++++++++++++++++++++++++++ > 1 file changed, 217 insertions(+) > > diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c > index dc3a0871809c..dd6d333ebcfa 100644 > --- a/drivers/platform/x86/oxpec.c > +++ b/drivers/platform/x86/oxpec.c > @@ -24,6 +24,7 @@ > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/processor.h> > +#include <acpi/battery.h> > > /* Handle ACPI lock mechanism */ > static u32 oxp_mutex; > @@ -87,6 +88,24 @@ static enum oxp_board board; > > #define OXP_TURBO_RETURN_VAL 0x00 /* Common return val */ > > +/* Battery bypass settings */ > +#define EC_CHARGE_CONTROL_BEHAVIOURS_X1 (BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) | \ > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE) | \ > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0)) > + > +#define OXP_X1_CHARGE_LIMIT_REG 0xA3 /* X1 charge limit (%) */ > +#define OXP_X1_CHARGE_BYPASS_REG 0xA4 /* X1 bypass charging */ > + > +#define OXP_X1_CHARGE_BYPASS_MASK_S0 0x01 > +/* > + * Cannot control S3, S5 individually. > + * X1 Mask is 0x0A, OneXFly F1Pro is just 0x02 > + * but the extra bit on the X1 does nothing. > + */ > +#define OXP_X1_CHARGE_BYPASS_MASK_S3S5 0x02 > +#define OXP_X1_CHARGE_BYPASS_MASK_ALWAYS (OXP_X1_CHARGE_BYPASS_MASK_S0 | \ > + OXP_X1_CHARGE_BYPASS_MASK_S3S5) > + > static const struct dmi_system_id dmi_table[] = { > { > .matches = { > @@ -434,6 +453,194 @@ static ssize_t tt_toggle_show(struct device *dev, > > static DEVICE_ATTR_RW(tt_toggle); > > +/* Callbacks for turbo toggle attribute */ This comment is not correct for the section. I think it was a copy/paste? > +static bool charge_behaviour_supported(void) Attribute groups support .is_visible. This blocks invocation from userspace, vice doing it in probe() manually. > +{ > + switch (board) { > + case oxp_x1: > + case oxp_fly: > + return 1; > + default: > + break; > + } > + return 0; > +} > + > +static ssize_t charge_behaviour_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t count) > +{ > + int ret; > + u8 reg; > + long val, s0, always; > + unsigned int available; > + Convention is to order variables in reverse xmas tree, with the longest line first and shortest line last. > + switch (board) { > + case oxp_x1: > + case oxp_fly: > + s0 = OXP_X1_CHARGE_BYPASS_MASK_S0; > + always = OXP_X1_CHARGE_BYPASS_MASK_ALWAYS; > + reg = OXP_X1_CHARGE_BYPASS_REG; > + available = EC_CHARGE_CONTROL_BEHAVIOURS_X1; > + break; > + default: > + return -EINVAL; > + } > + > + ret = power_supply_charge_behaviour_parse(available, buf); > + if (ret < 0) Does ret ever return > 0? I think you can just if (ret) > + return ret; > + > + switch (ret) { > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO: > + val = 0; > + break; > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0: > + val = s0; > + break; > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE: > + val = always; > + break; > + default: > + return -EINVAL; > + } > + > + ret = write_to_ec(reg, val); > + if (ret < 0) if (ret) > + return ret; > + > + return count; > +} > + > +static ssize_t charge_behaviour_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + int ret; > + u8 reg; > + long val, s0, always, sel; > + unsigned int available; > + Reverse xmas tree here too. > + switch (board) { > + case oxp_x1: > + case oxp_fly: > + s0 = OXP_X1_CHARGE_BYPASS_MASK_S0; > + always = OXP_X1_CHARGE_BYPASS_MASK_ALWAYS; > + reg = OXP_X1_CHARGE_BYPASS_REG; > + available = EC_CHARGE_CONTROL_BEHAVIOURS_X1; > + break; > + default: > + return -EINVAL; > + } > + > + ret = read_from_ec(reg, 1, &val); > + if (ret < 0) if (ret) > + return ret; > + > + if ((val & always) == always) > + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; > + else if ((val & s0) == s0) > + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0; > + else > + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; > + > + return power_supply_charge_behaviour_show(dev, available, sel, buf); > +} > + > +static DEVICE_ATTR_RW(charge_behaviour); > + > +static ssize_t charge_control_end_threshold_store(struct device *dev, > + struct device_attribute *attr, const char *buf, > + size_t count) > +{ > + u64 val, reg; > + int ret; > + > + ret = kstrtou64(buf, 10, &val); > + if (ret < 0) if (ret) > + return ret; > + if (val > 100) > + return -EINVAL; > + > + switch (board) { > + case oxp_x1: > + case oxp_fly: > + reg = OXP_X1_CHARGE_LIMIT_REG; > + break; > + default: > + return -EINVAL; > + } > + > + ret = write_to_ec(reg, val); > + if (ret < 0) > + return ret; > + > + return count; > +} > + > +static ssize_t charge_control_end_threshold_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + int ret; > + u8 reg; > + long val; > + Reverse xmas tree here too. > + switch (board) { > + case oxp_x1: > + case oxp_fly: > + reg = OXP_X1_CHARGE_LIMIT_REG; > + break; > + default: > + return -EINVAL; > + } > + > + ret = read_from_ec(reg, 1, &val); > + if (ret < 0) if (ret) Cheers, - Derek > + return ret; > + > + return sysfs_emit(buf, "%ld\n", val); > +} > + > +static DEVICE_ATTR_RW(charge_control_end_threshold); > + > +static int oxp_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + /* OneXPlayer devices only have one battery. */ > + if (strcmp(battery->desc->name, "BAT0") != 0 && > + strcmp(battery->desc->name, "BAT1") != 0 && > + strcmp(battery->desc->name, "BATC") != 0 && > + strcmp(battery->desc->name, "BATT") != 0) > + return -ENODEV; > + > + if (device_create_file(&battery->dev, > + &dev_attr_charge_control_end_threshold)) > + return -ENODEV; > + > + if (device_create_file(&battery->dev, > + &dev_attr_charge_behaviour)) { > + device_remove_file(&battery->dev, > + &dev_attr_charge_control_end_threshold); > + return -ENODEV; > + } > + > + return 0; > +} > + > +static int oxp_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + device_remove_file(&battery->dev, > + &dev_attr_charge_control_end_threshold); > + device_remove_file(&battery->dev, > + &dev_attr_charge_behaviour); > + return 0; > +} > + > +static struct acpi_battery_hook battery_hook = { > + .add_battery = oxp_battery_add, > + .remove_battery = oxp_battery_remove, > + .name = "OneXPlayer Battery", > +}; > + > /* PWM enable/disable functions */ > static int oxp_pwm_enable(void) > { > @@ -716,15 +923,25 @@ static int oxp_platform_probe(struct platform_device *pdev) > hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL, > &oxp_ec_chip_info, NULL); > > + if (charge_behaviour_supported()) > + battery_hook_register(&battery_hook); > + > return PTR_ERR_OR_ZERO(hwdev); > } > > +static void oxp_platform_remove(struct platform_device *device) > +{ > + if (charge_behaviour_supported()) > + battery_hook_unregister(&battery_hook); > +} > + > static struct platform_driver oxp_platform_driver = { > .driver = { > .name = "oxp-platform", > .dev_groups = oxp_ec_groups, > }, > .probe = oxp_platform_probe, > + .remove = oxp_platform_remove, > }; > > static struct platform_device *oxp_platform_device; > -- > 2.48.1 >
On Tue, 11 Mar 2025 at 00:51, Derek John Clark <derekjohn.clark@gmail.com> wrote: > > On Sun, Mar 9, 2025 at 4:21 AM Antheas Kapenekakis <lkml@antheas.dev> wrote: > > > > With the X1 (AMD), OneXPlayer added a charge limit and charge bypass to > > their devices. Charge limit allows for choosing an arbitrary battery > > charge setpoint in percentages. Charge bypass allows to instruct the > > device to stop charging either when it is in s0 or always. > > > > This feature was then extended for the F1Pro as well. OneXPlayer also > > released BIOS updates for the X1 Mini, X1 (Intel), and F1 devices that > > add support for this feature. Therefore, enable it for all F1 and > > X1 devices. > > > > As noted in your previous patch, I think checking for BIOS support is > a wise move here. > > > Add both of these under the standard sysfs battery endpoints for them, > > by looking for the battery. OneXPlayer devices have a single battery. > > > > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> > > --- > > drivers/platform/x86/oxpec.c | 217 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 217 insertions(+) > > > > diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c > > index dc3a0871809c..dd6d333ebcfa 100644 > > --- a/drivers/platform/x86/oxpec.c > > +++ b/drivers/platform/x86/oxpec.c > > @@ -24,6 +24,7 @@ > > #include <linux/module.h> > > #include <linux/platform_device.h> > > #include <linux/processor.h> > > +#include <acpi/battery.h> > > > > /* Handle ACPI lock mechanism */ > > static u32 oxp_mutex; > > @@ -87,6 +88,24 @@ static enum oxp_board board; > > > > #define OXP_TURBO_RETURN_VAL 0x00 /* Common return val */ > > > > +/* Battery bypass settings */ > > +#define EC_CHARGE_CONTROL_BEHAVIOURS_X1 (BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) | \ > > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE) | \ > > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0)) > > + > > +#define OXP_X1_CHARGE_LIMIT_REG 0xA3 /* X1 charge limit (%) */ > > +#define OXP_X1_CHARGE_BYPASS_REG 0xA4 /* X1 bypass charging */ > > + > > +#define OXP_X1_CHARGE_BYPASS_MASK_S0 0x01 > > +/* > > + * Cannot control S3, S5 individually. > > + * X1 Mask is 0x0A, OneXFly F1Pro is just 0x02 > > + * but the extra bit on the X1 does nothing. > > + */ > > +#define OXP_X1_CHARGE_BYPASS_MASK_S3S5 0x02 > > +#define OXP_X1_CHARGE_BYPASS_MASK_ALWAYS (OXP_X1_CHARGE_BYPASS_MASK_S0 | \ > > + OXP_X1_CHARGE_BYPASS_MASK_S3S5) > > + > > static const struct dmi_system_id dmi_table[] = { > > { > > .matches = { > > @@ -434,6 +453,194 @@ static ssize_t tt_toggle_show(struct device *dev, > > > > static DEVICE_ATTR_RW(tt_toggle); > > > > +/* Callbacks for turbo toggle attribute */ > > This comment is not correct for the section. I think it was a copy/paste? > > > +static bool charge_behaviour_supported(void) > > Attribute groups support .is_visible. This blocks invocation from > userspace, vice doing it in probe() manually. Unsure what this means. Instead of using is_visible, I block the battery attachment, which is preferable ATM as all devices that support battery features support all of them. If new devices have additional features not covered by this, we will have to move towards using is_visible. > > +{ > > + switch (board) { > > + case oxp_x1: > > + case oxp_fly: > > + return 1; > > + default: > > + break; > > + } > > + return 0; > > +} > > + > > +static ssize_t charge_behaviour_store(struct device *dev, > > + struct device_attribute *attr, const char *buf, > > + size_t count) > > +{ > > + int ret; > > + u8 reg; > > + long val, s0, always; > > + unsigned int available; > > + > > Convention is to order variables in reverse xmas tree, with the > longest line first and shortest line last. Sure > > + switch (board) { > > + case oxp_x1: > > + case oxp_fly: > > + s0 = OXP_X1_CHARGE_BYPASS_MASK_S0; > > + always = OXP_X1_CHARGE_BYPASS_MASK_ALWAYS; > > + reg = OXP_X1_CHARGE_BYPASS_REG; > > + available = EC_CHARGE_CONTROL_BEHAVIOURS_X1; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + ret = power_supply_charge_behaviour_parse(available, buf); > > + if (ret < 0) > > Does ret ever return > 0? I think you can just if (ret) This is how it is used in other platform drivers. I might be able to indulge you on the others though. > > + return ret; > > + > > + switch (ret) { > > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO: > > + val = 0; > > + break; > > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0: > > + val = s0; > > + break; > > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE: > > + val = always; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + ret = write_to_ec(reg, val); > > + if (ret < 0) > > if (ret) > > > + return ret; > > + > > + return count; > > +} > > + > > +static ssize_t charge_behaviour_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + int ret; > > + u8 reg; > > + long val, s0, always, sel; > > + unsigned int available; > > + > > Reverse xmas tree here too. > > > + switch (board) { > > + case oxp_x1: > > + case oxp_fly: > > + s0 = OXP_X1_CHARGE_BYPASS_MASK_S0; > > + always = OXP_X1_CHARGE_BYPASS_MASK_ALWAYS; > > + reg = OXP_X1_CHARGE_BYPASS_REG; > > + available = EC_CHARGE_CONTROL_BEHAVIOURS_X1; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + ret = read_from_ec(reg, 1, &val); > > + if (ret < 0) > > if (ret) > > > + return ret; > > + > > + if ((val & always) == always) > > + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; > > + else if ((val & s0) == s0) > > + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0; > > + else > > + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; > > + > > + return power_supply_charge_behaviour_show(dev, available, sel, buf); > > +} > > + > > +static DEVICE_ATTR_RW(charge_behaviour); > > + > > +static ssize_t charge_control_end_threshold_store(struct device *dev, > > + struct device_attribute *attr, const char *buf, > > + size_t count) > > +{ > > + u64 val, reg; > > + int ret; > > + > > + ret = kstrtou64(buf, 10, &val); > > + if (ret < 0) > > if (ret) > > > + return ret; > > + if (val > 100) > > + return -EINVAL; > > + > > + switch (board) { > > + case oxp_x1: > > + case oxp_fly: > > + reg = OXP_X1_CHARGE_LIMIT_REG; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + ret = write_to_ec(reg, val); > > + if (ret < 0) > > + return ret; > > + > > + return count; > > +} > > + > > +static ssize_t charge_control_end_threshold_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + int ret; > > + u8 reg; > > + long val; > > + > > Reverse xmas tree here too. > > > + switch (board) { > > + case oxp_x1: > > + case oxp_fly: > > + reg = OXP_X1_CHARGE_LIMIT_REG; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + ret = read_from_ec(reg, 1, &val); > > + if (ret < 0) > > if (ret) > > Cheers, > - Derek > > > + return ret; > > + > > + return sysfs_emit(buf, "%ld\n", val); > > +} > > + > > +static DEVICE_ATTR_RW(charge_control_end_threshold); > > + > > +static int oxp_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook) > > +{ > > + /* OneXPlayer devices only have one battery. */ > > + if (strcmp(battery->desc->name, "BAT0") != 0 && > > + strcmp(battery->desc->name, "BAT1") != 0 && > > + strcmp(battery->desc->name, "BATC") != 0 && > > + strcmp(battery->desc->name, "BATT") != 0) > > + return -ENODEV; > > + > > + if (device_create_file(&battery->dev, > > + &dev_attr_charge_control_end_threshold)) > > + return -ENODEV; > > + > > + if (device_create_file(&battery->dev, > > + &dev_attr_charge_behaviour)) { > > + device_remove_file(&battery->dev, > > + &dev_attr_charge_control_end_threshold); > > + return -ENODEV; > > + } > > + > > + return 0; > > +} > > + > > +static int oxp_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook) > > +{ > > + device_remove_file(&battery->dev, > > + &dev_attr_charge_control_end_threshold); > > + device_remove_file(&battery->dev, > > + &dev_attr_charge_behaviour); > > + return 0; > > +} > > + > > +static struct acpi_battery_hook battery_hook = { > > + .add_battery = oxp_battery_add, > > + .remove_battery = oxp_battery_remove, > > + .name = "OneXPlayer Battery", > > +}; > > + > > /* PWM enable/disable functions */ > > static int oxp_pwm_enable(void) > > { > > @@ -716,15 +923,25 @@ static int oxp_platform_probe(struct platform_device *pdev) > > hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL, > > &oxp_ec_chip_info, NULL); > > > > + if (charge_behaviour_supported()) > > + battery_hook_register(&battery_hook); > > + > > return PTR_ERR_OR_ZERO(hwdev); > > } > > > > +static void oxp_platform_remove(struct platform_device *device) > > +{ > > + if (charge_behaviour_supported()) > > + battery_hook_unregister(&battery_hook); > > +} > > + > > static struct platform_driver oxp_platform_driver = { > > .driver = { > > .name = "oxp-platform", > > .dev_groups = oxp_ec_groups, > > }, > > .probe = oxp_platform_probe, > > + .remove = oxp_platform_remove, > > }; > > > > static struct platform_device *oxp_platform_device; > > -- > > 2.48.1 > >
Hi Antheas, kernel test robot noticed the following build errors: [auto build test ERROR on groeck-staging/hwmon-next] [also build test ERROR on sre-power-supply/for-next amd-pstate/linux-next amd-pstate/bleeding-edge rafael-pm/linux-next rafael-pm/bleeding-edge linus/master v6.14-rc6 next-20250307] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Antheas-Kapenekakis/hwmon-oxp-sensors-Distinguish-the-X1-variants/20250309-192300 base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next patch link: https://lore.kernel.org/r/20250309112114.1177361-7-lkml%40antheas.dev patch subject: [PATCH v3 06/12] platform/x86: oxpec: Add charge threshold and behaviour to OneXPlayer config: x86_64-randconfig-074-20250311 (https://download.01.org/0day-ci/archive/20250311/202503112130.dl6b3XVs-lkp@intel.com/config) compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250311/202503112130.dl6b3XVs-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202503112130.dl6b3XVs-lkp@intel.com/ All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: battery_hook_register >>> referenced by oxpec.c:927 (drivers/platform/x86/oxpec.c:927) >>> vmlinux.o:(oxp_platform_probe) -- >> ld.lld: error: undefined symbol: battery_hook_unregister >>> referenced by oxpec.c:935 (drivers/platform/x86/oxpec.c:935) >>> vmlinux.o:(oxp_platform_remove)
diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c index dc3a0871809c..dd6d333ebcfa 100644 --- a/drivers/platform/x86/oxpec.c +++ b/drivers/platform/x86/oxpec.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/processor.h> +#include <acpi/battery.h> /* Handle ACPI lock mechanism */ static u32 oxp_mutex; @@ -87,6 +88,24 @@ static enum oxp_board board; #define OXP_TURBO_RETURN_VAL 0x00 /* Common return val */ +/* Battery bypass settings */ +#define EC_CHARGE_CONTROL_BEHAVIOURS_X1 (BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) | \ + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE) | \ + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0)) + +#define OXP_X1_CHARGE_LIMIT_REG 0xA3 /* X1 charge limit (%) */ +#define OXP_X1_CHARGE_BYPASS_REG 0xA4 /* X1 bypass charging */ + +#define OXP_X1_CHARGE_BYPASS_MASK_S0 0x01 +/* + * Cannot control S3, S5 individually. + * X1 Mask is 0x0A, OneXFly F1Pro is just 0x02 + * but the extra bit on the X1 does nothing. + */ +#define OXP_X1_CHARGE_BYPASS_MASK_S3S5 0x02 +#define OXP_X1_CHARGE_BYPASS_MASK_ALWAYS (OXP_X1_CHARGE_BYPASS_MASK_S0 | \ + OXP_X1_CHARGE_BYPASS_MASK_S3S5) + static const struct dmi_system_id dmi_table[] = { { .matches = { @@ -434,6 +453,194 @@ static ssize_t tt_toggle_show(struct device *dev, static DEVICE_ATTR_RW(tt_toggle); +/* Callbacks for turbo toggle attribute */ +static bool charge_behaviour_supported(void) +{ + switch (board) { + case oxp_x1: + case oxp_fly: + return 1; + default: + break; + } + return 0; +} + +static ssize_t charge_behaviour_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + int ret; + u8 reg; + long val, s0, always; + unsigned int available; + + switch (board) { + case oxp_x1: + case oxp_fly: + s0 = OXP_X1_CHARGE_BYPASS_MASK_S0; + always = OXP_X1_CHARGE_BYPASS_MASK_ALWAYS; + reg = OXP_X1_CHARGE_BYPASS_REG; + available = EC_CHARGE_CONTROL_BEHAVIOURS_X1; + break; + default: + return -EINVAL; + } + + ret = power_supply_charge_behaviour_parse(available, buf); + if (ret < 0) + return ret; + + switch (ret) { + case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO: + val = 0; + break; + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0: + val = s0; + break; + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE: + val = always; + break; + default: + return -EINVAL; + } + + ret = write_to_ec(reg, val); + if (ret < 0) + return ret; + + return count; +} + +static ssize_t charge_behaviour_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int ret; + u8 reg; + long val, s0, always, sel; + unsigned int available; + + switch (board) { + case oxp_x1: + case oxp_fly: + s0 = OXP_X1_CHARGE_BYPASS_MASK_S0; + always = OXP_X1_CHARGE_BYPASS_MASK_ALWAYS; + reg = OXP_X1_CHARGE_BYPASS_REG; + available = EC_CHARGE_CONTROL_BEHAVIOURS_X1; + break; + default: + return -EINVAL; + } + + ret = read_from_ec(reg, 1, &val); + if (ret < 0) + return ret; + + if ((val & always) == always) + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; + else if ((val & s0) == s0) + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_S0; + else + sel = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; + + return power_supply_charge_behaviour_show(dev, available, sel, buf); +} + +static DEVICE_ATTR_RW(charge_behaviour); + +static ssize_t charge_control_end_threshold_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + u64 val, reg; + int ret; + + ret = kstrtou64(buf, 10, &val); + if (ret < 0) + return ret; + + if (val > 100) + return -EINVAL; + + switch (board) { + case oxp_x1: + case oxp_fly: + reg = OXP_X1_CHARGE_LIMIT_REG; + break; + default: + return -EINVAL; + } + + ret = write_to_ec(reg, val); + if (ret < 0) + return ret; + + return count; +} + +static ssize_t charge_control_end_threshold_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int ret; + u8 reg; + long val; + + switch (board) { + case oxp_x1: + case oxp_fly: + reg = OXP_X1_CHARGE_LIMIT_REG; + break; + default: + return -EINVAL; + } + + ret = read_from_ec(reg, 1, &val); + if (ret < 0) + return ret; + + return sysfs_emit(buf, "%ld\n", val); +} + +static DEVICE_ATTR_RW(charge_control_end_threshold); + +static int oxp_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook) +{ + /* OneXPlayer devices only have one battery. */ + if (strcmp(battery->desc->name, "BAT0") != 0 && + strcmp(battery->desc->name, "BAT1") != 0 && + strcmp(battery->desc->name, "BATC") != 0 && + strcmp(battery->desc->name, "BATT") != 0) + return -ENODEV; + + if (device_create_file(&battery->dev, + &dev_attr_charge_control_end_threshold)) + return -ENODEV; + + if (device_create_file(&battery->dev, + &dev_attr_charge_behaviour)) { + device_remove_file(&battery->dev, + &dev_attr_charge_control_end_threshold); + return -ENODEV; + } + + return 0; +} + +static int oxp_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook) +{ + device_remove_file(&battery->dev, + &dev_attr_charge_control_end_threshold); + device_remove_file(&battery->dev, + &dev_attr_charge_behaviour); + return 0; +} + +static struct acpi_battery_hook battery_hook = { + .add_battery = oxp_battery_add, + .remove_battery = oxp_battery_remove, + .name = "OneXPlayer Battery", +}; + /* PWM enable/disable functions */ static int oxp_pwm_enable(void) { @@ -716,15 +923,25 @@ static int oxp_platform_probe(struct platform_device *pdev) hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL, &oxp_ec_chip_info, NULL); + if (charge_behaviour_supported()) + battery_hook_register(&battery_hook); + return PTR_ERR_OR_ZERO(hwdev); } +static void oxp_platform_remove(struct platform_device *device) +{ + if (charge_behaviour_supported()) + battery_hook_unregister(&battery_hook); +} + static struct platform_driver oxp_platform_driver = { .driver = { .name = "oxp-platform", .dev_groups = oxp_ec_groups, }, .probe = oxp_platform_probe, + .remove = oxp_platform_remove, }; static struct platform_device *oxp_platform_device;
With the X1 (AMD), OneXPlayer added a charge limit and charge bypass to their devices. Charge limit allows for choosing an arbitrary battery charge setpoint in percentages. Charge bypass allows to instruct the device to stop charging either when it is in s0 or always. This feature was then extended for the F1Pro as well. OneXPlayer also released BIOS updates for the X1 Mini, X1 (Intel), and F1 devices that add support for this feature. Therefore, enable it for all F1 and X1 devices. Add both of these under the standard sysfs battery endpoints for them, by looking for the battery. OneXPlayer devices have a single battery. Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> --- drivers/platform/x86/oxpec.c | 217 +++++++++++++++++++++++++++++++++++ 1 file changed, 217 insertions(+)