Re: [PATCH v4 3/6] efi_loader: Add SPI I/O protocol support

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

Hi Paul, 

I think the series overall is in a good state,  but we are trying to figure
out if we can avoid defining SPI uuid's in the DT.  OTOH U-Boot uses the DT
for it's config so that might be okay...

[...]

> +{
> +	struct efi_spi_peripheral *peripheral = bus->peripheral_list;
> +
> +	while (peripheral) {
> +		struct efi_spi_peripheral *next =
> +			peripheral->next_spi_peripheral;
> +		destroy_efi_spi_peripheral(peripheral);
> +		peripheral = next;
> +	}
> +	free(bus->friendly_name);
> +	free(bus);
> +}
> +
> +static efi_status_t efi_spi_new_handle(const efi_guid_t *guid, void *proto)
> +{
> +	efi_status_t status;
> +	efi_handle_t handle;
> +
> +	status = efi_create_handle(&handle);
> +	if (status != EFI_SUCCESS) {
> +		printf("Failed to create EFI handle\n");
> +		goto fail_1;
> +	}
> +
> +	status = efi_add_protocol(handle, guid, proto);

Apologies for this it's my fault,  but can you replace efi_add_protocol ->
efi_install_multiple_protocol_interfaces?
commit 05c4c9e21ae6 ("efi_loader: define internal implementations of install/uninstallmultiple")
has some details on why.  A bit annoying but the change is straightforward

> +	if (status != EFI_SUCCESS) {
> +		printf("Failed to add protocol\n");
> +		goto fail_2;
> +	}
> +
> +	return EFI_SUCCESS;
> +
> +fail_2:
> +	efi_delete_handle(handle);

Same here, just uninstall the protocol

> +fail_1:
> +	return status;
> +}
> +
> +static void
> +efi_spi_init_part(struct efi_spi_part *part,
> +		  struct spi_slave *target,
> +		  efi_string_t vendor_utf16,
> +		  efi_string_t part_number_utf16
> +)
> +{
> +	part->vendor = vendor_utf16;
> +	part->part_number = part_number_utf16;
> +	part->min_clock_hz = 0;
> +	part->max_clock_hz = target->max_hz;
> +	part->chip_select_polarity =
> +		(target->mode & SPI_CS_HIGH) ? true : false;
> +}
> +
 

Thanks!
/Ilias