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

Ilias Apalodimas
2022-12-13T07:15:57Z

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