[PATCH] power: supply: cros_charge-control: restore EC state on resume

[PATCH] power: supply: cros_charge-control: restore EC state on resume

[Andrew Pam]

The EC resets its charge control configuration when the AP powers off,
so configured sustainer thresholds and charge behaviour are lost across
hibernation.

Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
cros_charge-control: adopt EC charge state on probe") made the driver
initialise its cached state from the EC, but it does so only on probe,
and a resume is not a probe. Sysfs therefore keeps reporting the
configured values while the EC charges the battery to full. On a
Framework Laptop 16 (Ryzen AI 300, charge control command version 3)
with charge_control_end_threshold set to 80, a hibernate and resume
cycle leaves the attribute still reading 80 while the battery charges
past it to 100%.

Add a system sleep resume hook that reprograms the EC from the cached
state. The same EC behaviour and the same fix already exist for another
driver, see commit 2643187ccb86 ("platform/x86: ayaneo-ec: Add suspend
hook").

Reprogramming the EC is only strictly required after hibernation, but it
is cheap and idempotent, so hook every resume path rather than rely on
the EC preserving its state across each kind of sleep transition.

Note that re-reading the state from the EC on resume, by reusing
cros_chctl_init_state() or otherwise, would be wrong even on the command
versions that support GET: the EC has been reset to its defaults by the
time the hook runs, so adopting that state would silently discard the
configuration the user asked for. The driver's cached state is the only
remaining record of it, which is why it is pushed to the EC rather than
refreshed from it.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Andrew Pam <andrew@fastmailteam.com>
---
 drivers/power/supply/cros_charge-control.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/power/supply/cros_charge-control.c b/drivers/power/supply/cros_charge-control.c
index 07bcc6a9aeef..77023dcd1851 100644
--- a/drivers/power/supply/cros_charge-control.c
+++ b/drivers/power/supply/cros_charge-control.c
@@ -13,6 +13,7 @@
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
 #include <linux/types.h>
 
 #define EC_CHARGE_CONTROL_BEHAVIOURS	(BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)             | \
@@ -420,6 +421,7 @@ static int cros_chctl_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 	priv->cros_ec = cros_ec;
+	platform_set_drvdata(pdev, priv);
 
 	if (priv->cmd_version == 1)
 		priv->psy_ext = &cros_chctl_psy_ext_v1;
@@ -439,6 +441,23 @@ static int cros_chctl_probe(struct platform_device *pdev)
 	return devm_battery_hook_register(dev, &priv->battery_hook);
 }
 
