From patchwork Fri Oct 23 09:47:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gabriele Mazzotta X-Patchwork-Id: 7471281 Return-Path: X-Original-To: patchwork-platform-driver-x86@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3AFBB9F36A for ; Fri, 23 Oct 2015 09:47:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 233DE2083E for ; Fri, 23 Oct 2015 09:47:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F41A620838 for ; Fri, 23 Oct 2015 09:47:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751138AbbJWJr3 (ORCPT ); Fri, 23 Oct 2015 05:47:29 -0400 Received: from mail-wi0-f175.google.com ([209.85.212.175]:36016 "EHLO mail-wi0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750911AbbJWJr2 (ORCPT ); Fri, 23 Oct 2015 05:47:28 -0400 Received: by wicfx6 with SMTP id fx6so23507421wic.1 for ; Fri, 23 Oct 2015 02:47:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; bh=OwVDqxFMGjnSJDohNlKoniIOVHhjrUSf2pMssM5Xi2k=; b=dXLUd/K/zNhrJB6bRTInDacAsrL+BzIn8QgWYm7fOw1TmgyERE6Z2V1+lrhBH/adMz 7ewzI5D99L29iBJLZ9z2FgiMkgMz1nqD4axP9yPRolxpilLvzU+rC3lJYTAWycBg3IoC s/5osfz8ldVYbFGUtzTMqbzkg5SexS1l5Xx+/2g490dp0xhVRcYN4Sx0J+7jWFGHNdFH D5yI72Zi7r1fevyWL77dh+lExsK9+M+TCP9rjQwAmColZ1fIps0+wb03gopUeuIHAzWR WIliIcA5/4r7HBSs4Fh+lcKa4roigXGIQJbWv5UyzlRIjElRJjFfU14QAh1xwHfZ4ohX nErg== X-Received: by 10.194.77.146 with SMTP id s18mr4172515wjw.122.1445593646681; Fri, 23 Oct 2015 02:47:26 -0700 (PDT) Received: from [10.169.221.81] (nat131-dot1x.polimi.it. [131.175.28.131]) by smtp.gmail.com with ESMTPSA id bh5sm21721933wjb.42.2015.10.23.02.47.25 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 23 Oct 2015 02:47:26 -0700 (PDT) Subject: Re: [Bug 106031] Regression in 4.2.x: in airplane mode each time I open my laptop lid To: =?UTF-8?Q?Pali_Roh=c3=a1r?= References: <20151022074937.GA2581@malice.jf.intel.com> <20151022085117.GQ15219@pali> <5628BDF8.9030506@gmail.com> <20151022105018.GX15219@pali> <5628C069.8040902@gmail.com> <20151022130211.GA110029@vmdeb7> <5628E812.2070708@gmail.com> <20151022141710.GD15219@pali> <56297148.6060605@gmail.com> <20151023090027.GG15219@pali> Cc: Darren Hart , "platform-driver-x86@vger.kernel.org" , Alex Hung From: Gabriele Mazzotta Message-ID: <562A022D.1020302@gmail.com> Date: Fri, 23 Oct 2015 11:47:25 +0200 MIME-Version: 1.0 In-Reply-To: <20151023090027.GG15219@pali> Sender: platform-driver-x86-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. 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. --- 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 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",