Thanks Hans On 2021-05-27 5:12 a.m., Hans de Goede wrote: > 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. Ah - my bad. Thanks for fixing. > >> + 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. Ack. > > >> # 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. Ack - sorry I missed those. > >> --- /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 > Mark
