Message-ID: <20080414100018.ZZRA012@mailhub.coreip.homeip.net>
Date:	Mon, 14 Apr 2008 10:16:06 -0400
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Ivo van Doorn <ivdoorn@...il.com>
Cc:	Henrique de Moraes Holschuh <hmh@....eng.br>,
	linux-kernel@...r.kernel.org,
	"John W. Linville" <linville@...driver.com>
Subject: Re: [PATCH 4/8] rfkill: add read-write rfkill switch support

On Mon, Apr 14, 2008 at 02:00:41PM +0200, Ivo van Doorn wrote:
> On Monday 14 April 2008, Henrique de Moraes Holschuh wrote:
> > On Sat, 12 Apr 2008, Ivo van Doorn wrote:
> > > > Currently, rfkill supports only write-only rfkill switches.  There is no
> > > > provision for querying the current switch state from the hardware/firmware.
> > > 
> > > No that is by design, the input_polled_dev interface is there for polling.
> > 
> > I have looked into this, now.  And I have to say I am not impressed with
> > the use of the input system for this.  My guess is that somehow the need
> > to issue state changes for "the stuff the user slides/presses" got
> > confused with the need to keep the state of the "hardware and software
> > that enables/disables radios", just because both are called "rfkill
> > switches".
> >
> > The current flow for read/write switches (i.e. using input-polldev) is
> > this:
> > 
> > The players:
> > A: RFKILL class code: write-only with a state cache
> > B: rfkill-input
> > C: the code supporting the device registered with the rfkill class
> > D: something else, like a hardware signal line or firmware
> > 
> > The flow:
> > C: sets initial state, and registers rfkill device with A.
> > A: keeps state cache in sync with any changes that goes through it
> > D: toggles radio state directly, A doesn't know about it.
> > C: using pooling or some other method, finds out what D did.
> > C: issues an INPUT EVENT(!!) to force A to resync itself
> > B: traps the INPUT EVENT, and tells A to change radio state FOR ALL
> >    RADIOS of that type
> > A: toggles the radio to the state B wanted.
> > 
> > It is no surprise that b43 can't work right if it is using that flow.
> > 
> > Here's what is wrong with it:
> > 
> >   1. Usage of input events that matter to an entire group of drivers
> >      to syncronize something that is specific to a single device
> >
> >   2. Dependency of an OPTIONAL input handler to do it, and let's not
> >      forget this can become a MAJOR mess if userspace decides to butt
> >      in, which it is indeed *expected* to do
> 
> The input handler is optional, because userspace *should* actually do all
> the work. Userspace should listen to the registered input device and
> it is userspace that should select the input device it wants to listen to.
> (perhaps the laptop has 2 or more keys, but the user wants all interfaces
> to be toggled when any key has been pressed).
> 
> >   3. (some drivers like b43, not a rfkill class issue): the use of
> >      KEY_  events to set a desired state for a radio is wrong.  That
> >      event tells rfkill to toggle the radio state, and NOT to set it
> >      to a particular state.  One would need EV_SW events for this.
> 
> That is a rfkill issue due to lack of correct documentation and problems
> with the rfkill implementation. I wouldn't blame the drivers for that. ;)
> 
> >   4. Extremely heavy-weight, convoluted, complex information flow
> >      path, prone to races when there are multiple rfkill devices of
> >      the same type involved.
> > 
> >   5. Breaks completely if user_claim is used.
> > 

What exactly breaks with user_claim? I think the general issue is that
rfkill was originally not supposed to be used with switches that can't
be programmatically controlled. If we have a switch that mechanically
cuts off power to the RF transmitter rfkill can't relly do anything.
The input side (input polldev or other kind of button/switch/slider
inmplementation) still can issue KEY_WLAN or something to indicate that
user toggled the state and userspace or kernel may try to shut off the
rest of RF transmitters in the box but as far as this particlular
switch goes it is game over. I expected that here we'd just rely on
netif_carrier_* to notify the network side of things about interface
losing connection.

Unfortunately (or may be fortunately, I am not quite sure yet) rfkill
started to be used for mechanical type switches as well, disabling
user_claim option, so now we do need the "read-only" kind of rfkill
switch support that allows setting switch state directly from the
driver.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists