Re: [PATCH v2 2/3] platform/x86: dell-wmi-sysman: Use firmware_attributes_class helper

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 5/9/21 3:57 AM, Mark Pearson wrote:
> Update Dell WMI sysman driver to use newly implemented helper module.
> 
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

(note this will need to be adjusted for the function renames in patch 1/3
 but that is trivial, so feel free to keep my Reviewed-by after those changes).

Regards,

Hans

> ---
> Changes in v2:
>  - firmare_attributes_class.c added as reqested during review of the
>    proposed think_lmi.c driver. Labeling as V2 to keep series
>    consistent (hope that's correct).
>  - Note, compile tested only due to lack of HW to test on. Matching
>    changes were made in the think-lmi.c driver and worked. Please let me
>    know if any problems!
> 
>  drivers/platform/x86/dell/Kconfig              |  1 +
>  .../platform/x86/dell/dell-wmi-sysman/sysman.c | 18 ++++++++----------
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index e0a55337f..931f616ff 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -197,6 +197,7 @@ config DELL_WMI_SYSMAN
>  	depends on ACPI_WMI
>  	depends on DMI
>  	select NLS
> +	select FW_ATTR_CLASS
>  	help
>  	  This driver allows changing BIOS settings on many Dell machines from
>  	  2018 and newer without the use of any additional software.
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index c8d276d78..91ecf49ab 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -13,14 +13,11 @@
>  #include <linux/kernel.h>
>  #include <linux/wmi.h>
>  #include "dell-wmi-sysman.h"
> +#include "../../firmware_attributes_class.h"
>  
>  #define MAX_TYPES  4
>  #include <linux/nls.h>
>  
> -static struct class firmware_attributes_class = {
> -	.name = "firmware-attributes",
> -};
> -
>  struct wmi_sysman_priv wmi_priv = {
>  	.mutex = __MUTEX_INITIALIZER(wmi_priv.mutex),
>  };
> @@ -28,6 +25,7 @@ struct wmi_sysman_priv wmi_priv = {
>  /* reset bios to defaults */
>  static const char * const reset_types[] = {"builtinsafe", "lastknowngood", "factory", "custom"};
>  static int reset_option = -1;
> +struct class *fw_attr_class;
>  
>  
>  /**
> @@ -541,11 +539,11 @@ static int __init sysman_init(void)
>  		goto err_exit_bios_attr_pass_interface;
>  	}
>  
> -	ret = class_register(&firmware_attributes_class);
> +	ret = fw_attributes_class_register(&fw_attr_class);
>  	if (ret)
>  		goto err_exit_bios_attr_pass_interface;
>  
> -	wmi_priv.class_dev = device_create(&firmware_attributes_class, NULL, MKDEV(0, 0),
> +	wmi_priv.class_dev = device_create(fw_attr_class, NULL, MKDEV(0, 0),
>  				  NULL, "%s", DRIVER_NAME);
>  	if (IS_ERR(wmi_priv.class_dev)) {
>  		ret = PTR_ERR(wmi_priv.class_dev);
> @@ -602,10 +600,10 @@ static int __init sysman_init(void)
>  	release_attributes_data();
>  
>  err_destroy_classdev:
> -	device_destroy(&firmware_attributes_class, MKDEV(0, 0));
> +	device_destroy(fw_attr_class, MKDEV(0, 0));
>  
>  err_unregister_class:
> -	class_unregister(&firmware_attributes_class);
> +	fw_attributes_class_remove();
>  
>  err_exit_bios_attr_pass_interface:
>  	exit_bios_attr_pass_interface();
> @@ -619,8 +617,8 @@ static int __init sysman_init(void)
>  static void __exit sysman_exit(void)
>  {
>  	release_attributes_data();
> -	device_destroy(&firmware_attributes_class, MKDEV(0, 0));
> -	class_unregister(&firmware_attributes_class);
> +	device_destroy(fw_attr_class, MKDEV(0, 0));
> +	fw_attributes_class_remove();
>  	exit_bios_attr_set_interface();
>  	exit_bios_attr_pass_interface();
>  }
>