Hi Mark, Andy, Overall this looks pretty good. There are a few very small issues remaining, but they are so small that I've decided to fix them up and merge this into my review-hans branch with the issues fixed up. I plan to let this sit in review-hans a bit longer then usual to give you (Mark) a chance to check out the changes and ack them and to give Andy the time to check if his review remarks were addressed to his liking. I've put remarks inline / below about the few things which I've fixed up in this patch. I'll also reply to patch 3/3 with the fixups which I've done there. On 5/26/21 10:14 PM, Mark Pearson wrote: > This will be used by the Dell and Lenovo WMI management drivers to > prevent both drivers being active. > > Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx> > --- > Changes in v2: > - This is a new file requested as part of the review of the proposed > think_lmi.c driver. Labeling as V2 to keep series consistent > > Changes in v3: > - Set default in Kconfig, and removed help text > - Allow multiple modules to register with module. Change API names to > better reflect this. > > Changes in v4: > - version bump for consistency in series > > drivers/platform/x86/Kconfig | 4 ++ > drivers/platform/x86/Makefile | 1 + > .../platform/x86/firmware_attributes_class.c | 53 +++++++++++++++++++ > .../platform/x86/firmware_attributes_class.h | 13 +++++ > 4 files changed, 71 insertions(+) > create mode 100644 drivers/platform/x86/firmware_attributes_class.c > create mode 100644 drivers/platform/x86/firmware_attributes_class.h > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 2714f7c38..57da8352d 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1076,6 +1076,10 @@ config TOUCHSCREEN_DMI > the OS-image for the device. This option supplies the missing info. > Enable this for x86 tablets with Silead or Chipone touchscreens. > > +config FW_ATTR_CLASS > + tristate "Firmware attributes class helper module" This should be just " tristate" adding a string after the "tristate" makes this user selectable, I've dropped the string. > + default n > + > config INTEL_IMR > bool "Intel Isolated Memory Region support" > depends on X86_INTEL_QUARK && IOSF_MBI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index dcc8cdb95..147573f69 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -115,6 +115,7 @@ obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o > obj-$(CONFIG_I2C_MULTI_INSTANTIATE) += i2c-multi-instantiate.o > obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o > obj-$(CONFIG_TOUCHSCREEN_DMI) += touchscreen_dmi.o > +obj-$(CONFIG_FW_ATTR_CLASS) += firmware_attributes_class.o This was using spaces instead of tabs for indent before the += and it did not apply because of the "platform/x86: Rename hp-wireless to wireless-hotkey" patch in review-hans. > # Intel uncore drivers > obj-$(CONFIG_INTEL_IPS) += intel_ips.o > diff --git a/drivers/platform/x86/firmware_attributes_class.c b/drivers/platform/x86/firmware_attributes_class.c > new file mode 100644 > index 000000000..31393ce4d > --- /dev/null > +++ b/drivers/platform/x86/firmware_attributes_class.c This file had a couple of trailing empty lines which I've stripped. > --- /dev/null > +++ b/drivers/platform/x86/firmware_attributes_class.h Idem, and also the same for think-lmi.c from patch 3/3. Regards, Hans
