lkml.org 
[lkml]   [2017]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2017-10-05 10:48    [W:0.171 / U:0.152 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site