Re: [PATCH 01/14] dell-laptop: extract SMBIOS-related code to a separate module

From: Darren Hart
Date: Tue Feb 09 2016 - 11:51:18 EST


On Tue, Feb 09, 2016 at 09:33:03AM +0100, Pali RohÃr wrote:
> On Monday 08 February 2016 13:42:10 Darren Hart wrote:
> > Assuming the above is an accurate view, I don't see any reason to go beyond the
> > minimal change to the existing SMBIOS code to make it a usable API. If the need
> > arises, we can always make such optimizations and performance improvements
> > later. This is an internal API and we can change it whenever we need to so long
> > as we update the call sites.
>
> Problem is that now smbios code from dell-laptop.c is moved into
> dell-smbios.c and dell-smbios.h and LED subsystem starts using
> dell-smbios.h. In this case I'm thinking that we have something like API
> usable by other modules/subsystem. And I'm thinking if it is not better
> to create "correct" API now instead rewriting code in LED and platform
> subsystem again later... As this API needs to provide just 1 function,
> send command to Dell SMBIOS I think that API is still minimal. Currently
> we have another two functions alloc/free buffer (needed for send).

The internal kernel API changes all the time, we are not bound to it beyond
ensuring we update the internal users when we change it. I prefer not to
introduce complexity until we have to.

buffer = dell_smbios_get_buffer();
buffer->input[0] = token->location;
buffer->input[1] = token->value;
dell_smbios_send_request(1, 0);
dell_smbios_release_buffer();

The get_buffer and release_buffer also include the locking which is necessary
for a shared buffer. If you eliminate the shared buffer, then you have to have a
local buffer, which adds back code to create the buffer, initializize
it, free it if it's dynamic, etc.

So from that sense, MichaÅ's API seems at least as concise as the alternative,
and it introduces less change.

--
Darren Hart
Intel Open Source Technology Center