[PATCH] RFKILL: fix input layer initialisation

[PATCH] RFKILL: fix input layer initialisation

[Dmitry Baryshkov]

Initialise correctly last fields, so tasks can be actually executed

Also move rfkill-input initialisation to subsys level so that it's
available before generic drivers probing occurs.

Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
---
 net/rfkill/rfkill-input.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index e4b051d..e4540fc 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -181,6 +181,11 @@ static struct input_handler rfkill_handler = {
 
 static int __init rfkill_handler_init(void)
 {
+	unsigned long last_run = jiffies - msecs_to_jiffies(500);
+	rfkill_wlan.last = last_run;
+	rfkill_bt.last = last_run;
+	rfkill_uwb.last = last_run;
+	rfkill_wimax.last = last_run;
 	return input_register_handler(&rfkill_handler);
 }
 
@@ -190,5 +195,5 @@ static void __exit rfkill_handler_exit(void)
 	flush_scheduled_work();
 }
 
-module_init(rfkill_handler_init);
+subsys_initcall(rfkill_handler_init);
 module_exit(rfkill_handler_exit);
-- 
1.5.5.3


-- 
With best wishes
Dmitry

Re: [PATCH] RFKILL: fix input layer initialisation

[Ivo van Doorn]

On Thursday 19 June 2008, Dmitry Baryshkov wrote:
> Initialise correctly last fields, so tasks can be actually executed
> 
> Also move rfkill-input initialisation to subsys level so that it's
> available before generic drivers probing occurs.
> 
> Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
> ---
>  net/rfkill/rfkill-input.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
> index e4b051d..e4540fc 100644
> --- a/net/rfkill/rfkill-input.c
> +++ b/net/rfkill/rfkill-input.c
> @@ -181,6 +181,11 @@ static struct input_handler rfkill_handler = {
>  
>  static int __init rfkill_handler_init(void)
>  {
> +	unsigned long last_run = jiffies - msecs_to_jiffies(500);
> +	rfkill_wlan.last = last_run;
> +	rfkill_bt.last = last_run;
> +	rfkill_uwb.last = last_run;
> +	rfkill_wimax.last = last_run;

I'm not really fan of this approach
Can't this not be done someway in the DEFINE_RFKILL_TASK macro
where the entire structure is initialized?
That way it will automatically be set for all new rfkill task structures later.

>  	return input_register_handler(&rfkill_handler);
>  }
>  
> @@ -190,5 +195,5 @@ static void __exit rfkill_handler_exit(void)
>  	flush_scheduled_work();
>  }
>  
> -module_init(rfkill_handler_init);
> +subsys_initcall(rfkill_handler_init);
>  module_exit(rfkill_handler_exit);

This sounds good.

Ivo

Re: [PATCH] RFKILL: fix input layer initialisation

[Dmitry]

Hi,

2008/6/19 Ivo van Doorn <ivdoorn@gmail.com>:
> On Thursday 19 June 2008, Dmitry Baryshkov wrote:
>> Initialise correctly last fields, so tasks can be actually executed
>>
>> Also move rfkill-input initialisation to subsys level so that it's
>> available before generic drivers probing occurs.
>>
>> Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
>> ---
>>  net/rfkill/rfkill-input.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
>> index e4b051d..e4540fc 100644
>> --- a/net/rfkill/rfkill-input.c
>> +++ b/net/rfkill/rfkill-input.c
>> @@ -181,6 +181,11 @@ static struct input_handler rfkill_handler = {
>>
>>  static int __init rfkill_handler_init(void)
>>  {
>> +     unsigned long last_run = jiffies - msecs_to_jiffies(500);
>> +     rfkill_wlan.last = last_run;
>> +     rfkill_bt.last = last_run;
>> +     rfkill_uwb.last = last_run;
>> +     rfkill_wimax.last = last_run;
>
> I'm not really fan of this approach
> Can't this not be done someway in the DEFINE_RFKILL_TASK macro
> where the entire structure is initialized?
> That way it will automatically be set for all new rfkill task structures later.

IMHO it can't. the jiffies - mset_to_jiffies isn't a constant
initialiser; gcc won't permit such a thing

>
>>       return input_register_handler(&rfkill_handler);
>>  }
>>
>> @@ -190,5 +195,5 @@ static void __exit rfkill_handler_exit(void)
>>       flush_scheduled_work();
>>  }
>>
>> -module_init(rfkill_handler_init);
>> +subsys_initcall(rfkill_handler_init);
>>  module_exit(rfkill_handler_exit);
>
> This sounds good.
>
> Ivo
>



-- 
With best wishes
Dmitry

Re: [PATCH] RFKILL: fix input layer initialisation

[Ivo van Doorn]

