Re: [PATCH 2/2] platform/x86: dell-*wmi*: Relay failed initial probe to dependent drivers

[Darren Hart]

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