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

[Darren Hart <dvhart@infradead.org>]

On Thu, Oct 05, 2017 at 01:59:36PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> > Sent: Thursday, October 5, 2017 3:48 AM
> > To: Darren Hart <dvhart@infradead.org>
> > Cc: Limonciello, Mario <Mario_Limonciello@Dell.com>; LKML <linux-
> > kernel@vger.kernel.org>; Platform Driver <platform-driver-
> > x86@vger.kernel.org>; Andy Lutomirski <luto@kernel.org>;
> > quasisec@google.com; Pali Rohár <pali.rohar@gmail.com>; Rafael J. Wysocki
> > <rjw@rjwysocki.net>; mjg59@google.com; Christoph Hellwig <hch@lst.de>;
> > Greg KH <greg@kroah.com>
> > 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@infradead.org> 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@infradead.org> 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?

This was nagging me all night, and I was thinking the last time I had to
use this it was indeed a runtime dependency. Sorry for the noise on
this.

The kconfig needs to be setup such that dell-wmi-descriptor cannot be a
module if the dependent drivers are modules.

-- 
Darren Hart
VMware Open Source Technology Center