+static int cros_chctl_resume(struct device *dev)
+{
+	struct cros_chctl_priv *priv = dev_get_drvdata(dev);
+
+	/*
+	 * The EC resets its charge control state when the AP powers off, so the
+	 * configuration is lost across hibernation. As the driver never reads
+	 * the state back from the EC, the values exposed to userspace would
+	 * still describe the lost configuration. Reprogram the EC from the
+	 * cached state to keep both in sync.
+	 */
+	guard(mutex)(&priv->lock);
+	return cros_chctl_configure_ec(priv);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(cros_chctl_pm_ops, NULL, cros_chctl_resume);
+
 static const struct platform_device_id cros_chctl_id[] = {
 	{ .name = "cros-charge-control" },
 	{ }
@@ -447,6 +466,7 @@ MODULE_DEVICE_TABLE(platform, cros_chctl_id);
 
 static struct platform_driver cros_chctl_driver = {
 	.driver.name	= "cros-charge-control",
+	.driver.pm	= pm_sleep_ptr(&cros_chctl_pm_ops),
 	.probe		= cros_chctl_probe,
 	.id_table	= cros_chctl_id,
 };

base-commit: b33596a615d163372f5e7519338d860cbfc37217
-- 
2.53.0

Re: [PATCH] power: supply: cros_charge-control: restore EC state on resume

[Tzung-Bi Shih]

On Mon, Oct 05, 2026 at 09:56:06AM +1100, Andrew Pam wrote:
> Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
> cros_charge-control: adopt EC charge state on probe") made the driver

The commit 4fc88ba435da doesn't exist.

> initialise its cached state from the EC, but it does so only on probe,
                              ^^^^
To be precise, this should be "to".

> Note that re-reading the state from the EC on resume, by reusing
> cros_chctl_init_state() or otherwise, would be wrong even on the command

cros_chctl_init_state() doesn't exist.

>  #define EC_CHARGE_CONTROL_BEHAVIOURS	(BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)             | \
> @@ -420,6 +421,7 @@ static int cros_chctl_probe(struct platform_device *pdev)
>  
>  	priv->dev = dev;
>  	priv->cros_ec = cros_ec;
> +	platform_set_drvdata(pdev, priv);

Even though functionally identical, to be symmetric to cros_chctl_resume(),
maybe use:

    dev_set_drvdata(dev, priv)

> +static int cros_chctl_resume(struct device *dev)
> +{
> +	struct cros_chctl_priv *priv = dev_get_drvdata(dev);
> +
> +	/*
> +	 * The EC resets its charge control state when the AP powers off, so the
> +	 * configuration is lost across hibernation. As the driver never reads
> +	 * the state back from the EC, the values exposed to userspace would
> +	 * still describe the lost configuration. Reprogram the EC from the
> +	 * cached state to keep both in sync.
> +	 */
> +	guard(mutex)(&priv->lock);
> +	return cros_chctl_configure_ec(priv);

It may be worth adding a dev_err() or dev_warn() log if the configuration
fails.

> @@ -447,6 +466,7 @@ MODULE_DEVICE_TABLE(platform, cros_chctl_id);
>  
>  static struct platform_driver cros_chctl_driver = {
>  	.driver.name	= "cros-charge-control",
> +	.driver.pm	= pm_sleep_ptr(&cros_chctl_pm_ops),

To be neat,

    .driver = {
	.name = ...
	.pm = ...
    }

Re: [PATCH] power: supply: cros_charge-control: restore EC state on resume

[Andrew Pam]

On Mon, 5 Oct 2026, at 15:20, Tzung-Bi Shih wrote:
> On Mon, Oct 05, 2026 at 09:56:06AM +1100, Andrew Pam wrote:
> > Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
> > cros_charge-control: adopt EC charge state on probe") made the driver
> 
> The commit 4fc88ba435da doesn't exist.

Thanks for the review. The patch is against Sebastian's power-supply for-next, where that is Matt's patch you reviewed, applied on 10 September:

https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/commit/?id=4fc88ba435da

It isn't in mainline or chrome-platform yet, so I have made that explicit in the v2 commit message.

> > initialise its cached state from the EC, but it does so only on probe,
>                               ^^^^
> To be precise, this should be "to".

Reworded in v2 as "made the driver adopt the EC's charge state, but only at probe".

I have also taken your other three suggestions: dev_set_drvdata(), a dev_err() when reprogramming fails, and the nested .driver initialiser. v2 also fixes the resume hook's comment, which still said the driver never reads the state back from the EC. That hasn't been true since 4fc88ba435da. And v2 adds Fixes: and Cc: stable: the driver has had no resume handling since it was added in 6.11, so stable kernels lose the charge limit across hibernation too. v2 follows shortly.

Thanks,
Andrew

Re: [PATCH] power: supply: cros_charge-control: restore EC state on resume

[Tzung-Bi Shih]

On Mon, Oct 05, 2026 at 03:44:45PM +1100, Andrew Pam wrote:
> On Mon, 5 Oct 2026, at 15:20, Tzung-Bi Shih wrote:
> > On Mon, Oct 05, 2026 at 09:56:06AM +1100, Andrew Pam wrote:
> > > Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
> > > cros_charge-control: adopt EC charge state on probe") made the driver
> > 
> > The commit 4fc88ba435da doesn't exist.
> 
> Thanks for the review. The patch is against Sebastian's power-supply for-next, where that is Matt's patch you reviewed, applied on 10 September:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/commit/?id=4fc88ba435da
> 
> It isn't in mainline or chrome-platform yet, so I have made that explicit in the v2 commit message.
> 
> > > initialise its cached state from the EC, but it does so only on probe,
> >                               ^^^^
> > To be precise, this should be "to".
> 
> Reworded in v2 as "made the driver adopt the EC's charge state, but only at probe".

Ah, I see.  I misunderstood because the commit wasn't in my local tree.

[PATCH v2] power: supply: cros_charge-control: restore EC state on resume

[Andrew Pam]

The EC resets its charge control configuration when the AP powers off,
so configured sustainer thresholds and charge behaviour are lost across
hibernation.

Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
cros_charge-control: adopt EC charge state on probe"), in the
power-supply tree, made the driver adopt the EC's charge state, but only
at probe, and a resume is not a probe. Sysfs therefore keeps reporting
the configured values while the EC charges the battery to full. On a
Framework Laptop 16 (Ryzen AI 300, charge control command version 3)
with charge_control_end_threshold set to 80, a hibernate and resume
cycle leaves the attribute still reading 80 while the battery charges
past it to 100%.

Add a system sleep resume hook that reprograms the EC from the cached
state. The same EC behaviour and the same fix already exist for another
driver, see commit 2643187ccb86 ("platform/x86: ayaneo-ec: Add suspend
hook").

Reprogramming the EC is only strictly required after hibernation, but it
is cheap and idempotent, so hook every resume path rather than rely on
the EC preserving its state across each kind of sleep transition.

Note that re-reading the state from the EC on resume, by reusing
cros_chctl_init_state() or otherwise, would be wrong even on the command
versions that support GET: the EC has been reset to its defaults by the
time the hook runs, so adopting that state would silently discard the
configuration the user asked for. The driver's cached state is the only
remaining record of it, which is why it is pushed to the EC rather than
refreshed from it.

Fixes: c6ed48ef5259 ("power: supply: add ChromeOS EC based charge control driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Andrew Pam <andrew@fastmailteam.com>
---
Changes in v2:
- Use dev_set_drvdata() in probe, symmetric with dev_get_drvdata() in
  the resume hook (Tzung-Bi)
- Log an error if reprogramming the EC fails on resume (Tzung-Bi)
- Use a nested .driver initialiser in struct platform_driver (Tzung-Bi)
- Update the resume hook's comment, which still said the driver never
  reads the state back from the EC; untrue since 4fc88ba435da
- Add Fixes: and Cc: stable tags
- Commit message: say that 4fc88ba435da is in the power-supply tree, and
  reword its description

v1: https://lore.kernel.org/all/20261004225606.30577-1-andrew@fastmailteam.com/

 drivers/power/supply/cros_charge-control.c | 29 +++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/cros_charge-control.c b/drivers/power/supply/cros_charge-control.c
index 07bcc6a9aeef..e5841b40563e 100644
--- a/drivers/power/supply/cros_charge-control.c
+++ b/drivers/power/supply/cros_charge-control.c
@@ -13,6 +13,7 @@
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
 #include <linux/types.h>
 
 #define EC_CHARGE_CONTROL_BEHAVIOURS	(BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO)             | \
@@ -420,6 +421,7 @@ static int cros_chctl_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 	priv->cros_ec = cros_ec;
+	dev_set_drvdata(dev, priv);
 
 	if (priv->cmd_version == 1)
 		priv->psy_ext = &cros_chctl_psy_ext_v1;
@@ -439,6 +441,28 @@ static int cros_chctl_probe(struct platform_device *pdev)
 	return devm_battery_hook_register(dev, &priv->battery_hook);
 }
 
+static int cros_chctl_resume(struct device *dev)
+{
+	struct cros_chctl_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	/*
+	 * The EC resets its charge control state when the AP powers off, so the
+	 * configuration is lost across hibernation, while the values exposed to
+	 * userspace still describe it. The cached state is the only remaining
+	 * record of that configuration: reprogram the EC from it rather than
+	 * re-reading the EC, which would adopt the reset defaults instead.
+	 */
+	guard(mutex)(&priv->lock);
+	ret = cros_chctl_configure_ec(priv);
+	if (ret)
+		dev_err(dev, "failed to restore charge control state: %d\n", ret);
+
+	return ret;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(cros_chctl_pm_ops, NULL, cros_chctl_resume);
+
 static const struct platform_device_id cros_chctl_id[] = {
 	{ .name = "cros-charge-control" },
 	{ }
@@ -446,7 +470,10 @@ static const struct platform_device_id cros_chctl_id[] = {
 MODULE_DEVICE_TABLE(platform, cros_chctl_id);
 
 static struct platform_driver cros_chctl_driver = {
-	.driver.name	= "cros-charge-control",
+	.driver = {
+		.name	= "cros-charge-control",
+		.pm	= pm_sleep_ptr(&cros_chctl_pm_ops),
+	},
 	.probe		= cros_chctl_probe,
 	.id_table	= cros_chctl_id,
 };

base-commit: 5c1c602463d597c5189ddc5a73dece588ba2c57a
-- 
2.53.0

Re: [PATCH v2] power: supply: cros_charge-control: restore EC state on resume

[Tzung-Bi Shih]

On Mon, Oct 05, 2026 at 03:34:18PM +1100, Andrew Pam wrote:
> The EC resets its charge control configuration when the AP powers off,
> so configured sustainer thresholds and charge behaviour are lost across
> hibernation.
> 
> Nothing notices the reset. Commit 4fc88ba435da ("power: supply:
> cros_charge-control: adopt EC charge state on probe"), in the
> power-supply tree, made the driver adopt the EC's charge state, but only
> at probe, and a resume is not a probe. Sysfs therefore keeps reporting
> the configured values while the EC charges the battery to full. On a
> Framework Laptop 16 (Ryzen AI 300, charge control command version 3)
> with charge_control_end_threshold set to 80, a hibernate and resume
> cycle leaves the attribute still reading 80 while the battery charges
> past it to 100%.
> 
> Add a system sleep resume hook that reprograms the EC from the cached
> state. The same EC behaviour and the same fix already exist for another
> driver, see commit 2643187ccb86 ("platform/x86: ayaneo-ec: Add suspend
> hook").
> 
> Reprogramming the EC is only strictly required after hibernation, but it
> is cheap and idempotent, so hook every resume path rather than rely on
> the EC preserving its state across each kind of sleep transition.
> 
> Note that re-reading the state from the EC on resume, by reusing
> cros_chctl_init_state() or otherwise, would be wrong even on the command
> versions that support GET: the EC has been reset to its defaults by the
> time the hook runs, so adopting that state would silently discard the
> configuration the user asked for. The driver's cached state is the only
> remaining record of it, which is why it is pushed to the EC rather than
> refreshed from it.
> 
> Fixes: c6ed48ef5259 ("power: supply: add ChromeOS EC based charge control driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Andrew Pam <andrew@fastmailteam.com>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

You should send new versions (e.g., v2) independently next time, instead of
replying to your previous versions (e.g., v1).