RE: [PATCH v4 09/14] platform/x86: dell-smbios: Introduce dispatcher for SMM calls

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@xxxxxxxxxxxxx]
> Sent: Wednesday, October 4, 2017 8:58 PM
> To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>
> Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>; LKML <linux-
> kernel@xxxxxxxxxxxxxxx>; platform-driver-x86@xxxxxxxxxxxxxxx; Andy Lutomirski
> <luto@xxxxxxxxxx>; quasisec@xxxxxxxxxx; pali.rohar@xxxxxxxxx;
> rjw@xxxxxxxxxxxxx; mjg59@xxxxxxxxxx; hch@xxxxxx; Greg KH
> <greg@xxxxxxxxx>
> Subject: Re: [PATCH v4 09/14] platform/x86: dell-smbios: Introduce dispatcher for
> SMM calls
> 
> On Wed, Oct 04, 2017 at 05:48:35PM -0500, Mario Limonciello wrote:
> > This splits up the dell-smbios driver into two drivers:
> > * dell-smbios
> > * dell-smbios-smm
> >
> > dell-smbios can operate with multiple different dispatcher drivers to
> > perform SMBIOS operations.
> >
> > Also modify the interface that dell-laptop and dell-wmi use align to this
> > model more closely.  Rather than a single global buffer being allocated
> > for all drivers, each driver will allocate and be responsible for it's own
> > buffer. The pointer will be passed to the calling function and each
> > dispatcher driver will then internally copy it to the proper location to
> > perform it's call.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxxx>
> > ---
> >  MAINTAINERS                            |   6 ++
> >  drivers/platform/x86/Kconfig           |  17 ++-
> >  drivers/platform/x86/Makefile          |   1 +
> >  drivers/platform/x86/dell-laptop.c     | 191 ++++++++++++++++-----------------
> >  drivers/platform/x86/dell-smbios-smm.c | 136 +++++++++++++++++++++++
> >  drivers/platform/x86/dell-smbios-smm.h |  22 ++++
> >  drivers/platform/x86/dell-smbios.c     | 120 +++++++++++++--------
> >  drivers/platform/x86/dell-smbios.h     |  13 ++-
> >  drivers/platform/x86/dell-wmi.c        |   8 +-
> >  9 files changed, 361 insertions(+), 153 deletions(-)
> >  create mode 100644 drivers/platform/x86/dell-smbios-smm.c
> >  create mode 100644 drivers/platform/x86/dell-smbios-smm.h
> >
> ...
> 
> > +config DELL_SMBIOS_SMM
> > +       tristate "Dell SMBIOS calling interface (SMM implementation)"
> > +       depends on DCDBAS
> > +       default DCDBAS
> > +       select DELL_SMBIOS
> > +       ---help---
> > +       This provides an implementation for the Dell SMBIOS calling interface
> > +       communicated over ACPI-WMI.
> 
> Not over ACPI-WMI... over SMM.... right?

Yep, copy paste error.

> 
> 
> 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-
> laptop.c
> > index f42159fd2031..c648bbfcc394 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -85,6 +85,7 @@ static struct platform_driver platform_driver = {
> >  	}
> >  };
> >
> > +static struct calling_interface_buffer *buffer;
> >  static struct platform_device *platform_device;
> >  static struct backlight_device *dell_backlight_device;
> >  static struct rfkill *wifi_rfkill;
> > @@ -405,7 +406,6 @@ static const struct dmi_system_id dell_quirks[]
> __initconst = {
> >
> >  static int dell_rfkill_set(void *data, bool blocked)
> >  {
> > -	struct calling_interface_buffer *buffer;
> >  	int disable = blocked ? 1 : 0;
> >  	unsigned long radio = (unsigned long)data;
> >  	int hwswitch_bit = (unsigned long)data - 1;
> > @@ -413,19 +413,21 @@ static int dell_rfkill_set(void *data, bool blocked)
> >  	int status;
> >  	int ret;
> >
> > -	buffer = dell_smbios_get_buffer();
> > -
> > -	dell_smbios_send_request(17, 11);
> > +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> > +	buffer->class = 17;
> > +	buffer->select = 11;
> 
> Please move these three lines into a function, it's used far far too
> often to be open coded. The dell_smbios_send_request is a reasonable
> wrapper which you could just define here as well. This would minimize
> the change to the driver.
> 
OK I'll clean this up and make some functions.