Re: [PATCH v3 5/8] platform/x86/intel/ifs: Sysfs interface for Array BIST

[Hans de Goede]

Hi,

On 3/1/23 02:59, Jithu Joseph wrote:
> The interface to trigger Array BIST test and obtain its result
> is similar to the existing scan test. The only notable
> difference is that, Array BIST doesn't require any test content
> to be loaded. So binary load related options are not needed for
> this test.
> 
> Add sysfs interface for array BIST test, the testing support will
> be added by subsequent patch.
> 
> Signed-off-by: Jithu Joseph <jithu.joseph@xxxxxxxxx>
> Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
> ---
>  drivers/platform/x86/intel/ifs/ifs.h     |  1 +
>  drivers/platform/x86/intel/ifs/core.c    |  2 ++
>  drivers/platform/x86/intel/ifs/runtest.c | 10 +++++++++-
>  drivers/platform/x86/intel/ifs/sysfs.c   | 10 +++++++++-
>  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/ifs/ifs.h b/drivers/platform/x86/intel/ifs/ifs.h
> index b8b956e29653..f31966e291df 100644
> --- a/drivers/platform/x86/intel/ifs/ifs.h
> +++ b/drivers/platform/x86/intel/ifs/ifs.h
> @@ -250,5 +250,6 @@ extern bool *ifs_pkg_auth;
>  int ifs_load_firmware(struct device *dev);
>  int do_core_test(int cpu, struct device *dev);
>  extern struct attribute *plat_ifs_attrs[];
> +extern struct attribute *plat_ifs_array_attrs[];
>  
>  #endif
> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 2237aaba7078..c74accedfc8d 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -22,6 +22,7 @@ static const struct x86_cpu_id ifs_cpu_ids[] __initconst = {
>  MODULE_DEVICE_TABLE(x86cpu, ifs_cpu_ids);
>  
>  ATTRIBUTE_GROUPS(plat_ifs);
> +ATTRIBUTE_GROUPS(plat_ifs_array);
>  
>  bool *ifs_pkg_auth;
>  
> @@ -45,6 +46,7 @@ static struct ifs_device ifs_devices[] = {
>  		.misc = {
>  			.name = "intel_ifs_1",
>  			.minor = MISC_DYNAMIC_MINOR,
> +			.groups = plat_ifs_array_groups,
>  		},
>  	},
>  };
> diff --git a/drivers/platform/x86/intel/ifs/runtest.c b/drivers/platform/x86/intel/ifs/runtest.c
> index 0bfd8fcdd7e8..969b3e0946d5 100644
> --- a/drivers/platform/x86/intel/ifs/runtest.c
> +++ b/drivers/platform/x86/intel/ifs/runtest.c
> @@ -236,6 +236,7 @@ static void ifs_test_core(int cpu, struct device *dev)
>   */
>  int do_core_test(int cpu, struct device *dev)
>  {
> +	struct ifs_data *ifsd = ifs_get_data(dev);
>  	int ret = 0;
>  
>  	/* Prevent CPUs from being taken offline during the scan test */
> @@ -247,7 +248,14 @@ int do_core_test(int cpu, struct device *dev)
>  		goto out;
>  	}
>  
> -	ifs_test_core(cpu, dev);
> +	switch (ifsd->ro_info->test_num) {
> +	case IFS_TYPE_SAF:
> +		ifs_test_core(cpu, dev);
> +		break;
> +	case IFS_TYPE_ARRAY_BIST:
> +	default:
> +		return -EINVAL;
> +	}
>  out:
>  	cpus_read_unlock();
>  	return ret;
> diff --git a/drivers/platform/x86/intel/ifs/sysfs.c b/drivers/platform/x86/intel/ifs/sysfs.c
> index 2007d8054f04..88234798080a 100644
> --- a/drivers/platform/x86/intel/ifs/sysfs.c
> +++ b/drivers/platform/x86/intel/ifs/sysfs.c
> @@ -75,7 +75,7 @@ static ssize_t run_test_store(struct device *dev,
>  	if (down_interruptible(&ifs_sem))
>  		return -EINTR;
>  
> -	if (!ifsd->loaded)
> +	if (ifsd->ro_info->test_num != IFS_TYPE_ARRAY_BIST && !ifsd->loaded)
>  		rc = -EPERM;
>  	else
>  		rc = do_core_test(cpu, dev);
> @@ -149,3 +149,11 @@ struct attribute *plat_ifs_attrs[] = {
>  	&dev_attr_image_version.attr,
>  	NULL
>  };
> +
> +/* global array sysfs attributes */
> +struct attribute *plat_ifs_array_attrs[] = {
> +	&dev_attr_details.attr,
> +	&dev_attr_status.attr,
> +	&dev_attr_run_test.attr,
> +	NULL
> +};

Since you need 2 different groups now, please:

1. Add a "struct attribute **groups" member to struct ifs_hw_caps
2. Set it to the right attrs[] array for the CPU type in the
   2 const struct if_hw_caps instances.
3. in probe() add:

	ifs_device.misc_device.groups = ifs_device.hw_caps->groups;

before registering the misc_device.

Regards,

Hans