Re: [RFC] rfkill - Add support for input key to control wireless radio

["Dmitry Torokhov" <dmitry.torokhov@gmail.com>]

On 3/30/07, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> > > >
> > > > My only concern is where rfkill code should live. Since it is no
> > > > longer dependent on input core (embedded systems might disable
> > > > rfkill-input and use bare rfkill and control state from userspace)
> > > > it does not need to live in drivers/input.
> > >
> > > How about:
> > > rfkill -> drivers/misc
> >
> > Not in net?
>
> Well in drivers/net are the network drivers but not the irda and bluetooth drivers,
> those are located in different folders in drivers/ so I think misc would be the most
> suitable location.

We could also consider the ./net itself. rfkill is not a driver, it is
a facility.

> > >  - input_polldev
> > >        - Handlers polling, where the driver should check what the previous state was and the driver
> > >          should send the event to rfkill.
> >
> > This is the input device visible to userspace and kernel. It polls
> > state of the button and sends KEY_WLAN/KEY_BLUETOOTH events through
> > input layer. They get distributed through various input handlers such
> > as evdev and rfkill-input. Evdev provides route for events to
> > userspace where application can listen to events and then toggle RF
> > switches according to the current policy via
> > /sys/class/rfkill/rfkillXXX/state. Rfkill-input provides in-kernel
> > route for events into rfkill system. If rfkill-input is loaded
> > switches that are not claimed by userspace will be toggled
> > automatically.
> >
> > Does this make sense?
>
> Yes, but what if the user loads both modules or has them both compiled in?
> Shouldn't there be some protection against that, since both handlers should not
> be active at the same time.

Why not? evdev is just a delivery transport for input events to
userspace. Even if user wants the kernel to control state of RF
switches (which I expect most users woudl want) there still could be
applications interested in knowing that used turned off wireless. And
if userspace really wishes to control switch all by itself it can
"claim" it.

I guess what you are missing is that input event generated by a device
is pushed through every handler bound to the device so there is no way
for a "wrong" handler to "steals" an event from "right" handler. They
all work simultaneously.

> > > personally I would prefer enforcing drivers to call
> > > allocate()
> > > register()
> > > unregister()
> > > free()
> > >
> > > Especially with unregister() doing the same steps as free() (put_device)
> > > might be somwhat confusing. But might be just me. ;)
> > >
> >
> > I know but for refcounted objects you can't really tell when they will
> > actually be freed. It depends when their last user drops off.
>
> Then perhaps rfkill_register could call put_device() when it fails, and free()
> can be removed entirely. That way it would we prevent some driver
> to call free() anyway.
>

That would make error handling in ->probe() methods a bit unwieldy I
think - if you are using the standard "goto err_xxx; goto err_yyy;"
technique then you have to conditionally call rfkill_free(). Hovewer
if register simply fails and does not free anything and you call
rfkill_register() last (which you need to do because driver has to be
almost fully functional to be able to serve toggle_radio calls) you
can always call rfkill_free() if something fails.

-- 
Dmitry