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

[Krzysztof Kozlowski]

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...

Best regards,
Krzysztof