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

[Paul Barker]

Hi Ilias,

On 24/10/2022 12:54, Ilias Apalodimas wrote:
> 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...

What may make sense is to prefix the DT property names with `u-boot,`.
i.e. they'd become:

* u-boot,uefi-spi-vendor
* u-boot,uefi-spi-part-number
* u-boot,uefi-spi-io-guid

For the sandbox tests it makes sense to keep the configuration in
arch/sandbox/dts/test.dts. However for a real device such as the
SanCloud BBE it makes sense to keep the dts file in line with the Linux
kernel dts file and add the configuration to a new file
arch/arm/dts/am335x-sancloud-bbe-lite-u-boot.dtsi which will be picked
up by the build system. That keeps the configuration separate from the
upstream dts file and matches what we do for properties like
`u-boot,dm-spl`.

Does that sound like a good way forward?

> 
> [...]
> 
>> +{
>> +	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

I'll change this for v5.

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

As above.

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

Thanks,

-- 
Paul Barker
Principal Software Engineer
SanCloud Ltd

e: paul.barker at sancloud.com
w: https://sancloud.com/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xA67255DFCCE62ECD.asc
Type: application/pgp-keys
Size: 6758 bytes
Desc: OpenPGP public key
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20221025/58177c7b/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20221025/58177c7b/attachment.sig>