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.bar...@sancloud.com
w: https://sancloud.com/
OpenPGP_0xA67255DFCCE62ECD.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
