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,