Re: [PATCH 1/3] platform/dell-wmi: Fix driver interface version query

[Darren Hart <dvhart@infradead.org>]

On Tue, Aug 01, 2017 at 11:08:26AM -0700, Andy Lutomirski wrote:
> On Tue, Aug 1, 2017 at 8:43 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > 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 <luto@kernel.org>

> > 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.
> 
> I thought of that and intentionally didn't do it: I wanted to leave
> enable and the disable paired properly, and there's nothing that
> tracks the enabled state per device.  Also, it's at least
> theoretically possible to have more than one instance of dell-wmi in a
> system (my laptop already has *two* wmi busses), and moving this code
> to ->probe would break this.
> 
> (The current wmi.c code can't handle the same GUID on two busses, but
> it could easily be added if anyone ever finds a laptop that does
> that.)
> 

Better still, thanks Andy.

-- 
Darren Hart
VMware Open Source Technology Center