Re: [PATCH v2 2/8] firmware: smccc: Add support for Live Firmware Activation (LFA)

[Krzysztof Kozlowski]

On 18/03/2026 09:09, Krzysztof Kozlowski wrote:
> On Tue, Mar 17, 2026 at 11:33:28AM +0100, Andre Przywara wrote:
>> +
>> +/* A list of known GUIDs, to be shown in the "name" sysfs file. */
>> +static const struct fw_image_uuid {
>> +	const char *name;
>> +	const char *uuid;
>> +} fw_images_uuids[] = {
>> +	{
>> +		.name = "TF-A BL31 runtime",
>> +		.uuid = "47d4086d-4cfe-9846-9b95-2950cbbd5a00",
>> +	},
>> +	{
>> +		.name = "BL33 non-secure payload",
>> +		.uuid = "d6d0eea7-fcea-d54b-9782-9934f234b6e4",
>> +	},
>> +	{
>> +		.name = "TF-RMM",
>> +		.uuid = "6c0762a6-12f2-4b56-92cb-ba8f633606d9",
>> +	},
>> +};
>> +
>> +static struct kset *lfa_kset;
>> +static struct workqueue_struct *fw_images_update_wq;
>> +static struct work_struct fw_images_update_work;
>> +static struct attribute *image_default_attrs[LFA_ATTR_NR_IMAGES + 1];
> 
> Bunch of singletons here because (see later)...
> 
>> +
>> +static const struct attribute_group image_attr_group = {
>> +	.attrs = image_default_attrs,
>> +};
>> +
>> +static const struct attribute_group *image_default_groups[] = {
>> +	&image_attr_group,
>> +	NULL
>> +};
>> +
>> +static int __init lfa_init(void)
>> +{
>> +	struct arm_smccc_1_2_regs reg = { 0 };
>> +	int err;
>> +
>> +	reg.a0 = LFA_1_0_FN_GET_VERSION;
>> +	arm_smccc_1_2_invoke(&reg, &reg);
>> +	if (reg.a0 == -LFA_NOT_SUPPORTED) {
>> +		pr_info("Live Firmware activation: no firmware agent found\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pr_info("Live Firmware Activation: detected v%ld.%ld\n",
>> +		reg.a0 >> 16, reg.a0 & 0xffff);
>> +
>> +	fw_images_update_wq = alloc_workqueue("fw_images_update_wq",
>> +					      WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
>> +	if (!fw_images_update_wq) {
>> +		pr_err("Live Firmware Activation: Failed to allocate workqueue.\n");
>> +
>> +		return -ENOMEM;
>> +	}
>> +	INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
>> +
>> +	init_image_default_attrs();
>> +	lfa_kset = kset_create_and_add("lfa", NULL, firmware_kobj);
>> +	if (!lfa_kset)
>> +		return -ENOMEM;
>> +
>> +	err = update_fw_images_tree();
>> +	if (err != 0) {
>> +		kset_unregister(lfa_kset);
>> +		destroy_workqueue(fw_images_update_wq);
>> +	}
>> +
>> +	return err;
>> +}
>> +module_init(lfa_init);
> 
> You do not use driver model, but 199x style of modprobing and performing
> actions.
> 
> I do not understand why module load is already doign anything. This
> looks like misinterpretation/misuse of Linux driver model - in a way,
> you don't use it all and this is like back to 199x where modprobe was
> already meaning you bind drivers...

Although now going through further patches I found you implementing some
parts of driver model, so probably this split is just needing fix.

Modprobe must not do "arm_smccc_1_2_invoke" or any other device related
things. You only initialize your bus, just like every other bus driver
would do, but honestly this should not be a bus-like code.

Best regards,
Krzysztof