Thread (33 messages) 33 messages, 6 authors, 2016-02-09

Re: [PATCH v2 00/16] Common Dell SMBIOS API

From: Darren Hart <dvhart@infradead.org>
Date: 2016-02-08 20:46:55
Also in: lkml, platform-driver-x86

On Mon, Feb 08, 2016 at 11:29:20AM -0800, Greg Kroah-Hartman wrote:
On Mon, Feb 08, 2016 at 11:20:14AM -0800, Darren Hart wrote:
quoted
On Fri, Jan 22, 2016 at 03:48:51PM +0100, Pali Rohár wrote:
quoted
On Friday 22 January 2016 15:27:12 Michał Kępień wrote:
quoted
Note:

    In this series (both v1 and v2) I tried to stick to the overall
    concept used in dell-laptop, but in the v1 thread me and Pali also
    briefly discussed his alternative ideas [1][2] as to what this API
    could look like, so feel free to suggest a different approach.

[1] http://www.spinics.net/lists/platform-driver-x86/msg08260.html
[2] http://www.spinics.net/lists/platform-driver-x86/msg08268.html
I would like to hear opinion about dell-smbios API also from other
people. Darren, can you look and comment it?
This is an excellently prepared series, nice work Michał.

Most of my concerns were addressed by later patches in the series. I have pushed
a version of this 1/7 fixed per lkp (linux/io.h) and 7/16 with a corrected body
as I sent in reply to that patch. This is on my tree as the dell-smbios branch.

My only major concern is module load order dependencies. Inter-module
dependencies are frowned upon with good reason, the kernel load ordering is
non-deterministic and it's possible, for example, for dell-laptop to fail to
find the symbols exported by dell-smbios under certain conditions.

I have worked around this in the past with things like the following:

#ifdef MODULE
#ifdef CONFIG_FOO_MODULE
	if (request_module("foo"))
		return -ENODEV;
#endif
#endif

Something like the above may be necessary for dell-smbios in dell-laptop,
dell-wmi, and dell-leds now that they depend on the dell-smbios exported
functions.

Cc Greg in case there is a better way to handle this that I'm not aware of.
No need to request_module anything, the symbol resolution should pull
the dependant module in automagically when you do a 'modprobe',
otherwise the symbols would never be found.
OK, Thank you Greg. Based on this, I have no concerns that haven't been addressed in the
pdx86/dell-smbios branch.

Michał, did you have any changes you wanted to make?

Pali, are you happy enough with this to add your reviewed-by?

I'm satisfied with the series and can push to next tomorrow if we're all in
agreement.

-- 
Darren Hart
Intel Open Source Technology Center
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help