On Thu, Nov 09, 2017 at 05:34:39PM +0000, Mario.Limonciello@xxxxxxxx wrote:
> > > > > static int dell_wmi_probe(struct wmi_device *wdev)
> > > > > {
> > > > > struct dell_wmi_priv *priv;
> > > > > + int ret;
> > > > >
> > > > > if (!wmi_has_guid(DELL_WMI_DESCRIPTOR_GUID))
> > > > > return -ENODEV;
> > > >
> > > > Just one suggestion, is above check still needed in dell-wmi.c code?
> > > > I think that now it should be job of dell_wmi_get_descriptor_valid()
> > > > function to fully validate if dell wmi descriptor driver is able to
> > > > provide all needed information for dell-wmi driver.
> > > >
> > >
> > > Yes, I believe it's still needed because if the GUID isn't on the bus the probe
> > > routine for dell-wmi-descriptor won't run and dell_wmi_get_descriptor_valid()
> > > will continually return -EPROBE_DEFER.
> > >
> > > That's the exact problem this patch exists for (preventing infinite deferred
> > > probing).
> > >
> > > Perhaps a "cleaner" solution is to have the init routine of dell-wmi-descriptor
> > > do this wmi_has_guid check and set the descriptor_valid variable to -ENODEV
> > > so that "dependent" drivers don't need to.
> >
> > I understand. But I mean, if function dell_wmi_get_descriptor_valid()
> > should not do that check for DELL_WMI_DESCRIPTOR_GUID itself.
> >
> > Because every driver which would use dell-wmi-descriptor needs to
> > do that check.
>
> I'll move the check to dell_wmi_descriptor_valid(). That actually does remove
> the need for even including the GUID #define in a header file. All use will be
> contained now directly in dell-wmi-descriptor.c.
This is a worthwhile improvement.
--
Darren Hart
VMware Open Source Technology Center
