Re: [PATCH] rfkill: create useful userspace interface

[Marcel Holtmann <marcel@holtmann.org>]

Hi Alan,

> > I really don't understand why this is needed. What benefit does it give
> > us compared to just sent OP_CHANGE and OP_CHANGE as an update. My X200
> > for example does this anyway on suspend/resume.
> >   
> 
> This is required for boot only.  I have no reason for this event to be 
> generated on resume.
> 
> The same effect could be had by generating an OP_CHANGE on f_open, 
> _only_ when a platform driver has provided a value from NVS.  But it 
> does seem clearer to make it a different type of event.

that is my whole point. If the kernel driver wants to preserve these
then it can just issue OP_ADD to notify use about the current state of
the values. The OP_ADD gets send once you open /dev/rfkill and if they
not match with our policy we have to change them anyway. I really don't
see the need for an extra operation here. Let me ask this again, how is
it different from just sending the OP_ADD and then let rfkilld decide to
either use that value or enforce its own policy. If the wish is to
enforce policy you can't do anything about it anyway.

> > So what is rfkilld suppose to be doing when receiving this report? What
> > is the expected behavior? Why do we bother with multi-OS crap here? I am
> > really unclear what are we trying to solve here.
> 
> In order to replicate the kernel behavior, it is expected that you set 
> your internal state from this event.  E.g. when the user next presses 
> the wireless toggle key, you set the inverse of that internal state.
> 
> Since this event is generated by a platform driver, you can expect it to 
> be present following coldplug (the udev initscript).  If the event is 
> not present after coldplug, you may then issue OP_CHANGE yourself, to 
> e.g. restore the state from a file.  You would not be expected to handle 
> OP_NVS_REPORT after startup.  (Unless the daemon is restarted).
> 
> Replicating the kernel behavior will allow us to avoid causing a couple 
> of niggly little regressions on at least two platforms.  It preserves 
> the behavior when dual-booting (possibly between different linux 
> distros), and when the BIOS setup screen exposes the NVS state as an 
> option.  The new behavior you suggest will annoy any users who have 
> become used to these scenarios "just working". 
> 
> You may not use these platforms yourself.  But I'm as annoyed as 
> Henrique is, we don't want to impose regressions just because other 
> platforms don't implement the feature.
> 
> Why the fuss about implementing this, it seems easy enough?  Start 
> rfkilld after udev (like everything else).  If you get NVS_REPORT, then 
> use those states.  Fill in any other states from defaults or state files 
> and issue OP_CHANGE for them, just as you're already planning.  Ignore 
> any subsequent NVS_REPORTs.  That should cover it.
> 
> It's the cost for starting from a working implementation.  You benefit 
> from having existing drivers and users, you pay by not breaking them 
> without good reason.

I really don't care about current behavior, because that has been just a
hack anyway. And it happens to work if there is proper BIOS support. We
are at the point now where we stop working around a complicated and in
some cases broken implementation. Overloading it with weird special
cases is just wrong and so far I am not buying into any of the arguments
here. The point behind the whole effort from Johannes is to finally fix
RFKILL support. If it breaks current behavior, I couldn't care less in
some cases.

So Johannes and I talked about it a lot last week. And we will be doing
rfkilld so finally deprecated the broken idea of rfkill-input and move
the policy into userspace where it belongs. To make this clear, the
concept of cross-OS state keeping is broken. Having the BIOS or a
different OS dictate policy makes no sense.

Regards

Marcel