On Friday 20 June 2008, Dmitry wrote:
> Hi,
> 
> 2008/6/19 Ivo van Doorn <ivdoorn@gmail.com>:
> > On Thursday 19 June 2008, Dmitry Baryshkov wrote:
> >> Initialise correctly last fields, so tasks can be actually executed
> >>
> >> Also move rfkill-input initialisation to subsys level so that it's
> >> available before generic drivers probing occurs.
> >>
> >> Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>
> >> ---
> >>  net/rfkill/rfkill-input.c |    7 ++++++-
> >>  1 files changed, 6 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
> >> index e4b051d..e4540fc 100644
> >> --- a/net/rfkill/rfkill-input.c
> >> +++ b/net/rfkill/rfkill-input.c
> >> @@ -181,6 +181,11 @@ static struct input_handler rfkill_handler = {
> >>
> >>  static int __init rfkill_handler_init(void)
> >>  {
> >> +     unsigned long last_run = jiffies - msecs_to_jiffies(500);
> >> +     rfkill_wlan.last = last_run;
> >> +     rfkill_bt.last = last_run;
> >> +     rfkill_uwb.last = last_run;
> >> +     rfkill_wimax.last = last_run;
> >
> > I'm not really fan of this approach
> > Can't this not be done someway in the DEFINE_RFKILL_TASK macro
> > where the entire structure is initialized?
> > That way it will automatically be set for all new rfkill task structures later.
> 
> IMHO it can't. the jiffies - mset_to_jiffies isn't a constant
> initialiser; gcc won't permit such a thing

True, but can't we make it a function then? rfkill_init_task() or something.
If we group the initialization steps as closely together as possible it will
make it easier to add new tasks later.

> >>       return input_register_handler(&rfkill_handler);
> >>  }
> >>
> >> @@ -190,5 +195,5 @@ static void __exit rfkill_handler_exit(void)
> >>       flush_scheduled_work();
> >>  }
> >>
> >> -module_init(rfkill_handler_init);
> >> +subsys_initcall(rfkill_handler_init);
> >>  module_exit(rfkill_handler_exit);
> >
> > This sounds good.
> >
> > Ivo
> >
> 
> 
> 

Re: [PATCH] RFKILL: fix input layer initialisation

[Dmitry Torokhov]

Hi Dmitry,

On Thu, Jun 19, 2008 at 03:28:41PM +0400, Dmitry Baryshkov wrote:
> Initialise correctly last fields, so tasks can be actually executed
> 
> Also move rfkill-input initialisation to subsys level so that it's
> available before generic drivers probing occurs.
> 

Rfkill-input is an implementation of default policy of handling
wifi-related key presses, it is not mandatory and should not be
required for a driver to operate, it may even not be present in the
kernel and therefore module_init is the appropriate time for its
initialization. We don't expect user to start pushing WIFI buttons
while system just starting to boot up, do we?

-- 
Dmitry

Re: [PATCH] RFKILL: fix input layer initialisation

[Dmitry Baryshkov]

On Mon, Jun 23, 2008 at 09:22:39AM -0400, Dmitry Torokhov wrote:
> Hi Dmitry,
> 
> On Thu, Jun 19, 2008 at 03:28:41PM +0400, Dmitry Baryshkov wrote:
> > Initialise correctly last fields, so tasks can be actually executed
> > 
> > Also move rfkill-input initialisation to subsys level so that it's
> > available before generic drivers probing occurs.
> > 
> 
> Rfkill-input is an implementation of default policy of handling
> wifi-related key presses, it is not mandatory and should not be
> required for a driver to operate, it may even not be present in the
> kernel and therefore module_init is the appropriate time for its
> initialization. We don't expect user to start pushing WIFI buttons
> while system just starting to boot up, do we?

I do toggle the KEY_BLUETOOTH from the kernel driver early.
The preliminary patch can be found at
http://permalink.gmane.org/gmane.linux.ports.arm.kernel/43136

The idea was to drive the bluetooth chip attached to the UART from the
uart pm hook.


-- 
With best wishes
Dmitry

Re: [PATCH] RFKILL: fix input layer initialisation

[Dmitry Torokhov]

On Mon, Jun 23, 2008 at 06:40:24PM +0400, Dmitry Baryshkov wrote:
> On Mon, Jun 23, 2008 at 09:22:39AM -0400, Dmitry Torokhov wrote:
> > Hi Dmitry,
> > 
> > On Thu, Jun 19, 2008 at 03:28:41PM +0400, Dmitry Baryshkov wrote:
> > > Initialise correctly last fields, so tasks can be actually executed
> > > 
> > > Also move rfkill-input initialisation to subsys level so that it's
> > > available before generic drivers probing occurs.
> > > 
> > 
> > Rfkill-input is an implementation of default policy of handling
> > wifi-related key presses, it is not mandatory and should not be
> > required for a driver to operate, it may even not be present in the
> > kernel and therefore module_init is the appropriate time for its
> > initialization. We don't expect user to start pushing WIFI buttons
> > while system just starting to boot up, do we?
> 
> I do toggle the KEY_BLUETOOTH from the kernel driver early.
> The preliminary patch can be found at
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/43136
> 
> The idea was to drive the bluetooth chip attached to the UART from the
> uart pm hook.
> 

Is there an actual button there? If not you should not create input
device. You probably need to talk to Henrique (CCed) about how
properly integrate rfkill support.

-- 
Dmitry

Re: [PATCH] RFKILL: fix input layer initialisation

