RE: [PATCH v4 05/14] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver

[Mario.Limonciello]

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@xxxxxxxxx]
> Sent: Thursday, October 5, 2017 3:48 AM
> To: Darren Hart <dvhart@xxxxxxxxxxxxx>
> Cc: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>; LKML <linux-
> kernel@xxxxxxxxxxxxxxx>; Platform Driver <platform-driver-
> x86@xxxxxxxxxxxxxxx>; Andy Lutomirski <luto@xxxxxxxxxx>;
> quasisec@xxxxxxxxxx; Pali RohÃr <pali.rohar@xxxxxxxxx>; Rafael J. Wysocki
> <rjw@xxxxxxxxxxxxx>; mjg59@xxxxxxxxxx; Christoph Hellwig <hch@xxxxxx>;
> Greg KH <greg@xxxxxxxxx>
> Subject: Re: [PATCH v4 05/14] platform/x86: dell-wmi-descriptor: split WMI
> descriptor into it's own driver
> 
> On Thu, Oct 5, 2017 at 10:11 AM, Darren Hart <dvhart@xxxxxxxxxxxxx> wrote:
> > On Thu, Oct 05, 2017 at 08:29:10AM +0300, Andy Shevchenko wrote:
> >> On Thu, Oct 5, 2017 at 4:09 AM, Darren Hart <dvhart@xxxxxxxxxxxxx> wrote:
> 
> >> > You'll want to add something like:
> >> >
> >> > #ifdef CONFIG_DELL_WMI_DESCRIPTOR_MODULE
> >> >         if (request_module("dell_wmi_descriptor"))
> >> >                 /* FAIL */
> >> > #endif
> >> >
> >> > During init.
> >>
> >> I don't think #ifdef is needed.
> >
> > Without the ifdef, we can't distinguish between request_module failing
> > to load the module because it isn't available and because it is
> > built-in.
> >
> >>
> >> We may just request module.
> >>
> >> But looking in the code it seems that we simple need to select that
> >> module. No request_module will be needed.
> >
> > The select will ensure the module is built, but there is not guarantee
> > to module load order. The intent of the above is to ensure the symbols
> > from the required module are loaded.
> >
> >> Did I miss something?
> >
> > Or I did :-) Is there something about this module which ensures
> > dell_wmi_descriptor is loaded first?
> 
> If there is an optional *run-time* dependency we need to use somelike
> request_module(). For example, this is the case for idma64
> (drivers/dma) vs intel-lpss (drivers/mfd).
> 
> If it's mandatory run-time dependency, then we need to add stubs to
> the header and select the callee's module in Kconfig.
> 
> In case they are both modules, depmod keeps an ordering.
> 
> So, the corner case here is when the caller is builtin while callee is module.
> 
> This is a bit tricky to add to Kconfig (we also have such cases
> between I2C DesignWare and acpi_lpss AFAIR).
> 
> --
> With Best Regards,
> Andy Shevchenko

So I believe it should be a mandatory runtime dependency due to needing
the symbol dell_wmi_get_interface_version.  Depmod should be handling
this then no?