[PATCH v3 1/3] efi_loader: Add SPI I/O protocol support
Paul Barker
paul.barker at sancloud.com
Mon Oct 3 14:00:48 CEST 2022
On 26/09/2022 13:43, Ilias Apalodimas wrote:
> Hi Paul,
>
> On Wed, Sep 21, 2022 at 05:06:26PM +0100, Paul Barker wrote:
>> This addition allows UEFI applications running under u-boot to access
>> peripherals on SPI busses. It is based on the UEFI Platform
>> Initialization (PI) Specification, Version 1.7 Errata A (April 2020).
>> Only the core functionality required to discover SPI peripherals and
>> communicate with them is currently implemented. Other functionality such
>> as the legacy SPI controller interface and the ability to update the SPI
>> peripheral object associated with a particular SPI I/O protocol object
>> is currently unimplemented.
>>
>> The following protocols are defined:
>> * EFI_SPI_CONFIGURATION_PROTOCOL
>> * EFI_SPI_IO_PROTOCOL
>> * EFI_LEGACY_SPI_CONTROLLER_PROTOCOL
>>
>> Since there are no open source implementations of these protocols to use
>> as an example, educated guesses/hacks have been made in cases where the
>> UEFI PI specification is unclear and these are documented in comments.
>>
>> This implementation has been tested on the SanCloud BBE Lite and allowed
>> a UEFI test application to successfully communicate with a Micron
>> Authenta flash device connected via the SPI bus. It has also been tested
>> with the sandbox target using the included efi_selftest case.
>>
>> Signed-off-by: Paul Barker <paul.barker at sancloud.com>
>> ---
>> MAINTAINERS | 7 +
>> arch/sandbox/dts/test.dts | 13 +
>> include/efi_api.h | 4 +
>> include/efi_loader.h | 4 +
>> include/efi_spi_protocol.h | 166 +++++
>> lib/efi_loader/Kconfig | 8 +
>> lib/efi_loader/Makefile | 1 +
>> lib/efi_loader/efi_setup.c | 6 +
>> lib/efi_loader/efi_spi_protocol.c | 614 +++++++++++++++++++
>> lib/efi_selftest/Makefile | 1 +
>> lib/efi_selftest/efi_selftest_spi_protocol.c | 284 +++++++++
>> lib/uuid.c | 4 +
>> 12 files changed, 1112 insertions(+)
>> create mode 100644 include/efi_spi_protocol.h
>> create mode 100644 lib/efi_loader/efi_spi_protocol.c
>> create mode 100644 lib/efi_selftest/efi_selftest_spi_protocol.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 83346183ee4b..a58b2083a218 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -857,6 +857,13 @@ F: tools/efivar.py
>> F: tools/file2include.c
>> F: tools/mkeficapsule.c
>>
>> +EFI SPI SUPPORT
>> +M: Paul Barker <paul.barker at sancloud.com>
>> +S: Maintained
>> +F: include/efi_spi_protocol.h
>> +F: lib/efi_loader/efi_spi_protocol.c
>> +F: lib/efi_selftest/efi_selftest_spi_protocol.c
>> +
>> EFI VARIABLES VIA OP-TEE
>> M: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>> S: Maintained
>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>> index 2761588f0dad..05c3e0377ac4 100644
>> --- a/arch/sandbox/dts/test.dts
>> +++ b/arch/sandbox/dts/test.dts
>> @@ -1185,6 +1185,13 @@
>> compatible = "spansion,m25p16", "jedec,spi-nor";
>> spi-max-frequency = <40000000>;
>> sandbox,filename = "spi.bin";
>> +
>> + uefi-spi-vendor = "spansion";
>> + uefi-spi-part-number = "mt25p16";
>> +
>> + /* GUID in UEFI format: b881eb5d-ad92-4a48-8fdd-fa75a8e4c6b8 */
>> + uefi-spi-io-guid = [5d eb 81 b8 92 ad 48 4a
>> + 8f dd fa 75 a8 e4 c6 b8];
>> };
>> spi.bin at 1 {
>> reg = <1>;
>> @@ -1193,6 +1200,12 @@
>> sandbox,filename = "spi.bin";
>> spi-cpol;
>> spi-cpha;
>> +
>> + uefi-spi-vendor = "spansion";
>> + uefi-spi-part-number = "mt25p16";
>
> This is needed to identify the flash we want to access through the protocol
> right? We keep dumping info on the DT that I am not sure it belongs there.
In export_spi_peripheral(), we need to be able to get these values given
a struct udevice pointer. I can't think of anywhere else to put them
other than the device tree, we don't want to go back to hardcoding
things in board.c files. If there's somewhere better to store this info
then let me know.
>
>> + /* GUID in UEFI format: b6b39ecd-2b1f-a643-b8d7-3192d7cf7270 */
>> + uefi-spi-io-guid = [cd 9e b3 b6 1f 2b 43 a6
>> + b8 d7 31 92 d7 cf 72 70];
>> };
>> };
>>
>
> [...]
>
>> + */
>> +
>> +#define LOG_CATEGORY LOGC_EFI
>> +
>> +#include <common.h>
>> +#include <dm/device.h>
>> +#include <dm/device-internal.h>
>> +#include <dm/read.h>
>> +#include <efi.h>
>> +#include <efi_loader.h>
>> +#include <efi_spi_protocol.h>
>> +#include <malloc.h>
>> +#include <spi.h>
>> +
>> +static efi_string_t convert_efi_string(const char *str)
>> +{
>> + efi_string_t str_16, tmp;
>> + size_t sz_8, sz_16;
>> +
>> + sz_8 = strlen(str);
>> + sz_16 = utf8_utf16_strlen(str);
>> + str_16 = calloc(sz_16 + 1, 2);
>> + if (!str_16)
>> + return NULL;
>> +
>> + tmp = str_16;
>> + utf8_utf16_strcpy(&tmp, str);
>> +
>> + return str_16;
>> +}
>
> This seems useful overall, mind moving it to lib/efi_loader/efi_string.c
> and add sphinx style comments with the description? And while at it replace
> '2' with sizeof(u16)?
This is definitely possilble. I'll make the change in v4.
>
>> +
>
> [...]
>
>> +static efi_status_t EFIAPI
>> +efi_spi_io_transaction(const struct efi_spi_io_protocol *this,
>> + enum efi_spi_transaction_type transaction_type,
>> + bool debug_transaction,
>> + u32 clock_hz,
>> + u32 bus_width,
>> + u32 frame_size,
>> + u32 write_bytes,
>> + u8 *write_buffer,
>> + u32 read_bytes,
>> + u8 *read_buffer)
>> +{
>> + struct spi_slave *target;
>> + efi_status_t status = EFI_SUCCESS;
>> + int r;
>> +
>> + /* We ignore the bus_width and frame_size arguments to this function as the
>> + * appropriate bus configuration for the connected device will be performed
>> + * during spi_claim_bus().
>> + */
>> +
>> + /* TODO: Print transaction details if debug_transaction is true. */
>> +
>> + EFI_ENTRY("%p, %u, %u, %u, %u, %u, %u, %p, %u, %p",
>> + this, transaction_type, debug_transaction,
>> + clock_hz, bus_width, frame_size,
>> + write_bytes, write_buffer, read_bytes, read_buffer);
>> +
>> + if (!this)
>> + return EFI_EXIT(EFI_INVALID_PARAMETER);
>> +
>> + target = container_of(this, struct efi_spi_peripheral_priv, io_protocol)->target;
>> +
>> + if (clock_hz > this->spi_peripheral->max_clock_hz)
>> + return EFI_EXIT(EFI_UNSUPPORTED);
>> +
>> + r = spi_claim_bus(target);
>> + if (r != 0)
>> + return EFI_EXIT(EFI_DEVICE_ERROR);
>> + EFI_PRINT("SPI IO: Bus claimed\n");
>> +
>> + if (clock_hz) {
>> + EFI_PRINT("SPI IO: Setting clock rate to %u Hz\n", clock_hz);
>> + spi_get_ops(target->dev->parent)->set_speed(target->dev->parent, clock_hz);
>
> Is set_speed always implemented in the driver model or will we crash?
> I think we should be using spi_set_speed_mode()?
I'll modify this in v4.
>
>> + } else {
>> + EFI_PRINT("SPI IO: Using default clock rate\n");
>> + }
>> +
>> + switch (transaction_type) {
>> + case SPI_TRANSACTION_FULL_DUPLEX:
>> + EFI_PRINT("SPI IO: Full-duplex\n");
>> + if (write_bytes != read_bytes || !write_bytes || !write_buffer || !read_buffer) {
>> + status = EFI_INVALID_PARAMETER;
>> + break;
>> + }
>> + if (debug_transaction)
>> + dump_buffer("SPI IO: write", write_bytes, write_buffer);
>> + r = spi_xfer(target, 8 * write_bytes,
>> + write_buffer, read_buffer, SPI_XFER_ONCE);
>> + EFI_PRINT("SPI IO: xfer returned %d\n", r);
>> + if (debug_transaction)
>> + dump_buffer("SPI IO: read", read_bytes, read_buffer);
>> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
>> + break;
>> + case SPI_TRANSACTION_READ_ONLY:
>> + EFI_PRINT("SPI IO: Read-only\n");
>> + if (!read_bytes || !read_buffer) {
>> + status = EFI_INVALID_PARAMETER;
>> + break;
>> + }
>> + r = spi_xfer(target, 8 * read_bytes,
>> + NULL, read_buffer, SPI_XFER_ONCE);
>> + EFI_PRINT("SPI IO: xfer returned %d\n", r);
>> + if (debug_transaction)
>> + dump_buffer("SPI IO: read", read_bytes, read_buffer);
>> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
>> + break;
>> + case SPI_TRANSACTION_WRITE_ONLY:
>> + EFI_PRINT("SPI IO: Write-only\n");
>> + if (!write_bytes || !write_buffer) {
>> + status = EFI_INVALID_PARAMETER;
>> + break;
>> + }
>> + if (debug_transaction)
>> + dump_buffer("SPI IO: write", write_bytes, write_buffer);
>> + r = spi_xfer(target, 8 * write_bytes,
>> + write_buffer, NULL, SPI_XFER_ONCE);
>> + EFI_PRINT("SPI IO: xfer returned %d\n", r);
>> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
>> + break;
>> + case SPI_TRANSACTION_WRITE_THEN_READ:
>> + EFI_PRINT("SPI IO: Write-then-read\n");
>> + if (!write_bytes || !write_buffer || !read_bytes || !read_buffer) {
>> + status = EFI_INVALID_PARAMETER;
>> + break;
>> + }
>> + if (debug_transaction)
>> + dump_buffer("SPI IO: write", write_bytes, write_buffer);
>> + r = spi_xfer(target, 8 * write_bytes,
>> + write_buffer, NULL, SPI_XFER_BEGIN);
>> + EFI_PRINT("SPI IO: xfer [1/2] returned %d\n", r);
>> + if (r != 0) {
>> + status = EFI_DEVICE_ERROR;
>> + break;
>> + }
>> + r = spi_xfer(target, 8 * read_bytes,
>> + NULL, read_buffer, SPI_XFER_END);
>> + EFI_PRINT("SPI IO: xfer [2/2] returned %d\n", r);
>> + if (debug_transaction)
>> + dump_buffer("SPI IO: read", read_bytes, read_buffer);
>> + status = (r == 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
>> + break;
>> + default:
>> + status = EFI_INVALID_PARAMETER;
>> + break;
>> + }
>> +
>> + spi_release_bus(target);
>> + EFI_PRINT("SPI IO: Released bus\n");
>> + return EFI_EXIT(status);
>> +}
>> +
>> +static struct efi_device_path null_device_path = {
>> + .type = DEVICE_PATH_TYPE_END,
>> + .sub_type = DEVICE_PATH_SUB_TYPE_END,
>> + .length = 4
>> +};
>> +
>> +static struct efi_legacy_spi_controller_protocol
>> +dummy_legacy_spi_controller_protocol = {
>> + .maximum_offset = 0,
>> + .maximum_range_bytes = 0,
>> + .range_register_count = 0,
>> + .erase_block_opcode = legacy_erase_block_opcode,
>> + .write_status_prefix = legacy_write_status_prefix,
>> + .bios_base_address = legacy_bios_base_address,
>> + .clear_spi_protect = legacy_clear_spi_protect,
>> + .is_range_protected = legacy_is_range_protected,
>> + .protect_next_range = legacy_protect_next_range,
>> + .lock_controller = legacy_lock_controller
>> +};
>
> Keeping in mind all these return EFI_UNSUPPORTED can we get rid of them and
> set the legacy_spi_protocol to NULL? Or defining them is mandatory from the PI spec?
> Do you plan to implement it in the future?
The spec does not say anything about allowing NULLs here so I don't want
to assume that an arbitrary UEFI app will check the function pointers
are non-NULL before calling them. Hence the functions which return
EFI_UNSUPPORTED.
>
>> +
>> +static efi_guid_t efi_spi_configuration_guid = EFI_SPI_CONFIGURATION_GUID;
>> +
>> +static void destroy_efi_spi_peripheral(struct efi_spi_peripheral *peripheral)
>> +{
>> + struct efi_spi_peripheral_priv *priv =
>> + container_of(peripheral,
>> + struct efi_spi_peripheral_priv,
>> + peripheral);
>> + free(priv->peripheral.friendly_name);
>> + free(priv->part.vendor);
>> + free(priv->part.part_number);
>> + efi_delete_handle(priv->handle);
>
> Does this handle has any more protocols? In theory we shouldn't be calling
> this. Uninstalling protocols from a handler should take care of this for
> us. IOW if the protocol you uninstalled was the last one on the handler,
> we automatically delete it.
I can drop this call in v4.
>
>> + free(priv);
>> +}
>> +
>> +static void destroy_efi_spi_bus(struct efi_spi_bus *bus)
>> +{
>> + 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);
>> + if (status != EFI_SUCCESS) {
>> + printf("Failed to add protocol\n");
>> + goto fail_2;
>> + }
>> +
>> + return EFI_SUCCESS;
>> +
>> +fail_2:
>> + efi_delete_handle(handle);
>> +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;
>> +}
>> +
>> +static void
>> +efi_spi_init_peripheral(struct efi_spi_peripheral *peripheral,
>> + struct efi_spi_part *part,
>> + struct efi_spi_bus *bus,
>> + struct spi_slave *target,
>> + efi_guid_t *guid,
>> + efi_string_t name_utf16
>> +)
>> +{
>> + peripheral->friendly_name = name_utf16;
>> + peripheral->spi_part = part;
>> + peripheral->next_spi_peripheral = NULL;
>> + peripheral->spi_peripheral_driver_guid = guid;
>> + peripheral->max_clock_hz = target->max_hz;
>> + peripheral->clock_polarity = (target->mode & SPI_CPOL) ? true : false;
>> + peripheral->clock_phase = (target->mode & SPI_CPHA) ? true : false;
>> + peripheral->attributes = 0;
>> + peripheral->configuration_data = NULL;
>> + peripheral->spi_bus = bus;
>> + peripheral->chip_select = efi_spi_peripheral_chip_select;
>> + peripheral->chip_select_parameter = NULL;
>> +}
>> +
>> +static void
>> +efi_spi_append_peripheral(struct efi_spi_peripheral *peripheral,
>> + struct efi_spi_bus *bus
>> +)
>> +{
>> + if (bus->peripheral_list) {
>> + struct efi_spi_peripheral *tmp = bus->peripheral_list;
>> +
>> + while (tmp->next_spi_peripheral)
>> + tmp = tmp->next_spi_peripheral;
>> +
>> + tmp->next_spi_peripheral = peripheral;
>> + } else {
>> + bus->peripheral_list = peripheral;
>> + }
>> +}
>> +
>> +static void
>> +efi_spi_init_io_protocol(struct efi_spi_io_protocol *io_protocol,
>> + struct efi_spi_peripheral *peripheral,
>> + struct spi_slave *target
>> +)
>> +{
>> + u32 max_read, max_write;
>> +
>> + io_protocol->spi_peripheral = peripheral;
>> + io_protocol->original_spi_peripheral = peripheral;
>> + io_protocol->legacy_spi_protocol = &dummy_legacy_spi_controller_protocol;
>> + io_protocol->transaction = efi_spi_io_transaction;
>> + io_protocol->update_spi_peripheral = efi_spi_io_update_spi_peripheral;
>> +
>> + /* This is a bit of a hack. The EFI data structures do not allow us to
>> + * represent a frame size greater than 32 bits.
>> + */
>> + if (target->wordlen <= 32)
>> + io_protocol->frame_size_support_mask =
>> + 1 << (target->wordlen - 1);
>> + else
>> + io_protocol->frame_size_support_mask = 0;
>> +
>> + /* Again, this is a bit of a hack. The EFI data structures only allow
>> + * for a single maximum transfer size whereas the u-boot spi_slave
>> + * structure records maximum read transfer size and maximum write
>> + * transfer size separately. So we need to take the minimum of these two
>> + * values.
>> + *
>> + * In u-boot, a value of zero for these fields means there is no limit
>> + * on the transfer size. However in the UEFI PI spec a value of zero is
>> + * invalid so we'll use 0xFFFFFFFF as a substitute unlimited value.
>> + */
>> + max_write = target->max_write_size ? target->max_write_size : 0xFFFFFFFF;
>> + max_read = target->max_read_size ? target->max_read_size : 0xFFFFFFFF;
>> + io_protocol->maximum_transfer_bytes = (max_read > max_write) ? max_write : max_read;
>> +
>> + /* Hack++. Leave attributes set to zero since the flags listed in the
>> + * UEFI PI spec have no defined numerical values and so cannot be used.
>> + */
>> + io_protocol->attributes = 0;
>> +}
>> +
>> +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;
>> + struct udevice *dev_bus = dev->parent;
>> + struct spi_slave *target;
>> + const char *name = dev_read_name(dev);
>> + const char *vendor = dev_read_string(dev, "uefi-spi-vendor");
>> + const char *part_number = dev_read_string(dev, "uefi-spi-part-number");
>> + efi_guid_t *guid =
>> + (efi_guid_t *)dev_read_u8_array_ptr(dev, "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 = convert_efi_string(vendor);
>> + if (!vendor_utf16) {
>> + status = EFI_OUT_OF_RESOURCES;
>> + goto fail_2;
>> + }
>> +
>> + part_number_utf16 = convert_efi_string(part_number);
>> + if (!part_number_utf16) {
>> + status = EFI_OUT_OF_RESOURCES;
>> + goto fail_3;
>> + }
>> +
>> + name_utf16 = convert_efi_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_spi_new_handle(guid, &priv->io_protocol);
>> + if (status != EFI_SUCCESS)
>> + goto fail_5;
>> +
>> + log_debug("Added EFI_SPI_IO_PROTOCOL for %s with guid "
>> + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x\n",
>> + name,
>> + guid->b[3], guid->b[2], guid->b[1], guid->b[0],
>> + guid->b[5], guid->b[4], guid->b[7], guid->b[6],
>> + guid->b[8], guid->b[9], guid->b[10], guid->b[11],
>> + guid->b[12], guid->b[13], guid->b[14], guid->b[15]);
>> + 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) {
>> + printf("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 = convert_efi_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;
>> + struct efi_spi_configuration_protocol *proto;
>> + uint i;
>> +
>> + printf("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_spi_new_handle(&efi_spi_configuration_guid, proto);
>> + 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/efi_selftest/Makefile b/lib/efi_selftest/Makefile
>
>
> Would you mind splitting of the selftest on a different patchset?
I'll put it in a separate patch in v4.
>
>> index daac6c396820..2790fcd784e0 100644
>> --- a/lib/efi_selftest/Makefile
>> +++ b/lib/efi_selftest/Makefile
>> @@ -63,6 +63,7 @@ obj-$(CONFIG_EFI_LOADER_HII) += efi_selftest_hii.o
>> obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_selftest_rng.o
>> obj-$(CONFIG_EFI_GET_TIME) += efi_selftest_rtc.o
>> obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_selftest_tcg2.o
>> +obj-$(CONFIG_EFI_SPI_PROTOCOL) += efi_selftest_spi_protocol.o
>>
>> ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
>> obj-y += efi_selftest_fdt.o
>> diff --git a/lib/efi_selftest/efi_selftest_spi_protocol.c b/lib/efi_selftest/efi_selftest_spi_protocol.c
>> new file mode 100644
>> index 000000000000..946d04dbb557
>> --- /dev/null
>> +++ b/lib/efi_selftest/efi_selftest_spi_protocol.c
>> @@ -0,0 +1,284 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2022 Micron Technology, Inc.
>> + */
>> +
>> +#include <efi_selftest.h>
>> +#include <efi_spi_protocol.h>
>> +
>> +static struct efi_boot_services *boottime;
>> +static efi_guid_t efi_spi_configuration_guid = EFI_SPI_CONFIGURATION_GUID;
>> +
>> +static int setup(const efi_handle_t img_handle,
>> + const struct efi_system_table *systable)
>> +{
>> + boottime = systable->boottime;
>> + return EFI_ST_SUCCESS;
>> +}
>> +
>
> [...]
>
> Thanks!
> /Ilias
Thanks for the review.
--
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/20221003/20cb826a/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/20221003/20cb826a/attachment.sig>
More information about the U-Boot
mailing list