diff mbox

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

Message ID 5628BDF8.9030506@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Gabriele Mazzotta Oct. 22, 2015, 10:44 a.m. UTC
On 22/10/2015 10:51, Pali Rohár wrote:
> On Thursday 22 October 2015 10:26:47 Gabriele Mazzotta wrote:
>> I'll see if I can find a batter way to deal with this problem,
>> dell-laptop can detect the presence of an hardware switch.
>
> dell-rbtn.ko has acpi method CRBT which returns if notebook has hw switch or
> toggle key.
>
> And rfkill implementation in dell-laptop.ko does not work correctly on
> XPS machines. And because userspace implementation of smbios works fine,
> I think that problem is in kernel driver rather in BIOS/firmware...

I was thinking about something such as the following, which should favor
the hardware slider when available. This should work if my assumption is
correct, that is the laptops with a working RBTN method are the ones
having problems.

--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Pali Rohár Oct. 22, 2015, 10:50 a.m. UTC | #1
On Thursday 22 October 2015 12:44:08 Gabriele Mazzotta wrote:
> On 22/10/2015 10:51, Pali Rohár wrote:
> >On Thursday 22 October 2015 10:26:47 Gabriele Mazzotta wrote:
> >>I'll see if I can find a batter way to deal with this problem,
> >>dell-laptop can detect the presence of an hardware switch.
> >
> >dell-rbtn.ko has acpi method CRBT which returns if notebook has hw switch or
> >toggle key.
> >
> >And rfkill implementation in dell-laptop.ko does not work correctly on
> >XPS machines. And because userspace implementation of smbios works fine,
> >I think that problem is in kernel driver rather in BIOS/firmware...
> 
> I was thinking about something such as the following, which should favor
> the hardware slider when available. This should work if my assumption is
> correct, that is the laptops with a working RBTN method are the ones
> having problems.
> 

Hm... wait! There are machines with HW slider and type is RBTN_TOGGLE?

I thought that all machines with HW slider has type RBTN_SLIDER and
others have RBTN_TOGGLE.
Gabriele Mazzotta Oct. 22, 2015, 10:54 a.m. UTC | #2
On 22/10/2015 12:50, Pali Rohár wrote:
> On Thursday 22 October 2015 12:44:08 Gabriele Mazzotta wrote:
>> On 22/10/2015 10:51, Pali Rohár wrote:
>>> On Thursday 22 October 2015 10:26:47 Gabriele Mazzotta wrote:
>>>> I'll see if I can find a batter way to deal with this problem,
>>>> dell-laptop can detect the presence of an hardware switch.
>>>
>>> dell-rbtn.ko has acpi method CRBT which returns if notebook has hw switch or
>>> toggle key.
>>>
>>> And rfkill implementation in dell-laptop.ko does not work correctly on
>>> XPS machines. And because userspace implementation of smbios works fine,
>>> I think that problem is in kernel driver rather in BIOS/firmware...
>>
>> I was thinking about something such as the following, which should favor
>> the hardware slider when available. This should work if my assumption is
>> correct, that is the laptops with a working RBTN method are the ones
>> having problems.
>>
>
> Hm... wait! There are machines with HW slider and type is RBTN_TOGGLE?
>
> I thought that all machines with HW slider has type RBTN_SLIDER and
> others have RBTN_TOGGLE.

