diff mbox series

[v4,06/13] platform/x86: oxpec: Add charge threshold and behaviour to OneXPlayer

Message ID 20250311165406.331046-7-lkml@antheas.dev (mailing list archive)
State Changes Requested, archived
Headers show
Series hwmon: (oxpsensors) Add devices, features, fix ABI and move to platform/x86 | expand

Commit Message

Antheas Kapenekakis March 11, 2025, 4:53 p.m. UTC
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/Kconfig |   1 +
 drivers/platform/x86/oxpec.c | 217 +++++++++++++++++++++++++++++++++++
 2 files changed, 218 insertions(+)

Comments

Hans de Goede March 17, 2025, 12:32 p.m. UTC | #1
Hi,

On 11-Mar-25 17:53, Antheas Kapenekakis 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.

Again please don't use the word bypass, use inhibit instead.

> 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/Kconfig |   1 +
>  drivers/platform/x86/oxpec.c | 217 +++++++++++++++++++++++++++++++++++
>  2 files changed, 218 insertions(+)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 82cfc76bc5c9..f4d993658c01 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 dc3a0871809c..d73a10598d8f 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

Again avoid the word BYPASS please, if OneXPlayer are calling this bypass in their 
own documentation maybe add a note here when defining the registers that OneXPlayer
calls this bypass and then use inhibit from there on.

> +/*
> + * 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

Ok, so suspend is treated as off, but that is for S3 suspend, what about
s2idle, or does this hw not do s2idle ?

> +#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 charge behaviour attributes */
> +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)
> +{
> +	unsigned int available;
> +	long val, s0, always;
> +	int ret;
> +	u8 reg;
> +
> +	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;

Since these are always the same this does not seem useful, please
use the defines directly below.

> +	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)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t charge_behaviour_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	long val, s0, always, sel;
> +	unsigned int available;
> +	int ret;
> +	u8 reg;
> +
> +	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)
> +		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)
> +		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)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t charge_control_end_threshold_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	long val;
> +	int ret;
> +	u8 reg;
> +
> +	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)
> +		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",
> +};
> +

Since this is new code it should use the new power-supply extension support instead
of the old battery_hook mechanism:

https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6037802bbae892f3ad0c7b4c4faee39b967e32b0



>  /* 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;

Regards,

Hans
Antheas Kapenekakis March 17, 2025, 12:40 p.m. UTC | #2
On Mon, 17 Mar 2025 at 13:33, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 11-Mar-25 17:53, Antheas Kapenekakis 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.
>
> Again please don't use the word bypass, use inhibit instead.
>
> > 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/Kconfig |   1 +
> >  drivers/platform/x86/oxpec.c | 217 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 218 insertions(+)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 82cfc76bc5c9..f4d993658c01 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 dc3a0871809c..d73a10598d8f 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
>
> Again avoid the word BYPASS please, if OneXPlayer are calling this bypass in their
> own documentation maybe add a note here when defining the registers that OneXPlayer
> calls this bypass and then use inhibit from there on.

Sure, I can do that.

> > +/*
> > + * 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
>
> Ok, so suspend is treated as off, but that is for S3 suspend, what about
> s2idle, or does this hw not do s2idle ?

I will change it to OFF. Sorry, s3 is a typo it should be S4.

> > +#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 charge behaviour attributes */
> > +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)
> > +{
> > +     unsigned int available;
> > +     long val, s0, always;
> > +     int ret;
> > +     u8 reg;
> > +
> > +     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;
>
> Since these are always the same this does not seem useful, please
> use the defines directly below.

Ack

> > +     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)
> > +             return ret;
> > +
> > +     return count;
> > +}
> > +
> > +static ssize_t charge_behaviour_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +     long val, s0, always, sel;
> > +     unsigned int available;
> > +     int ret;
> > +     u8 reg;
> > +
> > +     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)
> > +             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)
> > +             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)
> > +             return ret;
> > +
> > +     return count;
> > +}
> > +
> > +static ssize_t charge_control_end_threshold_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +     long val;
> > +     int ret;
> > +     u8 reg;
> > +
> > +     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)
> > +             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",
> > +};
> > +
>
> Since this is new code it should use the new power-supply extension support instead
> of the old battery_hook mechanism:
>
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6037802bbae892f3ad0c7b4c4faee39b967e32b0
>

Ack

>
> >  /* 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;
>
> Regards,
>
> Hans
>
>
diff mbox series

Patch

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 82cfc76bc5c9..f4d993658c01 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 dc3a0871809c..d73a10598d8f 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 charge behaviour attributes */
+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)
+{
+	unsigned int available;
+	long val, s0, always;
+	int ret;
+	u8 reg;
+
+	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)
+		return ret;
+
+	return count;
+}
+
+static ssize_t charge_behaviour_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	long val, s0, always, sel;
+	unsigned int available;
+	int ret;
+	u8 reg;
+
+	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)
+		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)
+		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)
+		return ret;
+
+	return count;
+}
+
+static ssize_t charge_control_end_threshold_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	long val;
+	int ret;
+	u8 reg;
+
+	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)
+		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;