Re: [PATCH] rfkill: create useful userspace interface

[Alan Jenkins <alan-jenkins@tuffmail.co.uk>]

Marcel Holtmann wrote:
> Hi Henrique,
>
>   
>>>> I don't think we should expect userspace to know whether or not a device
>>>> has a persistent state.  Yes, it _could_ maintain whitelists, but why
>>>> should it have to if the driver already knows?
>>>>         
>>> If you want that, then the best approach seems an extra sysfs attribute
>>> for this. It is not intrusive on the event API and lets udev etc. have
>>> these information, too.
>>>       
>> I have no problems with either approach.  As long as the information of
>> which devices have restored their initial state from NVS is available to
>> userspace, it is enough.
>>     
>
> just to get the semantic right here. We are not telling userspace if a
> state has been restored or not. We are telling userspace that this
> specific RFKILL switch is capable of storing something in a persistent
> state over boot. There is a difference here.
>
> If a RFKILL driver claims it is capable of persistent storage then it
> better work or it should not claims it. Either it does it all the time
> or doesn't do it at all. Otherwise we end up in policy again and that is
> not the job of the kernel.
>
>   
>> Do note that this information also needs to be available for resume (state
>> should be checkpointed to NVS on sleep, and restored from NVS on resume.  I
>> believe tpacpi does this, but if it doesn't, I will fix it eventually).
>>     
>
> Correct. That is the job of the driver. If it is broken, that needs
> fixing.
>   

The core needs fixing too, currently it restores state for all devices
on resume.

This is my fault again, for coming up with scenarios you probably don't
care about :-).  The problem is that suspend to disk provides a
possibility that the state will change for _any_ driver.  It's more
obvious with rfkill-input and NVS, if the wireless toggle key is pressed
when the disk image is being written out.  But you can also contrive it
with no NVS and rfkilld, if rfkilld gets started in the initramfs of the
resume kernel.  We took the easy way out, rather than adding resume
handlers to all drivers, or trying to work out what the real design
problem was :-).

I hope that explains the issue.  I agree with your logic, I just want to
be clear that it needs more work on the rfkill core.  Drivers with NVS
should have a resume handler to call rfkill_set_sw_state(), but for this
to work the core will need to stop restoring their state (for NVS
drivers only).  As a detail, I think this behaviour difference with NVS
means it should be flagged with a more explicit API, e.g.
rfkill_init_persistent_sw_state().

rfkill-input would like another (even more intrusive) hack here to set
the global state on resume.  But I for one can live without it for the
transition.  I think NVS state change over suspend is much more of a
corner case.  At least on eeepc-laptop it only seems to happen if the
user does something relatively odd.  And the worst that will happen is
they have to press the wireless-toggle key a second time before it
starts working.

alan