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] [day] [month] [year] [list]

Message-ID: <CAKzfze_cBgxtekprHzAMxcpVCmp2UmJVA5j=4igdibgu0Rdp5g@mail.gmail.com>
Date:   Fri, 30 Sep 2016 15:57:45 -0700
From:   Matt Ranostay <mranostay@...il.com>
To:     Jacek Anaszewski <jacek.anaszewski@...il.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-leds@...r.kernel.org,
        Matt Ranostay <matt@...ostay.consulting>,
        Tony Lindgren <tony@...mide.com>,
        Richard Purdie <rpurdie@...ys.net>,
        Jacek Anaszewski <j.anaszewski@...sung.com>,
        Liam Breck <liam@...workimprov.net>
Subject: Re: [PATCH] leds: leds-pca963x: add power management support

On Fri, Sep 30, 2016 at 2:23 PM, Jacek Anaszewski
<jacek.anaszewski@...il.com> wrote:
> 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.

Understood.

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

Ok in this case it would make sense to use runtime PM..   which
shouldn't be too hard to do.  Will fix in v2

>
>>         },
>>         .probe  = pca963x_probe,
>>         .remove = pca963x_remove,
>>
>
> --
> Best regards,
> Jacek Anaszewski