Re: [PATCH v3 2/8] platform/x86/intel/ifs: IFS cleanup

[Hans de Goede]

Hi,

On 3/1/23 02:59, Jithu Joseph wrote:
> Cleanup incorporating misc review comments
> 
>  - Remove the subdirectory intel_ifs/0 for devicenode [1]
>  - Make plat_ifs_groups non static and use it directly without using a
>     function [2]
> 
> Link: https://lore.kernel.org/lkml/Y+4kQOtrHt5pdsSO@xxxxxxxxx/ [1]
> Link: https://lore.kernel.org/lkml/Y9nyxNesVHCUXAcH@xxxxxxxxx/  [2]
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

Please add my Reviewed-by to this patch for the next version of
the series, so that I know which patches I have already reviewed.

Regards,

Hans

> ---
>  drivers/platform/x86/intel/ifs/ifs.h   | 2 +-
>  drivers/platform/x86/intel/ifs/core.c  | 6 +++---
>  drivers/platform/x86/intel/ifs/sysfs.c | 9 +--------
>  3 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index e07463c794d4..ab168ddf28f1 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -246,6 +246,6 @@ static inline struct ifs_data *ifs_get_data(struct device *dev)
>  extern bool *ifs_pkg_auth;
>  int ifs_load_firmware(struct device *dev);
>  int do_core_test(int cpu, struct device *dev);
> -const struct attribute_group **ifs_get_groups(void);
> +extern struct attribute *plat_ifs_attrs[];
>  
>  #endif
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index b518b661daf0..62c44dbae757 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -20,6 +20,8 @@ static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
>  };
>  MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
>  
> +ATTRIBUTE_GROUPS(plat_ifs);
> +
>  bool *ifs_pkg_auth;
>  
>  static struct ifs_device ifs_device = {
> @@ -29,8 +31,8 @@ static struct ifs_device ifs_device = {
>  	},
>  	.misc = {
>  		.name = "intel_ifs_0",
> -		.nodename = "intel_ifs/0",
>  		.minor = MISC_DYNAMIC_MINOR,
> +		.groups = plat_ifs_groups,
>  	},
>  };
>  
> @@ -53,8 +55,6 @@ static int __init ifs_init(void)
>  	if (rdmsrl_safe(MSR_INTEGRITY_CAPS, &msrval))
>  		return -ENODEV;
>  
> -	ifs_device.misc.groups = ifs_get_groups();
> -
>  	if (!(msrval & BIT(ifs_device.ro_data.integrity_cap_bit)))
>  		return -ENODEV;
>  
> diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
> index ee636a76b083..2007d8054f04 100644
> --- a/drivers/platform/x86/intel/ifs/sysfs.c
> +++ b/drivers/platform/x86/intel/ifs/sysfs.c
> @@ -141,7 +141,7 @@ static ssize_t image_version_show(struct device *dev,
>  static DEVICE_ATTR_RO(image_version);
>  
>  /* global scan sysfs attributes */
> -static struct attribute *plat_ifs_attrs[] = {
> +struct attribute *plat_ifs_attrs[] = {
>  	&dev_attr_details.attr,
>  	&dev_attr_status.attr,
>  	&dev_attr_run_test.attr,
> @@ -149,10 +149,3 @@ static struct attribute *plat_ifs_attrs[] = {
>  	&dev_attr_image_version.attr,
>  	NULL
>  };
> -
> -ATTRIBUTE_GROUPS(plat_ifs);
> -
> -const struct attribute_group **ifs_get_groups(void)
> -{
> -	return plat_ifs_groups;
> -}