Yes, this is the problem I've been talking about ever since the work
on this driver started. AFAIK we don't know how to detect these laptops
(my XPS13 is one of them).
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darren Hart Oct. 22, 2015, 1:02 p.m. UTC | #3
On Thu, Oct 22, 2015 at 12:54:33PM +0200, Gabriele Mazzotta wrote:
> On 22/10/2015 12:50, Pali Rohár wrote:
> >On Thursday 22 October 2015 12:44:08 Gabriele Mazzotta wrote:
> >>On 22/10/2015 10:51, Pali Rohár wrote:
> >>>On Thursday 22 October 2015 10:26:47 Gabriele Mazzotta wrote:
> >>>>I'll see if I can find a batter way to deal with this problem,
> >>>>dell-laptop can detect the presence of an hardware switch.
> >>>
> >>>dell-rbtn.ko has acpi method CRBT which returns if notebook has hw switch or
> >>>toggle key.
> >>>
> >>>And rfkill implementation in dell-laptop.ko does not work correctly on
> >>>XPS machines. And because userspace implementation of smbios works fine,
> >>>I think that problem is in kernel driver rather in BIOS/firmware...
> >>
> >>I was thinking about something such as the following, which should favor
> >>the hardware slider when available. This should work if my assumption is
> >>correct, that is the laptops with a working RBTN method are the ones
> >>having problems.
> >>
> >
> >Hm... wait! There are machines with HW slider and type is RBTN_TOGGLE?
> >
> >I thought that all machines with HW slider has type RBTN_SLIDER and
> >others have RBTN_TOGGLE.
> 
> Yes, this is the problem I've been talking about ever since the work
> on this driver started. AFAIK we don't know how to detect these laptops
> (my XPS13 is one of them).

Where is the radio hw slider on this machine?

This patch would effectively ignore Fn-F2 (radio toggle) key on this machine
then - correct? I don't think that is desirable either.

Seems to me the desired behavior would be to restore the radio state on resume.
Consider the following table:

SW: Switch state
SS: State at suspend
SR: State after resume
 0: WiFi Enabled
 1: WiFi Disabled

State SW SS SR
--------------
a      0  0  0
b      0  1  1   (switch is enabled, but toggle key disabled wifi)
c      1  0  N/A (invalid state)
d      1  1  1

State "a" is where we are failing currently I believe?

Do we know if DELRBTN and DELLABCE are always a TOGGLE or a SLIDER respectively?
I'm wondering if these should be separate drivers.
Gabriele Mazzotta Oct. 22, 2015, 1:43 p.m. UTC | #4
On 22/10/2015 15:02, Darren Hart wrote:
> On Thu, Oct 22, 2015 at 12:54:33PM +0200, Gabriele Mazzotta wrote:
>> On 22/10/2015 12:50, Pali Rohár wrote:
>>> On Thursday 22 October 2015 12:44:08 Gabriele Mazzotta wrote:
>>>> On 22/10/2015 10:51, Pali Rohár wrote:
>>>>> On Thursday 22 October 2015 10:26:47 Gabriele Mazzotta wrote:
>>>>>> I'll see if I can find a batter way to deal with this problem,
>>>>>> dell-laptop can detect the presence of an hardware switch.
>>>>>
>>>>> dell-rbtn.ko has acpi method CRBT which returns if notebook has hw switch or
>>>>> toggle key.
>>>>>
>>>>> And rfkill implementation in dell-laptop.ko does not work correctly on
>>>>> XPS machines. And because userspace implementation of smbios works fine,
>>>>> I think that problem is in kernel driver rather in BIOS/firmware...
>>>>
>>>> I was thinking about something such as the following, which should favor
>>>> the hardware slider when available. This should work if my assumption is
>>>> correct, that is the laptops with a working RBTN method are the ones
>>>> having problems.
>>>>
>>>
>>> Hm... wait! There are machines with HW slider and type is RBTN_TOGGLE?
>>>
>>> I thought that all machines with HW slider has type RBTN_SLIDER and
>>> others have RBTN_TOGGLE.
>>
>> Yes, this is the problem I've been talking about ever since the work
>> on this driver started. AFAIK we don't know how to detect these laptops
>> (my XPS13 is one of them).
>
> Where is the radio hw slider on this machine?

It's the F2 key. Depending on the value passed to the RBTN, it acts as
hw slider or sw toggle. Currently dell-rbtn makes all the laptops with
both the hw slider and sw toggle use the sw toggle. In this way we are
sure that whenever CRBT returns either 1 or 0, we are using the sw
mechanism to disable the radios.

The problem is that the CRBT method of these laptops supporting both
the methods return 1 or 0 as the laptops that only have the sw toggle.
That's why we are calling RBTN to force the sw toggle.

