linux-kernel - Re: [PATCH] leds: leds-pca963x: add power management support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cb494ad2-c8fe-271a-8537-46c50964f82a@gmail.com>
Date:   Fri, 30 Sep 2016 23:23:45 +0200
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Matt Ranostay <mranostay@...il.com>, linux-kernel@...r.kernel.org,
        linux-leds@...r.kernel.org
Cc:     Matt Ranostay <matt@...ostay.consulting>,
        Tony Lindgren <tony@...mide.com>,
        Richard Purdie <rpurdie@...ys.net>,
        Jacek Anaszewski <j.anaszewski@...sung.com>
Subject: Re: [PATCH] leds: leds-pca963x: add power management support

Hi Matt,

Thanks for the patch.

On 09/30/2016 09:27 PM, Matt Ranostay wrote:
> Turn off LEDS on suspend and put device in low power state, and restore
> settings on resume.
>
> Cc: Tony Lindgren <tony@...mide.com>
> Cc: Richard Purdie <rpurdie@...ys.net>
> Cc: Jacek Anaszewski <j.anaszewski@...sung.com>
> Signed-off-by: Matt Ranostay <matt@...ostay.consulting>
> ---
>  drivers/leds/leds-pca963x.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
> index 407eba11e187..4840bdaa1b48 100644
> --- a/drivers/leds/leds-pca963x.c
> +++ b/drivers/leds/leds-pca963x.c
> @@ -382,6 +382,7 @@ static int pca963x_probe(struct i2c_client *client,
>
>  		pca963x[i].led_cdev.name = pca963x[i].name;
>  		pca963x[i].led_cdev.brightness_set_blocking = pca963x_led_set;
> +		pca963x[i].led_cdev.flags = LED_CORE_SUSPENDRESUME;
>
>  		if (pdata && pdata->blink_type == PCA963X_HW_BLINK)
>  			pca963x[i].led_cdev.blink_set = pca963x_blink_set;
> @@ -422,10 +423,40 @@ static int pca963x_remove(struct i2c_client *client)
>  	return 0;
>  }
>
> +static int pca963x_set_power(struct i2c_client *client, bool state)
> +{
> +	return i2c_smbus_write_byte_data(client, PCA963X_MODE1,
> +					 state ? 0 : BIT(4));
> +}
> +
> +static int pca963x_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	return pca963x_set_power(client, false);
> +}
> +
> +static int pca963x_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	int ret;
> +
> +	ret = pca963x_set_power(client, true);
> +
> +	/* wait for oscillator enabling */
> +	if (!ret)
> +		usleep_range(500, 1000);
> +
> +	return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(pca963x_pm, pca963x_suspend, pca963x_resume);
> +
>  static struct i2c_driver pca963x_driver = {
>  	.driver = {
>  		.name	= "leds-pca963x",
>  		.of_match_table = of_match_ptr(of_pca963x_match),
> +		.pm	= &pca963x_pm,

pm ops are initialized in led-class.c. If LED_CORE_SUSPENDRESUME
flag is set by a LED class driver, then the brightness will be
set to 0 on suspend and brought back on resume.

LED class driver's responsibility is to enter power down mode if
all LED class devices it exposes have their brightness set to 0.

Effectively, you should avoid defining pm ops, but instead
just track how many LEDs are on, and set the device in the power
down mode if brightness of the last active LED is set to LED_OFF.
Similarly the device should be powered up if any of LEDs brightness
is set to a value greater than 0.

>  	},
>  	.probe	= pca963x_probe,
>  	.remove	= pca963x_remove,
>

-- 
Best regards,
Jacek Anaszewski