| Message ID | 20250322103606.680401-15-lkml@antheas.dev |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: (oxpsensors) Add devices, features, fix ABI and move to platform/x86 | expand |
On Sat, 22 Mar 2025, Antheas Kapenekakis wrote: > With the X1 (AMD), OneXPlayer added a charge limit and charge inhibit > feature to their devices. Charge limit allows for choosing an arbitrary > battery charge setpoint in percentages. Charge ihibit allows to instruct > the device to stop charging either when it is awake 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. > > Reviewed-by: Thomas Weißschuh <linux@weissschuh.net> > Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev> > --- > drivers/platform/x86/Kconfig | 1 + > drivers/platform/x86/oxpec.c | 160 ++++++++++++++++++++++++++++++++++- > 2 files changed, 157 insertions(+), 4 deletions(-) > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 82cfc76bc5c9f..f4d993658c01f 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1189,6 +1189,7 @@ config SEL3350_PLATFORM > config OXP_EC > tristate "OneXPlayer EC platform control" > depends on ACPI_EC > + depends on ACPI_BATTERY > depends on HWMON > depends on X86 > help > diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c > index 0a5b33a842b24..e6ea702006ec5 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; > @@ -60,6 +61,7 @@ enum oxp_board { > }; > > static enum oxp_board board; > +static struct device *oxp_dev; > > /* Fan reading and PWM */ > #define OXP_SENSOR_FAN_REG 0x76 /* Fan reading is 2 registers long */ > @@ -93,6 +95,23 @@ static enum oxp_board board; > #define OXP_X1_TURBO_LED_OFF 0x01 > #define OXP_X1_TURBO_LED_ON 0x02 > > +/* Battery extension settings */ > +#define EC_CHARGE_CONTROL_BEHAVIOURS (BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) | \ > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE) | \ > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_AWAKE)) > + > +#define OXP_X1_CHARGE_LIMIT_REG 0xA3 /* X1 charge limit (%) */ > +#define OXP_X1_CHARGE_INHIBIT_REG 0xA4 /* X1 bypass charging */ > + > +#define OXP_X1_CHARGE_INHIBIT_MASK_AWAKE 0x01 > +/* > + * X1 Mask is 0x0A, OneXFly F1Pro is just 0x02 > + * but the extra bit on the X1 does nothing. > + */ > +#define OXP_X1_CHARGE_INHIBIT_MASK_OFF 0x02 > +#define OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS (OXP_X1_CHARGE_INHIBIT_MASK_AWAKE | \ > + OXP_X1_CHARGE_INHIBIT_MASK_OFF) > + > static const struct dmi_system_id dmi_table[] = { > { > .matches = { > @@ -507,6 +526,129 @@ static ssize_t tt_led_show(struct device *dev, > > static DEVICE_ATTR_RW(tt_led); > > +/* Callbacks for charge behaviour attributes */ > +static bool oxp_psy_ext_supported(void) > +{ > + switch (board) { > + case oxp_x1: > + case oxp_fly: > + return true; > + default: > + break; > + } > + return false; > +} > + > +static int oxp_psy_ext_get_prop(struct power_supply *psy, > + const struct power_supply_ext *ext, > + void *data, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + long raw_val; > + int ret; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: > + ret = read_from_ec(OXP_X1_CHARGE_LIMIT_REG, 1, &raw_val); > + if (ret) > + return ret; > + if (raw_val >= 0 && raw_val > 100) Is one of the signs wrong way around?? > + return -EINVAL; > + val->intval = raw_val; > + return 0; > + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: > + ret = read_from_ec(OXP_X1_CHARGE_INHIBIT_REG, 1, &raw_val); > + if (ret) > + return ret; > + if ((raw_val & OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS) == > + OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS) > + val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; > + else if ((raw_val & OXP_X1_CHARGE_INHIBIT_MASK_AWAKE) == > + OXP_X1_CHARGE_INHIBIT_MASK_AWAKE) > + val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_AWAKE; > + else > + val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; > + return 0; > + default: > + return -EINVAL; > + } > +} > + > +static int oxp_psy_ext_set_prop(struct power_supply *psy, > + const struct power_supply_ext *ext, > + void *data, > + enum power_supply_property psp, > + const union power_supply_propval *val) > +{ > + long raw_val; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: > + if (val->intval > 100) > + return -EINVAL; > + return write_to_ec(OXP_X1_CHARGE_LIMIT_REG, val->intval); > + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: > + switch (val->intval) { > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO: > + raw_val = 0; > + break; > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_AWAKE: > + raw_val = OXP_X1_CHARGE_INHIBIT_MASK_AWAKE; > + break; > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE: > + raw_val = OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS; > + break; > + default: > + return -EINVAL; > + } > + > + return write_to_ec(OXP_X1_CHARGE_INHIBIT_REG, raw_val); > + default: > + return -EINVAL; > + } > +} > + > +static int oxp_psy_prop_is_writeable(struct power_supply *psy, > + const struct power_supply_ext *ext, > + void *data, > + enum power_supply_property psp) > +{ > + return true; > +} > + > +static const enum power_supply_property oxp_psy_ext_props[] = { > + POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR, > + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, > +}; > + > +static const struct power_supply_ext oxp_psy_ext = { > + .name = "oxp-charge-control", > + .properties = oxp_psy_ext_props, > + .num_properties = ARRAY_SIZE(oxp_psy_ext_props), > + .charge_behaviours = EC_CHARGE_CONTROL_BEHAVIOURS, > + .get_property = oxp_psy_ext_get_prop, > + .set_property = oxp_psy_ext_set_prop, > + .property_is_writeable = oxp_psy_prop_is_writeable, > +}; > + > +static int oxp_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + return power_supply_register_extension(battery, &oxp_psy_ext, oxp_dev, NULL); > +} > + > +static int oxp_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + power_supply_unregister_extension(battery, &oxp_psy_ext); > + return 0; > +} > + > +static struct acpi_battery_hook battery_hook = { > + .add_battery = oxp_add_battery, > + .remove_battery = oxp_remove_battery, > + .name = "OneXPlayer Battery", > +}; > + > /* PWM enable/disable functions */ > static int oxp_pwm_enable(void) > { > @@ -845,12 +987,22 @@ static const struct hwmon_chip_info oxp_ec_chip_info = { > static int oxp_platform_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > - struct device *hwdev; > + int ret; > + > + oxp_dev = dev; > + ret = PTR_ERR_OR_ZERO(devm_hwmon_device_register_with_info( > + dev, "oxp_ec", NULL, &oxp_ec_chip_info, NULL)); > > - hwdev = devm_hwmon_device_register_with_info(dev, "oxp_ec", NULL, > - &oxp_ec_chip_info, NULL); > + if (ret) Using PTR_ERR_OR_ZERO() isn't IMO good change as it's meant for returning 0. Just preserve hwdev as it was and use IS_ERR(hwdev) here. > + return ret; > > - return PTR_ERR_OR_ZERO(hwdev); > + if (oxp_psy_ext_supported()) { > + ret = devm_battery_hook_register(dev, &battery_hook); > + if (ret) > + return ret; > + } > + > + return 0; > } > > static struct platform_driver oxp_platform_driver = { >
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 82cfc76bc5c9f..f4d993658c01f 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1189,6 +1189,7 @@ config SEL3350_PLATFORM config OXP_EC tristate "OneXPlayer EC platform control" depends on ACPI_EC + depends on ACPI_BATTERY depends on HWMON depends on X86 help diff --git a/drivers/platform/x86/oxpec.c b/drivers/platform/x86/oxpec.c index 0a5b33a842b24..e6ea702006ec5 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; @@ -60,6 +61,7 @@ enum oxp_board { }; static enum oxp_board board; +static struct device *oxp_dev; /* Fan reading and PWM */ #define OXP_SENSOR_FAN_REG 0x76 /* Fan reading is 2 registers long */ @@ -93,6 +95,23 @@ static enum oxp_board board; #define OXP_X1_TURBO_LED_OFF 0x01 #define OXP_X1_TURBO_LED_ON 0x02 +/* Battery extension settings */ +#define EC_CHARGE_CONTROL_BEHAVIOURS (BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) | \ + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE) | \ + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_AWAKE)) + +#define OXP_X1_CHARGE_LIMIT_REG 0xA3 /* X1 charge limit (%) */ +#define OXP_X1_CHARGE_INHIBIT_REG 0xA4 /* X1 bypass charging */ + +#define OXP_X1_CHARGE_INHIBIT_MASK_AWAKE 0x01 +/* + * X1 Mask is 0x0A, OneXFly F1Pro is just 0x02 + * but the extra bit on the X1 does nothing. + */ +#define OXP_X1_CHARGE_INHIBIT_MASK_OFF 0x02 +#define OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS (OXP_X1_CHARGE_INHIBIT_MASK_AWAKE | \ + OXP_X1_CHARGE_INHIBIT_MASK_OFF) + static const struct dmi_system_id dmi_table[] = { { .matches = { @@ -507,6 +526,129 @@ static ssize_t tt_led_show(struct device *dev, static DEVICE_ATTR_RW(tt_led); +/* Callbacks for charge behaviour attributes */ +static bool oxp_psy_ext_supported(void) +{ + switch (board) { + case oxp_x1: + case oxp_fly: + return true; + default: + break; + } + return false; +} + +static int oxp_psy_ext_get_prop(struct power_supply *psy, + const struct power_supply_ext *ext, + void *data, + enum power_supply_property psp, + union power_supply_propval *val) +{ + long raw_val; + int ret; + + switch (psp) { + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: + ret = read_from_ec(OXP_X1_CHARGE_LIMIT_REG, 1, &raw_val); + if (ret) + return ret; + if (raw_val >= 0 && raw_val > 100) + return -EINVAL; + val->intval = raw_val; + return 0; + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: + ret = read_from_ec(OXP_X1_CHARGE_INHIBIT_REG, 1, &raw_val); + if (ret) + return ret; + if ((raw_val & OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS) == + OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS) + val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; + else if ((raw_val & OXP_X1_CHARGE_INHIBIT_MASK_AWAKE) == + OXP_X1_CHARGE_INHIBIT_MASK_AWAKE) + val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_AWAKE; + else + val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; + return 0; + default: + return -EINVAL; + } +} + +static int oxp_psy_ext_set_prop(struct power_supply *psy, + const struct power_supply_ext *ext, + void *data, + enum power_supply_property psp, + const union power_supply_propval *val) +{ + long raw_val; + + switch (psp) { + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: + if (val->intval > 100) + return -EINVAL; + return write_to_ec(OXP_X1_CHARGE_LIMIT_REG, val->intval); + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: + switch (val->intval) { + case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO: + raw_val = 0; + break; + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE_AWAKE: + raw_val = OXP_X1_CHARGE_INHIBIT_MASK_AWAKE; + break; + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE: + raw_val = OXP_X1_CHARGE_INHIBIT_MASK_ALWAYS; + break; + default: + return -EINVAL; + } + + return write_to_ec(OXP_X1_CHARGE_INHIBIT_REG, raw_val); + default: + return -EINVAL; + } +} + +static int oxp_psy_prop_is_writeable(struct power_supply *psy, + const struct power_supply_ext *ext, + void *data, + enum power_supply_property psp) +{ + return true; +} + +static const enum power_supply_property oxp_psy_ext_props[] = { + POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR, + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, +}; + +static const struct power_supply_ext oxp_psy_ext = { + .name = "oxp-charge-control", + .properties = oxp_psy_ext_props, + .num_properties = ARRAY_SIZE(oxp_psy_ext_props), + .charge_behaviours = EC_CHARGE_CONTROL_BEHAVIOURS, + .get_property = oxp_psy_ext_get_prop, + .set_property = oxp_psy_ext_set_prop, + .property_is_writeable = oxp_psy_prop_is_writeable, +}; + +static int oxp_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook) +{ + return power_supply_register_extension(battery, &oxp_psy_ext, oxp_dev, NULL); +} + +static int oxp_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook) +{ + power_supply_unregister_extension(battery, &oxp_psy_ext); + return 0; +} + +static struct acpi_battery_hook battery_hook = { + .add_battery = oxp_add_battery, + .remove_battery = oxp_remove_battery, + .name = "OneXPlayer Battery", +}; + /* PWM enable/disable functions */ static int oxp_pwm_enable(void) { @@ -845,12 +987,22 @@ static const struct hwmon_chip_info oxp_ec_chip_info = { static int oxp_platform_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; - struct device *hwdev; + int ret; + + oxp_dev = dev; + ret = PTR_ERR_OR_ZERO(devm_hwmon_device_register_with_info( + dev, "oxp_ec", NULL, &oxp_ec_chip_info, NULL)); - hwdev = devm_hwmon_device_register_with_info(dev, "oxp_ec", NULL, - &oxp_ec_chip_info, NULL); + if (ret) + return ret; - return PTR_ERR_OR_ZERO(hwdev); + if (oxp_psy_ext_supported()) { + ret = devm_battery_hook_register(dev, &battery_hook); + if (ret) + return ret; + } + + return 0; } static struct platform_driver oxp_platform_driver = {