> This patch would effectively ignore Fn-F2 (radio toggle) key on this machine
> then - correct? I don't think that is desirable either.

No, this patch would force the hw slider when it's available instead
of blindly forcing the sw toggle. When the hw slider is in use,
everything is handled by the BIOS, so userspace/kernel don't have to
do anything.

> Seems to me the desired behavior would be to restore the radio state on resume.
> Consider the following table:
>
> SW: Switch state
> SS: State at suspend
> SR: State after resume
>   0: WiFi Enabled
>   1: WiFi Disabled
>
> State SW SS SR
> --------------
> a      0  0  0
> b      0  1  1   (switch is enabled, but toggle key disabled wifi)
> c      1  0  N/A (invalid state)
> d      1  1  1
>
> State "a" is where we are failing currently I believe?

dell-rbtn is simply listening to the ACPI notifications sent to
DELLABCE. These notifications are sent by the BIOS, both when users
press the function key and when it wants. In my case, the BIOS sends a
notification on resume (dell-rbtn sees this notification and sends an
input event to userspace), but the BIOS of some other laptops don't do
this.

Basically, dell-rbtn makes some laptop toggles the state of WiFi on
resume, but not all of them.

In my case:
if WiFi is ON on suspend, it's turned OFF on resume.
if WiFi is OFF on suspend, it's turned ON on resume.

Reading what Alex wrote some time ago [1], I guessed that the laptops 
that are misbehaving are those that can choose between sw toggle and hw
toggle.

> Do we know if DELRBTN and DELLABCE are always a TOGGLE or a SLIDER respectively?
> I'm wondering if these should be separate drivers.

I'm reading the acpidumps Alex provided some time ago [2] (and mine):
* XPS13 9333: DELLABCE, by default (without calling RBTN) SLIDER.
* Latitude E5440: DELLABCE, SLIDER
* Inspiron 7447: DELLABCE, TOGGLE
* Pali's laptop: DELLRBTN, SLIDER (correct?).

[1] https://marc.info/?l=linux-kernel&m=141941243829713
[2] http://people.canonical.com/~alexhung/dell-acpidump/
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Oct. 22, 2015, 2:17 p.m. UTC | #5
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).


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?


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).

* 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).

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?
Gabriele Mazzotta Oct. 22, 2015, 11:29 p.m. UTC | #6
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.

> 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.

> * 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.

> 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?
>
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Oct. 23, 2015, 9 a.m. UTC | #7
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).

> >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?

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.

> >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?
diff mbox

Patch

diff --git a/drivers/platform/x86/dell-rbtn.c 
b/drivers/platform/x86/dell-rbtn.c
index cd410e3..cf3c11c 100644
--- a/drivers/platform/x86/dell-rbtn.c
+++ b/drivers/platform/x86/dell-rbtn.c
@@ -321,6 +321,7 @@  static int rbtn_add(struct acpi_device *device)
  	struct rbtn_data *rbtn_data;
  	enum rbtn_type type;
  	int ret = 0;
+	bool has_hardware_slider; /* get this with SMI */

  	type = rbtn_check(device);
  	if (type == RBTN_UNKNOWN) {
@@ -328,7 +329,7 @@  static int rbtn_add(struct acpi_device *device)
  		return -EINVAL;
  	}

-	ret = rbtn_acquire(device, true);
+	ret = rbtn_acquire(device, !has_hardware_slider);
  	if (ret < 0) {
  		dev_err(&device->dev, "Cannot enable device\n");
  		return ret;
@@ -343,7 +344,10 @@  static int rbtn_add(struct acpi_device *device)

  	switch (rbtn_data->type) {
  	case RBTN_TOGGLE:
-		ret = rbtn_input_init(rbtn_data);
+		if (has_hardware_slider)
+			ret = 0;
+		else
+			ret = rbtn_input_init(rbtn_data);
  		break;
  	case RBTN_SLIDER:
  		if (auto_remove_rfkill && rbtn_chain_head.head)