Re: [RFC] rfkill: rewrite

Johannes Berg
2009-03-30T21:26:53Z

[-- Attachment #1: Type: text/plain, Size: 4088 bytes --]

On Mon, 2009-03-30 at 18:15 -0300, Henrique de Moraes Holschuh wrote:

> > +struct rfkill_ops {
> > +#if defined(CONFIG_RFKILL) || defined(CONFIG_RFKILL_MODULE)
> > + bool (*poll_hw_block)(void *data);
> > + bool (*query_state)(void *data);
> > + void (*set_block)(void *data, bool blocked);
> > #endif
>
> Missing error handling on set_block... could you change that to:
>
> int (*set_block)(void *data, bool blocked)
>
> with the usual kernel error semathics (0 = no error, < 0 = specific
> error)? And of course, do the error handling on the core?
>
> I don't know about wireless network drivers, but platform drivers want
> to be able to return errors, they _do_ happen when you are doing ACPI
> dances: EBUSY, EIO, ENOMEM...
>
> If something in userspace wants to block a transmitter, it better get
> an error back if the transmitter could not be blocked. The opposite
> (unblock) is less critical in the safety sense, but no less critical
> in the usability sense, so it wants proper error feedback as well.

I was considering that, but the current users of the API have no chance
to act on the error. It _might_ make sense to put it back when we add
the userspace API back, but what should userspace do in that case?

OTOH if it does return an error the sw block bit probably shouldn't be
set, so I guess I'll change that.

> > +/**
> > + * rfkill_register - Register a rfkill structure.
> > + * @rfkill: rfkill structure to be registered
> > + *
> > + * This function should be called by the network driver when the rfkill
>
> minor nit: s/network/rfkill controller/

something like that maybe.

> > -Important terms for the rfkill subsystem:
> > -
> > -In order to avoid confusion, we avoid the term "switch" in rfkill when it is
> > -referring to an electronic control circuit that enables or disables a
> > -transmitter. We reserve it for the physical device a human manipulates
> > -(which is an input device, by the way):
> > -
> > -rfkill switch:
> > -
> > - A physical device a human manipulates. Its state can be perceived by
> > - the kernel either directly (through a GPIO pin, ACPI GPE) or by its
> > - effect on a rfkill line of a wireless device.
> > -
> > -rfkill controller:
> > -
> > - A hardware circuit that controls the state of a rfkill line, which a
> > - kernel driver can interact with *to modify* that state (i.e. it has
> > - either write-only or read/write access).
> > -
> > -rfkill line:
> > -
> > - An input channel (hardware or software) of a wireless device, which
> > - causes a wireless transmitter to stop emitting energy (BLOCK) when it
> > - is active. Point of view is extremely important here: rfkill lines are
> > - always seen from the PoV of a wireless device (and its driver).
> > -
> > -soft rfkill line/software rfkill line:
> > -
> > - A rfkill line the wireless device driver can directly change the state
> > - of. Related to rfkill_state RFKILL_STATE_SOFT_BLOCKED.
> > -
> > -hard rfkill line/hardware rfkill line:
> > -
> > - A rfkill line that works fully in hardware or firmware, and that cannot
> > - be overridden by the kernel driver. The hardware device or the
> > - firmware just exports its status to the driver, but it is read-only.
> > - Related to rfkill_state RFKILL_STATE_HARD_BLOCKED.
>
> Are you sure you want to do away with the above definitions?
>
> The use of 'rfkill switch' for fundamentally different things caused a
> lot of confusion in the past. That's why I had to come up with
> 'rfkill controller', and 'rfkill line'. Maybe you can do away with
> the 'rfkill line' now, but controller/switch is something that is
> helpful to avoid getting input devices and non input devices mixed up.

This isn't actually helpful since the terms aren't very clearly used...

I think describing it as a "transmitter driver" is much better anyway.
And we don't need any sort of terms whether it's an rfkill line, or
whatever, it just doesn't matter -- all we need to know it's
hardware-off.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]