From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pali =?utf-8?B?Um9ow6Fy?= Subject: Re: [PATCH 1/3] platform/dell-wmi: Fix driver interface version query Date: Tue, 1 Aug 2017 17:43:12 +0200 Message-ID: <20170801154312.GA25574@pali> References: <0cc9b015ed366f3fd104d2de707083c6ca2c2a5a.1501601667.git.luto@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:38857 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751056AbdHAPnP (ORCPT ); Tue, 1 Aug 2017 11:43:15 -0400 Received: by mail-wm0-f67.google.com with SMTP id y206so3617425wmd.5 for ; Tue, 01 Aug 2017 08:43:14 -0700 (PDT) Content-Disposition: inline In-Reply-To: <0cc9b015ed366f3fd104d2de707083c6ca2c2a5a.1501601667.git.luto@kernel.org> Sender: platform-driver-x86-owner@vger.kernel.org List-ID: To: Andy Lutomirski Cc: Darren Hart , platform-driver-x86@vger.kernel.org On Tuesday 01 August 2017 08:37:26 Andy Lutomirski wrote: > When I converted dell-wmi to the new bus infrastructure, I left the > call to dell_wmi_check_descriptor_buffer() in dell_wmi_init(). This > could cause two problems: > > - An error message when loading the driver on a system without > dell-wmi. We'd try to read the event descriptor even if the WMI > GUID wasn't there. > > - A possible race if dell-wmi was loaded manually before wmi was > fully initialized. > > Fix it by moving the call to the probe function where it belongs. > > Fixes: bff589be59c5 ("platform/x86: dell-wmi: Convert to the WMI bus infrastructure") > Signed-off-by: Andy Lutomirski > --- > drivers/platform/x86/dell-wmi.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c > index f8978464df31..dad8f4afa17c 100644 > --- a/drivers/platform/x86/dell-wmi.c > +++ b/drivers/platform/x86/dell-wmi.c > @@ -626,7 +626,7 @@ static void dell_wmi_input_destroy(struct wmi_device *wdev) > * WMI Interface Version 8 4 > * WMI buffer length 12 4 4096 > */ > -static int __init dell_wmi_check_descriptor_buffer(void) > +static int dell_wmi_check_descriptor_buffer(void) > { > struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL }; > union acpi_object *obj; > @@ -717,9 +717,15 @@ static int dell_wmi_events_set_enabled(bool enable) > > static int dell_wmi_probe(struct wmi_device *wdev) > { > + int err; > + > struct dell_wmi_priv *priv = devm_kzalloc( > &wdev->dev, sizeof(struct dell_wmi_priv), GFP_KERNEL); > > + err = dell_wmi_check_descriptor_buffer(); > + if (err) > + return err; > + > dev_set_drvdata(&wdev->dev, priv); > > return dell_wmi_input_setup(wdev); > @@ -749,10 +755,6 @@ static int __init dell_wmi_init(void) > { > int err; > > - err = dell_wmi_check_descriptor_buffer(); > - if (err) > - return err; > - > dmi_check_system(dell_wmi_smbios_list); > > if (wmi_requires_smbios_request) { Hi! You should move also dell_wmi_events_set_enabled() into dell_wmi_probe() as there is no need to enable receiving events prior to creating input device. -- Pali Rohár pali.rohar@gmail.com