Re: [PATCH v5 1/3] efi_loader: Add SPI I/O protocol support

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Paul,

Apologies for the delayed reply.

[...]

> +static efi_status_t
> +export_spi_peripheral(struct efi_spi_bus *bus, struct udevice *dev)
> +{
> +	efi_string_t name_utf16, vendor_utf16, part_number_utf16;
> +	struct efi_spi_peripheral_priv *priv;
> +	efi_status_t status;
> +	efi_handle_t handle = NULL;
> +	struct udevice *dev_bus = dev->parent;
> +	struct spi_slave *target;
> +	const char *name = dev_read_name(dev);
> +	const char *vendor = dev_read_string(dev, "u-boot,uefi-spi-vendor");
> +	const char *part_number = dev_read_string(dev,
> +			"u-boot,uefi-spi-part-number");
> +	efi_guid_t *guid = (efi_guid_t *)dev_read_u8_array_ptr(dev,
> +			"u-boot,uefi-spi-io-guid", 16);
> +
> +	if (device_get_uclass_id(dev) == UCLASS_SPI_EMUL) {
> +		debug("Skipping emulated SPI peripheral %s\n", name);
> +		goto fail_1;
> +	}
> +
> +	if (!vendor || !part_number || !guid) {
> +		debug("Skipping SPI peripheral %s\n", name);
> +		status = EFI_UNSUPPORTED;
> +		goto fail_1;
> +	}
> +
> +	if (!device_active(dev)) {
> +		int ret = device_probe(dev);
> +		if (ret) {
> +			debug("Skipping SPI peripheral %s, probe failed\n",
> +			      name);
> +			goto fail_1;
> +		}
> +	}
> +
> +	target = dev_get_parent_priv(dev);
> +	if (!target) {
> +		debug("Skipping uninitialized SPI peripheral %s\n", name);
> +		status = EFI_UNSUPPORTED;
> +		goto fail_1;
> +	}
> +
> +	debug("Registering SPI dev %d:%d, name %s\n",
> +	      dev_bus->seq_, spi_chip_select(dev), name);
> +
> +	priv = calloc(1, sizeof(*priv));
> +	if (!priv) {
> +		status = EFI_OUT_OF_RESOURCES;
> +		goto fail_1;
> +	}
> +
> +	vendor_utf16 = efi_convert_string(vendor);
> +	if (!vendor_utf16) {
> +		status = EFI_OUT_OF_RESOURCES;
> +		goto fail_2;
> +	}
> +
> +	part_number_utf16 = efi_convert_string(part_number);
> +	if (!part_number_utf16) {
> +		status = EFI_OUT_OF_RESOURCES;
> +		goto fail_3;
> +	}
> +
> +	name_utf16 = efi_convert_string(name);
> +	if (!name_utf16) {
> +		status = EFI_OUT_OF_RESOURCES;
> +		goto fail_4;
> +	}
> +
> +	priv->target = target;
> +
> +	efi_spi_init_part(&priv->part, target, vendor_utf16, part_number_utf16);
> +
> +	efi_spi_init_peripheral(&priv->peripheral, &priv->part,
> +				bus, target, guid, name_utf16);
> +
> +	efi_spi_append_peripheral(&priv->peripheral, bus);
> +
> +	efi_spi_init_io_protocol(&priv->io_protocol, &priv->peripheral, target);
> +
> +	status = efi_install_multiple_protocol_interfaces(&handle, guid,
> +							  &priv->io_protocol,
> +							  NULL);

There's a protocols installed here as well as in
efi_spi_protocol_register().  But I don't see those being uninstalled
somewhere.  Shouldn't destroy_efi_spi_bus() call
efi_uninstall_multiple_protocol_interfaces() as well ?


> +	if (status != EFI_SUCCESS)
> +		goto fail_5;
> +
> +	debug("Added EFI_SPI_IO_PROTOCOL for %s with guid %pUl\n", name, guid);
> +	return EFI_SUCCESS;
> +
> +fail_5:
> +	free(name_utf16);
> +fail_4:
> +	free(part_number_utf16);
> +fail_3:
> +	free(vendor_utf16);
> +fail_2:
> +	free(priv);
> +fail_1:
> +	return status;
> +}
> +
> +static struct efi_spi_bus *export_spi_bus(int i)
> +{
> +	struct efi_spi_bus *bus;
> +	struct udevice *dev, *child;
> +	const char *name;
> +	int r;
> +
> +	r = uclass_get_device(UCLASS_SPI, i, &dev);
> +	if (r < 0) {
> +		debug("Failed to get SPI bus %d\n", i);
> +		goto fail_1;
> +	}
> +
> +	name = dev_read_name(dev);
> +	debug("Registering SPI bus %d, name %s\n", i, name);
> +
> +	bus = calloc(1, sizeof(*bus));
> +	if (!bus)
> +		goto fail_1;
> +
> +	bus->friendly_name = efi_convert_string(name);
> +	if (!bus->friendly_name)
> +		goto fail_2;
> +
> +	bus->peripheral_list = NULL;
> +	bus->clock = efi_spi_bus_clock;
> +	bus->clock_parameter = NULL;
> +
> +	/* For the purposes of the current implementation, we do not need to
> +	 * expose the hardware device path to users of the SPI I/O protocol.
> +	 */
> +	bus->controller_path = &null_device_path;
> +
> +	device_foreach_child(child, dev) {
> +		efi_status_t status = export_spi_peripheral(bus, child);
> +
> +		if (status == EFI_OUT_OF_RESOURCES)
> +			goto fail_3;
> +	}
> +
> +	return bus;
> +
> +fail_3:
> +	destroy_efi_spi_bus(bus);
> +fail_2:
> +	free(bus);
> +fail_1:
> +	return NULL;
> +}
> +
> +efi_status_t efi_spi_protocol_register(void)
> +{
> +	efi_status_t status;
> +	efi_handle_t handle = NULL;
> +	struct efi_spi_configuration_protocol *proto;
> +	uint i;
> +
> +	debug("Registering EFI_SPI_CONFIGURATION_PROTOCOL\n");
> +
> +	proto = calloc(1, sizeof(*proto));
> +	if (!proto) {
> +		status = EFI_OUT_OF_RESOURCES;
> +		goto fail_1;
> +	}
> +
> +	proto->bus_count = uclass_id_count(UCLASS_SPI);
> +	proto->bus_list = calloc(proto->bus_count, sizeof(*proto->bus_list));
> +	if (!proto->bus_list) {
> +		status = EFI_OUT_OF_RESOURCES;
> +		goto fail_2;
> +	}
> +
> +	for (i = 0; i < proto->bus_count; i++) {
> +		proto->bus_list[i] = export_spi_bus(i);
> +		if (!proto->bus_list[i])
> +			goto fail_3;
> +	}
> +
> +	status = efi_install_multiple_protocol_interfaces(&handle,
> +							  &efi_spi_configuration_guid,
> +							  proto, NULL);
> +	if (status != EFI_SUCCESS)
> +		goto fail_3;
> +
> +	return EFI_SUCCESS;
> +
> +fail_3:
> +	for (i = 0; i < proto->bus_count; i++) {
> +		if (proto->bus_list[i])
> +			destroy_efi_spi_bus(proto->bus_list[i]);
> +	}
> +	free(proto->bus_list);
> +fail_2:
> +	free(proto);
> +fail_1:
> +	return status;
> +}
> diff --git a/lib/uuid.c b/lib/uuid.c
> index 465e1ac38f57..3f723b732588 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -187,6 +187,10 @@ static const struct {
>  		"TCG2",
>  		EFI_TCG2_PROTOCOL_GUID,
>  		},
> +	{
> +		"SPI Protocol Stack",
> +		EFI_SPI_CONFIGURATION_GUID
> +	},
>  	{
>  		"System Partition",
>  		PARTITION_SYSTEM_GUID
> --
> 2.25.1
>


Regards
/Ilias