Re: [PATCH 04/14] platform/x86/intel/ifs: Remove image loading during init

[Sohil Mehta]

On 10/21/2022 1:34 PM, Jithu Joseph wrote:
> Existing implementation loads IFS test image during the driver
> init flow.
>
> Dropping test image loading from the init path improves
> module load time.
>
> Prior to starting IFS tests, the user has to load one of
> the IFS test images by writing to the current_batch sysfs file.

The kernel is still unaware of the 'current_batch' sysfs file. It will 
be introduced in patch 12. You can avoid the reference here.

> Removing IFS test image  loading during init also allows us to
> make ifs_sem static as it is used only within sysfs.c.

Does something like this sound better?

IFS test image is unnecessarily loaded during driver initialization. The 
user has to load one when starting the tests.

Drop image loading during ifs_init() and improve module load time. As a 
consequence, make ifs_sem static as it is only used within sysfs.c

> diff --git a/drivers/platform/x86/intel/ifs/core.c b/drivers/platform/x86/intel/ifs/core.c
> index 27204e3d674d..5fb7f655c291 100644
> --- a/drivers/platform/x86/intel/ifs/core.c
> +++ b/drivers/platform/x86/intel/ifs/core.c
> @@ -51,12 +51,8 @@ static int __init ifs_init(void)
>   	ifs_device.misc.groups = ifs_get_groups();
>   
>   	if ((msrval & BIT(ifs_device.data.integrity_cap_bit)) &&

Is there a reason to store the integrity cap constant in the device.data 
global structure?

.data = {
	.integrity_cap_bit = MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT,
},

IIUC, you are basically reading an MSR and testing a bit within? You 
already have an BIT macro (unused) for that.

#define MSR_INTEGRITY_CAPS_PERIODIC_BIST 
BIT(MSR_INTEGRITY_CAPS_PERIODIC_BIST_BIT)



> -	    !misc_register(&ifs_device.misc)) {
> -		down(&ifs_sem);
> -		ifs_load_firmware(ifs_device.misc.this_device);
> -		up(&ifs_sem);
> +	    !misc_register(&ifs_device.misc))
>   		return 0;
> -	}
>   
>   	return -ENODEV;
>   }

Sohil