From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752309AbdJ3NdD (ORCPT ); Mon, 30 Oct 2017 09:33:03 -0400 Received: from esa1.dell-outbound.iphmx.com ([68.232.153.90]:9417 "EHLO esa1.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751593AbdJ3NdA (ORCPT ); Mon, 30 Oct 2017 09:33:00 -0400 From: X-LoopCount0: from 10.166.132.195 X-IronPort-AV: E=Sophos;i="5.44,320,1505797200"; d="scan'208";a="1171824998" X-DLP: DLP_GlobalPCIDSS To: CC: , , , , , , , , , , Subject: RE: [PATCH v11 05/15] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver Thread-Topic: [PATCH v11 05/15] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver Thread-Index: AQHTUXTTauMHyQBw1EGBPpKIXVEeQKL8YmGg Date: Mon, 30 Oct 2017 13:32:57 +0000 Message-ID: <27e2d166f2ce461fb025ebf06a284d01@ausx13mpc124.AMER.DELL.COM> References: <20171030114652.kopuffflshq2ates@pali> In-Reply-To: <20171030114652.kopuffflshq2ates@pali> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.143.242.75] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id v9UDX81G031687 > -----Original Message----- > From: Pali Rohár [mailto:pali.rohar@gmail.com] > Sent: Monday, October 30, 2017 6:47 AM > To: Limonciello, Mario > Cc: dvhart@infradead.org; Andy Shevchenko ; > LKML ; platform-driver-x86@vger.kernel.org; Andy > Lutomirski ; quasisec@google.com; rjw@rjwysocki.net; > mjg59@google.com; hch@lst.de; Greg KH ; Alan Cox > > Subject: Re: [PATCH v11 05/15] platform/x86: dell-wmi-descriptor: split WMI > descriptor into it's own driver > > On Friday 20 October 2017 12:40:20 Mario Limonciello wrote: > > diff --git a/drivers/platform/x86/dell-wmi-descriptor.c > b/drivers/platform/x86/dell-wmi-descriptor.c > > new file mode 100644 > > index 000000000000..3204c408e261 > > --- /dev/null > > +++ b/drivers/platform/x86/dell-wmi-descriptor.c > > This dell-wmi-descriptor.c looks good now! > > > diff --git a/drivers/platform/x86/dell-wmi-descriptor.h > b/drivers/platform/x86/dell-wmi-descriptor.h > > new file mode 100644 > > index 000000000000..5f7b69c2c83a > > --- /dev/null > > +++ b/drivers/platform/x86/dell-wmi-descriptor.h > > @@ -721,7 +652,9 @@ static int dell_wmi_events_set_enabled(bool enable) > > static int dell_wmi_probe(struct wmi_device *wdev) > > { > > struct dell_wmi_priv *priv; > > - int err; > > + > > + if (!wmi_has_guid(DELL_WMI_DESCRIPTOR_GUID)) > > + return -ENODEV; > > > > priv = devm_kzalloc( > > &wdev->dev, sizeof(struct dell_wmi_priv), GFP_KERNEL); > > @@ -729,9 +662,8 @@ static int dell_wmi_probe(struct wmi_device *wdev) > > return -ENOMEM; > > dev_set_drvdata(&wdev->dev, priv); > > > > - err = dell_wmi_check_descriptor_buffer(wdev); > > - if (err) > > - return err; > > + if (!dell_wmi_get_interface_version(&priv->interface_version)) > > + return -EPROBE_DEFER; > > But here is still a problem. You added check that > DELL_WMI_DESCRIPTOR_GUID exists in APCI table, but it does not mean that > probe method of dell-wmi-descriptor cannot fail. > > With PROBE_DEFER, dell_wmi_probe function would be called later again > and again, even when probing dell-wmi-descriptor failed and so dell-wmi > in this case cannot work. > Yes it's possible that probe method can fail, but it depends on the reason for failure if it will fail again later. For example if not enough memory, it may work later. Or maybe user manually unbound from GUID, should continue to try until it's bound again. So in short, I believe this is the correct behavior to adopt.