linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fujitsu-laptop: Rework brightness of eco led
@ 2016-07-04 10:04 Matej Groma
  2016-07-04 11:29 ` Jonathan Woithe
  0 siblings, 1 reply; 3+ messages in thread
From: Matej Groma @ 2016-07-04 10:04 UTC (permalink / raw)
  To: jwoithe, dvhart; +Cc: platform-driver-x86, linux-kernel

For the sake of internal consistency, unset maximum brightness of eco
led and make it activatable only on values >= LED_FULL.

Signed-off-by: Matej Groma <matejgroma@gmail.com>
---
Here is a small statistics covering max brightness of platform-x86 leds:
1	other	unset
8	6	7
3/7 leds that have unset max_brightness are activated on nonzero value.
The other 4 are fujitsu-laptop leds. Maybe someone from led subsystem
would be willing to look at this and standardise guidelines on how and
when to use different brightness levels. Hopefully, this could then be
reverted.

 drivers/platform/x86/fujitsu-laptop.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 6ce8e78..61f39ab 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -212,7 +212,6 @@ static void eco_led_set(struct led_classdev *cdev,

 static struct led_classdev eco_led = {
  .name = "fujitsu::eco_led",
- .max_brightness = 1,
  .brightness_get = eco_led_get,
  .brightness_set = eco_led_set
 };
@@ -306,7 +305,7 @@ static void eco_led_set(struct led_classdev *cdev,
 	int curr;

 	curr = call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0);
-	if (brightness)
+	if (brightness >= LED_FULL)
 		call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr | ECO_LED_ON);
 	else
 		call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr & ~ECO_LED_ON);
@@ -352,7 +351,7 @@ static enum led_brightness eco_led_get(struct led_classdev *cdev)
 	enum led_brightness brightness = LED_OFF;

 	if (call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) & ECO_LED_ON)
-		brightness = cdev->max_brightness;
+		brightness = LED_FULL;

 	return brightness;
 }

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] fujitsu-laptop: Rework brightness of eco led
  2016-07-04 10:04 [PATCH] fujitsu-laptop: Rework brightness of eco led Matej Groma
@ 2016-07-04 11:29 ` Jonathan Woithe
  2016-07-06 18:31   ` Darren Hart
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Woithe @ 2016-07-04 11:29 UTC (permalink / raw)
  To: Matej Groma; +Cc: dvhart, platform-driver-x86, linux-kernel

On Mon, Jul 04, 2016 at 12:04:12PM +0200, Matej Groma wrote:
> For the sake of internal consistency, unset maximum brightness of eco
> led and make it activatable only on values >= LED_FULL.
> 
> Signed-off-by: Matej Groma <matejgroma@gmail.com>

Acked-by: Jonathan Woithe <jwoithe@just42.net>

> ---
> Here is a small statistics covering max brightness of platform-x86 leds:
> 1	other	unset
> 8	6	7
> 3/7 leds that have unset max_brightness are activated on nonzero value.
> The other 4 are fujitsu-laptop leds. Maybe someone from led subsystem
> would be willing to look at this and standardise guidelines on how and
> when to use different brightness levels. Hopefully, this could then be
> reverted.
> 
>  drivers/platform/x86/fujitsu-laptop.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 6ce8e78..61f39ab 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -212,7 +212,6 @@ static void eco_led_set(struct led_classdev *cdev,
> 
>  static struct led_classdev eco_led = {
>   .name = "fujitsu::eco_led",
> - .max_brightness = 1,
>   .brightness_get = eco_led_get,
>   .brightness_set = eco_led_set
>  };
> @@ -306,7 +305,7 @@ static void eco_led_set(struct led_classdev *cdev,
>  	int curr;
> 
>  	curr = call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0);
> -	if (brightness)
> +	if (brightness >= LED_FULL)
>  		call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr | ECO_LED_ON);
>  	else
>  		call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr & ~ECO_LED_ON);
> @@ -352,7 +351,7 @@ static enum led_brightness eco_led_get(struct led_classdev *cdev)
>  	enum led_brightness brightness = LED_OFF;
> 
>  	if (call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) & ECO_LED_ON)
> -		brightness = cdev->max_brightness;
> +		brightness = LED_FULL;
> 
>  	return brightness;
>  }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] fujitsu-laptop: Rework brightness of eco led
  2016-07-04 11:29 ` Jonathan Woithe
@ 2016-07-06 18:31   ` Darren Hart
  0 siblings, 0 replies; 3+ messages in thread
From: Darren Hart @ 2016-07-06 18:31 UTC (permalink / raw)
  To: Jonathan Woithe; +Cc: Matej Groma, platform-driver-x86, linux-kernel

On Mon, Jul 04, 2016 at 08:59:14PM +0930, Jonathan Woithe wrote:
> On Mon, Jul 04, 2016 at 12:04:12PM +0200, Matej Groma wrote:
> > For the sake of internal consistency, unset maximum brightness of eco
> > led and make it activatable only on values >= LED_FULL.
> > 
> > Signed-off-by: Matej Groma <matejgroma@gmail.com>
> 
> Acked-by: Jonathan Woithe <jwoithe@just42.net>

Queued, thank you.

> 
> > ---
> > Here is a small statistics covering max brightness of platform-x86 leds:
> > 1	other	unset
> > 8	6	7
> > 3/7 leds that have unset max_brightness are activated on nonzero value.
> > The other 4 are fujitsu-laptop leds. Maybe someone from led subsystem
> > would be willing to look at this and standardise guidelines on how and
> > when to use different brightness levels. Hopefully, this could then be
> > reverted.
> > 
> >  drivers/platform/x86/fujitsu-laptop.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > index 6ce8e78..61f39ab 100644
> > --- a/drivers/platform/x86/fujitsu-laptop.c
> > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > @@ -212,7 +212,6 @@ static void eco_led_set(struct led_classdev *cdev,
> > 
> >  static struct led_classdev eco_led = {
> >   .name = "fujitsu::eco_led",
> > - .max_brightness = 1,
> >   .brightness_get = eco_led_get,
> >   .brightness_set = eco_led_set
> >  };
> > @@ -306,7 +305,7 @@ static void eco_led_set(struct led_classdev *cdev,
> >  	int curr;
> > 
> >  	curr = call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0);
> > -	if (brightness)
> > +	if (brightness >= LED_FULL)
> >  		call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr | ECO_LED_ON);
> >  	else
> >  		call_fext_func(FUNC_LEDS, 0x1, ECO_LED, curr & ~ECO_LED_ON);
> > @@ -352,7 +351,7 @@ static enum led_brightness eco_led_get(struct led_classdev *cdev)
> >  	enum led_brightness brightness = LED_OFF;
> > 
> >  	if (call_fext_func(FUNC_LEDS, 0x2, ECO_LED, 0x0) & ECO_LED_ON)
> > -		brightness = cdev->max_brightness;
> > +		brightness = LED_FULL;
> > 
> >  	return brightness;
> >  }
> 

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-07-06 18:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 10:04 [PATCH] fujitsu-laptop: Rework brightness of eco led Matej Groma
2016-07-04 11:29 ` Jonathan Woithe
2016-07-06 18:31   ` Darren Hart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).