Re: [Bug 106031] Regression in 4.2.x: in airplane mode each time I open my laptop lid

[Gabriele Mazzotta <gabriele.mzt@gmail.com>]

On 23/10/2015 11:00, Pali Rohár wrote:
> On Friday 23 October 2015 01:29:12 Gabriele Mazzotta wrote:
>> On 22/10/2015 16:17, Pali Rohár wrote:
>>> On Thursday 22 October 2015 15:43:46 Gabriele Mazzotta wrote:
>>>> It's the F2 key. Depending on the value passed to the RBTN, it acts as
>>>> hw slider or sw toggle.
>>>
>>> Ok, we have differences in terminology and everybody understood this
>>> problem differently.
>>>
>>>
>>> To correct this situation, first define about what we are talking about:
>>>
>>> * HW slider: It is hardware slider switch which as two positions ON and
>>>               OFF. Position exactly defines hardware state of wireless
>>>               radio devices. Not possible (or should not) to remap.
>>>
>>> * SW hotkey toggle button: It is button or key, act in same way as any
>>>                             other key on keyboard, controlled by SW
>>>                             (if all drivers are installed, etc).
>>
>> Sorry for the confusion, I know I've been using "HW slider" improperly,
>> but I thought it was clear. Although it's a button, the BIOS makes it
>> behave like an hardware slider when configured to behave as such
>> through RBTN. The state of radio devices is changed by the BIOS when
>> the function key is pressed, this state is never changed until the user
>> presses the function key again (at least if we ignore the fact that we
>> can perform some special SMI calls), this state is saved in the EC and
>> both userspace and the kernel don't receive any event other than rfkill
>> events (if we don't consider the WMI events ignored by dell-wmi).
>>
>> I think this behavior is closer to the one of an hardware slider
>> rather than the one of a software hotkey, which I can still request
>> through the RBTN method.
>>
>>> Next I think that this hypothesis is truth:
>>>
>>> * Every machine on which is binded ACPI dell-rbtn.ko driver has either
>>>    HW slider or SW toggle. Not both!
>>>
>>> Correct?
>>
>> It depends on how you classify my laptop. The behavior changes
>> completely depending on the value passed to RBTN.
>>
>> Obviously it can behave either as HW slider (again, not an actual
>> slider) or SW toggle, but I can choose between the two.
>>
>
> If it has keyboard button and not switch with visible two positions, it
> is "toggle button" (from HW perspective).

Yes, sorry for the confusion.

>>> Then follows my expected behaviour for HW slider:
>>>
>>> * State of HW slider position is exported by kernel as rfkill device.
>>>
>>> * In any case HW slider position (ON/OFF) match hard rfkill state device
>>>    (rfkill device state is correct also after resume, wake from hibernate).
>>>
>>> * Immediately after user change position of HW slider, rfkill device
>>>    reflect it.
>>>
>>>
>>> And then expected behaviour for SW toggle button:
>>>
>>> * Every time when user press it, kernel just send input event "wireless
>>>    key pressed" to userspace.
>>>
>>> * Kernel does not send event "wireless key pressed" when user did not
>>>    pressed it (e.g. after resuming from suspend, waking from hibernate).
>>
>> Yes, but we don't know exactly when the user pressed the button. As I
>> said, the BIOS might send an event that triggers an input event even if
>> the user didn't press anything.
>>
>
> Ok. This is strange if BIOS send event "changed" even if it was not
> really changed.
>
> Is BIOS sending this incorrect event only after waking from suspend? Or
> it is also randomly at any time?

I looked at the ACPI table. It happens only on resume (or when the user
presses the function key).

> If we know that it send incorrectly only after suspend, we can drop this
> event. But if it is completely randomly, we probably cannot do anything
> (and just do not use driver in this case).
>
>>> * Kernel should provide rfkill devices to "soft" block appropriate
>>>    wireless cards.
>>>
>>> * Make sure that BIOS/firmware in any case does not change radio rfkill
>>>    state of any wireless card and wireless cards stay in same state as
>>>    before (no enable/disable or changing hard/soft rfkill state).
>>
>> This is the problem. We can't differentiate notifications sent to
>> DELLABCE/DELLRBTN by the BIOS because it felt like it was right to do so
>> (e.g. after resuming) or because the user pressed the function key.
>>
>
> In my opinion it is better to ignore user key press after resume, if it
> fix our problem. Better as false-positive event.

The following appears to work really well. The notification arrives
before rbtn_resume() has been executed, so the extra event is ignored.

diff --git a/drivers/platform/x86/dell-rbtn.c 
b/drivers/platform/x86/dell-rbtn.c
index cd410e3..1d64b72 100644
--- a/drivers/platform/x86/dell-rbtn.c
+++ b/drivers/platform/x86/dell-rbtn.c
@@ -28,6 +28,7 @@ struct rbtn_data {
  	enum rbtn_type type;
  	struct rfkill *rfkill;
  	struct input_dev *input_dev;
+	bool suspended;
  };


@@ -220,9 +221,33 @@ static const struct acpi_device_id rbtn_ids[] = {
  	{ "", 0 },
  };

+#ifdef CONFIG_PM_SLEEP
+static int rbtn_suspend(struct device *dev)
+{
+	struct acpi_device *device = to_acpi_device(dev);
+	struct rbtn_data *rbtn_data = acpi_driver_data(device);
+
+	rbtn_data->suspended = true;
+
+	return 0;
+}
+
+static int rbtn_resume(struct device *dev)
+{
+	struct acpi_device *device = to_acpi_device(dev);
+	struct rbtn_data *rbtn_data = acpi_driver_data(device);
+
+	rbtn_data->suspended = false;
+
+	return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(rbtn_pm_ops, rbtn_suspend, rbtn_resume);
+
  static struct acpi_driver rbtn_driver = {
  	.name = "dell-rbtn",
  	.ids = rbtn_ids,
+	.drv.pm = &rbtn_pm_ops,
  	.ops = {
  		.add = rbtn_add,
  		.remove = rbtn_remove,
@@ -384,6 +409,9 @@ static void rbtn_notify(struct acpi_device *device, 
u32 event)
  {
  	struct rbtn_data *rbtn_data = device->driver_data;

+	if (rbtn_data->suspended)
+		return;
+
  	if (event != 0x80) {
  		dev_info(&device->dev, "Received unknown event (0x%x)\n",
  			 event);

>>> If last sentence is not truth, then kernel must not send "wireless key
>>> pressed" event to userspace and act like "no key was pressed".
>>>
>>>
>>> Make this sense? Or are there any objections about this behaviour?
>>>
>
> So if you are OK with above my description (from previous email), then
> we should fix dell-rbtn to act as it (if it is not implemented already
> and correctly).
>
> So my next question is: Does dell-rbtn behave like my description? If
> not what are differences and what needs to be fixed?

Everything works as expected. We only have to deal with this extra
notification.