2009-01-16 19:53:10

by Werner Almesberger

[permalink] [raw]
Subject: rfkill: life after driver death

First of all, thanks for the advice on permissible rfkill semantics !
I've implemented rfkill for the AR6k driver on the FreeRunner.

Now there's another interesting problem that seems to want a more
general solution: how to handle devices that regularly disappear and
then get re-created ?


Problem statement:

In our case, the AR6k module sits on the SDIO bus. When we suspend,
the SDIO host driver does a sdio_bus_remove, which removes the entire
AR6k device. This would also include any rfkill device it owns.

Since rfkill is supposed to restore the previous state on resume, I
let a platform driver "own" the rfkill device instead, with a private
interface between the AR6k driver and this platform driver to perform
the actual blocking/unblocking and also to tell the AR6k driver
whether it is allowed to activate the SDIO function or not on resume.

We don't have a simple transmitter on/off line, so we control the
transmitter with the next simplest mechanism, which is through SDIO
function activation.

Furthermore, it's not only suspend/resume that makes the driver
disappear, but user space may also do other things that cause it to
be removed, e.g., unbind through sysfs or module removal.


My present hac^H^H^Hsolution:

This approach works quite satisfyingly, except that the private
interface between driver and platform isn't nice. I could apply some
window dressing to make it look a bit nicer, but I think this is
something that would better be handled by the general rfkill
infrastructure.

What the interface does is to track rfkill changes while the driver
is absent, and notify the driver in a race-free way of the rfkill
state at the time of driver initialization. The initialization of
the driver looks as follows:

my_driver_probe()
{
...
not_blocked = query_rfkill_and_block_further_rfkill_changes();
if (not_blocked) {
err = finish_initializing_my_driver();
if (err) {
unlock_rfkill_changes();
goto failure;
}
atomically_setup_rfkill_handler_and_unblock_changes(
my_driver_rfkill_handler);
}
...
}


Proposed generalization:

Since there are plenty of other hotpluggable devices around, I think
persistent state would generally be desirable for rfkill. Maybe
something along the lines of allowing a driver to detach its rfkill
device and to let a later instance to re-attach to it. Example:

struct rfkill *rfkill_try_reattach(const char *name)

Find the named rfkill device, block further state changes, and
return a pointer to it. If no device is found, return NULL.
On success, caller can now use rfkill->state to decide what to
do. On failure, it proceeds by creating a new rfkill device.

void rfkill_attach_handler(struct rfkill *rfkill, int (*toggle_radio)(...),
void *data);

Attach a toggle function to a rfkill device previously returned
by rfkill_try_reattach, and unblock rfkill state changes.

void rfkill_detach(struct rfkill *rfkill);

Detach a driver from a rfkill device. A new driver instance for
the same device can later re-attach to the rfkill device with
rfkill_try_reattach.

Does this sound reasonable so far ?

One potential issue would be name collisions, e.g., two drivers trying
to attach to the same rfkill device. I can think of three possible
solutions for this:

1) require (and enforce) unique names,
2) allow multiple rfkill devices with the same name, or
3) allow multiple devices to share the same rfkill device.

The semantics of 3) would be a bit tricky. 1) would be a bit of a trap,
because it would be just too convenient to assume there's only one
device. In case 2), the kernel would not know to distinguish among
rfkill devices with the same name, so rfkill_try_reattach would yield
an arbitrary one, or NULL if there is one but it's already attached.
This passes the problem of maintaining a consistent state up to user
space. I kind of like that ;-)

Opinions ?

- Werner


2009-01-16 20:59:35

by Werner Almesberger

[permalink] [raw]
Subject: Re: rfkill: life after driver death

Henrique de Moraes Holschuh wrote:
> You want the rfkill core to track per-device-instance rfkill states, or
> would the per-type tracking it does already be enough?

This is a good question :-) As long as we're only concerned with making
sure no RF power goes into the air, even a global switch for all RF
devices may be sufficient.

There may also be cases where only certain bands or signal strenghts
are prohibited. E.g., BT may be tolerated while GSM isn't. The
current rfkill architecture covers this.

At the other end of the spectrum you have people who use rfkill as
a poor man's power management. These would definitely want a more
fine-grained control.

There may be more usage stories, with yet different requirements.

I think may uneasiness with the current situation comes mainly from
having an unreliable per-device rfkill control. You can switch it on
or off, but whether this setting lasts across suspend/resume depends
on the implementation. There's also a contradiction with rfkill.txt,
section 3.1, bullet 6: "During resume, rfkill will try to restore its
previous state."

