From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3BFFAC07E9D for ; Mon, 26 Sep 2022 13:13:27 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7537D80488; Mon, 26 Sep 2022 15:13:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="bn4qYpLT"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CE92380488; Mon, 26 Sep 2022 15:13:23 +0200 (CEST) Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id CD8C880488 for ; Mon, 26 Sep 2022 15:13:19 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1664197998; bh=A/eezEXr7TTqeFW7e+Q+4xKzXcdLrA7sdKdlzb9CDec=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=bn4qYpLT2oJ+BsLTPrKaCG/NKJ3YP+XmeGafSUKpw0PMOQqNO43503gpk9IGW7RTB zbx8SVNzKoco7HKKNO9i3xJlVrxATu0TWoK2FyT1+VCaTo6El/LdFYAMmSDbv59ttc RK9nB4zBAsPtM5GZxntmV8cMWQ86Sh6+kBF75qWg= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.67] ([84.118.157.2]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MulqN-1pToBJ19dr-00rssI; Mon, 26 Sep 2022 15:13:18 +0200 Message-ID: <8f5bcacf-65b1-42aa-ff7f-d770b93e3cec@gmx.de> Date: Mon, 26 Sep 2022 15:13:03 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 Subject: Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support To: Paul Barker Cc: u-boot@lists.denx.de, Tom Rini , Ilias Apalodimas References: <20220921160628.4166966-1-paul.barker@sancloud.com> <20220921160628.4166966-2-paul.barker@sancloud.com> Content-Language: en-US From: Heinrich Schuchardt In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:YHoeJYFJ9b4G0ASLGbYMBUtY5EYJFhcDsepvGCRpx2GDn1cMVJD h3cihYsYkjqzTwJqewK8aEYcsfP+EZTx27kcIz6i7ZMqaRCYmVXWe4KQCuclz2+xOCdXLHL rD6msgeHbcHSjk4EJcR1gvCMvs9d8gEBUm/yTiNLGsO1a5NL82H3VlEsYxHXlCYUtIlakgr bhW2gSz6OBp5d9qK6Bxaw== X-UI-Out-Filterresults: notjunk:1;V03:K0:MjtXy85toEM=:P7Mx1b6kO4yXVulru4ginw rEe0SH0CGSMwb5jhbaOth+7p6daTrUfOPr3qrPm/2eZWnfUcr/9Fgbfi1HRZdY5MEIpkhxSv/ Jusi2yIN3Gj9GihJEgdFfOPhACCCgyTqVXYLn7UF2M3Ry7Jn2oi8CnIUydJDT3UfcWkVFbOJc SPRJNu1Z6o20MSwgkkLr5THR8++UBA/p7Xz62+cpwxCCd7z5UCGcW5V+NMLES6G/YVE5sY1JA 0O4/WrIWhJ6mvUH0gYeOnr2vVWh7sy5mjmddN3UNkNs2Q40VFo2cYgHXnPmrFzZ1p08EEcSzr bP2ifLAsii31KQMiXVrpTxwpJmuOmhQpbLEKFASGjLgPOghaibpjjjgegGNrHSndwvhfeh6mo R/6F+Z+CdyRI5hOIttVMOzSLcSlqxVXh5iOA2snWc7rpOOHnqGEJZop8QLH497ls/dl0BsPtr ARdhj9c1GynzbloW5+bFzODi/vZ0uWURCQqCMRGzWU5ASlZWp0JJV4lIVIz/I10MiK4OjbBlD Ca9753ybg4mCGfbrpptha5XyUiE29UYkznGaZrwxfhWC1rQjMWrXfAU4DZ9PQhgcABlQoDlHs CzgU9WJnWDbDeWC4m6KHrOS914y0Bzz10UnNOKDoGMrx5r0/k6lETCwppYymG0i/+IGY1+cpm zwAdp5KvQ0+9f5bCW0LTrJzwZPQVqNaFSkNMJ9bkGxseuS5FFy15aRf0dGn4gGwJxqo71I2+k w2jitw0YMBXEqh/Y3aVTlDQ/PWa1a+2CaEFVl29oiOZnD+Eiyv0SZpuhgnp5G9pWuYj/Lkz03 hUJkQw165kmSPmM5azgy4ZRz97rFSPLKHO0nEUnR42AXvrQnpzC2KsmZ4rBDGaQEd8hQYCvDt 9TyZxihMeJPI8yPOU/7JXpAk2fafHkYVV4xOIiuYihKHEBM3hbreDhJ674H5fORtVTrcHJFvA rrnO5M72+FAUHpo86YFrfF1GN2xaWSgBhQiD3WRwH1qXtVsG0NfPVLEStFcJloq1ZdM6y2zAX p2sCfPoDF1vbvKU/WDRcMdAx2kMLa+bHoLDmNxXJcz7NP1qsmt0of5PAznBJFUxaxnOKH7pQe gXGsjDJhP4tv6iWfYmB8cMjDMws/WbYpzcpqXMNUaeJh+YonLF72QwTwrSbBopou9jtCQ8WzN uI7bvoIegwSshSsDzn5Rl9QC01 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 9/26/22 14: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 suc= h >> as the legacy SPI controller interface and the ability to update the SP= I >> 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 us= e >> 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 allowe= d >> a UEFI test application to successfully communicate with a Micron >> Authenta flash device connected via the SPI bus. It has also been teste= d >> with the sandbox target using the included efi_selftest case. >> >> Signed-off-by: Paul Barker >> --- >> 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 >> +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 >> 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 =3D "spansion,m25p16", "jedec,spi-nor"; >> spi-max-frequency =3D <40000000>; >> sandbox,filename =3D "spi.bin"; >> + >> + uefi-spi-vendor =3D "spansion"; >> + uefi-spi-part-number =3D "mt25p16"; >> + >> + /* GUID in UEFI format: b881eb5d-ad92-4a48-8fdd-fa75a8e4c6b8 */ >> + uefi-spi-io-guid =3D [5d eb 81 b8 92 ad 48 4a >> + 8f dd fa 75 a8 e4 c6 b8]; >> }; >> spi.bin@1 { >> reg =3D <1>; >> @@ -1193,6 +1200,12 @@ >> sandbox,filename =3D "spi.bin"; >> spi-cpol; >> spi-cpha; >> + >> + uefi-spi-vendor =3D "spansion"; >> + uefi-spi-part-number =3D "mt25p16"; > > This is needed to identify the flash we want to access through the proto= col > right? We keep dumping info on the DT that I am not sure it belongs ther= e. > >> + /* GUID in UEFI format: b6b39ecd-2b1f-a643-b8d7-3192d7cf7270 */ >> + uefi-spi-io-guid =3D [cd 9e b3 b6 1f 2b 43 a6 >> + b8 d7 31 92 d7 cf 72 70]; >> }; >> }; >> > > [...] > >> + */ >> + >> +#define LOG_CATEGORY LOGC_EFI >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +static efi_string_t convert_efi_string(const char *str) >> +{ >> + efi_string_t str_16, tmp; >> + size_t sz_8, sz_16; >> + >> + sz_8 =3D strlen(str); sz_8 seems to be unused. >> + sz_16 =3D utf8_utf16_strlen(str); >> + str_16 =3D calloc(sz_16 + 1, 2); >> + if (!str_16) >> + return NULL; >> + >> + tmp =3D 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 rep= lace > '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 =3D 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 per= formed >> + * 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 =3D container_of(this, struct efi_spi_peripheral_priv, io_prot= ocol)->target; >> + >> + if (clock_hz > this->spi_peripheral->max_clock_hz) >> + return EFI_EXIT(EFI_UNSUPPORTED); >> + >> + r =3D spi_claim_bus(target); >> + if (r !=3D 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, clo= ck_hz); > > Is set_speed always implemented in the driver model or will we crash? > I think we should be using spi_set_speed_mode()? mtk_snor.c does not have set_speed(). > >> + } 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 !=3D read_bytes || !write_bytes || !write_buffer || = !read_buffer) { >> + status =3D EFI_INVALID_PARAMETER; >> + break; >> + } >> + if (debug_transaction) >> + dump_buffer("SPI IO: write", write_bytes, write_buffer); >> + r =3D 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 =3D (r =3D=3D 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 =3D EFI_INVALID_PARAMETER; >> + break; >> + } >> + r =3D 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 =3D (r =3D=3D 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 =3D EFI_INVALID_PARAMETER; >> + break; >> + } >> + if (debug_transaction) >> + dump_buffer("SPI IO: write", write_bytes, write_buffer); >> + r =3D spi_xfer(target, 8 * write_bytes, >> + write_buffer, NULL, SPI_XFER_ONCE); >> + EFI_PRINT("SPI IO: xfer returned %d\n", r); >> + status =3D (r =3D=3D 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 =3D EFI_INVALID_PARAMETER; >> + break; >> + } >> + if (debug_transaction) >> + dump_buffer("SPI IO: write", write_bytes, write_buffer); >> + r =3D 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 !=3D 0) { >> + status =3D EFI_DEVICE_ERROR; >> + break; >> + } >> + r =3D 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 =3D (r =3D=3D 0) ? EFI_SUCCESS : EFI_DEVICE_ERROR; >> + break; >> + default: >> + status =3D 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 =3D { >> + .type =3D DEVICE_PATH_TYPE_END, >> + .sub_type =3D DEVICE_PATH_SUB_TYPE_END, >> + .length =3D 4 >> +}; >> + >> +static struct efi_legacy_spi_controller_protocol >> +dummy_legacy_spi_controller_protocol =3D { >> + .maximum_offset =3D 0, >> + .maximum_range_bytes =3D 0, >> + .range_register_count =3D 0, >> + .erase_block_opcode =3D legacy_erase_block_opcode, >> + .write_status_prefix =3D legacy_write_status_prefix, >> + .bios_base_address =3D legacy_bios_base_address, >> + .clear_spi_protect =3D legacy_clear_spi_protect, >> + .is_range_protected =3D legacy_is_range_protected, >> + .protect_next_range =3D legacy_protect_next_range, >> + .lock_controller =3D 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? What do you mean by setting to NULL? You simply would not install the protocol interface on any handle. If there is no legacy SPI controller, it does not make sense to install the protocol. > >> + >> +static efi_guid_t efi_spi_configuration_guid =3D EFI_SPI_CONFIGURATION= _GUID; >> + >> +static void destroy_efi_spi_peripheral(struct efi_spi_peripheral *peri= pheral) >> +{ >> + struct efi_spi_peripheral_priv *priv =3D >> + 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 calli= ng > this. Uninstalling protocols from a handler should take care of this fo= r > 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 =3D bus->peripheral_list; >> + >> + while (peripheral) { >> + struct efi_spi_peripheral *next =3D >> + peripheral->next_spi_peripheral; >> + destroy_efi_spi_peripheral(peripheral); >> + peripheral =3D next; >> + } >> + free(bus->friendly_name); >> + free(bus); >> +} >> + >> +static efi_status_t efi_spi_new_handle(const efi_guid_t *guid, void *p= roto) >> +{ >> + efi_status_t status; >> + efi_handle_t handle; >> + >> + status =3D efi_create_handle(&handle); >> + if (status !=3D EFI_SUCCESS) { >> + printf("Failed to create EFI handle\n"); >> + goto fail_1; >> + } >> + >> + status =3D efi_add_protocol(handle, guid, proto); >> + if (status !=3D 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 =3D vendor_utf16; >> + part->part_number =3D part_number_utf16; >> + part->min_clock_hz =3D 0; >> + part->max_clock_hz =3D target->max_hz; >> + part->chip_select_polarity =3D >> + (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 =3D name_utf16; >> + peripheral->spi_part =3D part; >> + peripheral->next_spi_peripheral =3D NULL; >> + peripheral->spi_peripheral_driver_guid =3D guid; >> + peripheral->max_clock_hz =3D target->max_hz; >> + peripheral->clock_polarity =3D (target->mode & SPI_CPOL) ? true : fal= se; >> + peripheral->clock_phase =3D (target->mode & SPI_CPHA) ? true : false; >> + peripheral->attributes =3D 0; >> + peripheral->configuration_data =3D NULL; >> + peripheral->spi_bus =3D bus; >> + peripheral->chip_select =3D efi_spi_peripheral_chip_select; >> + peripheral->chip_select_parameter =3D 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 =3D bus->peripheral_list; >> + >> + while (tmp->next_spi_peripheral) >> + tmp =3D tmp->next_spi_peripheral; >> + >> + tmp->next_spi_peripheral =3D peripheral; >> + } else { >> + bus->peripheral_list =3D 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 =3D peripheral; >> + io_protocol->original_spi_peripheral =3D peripheral; >> + io_protocol->legacy_spi_protocol =3D &dummy_legacy_spi_controller_pro= tocol; >> + io_protocol->transaction =3D efi_spi_io_transaction; >> + io_protocol->update_spi_peripheral =3D efi_spi_io_update_spi_peripher= al; >> + >> + /* This is a bit of a hack. The EFI data structures do not allow us t= o >> + * represent a frame size greater than 32 bits. >> + */ >> + if (target->wordlen <=3D 32) >> + io_protocol->frame_size_support_mask =3D >> + 1 << (target->wordlen - 1); >> + else >> + io_protocol->frame_size_support_mask =3D 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 limi= t >> + * 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 =3D target->max_write_size ? target->max_write_size : 0xFFF= FFFFF; >> + max_read =3D target->max_read_size ? target->max_read_size : 0xFFFFFF= FF; >> + io_protocol->maximum_transfer_bytes =3D (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 use= d. >> + */ >> + io_protocol->attributes =3D 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 =3D dev->parent; >> + struct spi_slave *target; >> + const char *name =3D dev_read_name(dev); >> + const char *vendor =3D dev_read_string(dev, "uefi-spi-vendor"); >> + const char *part_number =3D dev_read_string(dev, "uefi-spi-part-numbe= r"); >> + efi_guid_t *guid =3D >> + (efi_guid_t *)dev_read_u8_array_ptr(dev, "uefi-spi-io-guid", 16); >> + >> + if (device_get_uclass_id(dev) =3D=3D 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 =3D EFI_UNSUPPORTED; >> + goto fail_1; >> + } >> + >> + if (!device_active(dev)) { >> + int ret =3D device_probe(dev); >> + if (ret) { >> + debug("Skipping SPI peripheral %s, probe failed\n", name); >> + goto fail_1; >> + } >> + } >> + >> + target =3D dev_get_parent_priv(dev); >> + if (!target) { >> + debug("Skipping uninitialized SPI peripheral %s\n", name); >> + status =3D EFI_UNSUPPORTED; >> + goto fail_1; >> + } >> + >> + debug("Registering SPI dev %d:%d, name %s\n", >> + dev_bus->seq_, spi_chip_select(dev), name); >> + >> + priv =3D calloc(1, sizeof(*priv)); >> + if (!priv) { >> + status =3D EFI_OUT_OF_RESOURCES; >> + goto fail_1; >> + } >> + >> + vendor_utf16 =3D convert_efi_string(vendor); >> + if (!vendor_utf16) { >> + status =3D EFI_OUT_OF_RESOURCES; >> + goto fail_2; >> + } >> + >> + part_number_utf16 =3D convert_efi_string(part_number); >> + if (!part_number_utf16) { >> + status =3D EFI_OUT_OF_RESOURCES; >> + goto fail_3; >> + } >> + >> + name_utf16 =3D convert_efi_string(name); >> + if (!name_utf16) { >> + status =3D EFI_OUT_OF_RESOURCES; >> + goto fail_4; >> + } >> + >> + priv->target =3D target; >> + >> + efi_spi_init_part(&priv->part, target, vendor_utf16, part_number_utf1= 6); >> + >> + 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, targe= t); >> + >> + status =3D efi_spi_new_handle(guid, &priv->io_protocol); >> + if (status !=3D 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 =3D uclass_get_device(UCLASS_SPI, i, &dev); >> + if (r < 0) { >> + printf("Failed to get SPI bus %d\n", i); >> + goto fail_1; >> + } >> + >> + name =3D dev_read_name(dev); >> + debug("Registering SPI bus %d, name %s\n", i, name); >> + >> + bus =3D calloc(1, sizeof(*bus)); >> + if (!bus) >> + goto fail_1; >> + >> + bus->friendly_name =3D convert_efi_string(name); >> + if (!bus->friendly_name) >> + goto fail_2; >> + >> + bus->peripheral_list =3D NULL; >> + bus->clock =3D efi_spi_bus_clock; >> + bus->clock_parameter =3D 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 =3D &null_device_path; >> + >> + device_foreach_child(child, dev) { >> + efi_status_t status =3D export_spi_peripheral(bus, child); >> + >> + if (status =3D=3D 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 =3D calloc(1, sizeof(*proto)); >> + if (!proto) { >> + status =3D EFI_OUT_OF_RESOURCES; >> + goto fail_1; >> + } >> + >> + proto->bus_count =3D uclass_id_count(UCLASS_SPI); >> + proto->bus_list =3D calloc(proto->bus_count, sizeof(*proto->bus_list)= ); >> + if (!proto->bus_list) { >> + status =3D EFI_OUT_OF_RESOURCES; >> + goto fail_2; >> + } >> + >> + for (i =3D 0; i < proto->bus_count; i++) { >> + proto->bus_list[i] =3D export_spi_bus(i); >> + if (!proto->bus_list[i]) >> + goto fail_3; >> + } >> + >> + status =3D efi_spi_new_handle(&efi_spi_configuration_guid, proto); >> + if (status !=3D EFI_SUCCESS) >> + goto fail_3; >> + >> + return EFI_SUCCESS; >> + >> +fail_3: >> + for (i =3D 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? Different patch not patch set. Best regards Heinrich > >> index daac6c396820..2790fcd784e0 100644 >> --- a/lib/efi_selftest/Makefile >> +++ b/lib/efi_selftest/Makefile >> @@ -63,6 +63,7 @@ obj-$(CONFIG_EFI_LOADER_HII) +=3D efi_selftest_hii.o >> obj-$(CONFIG_EFI_RNG_PROTOCOL) +=3D efi_selftest_rng.o >> obj-$(CONFIG_EFI_GET_TIME) +=3D efi_selftest_rtc.o >> obj-$(CONFIG_EFI_TCG2_PROTOCOL) +=3D efi_selftest_tcg2.o >> +obj-$(CONFIG_EFI_SPI_PROTOCOL) +=3D efi_selftest_spi_protocol.o >> >> ifeq ($(CONFIG_GENERATE_ACPI_TABLE),) >> obj-y +=3D efi_selftest_fdt.o >> diff --git a/lib/efi_selftest/efi_selftest_spi_protocol.c b/lib/efi_sel= ftest/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 >> +#include >> + >> +static struct efi_boot_services *boottime; >> +static efi_guid_t efi_spi_configuration_guid =3D EFI_SPI_CONFIGURATION= _GUID; >> + >> +static int setup(const efi_handle_t img_handle, >> + const struct efi_system_table *systable) >> +{ >> + boottime =3D systable->boottime; >> + return EFI_ST_SUCCESS; >> +} >> + > > [...] > > Thanks! > /Ilias