[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <D9TQ460OHUEU.LEUNQX49TLS3@gmail.com>
Date: Sun, 11 May 2025 20:34:50 -0300
From: "Kurt Borja" <kuurtb@...il.com>
To: "Antheas Kapenekakis" <lkml@...heas.dev>,
<platform-driver-x86@...r.kernel.org>
Cc: "Armin Wolf" <W_Armin@....de>, "Jonathan Corbet" <corbet@....net>, "Hans
de Goede" <hdegoede@...hat.com>, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com>, "Jean Delvare" <jdelvare@...e.com>,
"Guenter Roeck" <linux@...ck-us.net>, <linux-doc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-hwmon@...r.kernel.org>
Subject: Re: [PATCH v1 07/10] platform/x86: msi-wmi-platform: Add
charge_threshold support
On Sun May 11, 2025 at 5:44 PM -03, Antheas Kapenekakis wrote:
> The battery of MSI laptops supports charge threshold. Add support for it.
>
> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> ---
> drivers/platform/x86/Kconfig | 1 +
> drivers/platform/x86/msi-wmi-platform.c | 110 ++++++++++++++++++++++++
> 2 files changed, 111 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index fd3da718731e7..51a34ab476ffc 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -744,6 +744,7 @@ config MSI_WMI
>
> config MSI_WMI_PLATFORM
> tristate "MSI WMI Platform features"
> + depends on ACPI_BATTERY
> depends on ACPI_WMI
> depends on HWMON
> select ACPI_PLATFORM_PROFILE
> diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
> index 6498f4b44fe53..46928fb4da8a6 100644
> --- a/drivers/platform/x86/msi-wmi-platform.c
> +++ b/drivers/platform/x86/msi-wmi-platform.c
> @@ -31,6 +31,7 @@
> #include <linux/sysfs.h>
> #include <linux/types.h>
> #include <linux/wmi.h>
> +#include <acpi/battery.h>
>
> #include <linux/unaligned.h>
>
> @@ -79,6 +80,7 @@
> /* Get_Data() and Set_Data() Params */
> #define MSI_PLATFORM_PL1_ADDR 0x50
> #define MSI_PLATFORM_PL2_ADDR 0x51
> +#define MSI_PLATFORM_BAT_ADDR 0xd7
>
> static bool force;
> module_param_unsafe(force, bool, 0);
> @@ -118,6 +120,7 @@ enum msi_wmi_platform_method {
>
> struct msi_wmi_platform_quirk {
> bool shift_mode; /* Shift mode is supported */
> + bool charge_threshold; /* Charge threshold is supported */
> int pl_min; /* Minimum PLx value */
> int pl1_max; /* Maximum PL1 value */
> int pl2_max; /* Maximum PL2 value */
> @@ -128,6 +131,7 @@ struct msi_wmi_platform_data {
> struct msi_wmi_platform_quirk *quirks;
> struct mutex wmi_lock; /* Necessary when calling WMI methods */
> struct device *ppdev;
> + struct acpi_battery_hook battery_hook;
> struct device *fw_attrs_dev;
> struct kset *fw_attrs_kset;
> };
> @@ -211,12 +215,14 @@ static const char * const msi_wmi_platform_debugfs_names[] = {
> static struct msi_wmi_platform_quirk quirk_default = {};
> static struct msi_wmi_platform_quirk quirk_gen1 = {
> .shift_mode = true,
> + .charge_threshold = true,
> .pl_min = 8,
> .pl1_max = 43,
> .pl2_max = 45
> };
> static struct msi_wmi_platform_quirk quirk_gen2 = {
> .shift_mode = true,
> + .charge_threshold = true,
> .pl_min = 8,
> .pl1_max = 30,
> .pl2_max = 37
> @@ -1013,6 +1019,94 @@ static int msi_wmi_fw_attrs_init(struct msi_wmi_platform_data *data)
> return 0;
> }
>
> +static int msi_platform_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)
> +{
> + struct msi_wmi_platform_data *msi = data;
> + u8 buffer[32] = { 0 };
> + int ret;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> + buffer[0] = MSI_PLATFORM_BAT_ADDR;
> + ret = msi_wmi_platform_query(msi, MSI_PLATFORM_GET_DATA,
> + buffer, sizeof(buffer));
> + if (ret)
> + return ret;
> +
> + val->intval = buffer[1] & ~BIT(7);
I think BIT(7) should be a named define.
> + if (val->intval > 100)
> + return -EINVAL;
> +
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int msi_platform_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)
> +{
> + struct msi_wmi_platform_data *msi = data;
> + u8 buffer[32] = { 0 };
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> + if (val->intval > 100)
> + return -EINVAL;
> + buffer[0] = MSI_PLATFORM_BAT_ADDR;
> + buffer[1] = val->intval | BIT(7);
> + return msi_wmi_platform_query(msi, MSI_PLATFORM_SET_DATA,
> + buffer, sizeof(buffer));
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int
> +msi_platform_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[] = {
Maybe s/oxp/msi/ ?
--
~ Kurt
> + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> +};
> +
> +static const struct power_supply_ext msi_platform_psy_ext = {
> + .name = "msi-platform-charge-control",
> + .properties = oxp_psy_ext_props,
> + .num_properties = ARRAY_SIZE(oxp_psy_ext_props),
> + .get_property = msi_platform_psy_ext_get_prop,
> + .set_property = msi_platform_psy_ext_set_prop,
> + .property_is_writeable = msi_platform_psy_prop_is_writeable,
> +};
> +
> +static int msi_wmi_platform_battery_add(struct power_supply *battery,
> + struct acpi_battery_hook *hook)
> +{
> + struct msi_wmi_platform_data *data =
> + container_of(hook, struct msi_wmi_platform_data, battery_hook);
> +
> + return power_supply_register_extension(battery, &msi_platform_psy_ext,
> + &data->wdev->dev, data);
> +}
> +
> +static int msi_wmi_platform_battery_remove(struct power_supply *battery,
> + struct acpi_battery_hook *hook)
> +{
> + power_supply_unregister_extension(battery, &msi_platform_psy_ext);
> + return 0;
> +}
> +
> static ssize_t msi_wmi_platform_debugfs_write(struct file *fp, const char __user *input,
> size_t length, loff_t *offset)
> {
> @@ -1245,6 +1339,13 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
> if (ret < 0)
> return ret;
>
> + if (data->quirks->charge_threshold) {
> + data->battery_hook.name = "MSI Battery";
> + data->battery_hook.add_battery = msi_wmi_platform_battery_add;
> + data->battery_hook.remove_battery = msi_wmi_platform_battery_remove;
> + battery_hook_register(&data->battery_hook);
> + }
> +
> msi_wmi_platform_debugfs_init(data);
>
> msi_wmi_platform_profile_setup(data);
> @@ -1252,6 +1353,14 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
> return msi_wmi_platform_hwmon_init(data);
> }
>
> +static void msi_wmi_platform_remove(struct wmi_device *wdev)
> +{
> + struct msi_wmi_platform_data *data = dev_get_drvdata(&wdev->dev);
> +
> + if (data->quirks->charge_threshold)
> + battery_hook_unregister(&data->battery_hook);
> +}
> +
> static const struct wmi_device_id msi_wmi_platform_id_table[] = {
> { MSI_PLATFORM_GUID, NULL },
> { }
> @@ -1265,6 +1374,7 @@ static struct wmi_driver msi_wmi_platform_driver = {
> },
> .id_table = msi_wmi_platform_id_table,
> .probe = msi_wmi_platform_probe,
> + .remove = msi_wmi_platform_remove,
> .no_singleton = true,
> };
> module_wmi_driver(msi_wmi_platform_driver);
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists