[PATCH 2/2] RFKILL: set the status of the leds on activation.

[PATCH 2/2] RFKILL: set the status of the leds on activation.

[Dmitry Baryshkov]

Provide default activate function to set the state of the led
when the led becomes bound to the trigger

Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
-- 
 This patch depends on the patch "leds: make sure led->trigger is set
 earlier" which was staged in -mm for some time and recently got merged
 into leds tree.
---
 net/rfkill/rfkill.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index b247677..e9010ff 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -57,6 +57,16 @@ static void rfkill_led_trigger(struct rfkill *rfkill,
 #endif /* CONFIG_RFKILL_LEDS */
 }
 
+#ifdef CONFIG_RFKILL_LEDS
+static void rfkill_led_trigger_activate(struct led_classdev *led)
+{
+	struct rfkill *rfkill = container_of(led->trigger,
+			struct rfkill, led_trigger);
+
+	rfkill_led_trigger(rfkill, rfkill->state);
+}
+#endif /* CONFIG_RFKILL_LEDS */
+
 static int rfkill_toggle_radio(struct rfkill *rfkill,
 				enum rfkill_state state)
 {
@@ -357,6 +367,8 @@ static void rfkill_led_trigger_register(struct rfkill *rfkill)
 
 	if (!rfkill->led_trigger.name)
 		rfkill->led_trigger.name = rfkill->dev.bus_id;
+	if (!rfkill->led_trigger.activate)
+		rfkill->led_trigger.activate = rfkill_led_trigger_activate;
 	error = led_trigger_register(&rfkill->led_trigger);
 	if (error)
 		rfkill->led_trigger.name = NULL;
-- 
1.5.6.2


-- 
With best wishes
Dmitry

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Henrique de Moraes Holschuh]

On Tue, 22 Jul 2008, Dmitry Baryshkov wrote:
> Provide default activate function to set the state of the led
> when the led becomes bound to the trigger

I think I understand what this is for, but I better ask to be sure...
What does it do, what is the use case?

>  This patch depends on the patch "leds: make sure led->trigger is set
>  earlier" which was staged in -mm for some time and recently got merged
>  into leds tree.

Will it get in mainline for 2.6.27-rc?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Dmitry]

Hi,

2008/7/22 Henrique de Moraes Holschuh <hmh@hmh.eng.br>:
> On Tue, 22 Jul 2008, Dmitry Baryshkov wrote:
>> Provide default activate function to set the state of the led
>> when the led becomes bound to the trigger
>
> I think I understand what this is for, but I better ask to be sure...
> What does it do, what is the use case?

Register rfkill-> registers led trigger
Register led with rfkill trigger.

However as rfkill doesn't provide the led_trigger->activate callback,
the led won't be in the proper state till the next rfkill state change.
With this patch, when led_trigger_set calls led_trigger->activate,
leds is set up with correct brightness.

>>  This patch depends on the patch "leds: make sure led->trigger is set
>>  earlier" which was staged in -mm for some time and recently got merged
>>  into leds tree.
>
> Will it get in mainline for 2.6.27-rc?

I hope so. It actually depends on Richard Purdie (cc'ed)

>
> --
>  "One disk to rule them all, One disk to find them. One disk to bring
>  them all and in the darkness grind them. In the Land of Redmond
>  where the shadows lie." -- The Silicon Valley Tarot
>  Henrique Holschuh
>



-- 
With best wishes
Dmitry

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Henrique de Moraes Holschuh]

Oi Tue Jul 2008, Dmitry wrote:
> 2008/7/22 Henrique de Moraes Holschuh <hmh@hmh.eng.br>:
> > On Tue, 22 Jul 2008, Dmitry Baryshkov wrote:
> >> Provide default activate function to set the state of the led
> >> when the led becomes bound to the trigger
> >
> > I think I understand what this is for, but I better ask to be sure...
> > What does it do, what is the use case?
> 
> Register rfkill-> registers led trigger
> Register led with rfkill trigger.
> 
> However as rfkill doesn't provide the led_trigger->activate callback,
> the led won't be in the proper state till the next rfkill state change.
> With this patch, when led_trigger_set calls led_trigger->activate,
> leds is set up with correct brightness.

Thought as much.  Please add that to the patch description...

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Now you just need Ivo's ACKs, since mine don't mean much :-)

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Richard Purdie]

On Tue, 2008-07-22 at 15:33 +0400, Dmitry wrote:
> 2008/7/22 Henrique de Moraes Holschuh <hmh@hmh.eng.br>:
> >>  This patch depends on the patch "leds: make sure led->trigger is set
> >>  earlier" which was staged in -mm for some time and recently got merged
> >>  into leds tree.
> >
> > Will it get in mainline for 2.6.27-rc?
> 
> I hope so. It actually depends on Richard Purdie (cc'ed)

That patch is queued and I'll be sending a pull request for the leds and
backlight trees shortly.

Cheers,

Richard

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Ivo van Doorn]

On Tuesday 22 July 2008, Dmitry Baryshkov wrote:
> Provide default activate function to set the state of the led
> when the led becomes bound to the trigger
> 
> Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
> Cc: Ivo van Doorn <IvDoorn@gmail.com>
> Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

> -- 
>  This patch depends on the patch "leds: make sure led->trigger is set
>  earlier" which was staged in -mm for some time and recently got merged
>  into leds tree.
> ---
>  net/rfkill/rfkill.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
> index b247677..e9010ff 100644
> --- a/net/rfkill/rfkill.c
> +++ b/net/rfkill/rfkill.c
> @@ -57,6 +57,16 @@ static void rfkill_led_trigger(struct rfkill *rfkill,
>  #endif /* CONFIG_RFKILL_LEDS */
>  }
>  
> +#ifdef CONFIG_RFKILL_LEDS
> +static void rfkill_led_trigger_activate(struct led_classdev *led)
> +{
> +	struct rfkill *rfkill = container_of(led->trigger,
> +			struct rfkill, led_trigger);
> +
> +	rfkill_led_trigger(rfkill, rfkill->state);
> +}
> +#endif /* CONFIG_RFKILL_LEDS */
> +
>  static int rfkill_toggle_radio(struct rfkill *rfkill,
>  				enum rfkill_state state)
>  {
> @@ -357,6 +367,8 @@ static void rfkill_led_trigger_register(struct rfkill *rfkill)
>  
>  	if (!rfkill->led_trigger.name)
>  		rfkill->led_trigger.name = rfkill->dev.bus_id;
> +	if (!rfkill->led_trigger.activate)
> +		rfkill->led_trigger.activate = rfkill_led_trigger_activate;
>  	error = led_trigger_register(&rfkill->led_trigger);
>  	if (error)
>  		rfkill->led_trigger.name = NULL;
> -- 
> 1.5.6.2
> 
> 

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Dmitry Baryshkov]

On Tue, Jul 22, 2008 at 07:11:35PM +0200, Ivo van Doorn wrote:
> On Tuesday 22 July 2008, Dmitry Baryshkov wrote:
> > Provide default activate function to set the state of the led
> > when the led becomes bound to the trigger
> > 
> > Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
> > Cc: Ivo van Doorn <IvDoorn@gmail.com>
> > Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> 
> Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

Sorry to bother you, but can I expect these patches to be merged
before 2.6.27, or should I wait for 2.6.28 window?

-- 
With best wishes
Dmitry

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Ivo van Doorn]

On Sunday 27 July 2008, Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2008 at 07:11:35PM +0200, Ivo van Doorn wrote:
> > On Tuesday 22 July 2008, Dmitry Baryshkov wrote:
> > > Provide default activate function to set the state of the led
> > > when the led becomes bound to the trigger
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
> > > Cc: Ivo van Doorn <IvDoorn@gmail.com>
> > > Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> > 
> > Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> 
> Sorry to bother you, but can I expect these patches to be merged
> before 2.6.27, or should I wait for 2.6.28 window?

Well they are not really bugfixes, but I'll let John decide to which tree
they should be merged.

Ivo

Re: [PATCH 2/2] RFKILL: set the status of the leds on activation.

[Henrique de Moraes Holschuh]

On Sun, 27 Jul 2008, Ivo van Doorn wrote:
> On Sunday 27 July 2008, Dmitry Baryshkov wrote:
> > On Tue, Jul 22, 2008 at 07:11:35PM +0200, Ivo van Doorn wrote:
> > > On Tuesday 22 July 2008, Dmitry Baryshkov wrote:
> > > > Provide default activate function to set the state of the led
> > > > when the led becomes bound to the trigger
> > > > 
> > > > Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
> > > > Cc: Ivo van Doorn <IvDoorn@gmail.com>
> > > > Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> > > 
> > > Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
> > 
> > Sorry to bother you, but can I expect these patches to be merged
> > before 2.6.27, or should I wait for 2.6.28 window?
> 
> Well they are not really bugfixes, but I'll let John decide to which tree
> they should be merged.

FWIW, I consider them to be both the kind of small thing that should go in
for 2.6.27.  It considerably enhances the LED usability of rfkill.

And I think patch 2/2 qualifies as a bug fix.  I believe it makes sense even
without 1/2?

BTW, Dmitry, since you seem to use rfkill LED, could I convince you to try
your hand at moving all the LED functionality from the current "hooks inside
the code paths" we use in rfkill.c to a "rfkill notify handler-based"
approach?  The could should get cleaner and maybe even lose some LOC with
that refactoring...  Maybe it can even go into a separate module.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh