* [PATCH v3] leds: pca963x: enable low-power state
@ 2016-10-27 4:04 ` Matt Ranostay
2016-10-27 4:07 ` Matt Ranostay
2016-10-27 9:13 ` Jacek Anaszewski
0 siblings, 2 replies; 4+ messages in thread
From: Matt Ranostay @ 2016-10-27 4:04 UTC (permalink / raw)
To: linux-leds
Cc: Matt Ranostay, Peter Meerwald, Ricardo Ribalda, Tony Lindgren,
Jacek Anaszewski
Allow chip to enter low power state when no LEDs are being lit or in
blink mode.
Cc: Peter Meerwald <p.meerwald@bct-electronic.com>,
Cc: Ricardo Ribalda <ricardo.ribalda@gmail.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
drivers/leds/leds-pca963x.c | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
index b6ce1f2ec33e..e1767369776f 100644
--- a/drivers/leds/leds-pca963x.c
+++ b/drivers/leds/leds-pca963x.c
@@ -103,6 +103,7 @@ struct pca963x {
struct mutex mutex;
struct i2c_client *client;
struct pca963x_led *leds;
+ int leds_on;
};
struct pca963x_led {
@@ -180,14 +181,40 @@ static void pca963x_blink(struct pca963x_led *pca963x)
mutex_unlock(&pca963x->chip->mutex);
}
+static int pca963x_power_state(struct pca963x_led *pca963x)
+{
+ int i, ret = 0, leds_on = 0;
+ int prev_leds = pca963x->chip->leds_on;
+
+ for (i = 0; i < pca963x->chip->chipdef->n_leds; i++) {
+ if (pca963x->chip->leds[i].led_cdev.brightness > 0)
+ leds_on++;
+ }
+
+ if (leds_on != prev_leds && !(leds_on && prev_leds)) {
+ ret = i2c_smbus_write_byte_data(pca963x->chip->client,
+ PCA963X_MODE1,
+ leds_on ? 0 : BIT(4));
+ }
+
+ pca963x->chip->leds_on = leds_on;
+
+ return ret;
+}
+
static int pca963x_led_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
struct pca963x_led *pca963x;
+ int ret;
pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);
- return pca963x_brightness(pca963x, value);
+ ret = pca963x_brightness(pca963x, value);
+ if (ret < 0)
+ return ret;
+
+ return pca963x_power_state(pca963x);
}
static unsigned int pca963x_period_scale(struct pca963x_led *pca963x,
@@ -403,8 +430,8 @@ static int pca963x_probe(struct i2c_client *client,
goto exit;
}
- /* Disable LED all-call address and set normal mode */
- i2c_smbus_write_byte_data(client, PCA963X_MODE1, 0x00);
+ /* Disable LED all-call address, and power down initially */
+ i2c_smbus_write_byte_data(client, PCA963X_MODE1, BIT(4));
if (pdata) {
/* Configure output: open-drain or totem pole (push-pull) */
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] leds: pca963x: enable low-power state
2016-10-27 4:04 ` [PATCH v3] leds: pca963x: enable low-power state Matt Ranostay
@ 2016-10-27 4:07 ` Matt Ranostay
2016-10-27 9:13 ` Jacek Anaszewski
1 sibling, 0 replies; 4+ messages in thread
From: Matt Ranostay @ 2016-10-27 4:07 UTC (permalink / raw)
To: Matt Ranostay
Cc: linux-leds, Peter Meerwald, Ricardo Ribalda, Tony Lindgren,
Jacek Anaszewski
Forgot the change log in my email...
On Wed, Oct 26, 2016 at 9:04 PM, Matt Ranostay <mranostay@gmail.com> wrote:
> Allow chip to enter low power state when no LEDs are being lit or in
> blink mode.
>
> Cc: Peter Meerwald <p.meerwald@bct-electronic.com>,
> Cc: Ricardo Ribalda <ricardo.ribalda@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
Changes from v1:
* remove runtime pm
* count leds that are off, if all then enter low-power state
Changes from v2:
* add reference count of leds to reduce i2c transactions
> drivers/leds/leds-pca963x.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
> index b6ce1f2ec33e..e1767369776f 100644
> --- a/drivers/leds/leds-pca963x.c
> +++ b/drivers/leds/leds-pca963x.c
> @@ -103,6 +103,7 @@ struct pca963x {
> struct mutex mutex;
> struct i2c_client *client;
> struct pca963x_led *leds;
> + int leds_on;
> };
>
> struct pca963x_led {
> @@ -180,14 +181,40 @@ static void pca963x_blink(struct pca963x_led *pca963x)
> mutex_unlock(&pca963x->chip->mutex);
> }
>
> +static int pca963x_power_state(struct pca963x_led *pca963x)
> +{
> + int i, ret = 0, leds_on = 0;
> + int prev_leds = pca963x->chip->leds_on;
> +
> + for (i = 0; i < pca963x->chip->chipdef->n_leds; i++) {
> + if (pca963x->chip->leds[i].led_cdev.brightness > 0)
> + leds_on++;
> + }
> +
> + if (leds_on != prev_leds && !(leds_on && prev_leds)) {
> + ret = i2c_smbus_write_byte_data(pca963x->chip->client,
> + PCA963X_MODE1,
> + leds_on ? 0 : BIT(4));
> + }
> +
> + pca963x->chip->leds_on = leds_on;
> +
> + return ret;
> +}
> +
> static int pca963x_led_set(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> struct pca963x_led *pca963x;
> + int ret;
>
> pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);
>
> - return pca963x_brightness(pca963x, value);
> + ret = pca963x_brightness(pca963x, value);
> + if (ret < 0)
> + return ret;
> +
> + return pca963x_power_state(pca963x);
> }
>
> static unsigned int pca963x_period_scale(struct pca963x_led *pca963x,
> @@ -403,8 +430,8 @@ static int pca963x_probe(struct i2c_client *client,
> goto exit;
> }
>
> - /* Disable LED all-call address and set normal mode */
> - i2c_smbus_write_byte_data(client, PCA963X_MODE1, 0x00);
> + /* Disable LED all-call address, and power down initially */
> + i2c_smbus_write_byte_data(client, PCA963X_MODE1, BIT(4));
>
> if (pdata) {
> /* Configure output: open-drain or totem pole (push-pull) */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] leds: pca963x: enable low-power state
2016-10-27 4:04 ` [PATCH v3] leds: pca963x: enable low-power state Matt Ranostay
2016-10-27 4:07 ` Matt Ranostay
@ 2016-10-27 9:13 ` Jacek Anaszewski
2016-10-27 18:19 ` Matt Ranostay
1 sibling, 1 reply; 4+ messages in thread
From: Jacek Anaszewski @ 2016-10-27 9:13 UTC (permalink / raw)
To: Matt Ranostay, linux-leds
Cc: Matt Ranostay, Peter Meerwald, Ricardo Ribalda, Tony Lindgren
On 10/27/2016 06:04 AM, Matt Ranostay wrote:
> Allow chip to enter low power state when no LEDs are being lit or in
> blink mode.
>
> Cc: Peter Meerwald <p.meerwald@bct-electronic.com>,
> Cc: Ricardo Ribalda <ricardo.ribalda@gmail.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
> drivers/leds/leds-pca963x.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
> index b6ce1f2ec33e..e1767369776f 100644
> --- a/drivers/leds/leds-pca963x.c
> +++ b/drivers/leds/leds-pca963x.c
> @@ -103,6 +103,7 @@ struct pca963x {
> struct mutex mutex;
> struct i2c_client *client;
> struct pca963x_led *leds;
> + int leds_on;
> };
>
> struct pca963x_led {
> @@ -180,14 +181,40 @@ static void pca963x_blink(struct pca963x_led *pca963x)
> mutex_unlock(&pca963x->chip->mutex);
> }
>
> +static int pca963x_power_state(struct pca963x_led *pca963x)
> +{
> + int i, ret = 0, leds_on = 0;
> + int prev_leds = pca963x->chip->leds_on;
> +
> + for (i = 0; i < pca963x->chip->chipdef->n_leds; i++) {
> + if (pca963x->chip->leds[i].led_cdev.brightness > 0)
> + leds_on++;
> + }
You're still executing this loop on every brightness setting.
It could be avoided if you cached current LED states in struct pca963x
using bit flags and set_bit()/clear_bit().
In pca963x_led_set() you would have to set bit related to
pca963x->led_num if brightness > 0 and clear it otherwise.
Then, you would have to check if any bit is on with a single
readout, and update the power mode basing on that if needed.
> +
> + if (leds_on != prev_leds && !(leds_on && prev_leds)) {
> + ret = i2c_smbus_write_byte_data(pca963x->chip->client,
> + PCA963X_MODE1,
> + leds_on ? 0 : BIT(4));
> + }
> +
> + pca963x->chip->leds_on = leds_on;
> +
> + return ret;
> +}
> +
> static int pca963x_led_set(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> struct pca963x_led *pca963x;
> + int ret;
>
> pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);
>
> - return pca963x_brightness(pca963x, value);
> + ret = pca963x_brightness(pca963x, value);
> + if (ret < 0)
> + return ret;
> +
> + return pca963x_power_state(pca963x);
> }
>
> static unsigned int pca963x_period_scale(struct pca963x_led *pca963x,
> @@ -403,8 +430,8 @@ static int pca963x_probe(struct i2c_client *client,
> goto exit;
> }
>
> - /* Disable LED all-call address and set normal mode */
> - i2c_smbus_write_byte_data(client, PCA963X_MODE1, 0x00);
> + /* Disable LED all-call address, and power down initially */
> + i2c_smbus_write_byte_data(client, PCA963X_MODE1, BIT(4));
>
> if (pdata) {
> /* Configure output: open-drain or totem pole (push-pull) */
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] leds: pca963x: enable low-power state
2016-10-27 9:13 ` Jacek Anaszewski
@ 2016-10-27 18:19 ` Matt Ranostay
0 siblings, 0 replies; 4+ messages in thread
From: Matt Ranostay @ 2016-10-27 18:19 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Matt Ranostay, linux-leds, Peter Meerwald, Ricardo Ribalda,
Tony Lindgren
On Thu, Oct 27, 2016 at 2:13 AM, Jacek Anaszewski
<j.anaszewski@samsung.com> wrote:
> On 10/27/2016 06:04 AM, Matt Ranostay wrote:
>>
>> Allow chip to enter low power state when no LEDs are being lit or in
>> blink mode.
>>
>> Cc: Peter Meerwald <p.meerwald@bct-electronic.com>,
>> Cc: Ricardo Ribalda <ricardo.ribalda@gmail.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> ---
>> drivers/leds/leds-pca963x.c | 33 ++++++++++++++++++++++++++++++---
>> 1 file changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
>> index b6ce1f2ec33e..e1767369776f 100644
>> --- a/drivers/leds/leds-pca963x.c
>> +++ b/drivers/leds/leds-pca963x.c
>> @@ -103,6 +103,7 @@ struct pca963x {
>> struct mutex mutex;
>> struct i2c_client *client;
>> struct pca963x_led *leds;
>> + int leds_on;
>> };
>>
>> struct pca963x_led {
>> @@ -180,14 +181,40 @@ static void pca963x_blink(struct pca963x_led
>> *pca963x)
>> mutex_unlock(&pca963x->chip->mutex);
>> }
>>
>> +static int pca963x_power_state(struct pca963x_led *pca963x)
>> +{
>> + int i, ret = 0, leds_on = 0;
>> + int prev_leds = pca963x->chip->leds_on;
>> +
>> + for (i = 0; i < pca963x->chip->chipdef->n_leds; i++) {
>> + if (pca963x->chip->leds[i].led_cdev.brightness > 0)
>> + leds_on++;
>> + }
>
>
> You're still executing this loop on every brightness setting.
> It could be avoided if you cached current LED states in struct pca963x
> using bit flags and set_bit()/clear_bit().
Gah should have thought of that... fixing in v4..
>
> In pca963x_led_set() you would have to set bit related to
> pca963x->led_num if brightness > 0 and clear it otherwise.
>
> Then, you would have to check if any bit is on with a single
> readout, and update the power mode basing on that if needed.
>
>
>> +
>> + if (leds_on != prev_leds && !(leds_on && prev_leds)) {
>> + ret = i2c_smbus_write_byte_data(pca963x->chip->client,
>> + PCA963X_MODE1,
>> + leds_on ? 0 : BIT(4));
>> + }
>> +
>> + pca963x->chip->leds_on = leds_on;
>> +
>> + return ret;
>> +}
>> +
>> static int pca963x_led_set(struct led_classdev *led_cdev,
>> enum led_brightness value)
>> {
>> struct pca963x_led *pca963x;
>> + int ret;
>>
>> pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);
>>
>> - return pca963x_brightness(pca963x, value);
>> + ret = pca963x_brightness(pca963x, value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return pca963x_power_state(pca963x);
>> }
>>
>> static unsigned int pca963x_period_scale(struct pca963x_led *pca963x,
>> @@ -403,8 +430,8 @@ static int pca963x_probe(struct i2c_client *client,
>> goto exit;
>> }
>>
>> - /* Disable LED all-call address and set normal mode */
>> - i2c_smbus_write_byte_data(client, PCA963X_MODE1, 0x00);
>> + /* Disable LED all-call address, and power down initially */
>> + i2c_smbus_write_byte_data(client, PCA963X_MODE1, BIT(4));
>>
>> if (pdata) {
>> /* Configure output: open-drain or totem pole (push-pull)
>> */
>>
>
>
> --
> Best regards,
> Jacek Anaszewski
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-27 18:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20161027040451epcas4p2fb5c7c652494310445b94fe8b7ffa8a8@epcas4p2.samsung.com>
2016-10-27 4:04 ` [PATCH v3] leds: pca963x: enable low-power state Matt Ranostay
2016-10-27 4:07 ` Matt Ranostay
2016-10-27 9:13 ` Jacek Anaszewski
2016-10-27 18:19 ` Matt Ranostay
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.