Re: [PATCH] rfkill: create useful userspace interface

[Marcel Holtmann <marcel@holtmann.org>]

Hi Henrique,

> > > > We just need to fix the platform drivers then. They should not set
> > > > global states since that is not what they are controlling. They control
> > > 
> > > We should change things, yes.  So that the platform stores the global
> > > state.  That was half-broken on the old core (the platform stored the
> > > state of its own device, which could be out of sync with the global
> > > state), but the part of it setting the global state is correct.
> > > 
> > > That needs a new in-kernel API to tie the core to platform drivers
> > > capable of storing global states without causing problems when drivers
> > > are unloaded, but it is not hard.
> > > 
> > > As for NVS events, they have a clear use case: to let rfkilld know which
> > > global states it could leave alone the first time it loads, and which
> > > ones have to be restored...
> > 
> > show me an example of a platform device that stores the global state. I
> > think you are confusing the word platform as in system with a platform
> > device. The ThinkPad Bluetooth and WWAN switches are platform devices
> > and control each one specific device. Same goes for the EeePC. They are
> > not controlling a global state.
> 
> I don't know what big difference you see between the two uses of "platform",
> but I will just work around it to get something useful out of this mail.
> 
> The laptop stores in NVS the state of its 'switches'.  This is as close as
> one gets from 'storing the global state'.   When the laptop boots,
> these devices get set by the firware to the state in NVS.  It is the best
> place to store global state, because these devices will be in their proper
> state (i.e. matching what will be the global state when the rfkill core
> loads) all the time.  It also gives you for free multi-OS/multi-kernel state
> storage for these devices, and compatibility with BIOSes that let you define
> the initial state for the devices in the firmware configuration, etc.

it stores the state of its switches and why should these be enforced as
a global state? Who says that this is a global state? For me that sounds
like policy.

> There.  I didn't use the 'p' word once.  Now, please tell me why show we
> just dump the storage done by the firmware in NVS, and instead store the
> global state for those switches in userspace?

We are getting OP_ADD for these switches when opening /dev/rfkill and it
is up to a userspace policy to either enforce this on all attached
devices or not.

> If it was a extremely complicated and fragile thing to do, it would be a
> different matter, but it isn't.  It isn't even ugly code, even after we fix
> it to be all about global states.

This is not about ugly code or anything alike. We wanna get the RFKILL
subsystem cleaned up now. And just overloading it with things that might
be a good idea is bad. Enhancing it later when it is actually needed is
way easy. And I am not convinced even a little bit that we should treat
the firmware stored states as global. They are not global states. You
are just abusing them as this.

Regards

Marcel