From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932696AbcEXT53 (ORCPT ); Tue, 24 May 2016 15:57:29 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:60698 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932650AbcEXT51 (ORCPT ); Tue, 24 May 2016 15:57:27 -0400 Date: Tue, 24 May 2016 12:57:26 -0700 From: Darren Hart To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: Andrei Borzenkov , Gabriele Mazzotta , "Rafael J. Wysocki" , "D. Jared Dominguez" , "platform-driver-x86@vger.kernel.org" , Alex Hung , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3] dell-rbtn: Ignore ACPI notifications if device is suspended Message-ID: <20160524195726.GM2735@f23x64.localdomain> References: <1457740175-8327-1-git-send-email-gabriele.mzt@gmail.com> <201605240006.03580@pali> <20160523221715.GE2735@f23x64.localdomain> <201605240022.28140@pali> <1d56338e-6312-371d-ace7-96f762e18bba@gmail.com> <5743CF19.8050404@gmail.com> <20160524070938.GG29844@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160524070938.GG29844@pali> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 24, 2016 at 09:09:38AM +0200, Pali Rohár wrote: > On Tuesday 24 May 2016 06:48:41 Andrei Borzenkov wrote: > > 24.05.2016 02:03, Gabriele Mazzotta пишет: > > > On 24/05/2016 00:22, Pali Rohár wrote: ... > > > +#ifdef CONFIG_PM_SLEEP > > > +static void ACPI_SYSTEM_XFACE rbtn_acpi_clear_flag(void *context) > > I would rename this function to rbtn_clear_suspended_flag. > ... > > > + /* > > > + * Upon resume, some BIOSes autonomously send an ACPI notification You can drop "autonomously", it reads a bit awkwardly, and doesn't add any information. > > > + * that triggers an unwanted input event. In order to ignore it, > > > + * we use a flag that we set at suspend and clear once we have > > > + * received the extra notification. Since ACPI notifications are > > > + * delivered asynchronously to drivers, we clear the flag from the > > > + * workqueue used to deliver the notifications. This should be enough > > > + * to guarantee that the flag is cleared only after we received the > > > + * extra notification, if any. > > > + */ > > > > "guarantee" is rather strong word here. We really do not know anything > > how and when these notifications are generated by firmware, so can only > > hope. But otherwise this explains what this patch intends to do (so that > > even me finally understood it :) > > Yes, thats better. > > > > + status = acpi_os_execute(OSL_NOTIFY_HANDLER, > > > + rbtn_acpi_clear_flag, rbtn_data); > > > + if (ACPI_FAILURE(status)) > > > + rbtn_data->suspended = false; > > And here rbtn_clear_suspended_flag(rbtn_data) call instead direct > assignment. > I'm dropping this from the queue, and awaiting an updated version with the requested changes (these from Pali, and the issue raised about "guarantee" being too strong). Thanks, -- Darren Hart Intel Open Source Technology Center From mboxrd@z Thu Jan 1 00:00:00 1970 From: Darren Hart Subject: Re: [PATCH v3] dell-rbtn: Ignore ACPI notifications if device is suspended Date: Tue, 24 May 2016 12:57:26 -0700 Message-ID: <20160524195726.GM2735@f23x64.localdomain> References: <1457740175-8327-1-git-send-email-gabriele.mzt@gmail.com> <201605240006.03580@pali> <20160523221715.GE2735@f23x64.localdomain> <201605240022.28140@pali> <1d56338e-6312-371d-ace7-96f762e18bba@gmail.com> <5743CF19.8050404@gmail.com> <20160524070938.GG29844@pali> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from bombadil.infradead.org ([198.137.202.9]:60698 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932650AbcEXT51 (ORCPT ); Tue, 24 May 2016 15:57:27 -0400 Content-Disposition: inline In-Reply-To: <20160524070938.GG29844@pali> Sender: platform-driver-x86-owner@vger.kernel.org List-ID: To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: Andrei Borzenkov , Gabriele Mazzotta , "Rafael J. Wysocki" , "D. Jared Dominguez" , "platform-driver-x86@vger.kernel.org" , Alex Hung , "linux-kernel@vger.kernel.org" On Tue, May 24, 2016 at 09:09:38AM +0200, Pali Roh=C3=A1r wrote: > On Tuesday 24 May 2016 06:48:41 Andrei Borzenkov wrote: > > 24.05.2016 02:03, Gabriele Mazzotta =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > > > On 24/05/2016 00:22, Pali Roh=C3=A1r wrote: =2E.. > > > +#ifdef CONFIG_PM_SLEEP > > > +static void ACPI_SYSTEM_XFACE rbtn_acpi_clear_flag(void *context= ) >=20 > I would rename this function to rbtn_clear_suspended_flag. >=20 =2E.. > > > + /* > > > + * Upon resume, some BIOSes autonomously send an ACPI notificat= ion You can drop "autonomously", it reads a bit awkwardly, and doesn't add = any information. > > > + * that triggers an unwanted input event. In order to ignore it= , > > > + * we use a flag that we set at suspend and clear once we have > > > + * received the extra notification. Since ACPI notifications ar= e > > > + * delivered asynchronously to drivers, we clear the flag from = the > > > + * workqueue used to deliver the notifications. This should be = enough > > > + * to guarantee that the flag is cleared only after we received= the > > > + * extra notification, if any. > > > + */ > >=20 > > "guarantee" is rather strong word here. We really do not know anyth= ing > > how and when these notifications are generated by firmware, so can = only > > hope. But otherwise this explains what this patch intends to do (so= that > > even me finally understood it :) >=20 > Yes, thats better. >=20 > > > + status =3D acpi_os_execute(OSL_NOTIFY_HANDLER, > > > + rbtn_acpi_clear_flag, rbtn_data); > > > + if (ACPI_FAILURE(status)) > > > + rbtn_data->suspended =3D false; >=20 > And here rbtn_clear_suspended_flag(rbtn_data) call instead direct > assignment. >=20 I'm dropping this from the queue, and awaiting an updated version with = the requested changes (these from Pali, and the issue raised about "guarant= ee" being too strong). Thanks, --=20 Darren Hart Intel Open Source Technology Center