[Henrique de Moraes Holschuh]

On Mon, 23 Jun 2008, Dmitry Torokhov wrote:
> On Mon, Jun 23, 2008 at 06:40:24PM +0400, Dmitry Baryshkov wrote:
> > On Mon, Jun 23, 2008 at 09:22:39AM -0400, Dmitry Torokhov wrote:
> > > Hi Dmitry,
> > > 
> > > On Thu, Jun 19, 2008 at 03:28:41PM +0400, Dmitry Baryshkov wrote:
> > > > Initialise correctly last fields, so tasks can be actually executed
> > > > 
> > > > Also move rfkill-input initialisation to subsys level so that it's
> > > > available before generic drivers probing occurs.
> > > > 
> > > 
> > > Rfkill-input is an implementation of default policy of handling
> > > wifi-related key presses, it is not mandatory and should not be
> > > required for a driver to operate, it may even not be present in the
> > > kernel and therefore module_init is the appropriate time for its
> > > initialization. We don't expect user to start pushing WIFI buttons
> > > while system just starting to boot up, do we?
> > 
> > I do toggle the KEY_BLUETOOTH from the kernel driver early.
> > The preliminary patch can be found at
> > http://permalink.gmane.org/gmane.linux.ports.arm.kernel/43136
> > 
> > The idea was to drive the bluetooth chip attached to the UART from the
> > uart pm hook.
> 
> Is there an actual button there? If not you should not create input
> device. You probably need to talk to Henrique (CCed) about how
> properly integrate rfkill support.

I'd be glad to help.  Torokhov got me in the loop because of an ongoing
rfkill enhancement and documentation enfort I am pushing for, that is being
fleshed out in linux-wireless.

Please describe to me *what* your driver needs to do.  As Torokhov said, it
is very weird that it would need rfkill-input for anything.  I know of no
valid use case to issue KEY_* input events except if you are a driver for
something that has a button/key that was pressed (i.e. a driver for an input
device)... and rfkill-input doesn't matter at all for these.

If your driver is issuing KEY_BLUETOOTH to sync state, that's a layering
violation, and lot allowed.  I may be able to help you fix it through the
patches being reviewed in linux-wireless.

If it is something else, I will try to help, but I need to understand the
details of what your driver is trying to accomplish, first.

-- 
  "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] RFKILL: fix input layer initialisation

[Dmitry]

Hi,

2008/6/23, Henrique de Moraes Holschuh <hmh@hmh.eng.br>:
> On Mon, 23 Jun 2008, Dmitry Torokhov wrote:
>  > On Mon, Jun 23, 2008 at 06:40:24PM +0400, Dmitry Baryshkov wrote:
>  > > On Mon, Jun 23, 2008 at 09:22:39AM -0400, Dmitry Torokhov wrote:
>  > > > Hi Dmitry,
>  > > >
>  > > > On Thu, Jun 19, 2008 at 03:28:41PM +0400, Dmitry Baryshkov wrote:
>  > > > > Initialise correctly last fields, so tasks can be actually executed
>  > > > >
>  > > > > Also move rfkill-input initialisation to subsys level so that it's
>  > > > > available before generic drivers probing occurs.
>  > > > >
>  > > >
>  > > > Rfkill-input is an implementation of default policy of handling
>  > > > wifi-related key presses, it is not mandatory and should not be
>  > > > required for a driver to operate, it may even not be present in the
>  > > > kernel and therefore module_init is the appropriate time for its
>  > > > initialization. We don't expect user to start pushing WIFI buttons
>  > > > while system just starting to boot up, do we?
>  > >
>  > > I do toggle the KEY_BLUETOOTH from the kernel driver early.
>  > > The preliminary patch can be found at
>  > > http://permalink.gmane.org/gmane.linux.ports.arm.kernel/43136
>  > >
>  > > The idea was to drive the bluetooth chip attached to the UART from the
>  > > uart pm hook.
>  >
>  > Is there an actual button there? If not you should not create input
>  > device. You probably need to talk to Henrique (CCed) about how
>  > properly integrate rfkill support.
>
>
> I'd be glad to help.  Torokhov got me in the loop because of an ongoing
>  rfkill enhancement and documentation enfort I am pushing for, that is being
>  fleshed out in linux-wireless.
>
>  Please describe to me *what* your driver needs to do.  As Torokhov said, it
>  is very weird that it would need rfkill-input for anything.  I know of no
>  valid use case to issue KEY_* input events except if you are a driver for
>  something that has a button/key that was pressed (i.e. a driver for an input
>  device)... and rfkill-input doesn't matter at all for these.
>
>  If your driver is issuing KEY_BLUETOOTH to sync state, that's a layering
>  violation, and lot allowed.  I may be able to help you fix it through the
>  patches being reviewed in linux-wireless.

Yes, probably that's the correct description :)
The driver is tracking the state of serial port and respectievly
powering the bt chip
up or down. I'll check the linux-wireless list.

>
>  If it is something else, I will try to help, but I need to understand the
>  details of what your driver is trying to accomplish, first.

Thank you for your help!

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