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

[Tzung-Bi Shih <tzungbi@kernel.org>]

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 = ...
    }