But I think the per-type controls could in fact be good enough for
all practical purposes. In the worst case, a dedicated controller
could just turn everything off and activate things on a case-by-case
basis.

But then we're a few sysfs interfaces short. Is there any specific
reason why there is a control for setting the global default state
(/sys/module/rfkill/parameters) but there are no controls for:

- setting the default state by type,
- setting the current state by type, and
- setting the current state ?

> But there are other questions: why? You *NEED* somewhere to attach the
> rfkill kobjects to, and that somewhere can't very well be a device that is
> getting hotunplugged, as it will disappear. So, you will need something
> like the platform device you used, anyway...

In my scenario, a driver calling rfkill_detach would say "I might be
back".

But I think I like the idea of making the per-type controls more
powerful.

- Werner

2009-01-16 22:01:40

by Werner Almesberger

[permalink] [raw]
Subject: Re: rfkill: life after driver death

Henrique de Moraes Holschuh wrote:
> Yeah, they are sitting in my queue, but I dislike them so much I always find
> a reason not to post them and do something else in thinkpad-acpi, instead...
>
> I will see if I can bring myself to post them during the weekend.

Great, thanks !

> Then, it is best that we stick with what already is there (the per-type
> control).

Sounds like a plan ;-)

- Werner

Subject: Re: rfkill: life after driver death

On Fri, 16 Jan 2009, Werner Almesberger wrote:
> Since rfkill is supposed to restore the previous state on resume, I
> let a platform driver "own" the rfkill device instead, with a private
> interface between the AR6k driver and this platform driver to perform
> the actual blocking/unblocking and also to tell the AR6k driver
> whether it is allowed to activate the SDIO function or not on resume.

This is the way to do it, yes. In fact, that's how most (all?) Bluetooth
and WWAN rfkilling is done...

> Furthermore, it's not only suspend/resume that makes the driver
> disappear, but user space may also do other things that cause it to
> be removed, e.g., unbind through sysfs or module removal.

Indeed.

> My present hac^H^H^Hsolution:
>
> This approach works quite satisfyingly, except that the private
> interface between driver and platform isn't nice. I could apply some
> window dressing to make it look a bit nicer, but I think this is
> something that would better be handled by the general rfkill
> infrastructure.
>
> What the interface does is to track rfkill changes while the driver
> is absent, and notify the driver in a race-free way of the rfkill
> state at the time of driver initialization. The initialization of
> the driver looks as follows:

It sort of already does that. Look at the rfkill core, it tracks the
"desired rfkill state" of each device *TYPE*, and when you register a new
rfkill switch, it calls rfkill_toggle_radio() on it to match the "desired
rfkill state" for that type.

> Proposed generalization:
>
> Since there are plenty of other hotpluggable devices around, I think
> persistent state would generally be desirable for rfkill. Maybe
> something along the lines of allowing a driver to detach its rfkill
> device and to let a later instance to re-attach to it. Example:

You want the rfkill core to track per-device-instance rfkill states, or
would the per-type tracking it does already be enough?

But there are other questions: why? You *NEED* somewhere to attach the
rfkill kobjects to, and that somewhere can't very well be a device that is
getting hotunplugged, as it will disappear. So, you will need something
like the platform device you used, anyway...

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

Subject: Re: rfkill: life after driver death

On Fri, 16 Jan 2009, Werner Almesberger wrote:
> But then we're a few sysfs interfaces short. Is there any specific
> reason why there is a control for setting the global default state
> (/sys/module/rfkill/parameters) but there are no controls for:
>
> - setting the default state by type,
> - setting the current state by type, and
> - setting the current state ?

Yeah, they are sitting in my queue, but I dislike them so much I always find
a reason not to post them and do something else in thinkpad-acpi, instead...

I will see if I can bring myself to post them during the weekend.

Anyway, right now access to those is only possible through rfkill-input and
input-events (for those types that HAVE an input event related to them...).

> > But there are other questions: why? You *NEED* somewhere to attach the
> > rfkill kobjects to, and that somewhere can't very well be a device that is
> > getting hotunplugged, as it will disappear. So, you will need something
> > like the platform device you used, anyway...
>
> In my scenario, a driver calling rfkill_detach would say "I might be
> back".
>
> But I think I like the idea of making the per-type controls more
> powerful.

Then, it is best that we stick with what already is there (the per-type
control).

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