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.bar...@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.bar...@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.apalodi...@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@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. 

> +                     /* 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)?

> +

[...]

> +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()?

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

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

> +     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;
> +}
> +
v> +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?  

> 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

Reply via email to