* [PATCH v3 0/3] Support UEFI SPI I/O protocol
@ 2022-09-21 16:06 Paul Barker
2022-09-21 16:06 ` [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support Paul Barker
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Paul Barker @ 2022-09-21 16:06 UTC (permalink / raw)
To: u-boot, Heinrich Schuchardt, Tom Rini, Ilias Apalodimas; +Cc: Paul Barker
These patches add support for the UEFI SPI I/O protocol defined in the
UEFI Platform Initialization (PI) Specification, Version 1.7 Errata A
(April 2020). This allows a UEFI application to interact with devices
on the SPI bus.
This support is initially intended to be used to communicate with SPI
devices from "pre-boot" UEFI applications which perform setup before
the main OS is loaded. Other use cases may also be supported, however
this is not intended to be a replacement for UEFI capsule updates.
The "pre-boot" UEFI application which we are currently developing will
interact with a Micron Authenta[1] flash device, sending vendor-specific
commands over the SPI bus to make use of Authenta security features, to
verify flash integrity and to generate ephemeral keys based on the flash
contents.
The code here is self-contained and easy to enable/disable at compile
time. Compilation testing with am335x_evm_defconfig shows that the size
of u-boot.img increases by less than 2kB when CONFIG_EFI_SPI_PROTOCOL
is enabled.
[1]: https://www.micron.com/-/media/client/global/documents/products/product-flyer/authenta_technology_solutions_brief.pdf
Changes since v2:
* Added description to efi_spi_protocol.h.
* Moved definition of EFI_SPI_CONFIGURATION_GUID to efi_api.h and
added it to the list in lib/uuid.c.
* Fixed UEFI GUID byte order in test.dts and in debug output.
* Use log_debug() instead of printf() for debug output.
* Add test cases to confirm that efi_legacy_spi_controller_protocol
functions return EFI_UNSUPPORTED (as they are not currently
implemented)
Changes since v1:
* Skip emulated SPI peripherals. These appear as duplicates within the
list of devices on the bus when using the sandbox SPI drivers.
* Make most printf statements debug only.
* Add efi_seltest unit test.
* Do not enable config EFI_SPI_PROTOCOL by default.
* Mark functions with EFIAPI where necessary.
* Handle debug_transaction argument to efi_spi_io_transaction().
* Handle a value of zero for target->max_read_size &
target->max_write_size.
* Probe inactive SPI devices when initializing the EFI SPI protocol to
ensure that dev_get_parent_priv() returns valid data and the exported
devices are ready to use.
Paul Barker (3):
efi_loader: Add SPI I/O protocol support
arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export
am335x_evm_defconfig: Enable Micron SPI flash support
MAINTAINERS | 7 +
arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 +-
arch/sandbox/dts/test.dts | 13 +
configs/am335x_evm_defconfig | 1 +
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 +
14 files changed, 1121 insertions(+), 2 deletions(-)
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
--
2.25.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-21 16:06 [PATCH v3 0/3] Support UEFI SPI I/O protocol Paul Barker @ 2022-09-21 16:06 ` Paul Barker 2022-09-26 12:43 ` Ilias Apalodimas 2022-09-26 13:52 ` Heinrich Schuchardt 2022-09-21 16:06 ` [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export Paul Barker 2022-09-21 16:06 ` [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support Paul Barker 2 siblings, 2 replies; 20+ messages in thread From: Paul Barker @ 2022-09-21 16:06 UTC (permalink / raw) To: u-boot, Heinrich Schuchardt, Tom Rini, Ilias Apalodimas; +Cc: Paul Barker 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@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@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@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"; + /* 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]; }; }; diff --git a/include/efi_api.h b/include/efi_api.h index 9bb0d44ac8d5..eebefbe1585a 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -454,6 +454,10 @@ struct efi_runtime_services { EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \ 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) +#define EFI_SPI_CONFIGURATION_GUID \ + EFI_GUID(0x85a6d3e6, 0xb65b, 0x4afc, \ + 0xb3, 0x8f, 0xc6, 0xd5, 0x4a, 0xf6, 0xdd, 0xc8) + #define RISCV_EFI_BOOT_PROTOCOL_GUID \ EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, \ 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf) diff --git a/include/efi_loader.h b/include/efi_loader.h index 545ba06d9466..150fa3cdc00b 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -541,6 +541,10 @@ efi_status_t efi_rng_register(void); efi_status_t efi_tcg2_register(void); /* Called by efi_init_obj_list() to install RISCV_EFI_BOOT_PROTOCOL */ efi_status_t efi_riscv_register(void); +/* Called by efi_init_obj_list() to install EFI_SPI_CONFIGURATION_PROTOCOL & + * EFI_SPI_IO_PROTOCOL + */ +efi_status_t efi_spi_protocol_register(void); /* Called by efi_init_obj_list() to do initial measurement */ efi_status_t efi_tcg2_do_initial_measurement(void); /* measure the pe-coff image, extend PCR and add Event Log */ diff --git a/include/efi_spi_protocol.h b/include/efi_spi_protocol.h new file mode 100644 index 000000000000..1a247a6a6f85 --- /dev/null +++ b/include/efi_spi_protocol.h @@ -0,0 +1,166 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent */ +/* + * Copyright (c) 2017, Intel Corporation. All rights reserved. + * Copyright (c) 2022 Micron Technology, Inc. + * + * This header is based on code from EDK II, with modifications to match the + * u-boot coding style. It defines data structures for the following protocols + * found in the UEFI Platform Initialization (PI) Specification, version 1.7 + * errata A: + * + * - EFI_SPI_CONFIGURATION_PROTOCOL + * - EFI_SPI_IO_PROTOCOL + * - EFI_LEGACY_SPI_CONTROLLER_PROTOCOL (required by the I/O protocol as the + * spec does not appear to allow the legacy_spi_protocol pointer to be NULL + * in struct efi_spi_io_protocol) + */ + +#ifndef _EFI_SPI_PROTOCOL_H +#define _EFI_SPI_PROTOCOL_H + +#include <efi.h> +#include <efi_api.h> + +struct efi_spi_peripheral; + +struct efi_spi_part { + efi_string_t vendor; + efi_string_t part_number; + u32 min_clock_hz; + u32 max_clock_hz; + bool chip_select_polarity; +}; + +struct efi_spi_bus { + efi_string_t friendly_name; + struct efi_spi_peripheral *peripheral_list; + struct efi_device_path *controller_path; + + efi_status_t + (EFIAPI * clock)(const struct efi_spi_peripheral *spi_peripheral, + u32 *clock_hz); + + void *clock_parameter; +}; + +struct efi_spi_peripheral { + struct efi_spi_peripheral *next_spi_peripheral; + efi_string_t friendly_name; + efi_guid_t *spi_peripheral_driver_guid; + struct efi_spi_part *spi_part; + u32 max_clock_hz; + bool clock_polarity; + bool clock_phase; + u32 attributes; + void *configuration_data; + struct efi_spi_bus *spi_bus; + + efi_status_t + (EFIAPI * chip_select)(const struct efi_spi_peripheral *spi_peripheral, + bool pin_value); + + void *chip_select_parameter; +}; + +struct efi_spi_configuration_protocol { + u32 bus_count; + struct efi_spi_bus **bus_list; +}; + +#define EFI_LEGACY_SPI_CONTROLLER_GUID \ + EFI_GUID(0x39136fc7, 0x1a11, 0x49de, \ + 0xbf, 0x35, 0x0e, 0x78, 0xdd, 0xb5, 0x24, 0xfc) + +struct efi_legacy_spi_controller_protocol; + +struct efi_legacy_spi_controller_protocol { + u32 maximum_offset; + u32 maximum_range_bytes; + u32 range_register_count; + + efi_status_t + (EFIAPI * erase_block_opcode)(const struct efi_legacy_spi_controller_protocol *this, + u8 erase_block_opcode); + + efi_status_t + (EFIAPI * write_status_prefix)(const struct efi_legacy_spi_controller_protocol *this, + u8 write_status_prefix); + + efi_status_t + (EFIAPI * bios_base_address)(const struct efi_legacy_spi_controller_protocol *this, + u32 bios_base_address); + + efi_status_t + (EFIAPI * clear_spi_protect)(const struct efi_legacy_spi_controller_protocol *this); + + bool + (EFIAPI * is_range_protected)(const struct efi_legacy_spi_controller_protocol *this, + u32 bios_address, + u32 blocks_to_protect); + + efi_status_t + (EFIAPI * protect_next_range)(const struct efi_legacy_spi_controller_protocol *this, + u32 bios_address, + u32 blocks_to_protect); + + efi_status_t + (EFIAPI * lock_controller)(const struct efi_legacy_spi_controller_protocol *this); +}; + +struct efi_spi_io_protocol; + +enum efi_spi_transaction_type { + SPI_TRANSACTION_FULL_DUPLEX, + SPI_TRANSACTION_WRITE_ONLY, + SPI_TRANSACTION_READ_ONLY, + SPI_TRANSACTION_WRITE_THEN_READ +}; + +struct efi_spi_bus_transaction { + struct efi_spi_peripheral *spi_peripheral; + enum efi_spi_transaction_type transaction_type; + bool debug_transaction; + u32 bus_width; + u32 frame_size; + u32 write_bytes; + u8 *write_buffer; + u32 read_bytes; + u8 *read_buffer; +}; + +struct efi_spi_io_protocol { + struct efi_spi_peripheral *spi_peripheral; + struct efi_spi_peripheral *original_spi_peripheral; + u32 frame_size_support_mask; + u32 maximum_transfer_bytes; + u32 attributes; + struct efi_legacy_spi_controller_protocol *legacy_spi_protocol; + + efi_status_t + (EFIAPI * 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); + + efi_status_t + (EFIAPI * update_spi_peripheral)(struct efi_spi_io_protocol *this, + struct efi_spi_peripheral *spi_peripheral); +}; + +struct efi_spi_peripheral_priv { + struct efi_spi_peripheral peripheral; + struct efi_spi_part part; + struct efi_spi_io_protocol io_protocol; + efi_handle_t handle; + struct spi_slave *target; +}; + +efi_status_t efi_spi_protocol_register(void); + +#endif diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index b8fb2701a74e..a9533eac4cbc 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -412,4 +412,12 @@ config EFI_RISCV_BOOT_PROTOCOL replace the transfer via the device-tree. The latter is not possible on systems using ACPI. +config EFI_SPI_PROTOCOL + bool "EFI SPI protocol support" + depends on DM_SPI + help + Provide implementations of EFI_SPI_CONFIGURATION_PROTOCOL and + EFI_SPI_IO_PROTOCOL to allow UEFI applications to access devices + connected via SPI bus. + endif diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index e187d2a914f9..8b77dddfbb8e 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -74,6 +74,7 @@ obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o +obj-$(CONFIG_EFI_SPI_PROTOCOL) += efi_spi_protocol.o obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o obj-$(CONFIG_EFI_ECPT) += efi_conformance.o diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index c633fcd91e35..1fb7f6497b1a 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -309,6 +309,12 @@ efi_status_t efi_init_obj_list(void) goto out; } + if (IS_ENABLED(CONFIG_EFI_SPI_PROTOCOL)) { + ret = efi_spi_protocol_register(); + if (ret != EFI_SUCCESS) + goto out; + } + /* Secure boot */ ret = efi_init_secure_boot(); if (ret != EFI_SUCCESS) diff --git a/lib/efi_loader/efi_spi_protocol.c b/lib/efi_loader/efi_spi_protocol.c new file mode 100644 index 000000000000..94c16b0549b9 --- /dev/null +++ b/lib/efi_loader/efi_spi_protocol.c @@ -0,0 +1,614 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2022 Micron Technology, Inc. + */ + +#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; +} + +static void dump_buffer(const char *msg, u32 length, u8 *buffer) +{ + u32 i; + EFI_PRINT("%s %d bytes:", msg, length); + for (i = 0; i < length; i++) + EFI_PRINT(" %02x", buffer[i]); + EFI_PRINT("\n"); +} + +static efi_status_t EFIAPI +efi_spi_bus_clock(const struct efi_spi_peripheral *spi_peripheral, + u32 *clock_hz) +{ + EFI_ENTRY("%p, %p", spi_peripheral, clock_hz); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +efi_spi_peripheral_chip_select(const struct efi_spi_peripheral *spi_peripheral, + bool pin_value) +{ + EFI_ENTRY("%p, %d", spi_peripheral, pin_value); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +legacy_erase_block_opcode(const struct efi_legacy_spi_controller_protocol *this, + u8 erase_block_opcode) +{ + EFI_ENTRY("%p, %u", this, erase_block_opcode); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +legacy_write_status_prefix(const struct efi_legacy_spi_controller_protocol *this, + u8 write_status_prefix) +{ + EFI_ENTRY("%p, %u", this, write_status_prefix); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +legacy_bios_base_address(const struct efi_legacy_spi_controller_protocol *this, + u32 bios_base_address) +{ + EFI_ENTRY("%p, %u", this, bios_base_address); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +legacy_clear_spi_protect(const struct efi_legacy_spi_controller_protocol *this) +{ + EFI_ENTRY("%p", this); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static bool EFIAPI +legacy_is_range_protected(const struct efi_legacy_spi_controller_protocol *this, + u32 bios_address, + u32 blocks_to_protect) +{ + EFI_ENTRY("%p, %u, %u", this, bios_address, blocks_to_protect); + return EFI_EXIT(false); +} + +static efi_status_t EFIAPI +legacy_protect_next_range(const struct efi_legacy_spi_controller_protocol *this, + u32 bios_address, + u32 blocks_to_protect) +{ + EFI_ENTRY("%p, %u, %u", this, bios_address, blocks_to_protect); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +legacy_lock_controller(const struct efi_legacy_spi_controller_protocol *this) +{ + EFI_ENTRY("%p", this); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +static efi_status_t EFIAPI +efi_spi_io_update_spi_peripheral(struct efi_spi_io_protocol *this, + struct efi_spi_peripheral *spi_peripheral) +{ + EFI_ENTRY("%p, %p", this, spi_peripheral); + return EFI_EXIT(EFI_UNSUPPORTED); +} + +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); + } 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 +}; + +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); + 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 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; +} + +static int test_peripheral(struct efi_spi_peripheral *p, struct efi_spi_bus *bus) +{ + struct efi_spi_io_protocol *io_protocol; + u8 req[5], resp[5]; + efi_status_t ret; + + if (!p->friendly_name) { + efi_st_error("SPI peripheral lacks a friendly name\n"); + return EFI_ST_FAILURE; + } + + if (!p->spi_peripheral_driver_guid) { + efi_st_error("SPI peripheral lacks driver GUID\n"); + return EFI_ST_FAILURE; + } + + if (!p->spi_part) { + efi_st_error("SPI peripheral lacks SPI part definition\n"); + return EFI_ST_FAILURE; + } + + if (!p->max_clock_hz) { + efi_st_error("SPI peripheral has a max clock rate of zero\n"); + return EFI_ST_FAILURE; + } + + if (!p->spi_bus) { + efi_st_error("SPI peripheral lack pointer to SPI bus\n"); + return EFI_ST_FAILURE; + } + + if (p->spi_bus != bus) { + efi_st_error("SPI peripheral spi_bus pointer points to the wrong bus\n"); + return EFI_ST_FAILURE; + } + + if (!p->chip_select) { + efi_st_error("SPI peripheral lacks chip_select function\n"); + return EFI_ST_FAILURE; + } + + if (!p->spi_part->vendor) { + efi_st_error("SPI part lacks vendor string\n"); + return EFI_ST_FAILURE; + } + + if (!p->spi_part->part_number) { + efi_st_error("SPI part lacks part number string\n"); + return EFI_ST_FAILURE; + } + + if (p->spi_part->min_clock_hz > p->spi_part->max_clock_hz) { + efi_st_error("SPI part min clock rate is greater than max clock rate\n"); + return EFI_ST_FAILURE; + } + + if (p->spi_part->max_clock_hz != p->max_clock_hz) { + efi_st_error("SPI part max clock rate does not match peripheral max clock rate\n"); + return EFI_ST_FAILURE; + } + + ret = boottime->locate_protocol(p->spi_peripheral_driver_guid, + NULL, (void **)&io_protocol); + if (ret != EFI_SUCCESS) { + efi_st_error("SPI IO protocol not available\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->spi_peripheral != p) { + efi_st_error("SPI IO protocol spi_peripheral pointer points to the wrong peripheral\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->original_spi_peripheral != p) { + efi_st_error("SPI IO protocol original_spi_peripheral pointer points to the wrong peripheral\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->maximum_transfer_bytes) { + efi_st_error("SPI IO protocol has zero maximum transfer size\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol) { + efi_st_error("SPI IO protocol lacks legacy SPI protocol\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->transaction) { + efi_st_error("SPI IO protocol lacks transaction function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->update_spi_peripheral) { + efi_st_error("SPI IO protocol lacks update_spi_peripheral function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->erase_block_opcode) { + efi_st_error("SPI legacy controller protocol lacks erase_block_opcode function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->erase_block_opcode( + io_protocol->legacy_spi_protocol, + 0) != EFI_UNSUPPORTED) { + efi_st_error("Incorrect return value from SPI legacy controller protocol erase_block_opcode function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->write_status_prefix) { + efi_st_error("SPI legacy controller protocol lacks write_status_prefix function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->write_status_prefix( + io_protocol->legacy_spi_protocol, + 0) != EFI_UNSUPPORTED) { + efi_st_error("Incorrect return value from SPI legacy controller protocol write_status_prefix function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->bios_base_address) { + efi_st_error("SPI legacy controller protocol lacks bios_base_address function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->bios_base_address( + io_protocol->legacy_spi_protocol, + 0) != EFI_UNSUPPORTED) { + efi_st_error("Incorrect return value from SPI legacy controller protocol bios_base_address function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->clear_spi_protect) { + efi_st_error("SPI legacy controller protocol lacks clear_spi_protect function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->clear_spi_protect( + io_protocol->legacy_spi_protocol) != EFI_UNSUPPORTED) { + efi_st_error("Incorrect return value from SPI legacy controller protocol clear_spi_protect function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->is_range_protected) { + efi_st_error("SPI legacy controller protocol lacks is_range_protected function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->is_range_protected( + io_protocol->legacy_spi_protocol, + 0, 0)) { + efi_st_error("Incorrect return value from SPI legacy controller protocol is_range_protected function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->protect_next_range) { + efi_st_error("SPI legacy controller protocol lacks protect_next_range function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->protect_next_range( + io_protocol->legacy_spi_protocol, + 0, 0) != EFI_UNSUPPORTED) { + efi_st_error("Incorrect return value from SPI legacy controller protocol protect_next_range function\n"); + return EFI_ST_FAILURE; + } + + if (!io_protocol->legacy_spi_protocol->lock_controller) { + efi_st_error("SPI legacy controller protocol lacks lock_controller function\n"); + return EFI_ST_FAILURE; + } + + if (io_protocol->legacy_spi_protocol->lock_controller( + io_protocol->legacy_spi_protocol) != EFI_UNSUPPORTED) { + efi_st_error("Incorrect return value from SPI legacy controller protocol lock_controller function\n"); + return EFI_ST_FAILURE; + } + + req[0] = 0x9f; + ret = io_protocol->transaction(io_protocol, + SPI_TRANSACTION_FULL_DUPLEX, + false, 0, 1, 8, + sizeof(req), req, + sizeof(resp), resp); + if (ret != EFI_SUCCESS) { + efi_st_error("SPI transaction failed\n"); + return EFI_ST_FAILURE; + } + + if ((resp[0] != 0xff) || (resp[1] != 0x20) || (resp[2] != 0x20) || (resp[3] != 0x15)) { + efi_st_error("Incorrect response from sandbox SPI flash emulator\n"); + return EFI_ST_FAILURE; + } + + return EFI_ST_SUCCESS; +} + +static int test_bus(struct efi_spi_bus *bus) +{ + struct efi_spi_peripheral *p; + int status; + + if (!bus->friendly_name) { + efi_st_error("SPI bus lacks a friendly name\n"); + return EFI_ST_FAILURE; + } + + if (!bus->peripheral_list) { + efi_st_error("SPI bus has zero peripherals\n"); + return EFI_ST_FAILURE; + } + + if (!bus->clock) { + efi_st_error("SPI bus lacks clock function\n"); + return EFI_ST_FAILURE; + } + + for (p = bus->peripheral_list; p; p = p->next_spi_peripheral) { + status = test_peripheral(p, bus); + if (status) { + efi_st_error("Failed testing SPI peripheral\n"); + return status; + } + } + + return EFI_ST_SUCCESS; +} + +static int execute(void) +{ + struct efi_spi_configuration_protocol *spi; + efi_status_t ret; + int status; + u32 i; + + ret = boottime->locate_protocol(&efi_spi_configuration_guid, + NULL, (void **)&spi); + if (ret != EFI_SUCCESS) { + efi_st_error("SPI configuration protocol not available\n"); + return EFI_ST_FAILURE; + } + + if (!spi->bus_count) { + efi_st_error("SPI configuration protocol has zero busses\n"); + return EFI_ST_FAILURE; + } + + for (i = 0; i < spi->bus_count; i++) { + status = test_bus(spi->bus_list[i]); + if (status) { + efi_st_error("Failed testing SPI bus %d\n", i); + return status; + } + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(spi_protocol) = { + .name = "SPI protocol", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, +}; diff --git a/lib/uuid.c b/lib/uuid.c index 465e1ac38f57..3f723b732588 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -187,6 +187,10 @@ static const struct { "TCG2", EFI_TCG2_PROTOCOL_GUID, }, + { + "SPI Protocol Stack", + EFI_SPI_CONFIGURATION_GUID + }, { "System Partition", PARTITION_SYSTEM_GUID -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-21 16:06 ` [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support Paul Barker @ 2022-09-26 12:43 ` Ilias Apalodimas 2022-09-26 13:13 ` Heinrich Schuchardt 2022-10-03 12:00 ` Paul Barker 2022-09-26 13:52 ` Heinrich Schuchardt 1 sibling, 2 replies; 20+ messages in thread From: Ilias Apalodimas @ 2022-09-26 12:43 UTC (permalink / raw) To: Paul Barker; +Cc: u-boot, Heinrich Schuchardt, Tom Rini 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@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@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@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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-26 12:43 ` Ilias Apalodimas @ 2022-09-26 13:13 ` Heinrich Schuchardt 2022-09-26 13:40 ` Ilias Apalodimas 2022-10-03 12:05 ` Paul Barker 2022-10-03 12:00 ` Paul Barker 1 sibling, 2 replies; 20+ messages in thread From: Heinrich Schuchardt @ 2022-09-26 13:13 UTC (permalink / raw) To: Paul Barker; +Cc: u-boot, Tom Rini, Ilias Apalodimas 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 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@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@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@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_8 seems to be unused. >> + 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()? 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 != 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? 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 = 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? 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) += 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-26 13:13 ` Heinrich Schuchardt @ 2022-09-26 13:40 ` Ilias Apalodimas 2022-10-03 12:05 ` Paul Barker 1 sibling, 0 replies; 20+ messages in thread From: Ilias Apalodimas @ 2022-09-26 13:40 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Paul Barker, u-boot, Tom Rini Hi Heinrich, [...] > > > +}; > > > + > > > +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? > > 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. There's no protocol installation for this, The protocl we install is efi_spi_io_protocol and one of it's struct members is that legacy spi protocol [...] Cheers /Ilias ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-26 13:13 ` Heinrich Schuchardt 2022-09-26 13:40 ` Ilias Apalodimas @ 2022-10-03 12:05 ` Paul Barker 1 sibling, 0 replies; 20+ messages in thread From: Paul Barker @ 2022-10-03 12:05 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: u-boot, Tom Rini, Ilias Apalodimas [-- Attachment #1.1.1: Type: text/plain, Size: 611 bytes --] Addressing only the comments which I've not covered in my reply to Ilias' email: On 26/09/2022 14:13, Heinrich Schuchardt wrote: > On 9/26/22 14:43, Ilias Apalodimas wrote: >> On Wed, Sep 21, 2022 at 05:06:26PM +0100, Paul Barker wrote: >>> +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_8 seems to be unused. I'll drop this in v4. Thanks, -- Paul Barker Principal Software Engineer SanCloud Ltd e: paul.barker@sancloud.com w: https://sancloud.com/ [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 6865 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-26 12:43 ` Ilias Apalodimas 2022-09-26 13:13 ` Heinrich Schuchardt @ 2022-10-03 12:00 ` Paul Barker 1 sibling, 0 replies; 20+ messages in thread From: Paul Barker @ 2022-10-03 12:00 UTC (permalink / raw) To: Ilias Apalodimas; +Cc: u-boot, Heinrich Schuchardt, Tom Rini [-- Attachment #1.1.1: Type: text/plain, Size: 23931 bytes --] 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@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@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@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. 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@sancloud.com w: https://sancloud.com/ [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 6865 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-21 16:06 ` [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support Paul Barker 2022-09-26 12:43 ` Ilias Apalodimas @ 2022-09-26 13:52 ` Heinrich Schuchardt 2022-10-03 12:05 ` Paul Barker 1 sibling, 1 reply; 20+ messages in thread From: Heinrich Schuchardt @ 2022-09-26 13:52 UTC (permalink / raw) To: Paul Barker; +Cc: u-boot, Tom Rini, Ilias Apalodimas On 9/21/22 18:06, Paul Barker wrote: > + > + 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; This should be log_debug(Added EFI_SPI_IO_PROTOCOL for %s with guid %pD\n", guid); Best regards Heinrich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-09-26 13:52 ` Heinrich Schuchardt @ 2022-10-03 12:05 ` Paul Barker 2022-10-04 16:31 ` Paul Barker 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2022-10-03 12:05 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: u-boot, Tom Rini, Ilias Apalodimas [-- Attachment #1.1.1: Type: text/plain, Size: 936 bytes --] On 26/09/2022 14:52, Heinrich Schuchardt wrote: > On 9/21/22 18:06, Paul Barker wrote: >> + >> + 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; > > This should be > > log_debug(Added EFI_SPI_IO_PROTOCOL for %s with guid %pD\n", guid); I'll address this in v4. > > Best regards > > Heinrich Thanks for the review comments. -- Paul Barker Principal Software Engineer SanCloud Ltd e: paul.barker@sancloud.com w: https://sancloud.com/ [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 6865 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-10-03 12:05 ` Paul Barker @ 2022-10-04 16:31 ` Paul Barker 2022-10-04 17:25 ` Heinrich Schuchardt 0 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2022-10-04 16:31 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: u-boot, Tom Rini, Ilias Apalodimas [-- Attachment #1.1.1: Type: text/plain, Size: 1004 bytes --] On 03/10/2022 13:05, Paul Barker wrote: > On 26/09/2022 14:52, Heinrich Schuchardt wrote: >> On 9/21/22 18:06, Paul Barker wrote: >>> + >>> + 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; >> >> This should be >> >> log_debug(Added EFI_SPI_IO_PROTOCOL for %s with guid %pD\n", guid); > > I'll address this in v4. After a quick look in lib/vsprintf.c, I assume you meant %pUl here. Thanks, -- Paul Barker Principal Software Engineer SanCloud Ltd e: paul.barker@sancloud.com w: https://sancloud.com/ [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 6865 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support 2022-10-04 16:31 ` Paul Barker @ 2022-10-04 17:25 ` Heinrich Schuchardt 0 siblings, 0 replies; 20+ messages in thread From: Heinrich Schuchardt @ 2022-10-04 17:25 UTC (permalink / raw) To: Paul Barker; +Cc: u-boot, Tom Rini, Ilias Apalodimas On 10/4/22 18:31, Paul Barker wrote: > On 03/10/2022 13:05, Paul Barker wrote: >> On 26/09/2022 14:52, Heinrich Schuchardt wrote: >>> On 9/21/22 18:06, Paul Barker wrote: >>>> + >>>> + 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; >>> >>> This should be >>> >>> log_debug(Added EFI_SPI_IO_PROTOCOL for %s with guid %pD\n", guid); >> >> I'll address this in v4. > > After a quick look in lib/vsprintf.c, I assume you meant %pUl here. yes > > Thanks, > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export 2022-09-21 16:06 [PATCH v3 0/3] Support UEFI SPI I/O protocol Paul Barker 2022-09-21 16:06 ` [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support Paul Barker @ 2022-09-21 16:06 ` Paul Barker 2022-09-21 16:15 ` Tom Rini 2022-09-21 16:06 ` [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support Paul Barker 2 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2022-09-21 16:06 UTC (permalink / raw) To: u-boot, Heinrich Schuchardt, Tom Rini, Ilias Apalodimas; +Cc: Paul Barker Add properties to the Authenta SPI flash device node to enable access by a UEFI application using a fixed GUID. Also specify that this device is JEDEC compatible so that it is correctly initialized when running `sf probe`. Signed-off-by: Paul Barker <paul.barker@sancloud.com> --- arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite.dts b/arch/arm/dts/am335x-sancloud-bbe-lite.dts index d6ef19311a91..f1ff9d6024cb 100644 --- a/arch/arm/dts/am335x-sancloud-bbe-lite.dts +++ b/arch/arm/dts/am335x-sancloud-bbe-lite.dts @@ -37,14 +37,20 @@ pinctrl-names = "default"; pinctrl-0 = <&bb_spi0_pins>; - channel@0 { + authenta-flash@0 { #address-cells = <1>; #size-cells = <0>; - compatible = "micron,spi-authenta"; + compatible = "micron,spi-authenta", "jedec,spi-nor"; reg = <0>; spi-max-frequency = <16000000>; spi-cpha; + + uefi-spi-vendor = "micron"; + uefi-spi-part-number = "mt25ql128abb"; + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ + uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 + b3 41 88 1f e1 8e 7f 7d]; }; }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export 2022-09-21 16:06 ` [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export Paul Barker @ 2022-09-21 16:15 ` Tom Rini 2022-09-26 11:33 ` Ilias Apalodimas 0 siblings, 1 reply; 20+ messages in thread From: Tom Rini @ 2022-09-21 16:15 UTC (permalink / raw) To: Paul Barker; +Cc: u-boot, Heinrich Schuchardt, Ilias Apalodimas [-- Attachment #1: Type: text/plain, Size: 1503 bytes --] On Wed, Sep 21, 2022 at 05:06:27PM +0100, Paul Barker wrote: > Add properties to the Authenta SPI flash device node to enable access by > a UEFI application using a fixed GUID. Also specify that this device is > JEDEC compatible so that it is correctly initialized when running > `sf probe`. > > Signed-off-by: Paul Barker <paul.barker@sancloud.com> > --- > arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite.dts b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > index d6ef19311a91..f1ff9d6024cb 100644 > --- a/arch/arm/dts/am335x-sancloud-bbe-lite.dts > +++ b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > @@ -37,14 +37,20 @@ > pinctrl-names = "default"; > pinctrl-0 = <&bb_spi0_pins>; > > - channel@0 { > + authenta-flash@0 { > #address-cells = <1>; > #size-cells = <0>; > > - compatible = "micron,spi-authenta"; > + compatible = "micron,spi-authenta", "jedec,spi-nor"; > > reg = <0>; > spi-max-frequency = <16000000>; > spi-cpha; > + > + uefi-spi-vendor = "micron"; > + uefi-spi-part-number = "mt25ql128abb"; > + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ > + uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 > + b3 41 88 1f e1 8e 7f 7d]; > }; > }; Are we far enough along with part one of this series to talk about getting these properties in the upstream binding document now? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export 2022-09-21 16:15 ` Tom Rini @ 2022-09-26 11:33 ` Ilias Apalodimas 2022-09-26 13:13 ` Tom Rini 0 siblings, 1 reply; 20+ messages in thread From: Ilias Apalodimas @ 2022-09-26 11:33 UTC (permalink / raw) To: Tom Rini; +Cc: Paul Barker, u-boot, Heinrich Schuchardt Hi Tom On Wed, 21 Sept 2022 at 19:15, Tom Rini <trini@konsulko.com> wrote: > > On Wed, Sep 21, 2022 at 05:06:27PM +0100, Paul Barker wrote: > > Add properties to the Authenta SPI flash device node to enable access by > > a UEFI application using a fixed GUID. Also specify that this device is > > JEDEC compatible so that it is correctly initialized when running > > `sf probe`. > > > > Signed-off-by: Paul Barker <paul.barker@sancloud.com> > > --- > > arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite.dts b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > index d6ef19311a91..f1ff9d6024cb 100644 > > --- a/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > +++ b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > @@ -37,14 +37,20 @@ > > pinctrl-names = "default"; > > pinctrl-0 = <&bb_spi0_pins>; > > > > - channel@0 { > > + authenta-flash@0 { > > #address-cells = <1>; > > #size-cells = <0>; > > > > - compatible = "micron,spi-authenta"; > > + compatible = "micron,spi-authenta", "jedec,spi-nor"; > > > > reg = <0>; > > spi-max-frequency = <16000000>; > > spi-cpha; > > + > > + uefi-spi-vendor = "micron"; > > + uefi-spi-part-number = "mt25ql128abb"; > > + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ > > + uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 > > + b3 41 88 1f e1 8e 7f 7d]; > > }; > > }; > > Are we far enough along with part one of this series to talk about > getting these properties in the upstream binding document now? You mean those bindings being part of the DT spec? Thanks /Ilias > > -- > Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export 2022-09-26 11:33 ` Ilias Apalodimas @ 2022-09-26 13:13 ` Tom Rini 2022-09-26 13:33 ` Heinrich Schuchardt 0 siblings, 1 reply; 20+ messages in thread From: Tom Rini @ 2022-09-26 13:13 UTC (permalink / raw) To: Ilias Apalodimas; +Cc: Paul Barker, u-boot, Heinrich Schuchardt [-- Attachment #1: Type: text/plain, Size: 2143 bytes --] On Mon, Sep 26, 2022 at 02:33:23PM +0300, Ilias Apalodimas wrote: > Hi Tom > > On Wed, 21 Sept 2022 at 19:15, Tom Rini <trini@konsulko.com> wrote: > > > > On Wed, Sep 21, 2022 at 05:06:27PM +0100, Paul Barker wrote: > > > Add properties to the Authenta SPI flash device node to enable access by > > > a UEFI application using a fixed GUID. Also specify that this device is > > > JEDEC compatible so that it is correctly initialized when running > > > `sf probe`. > > > > > > Signed-off-by: Paul Barker <paul.barker@sancloud.com> > > > --- > > > arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 ++++++++-- > > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite.dts b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > > index d6ef19311a91..f1ff9d6024cb 100644 > > > --- a/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > > +++ b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > > @@ -37,14 +37,20 @@ > > > pinctrl-names = "default"; > > > pinctrl-0 = <&bb_spi0_pins>; > > > > > > - channel@0 { > > > + authenta-flash@0 { > > > #address-cells = <1>; > > > #size-cells = <0>; > > > > > > - compatible = "micron,spi-authenta"; > > > + compatible = "micron,spi-authenta", "jedec,spi-nor"; > > > > > > reg = <0>; > > > spi-max-frequency = <16000000>; > > > spi-cpha; > > > + > > > + uefi-spi-vendor = "micron"; > > > + uefi-spi-part-number = "mt25ql128abb"; > > > + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ > > > + uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 > > > + b3 41 88 1f e1 8e 7f 7d]; > > > }; > > > }; > > > > Are we far enough along with part one of this series to talk about > > getting these properties in the upstream binding document now? > > You mean those bindings being part of the DT spec? Yes, getting these added to Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export 2022-09-26 13:13 ` Tom Rini @ 2022-09-26 13:33 ` Heinrich Schuchardt 2022-09-26 19:13 ` Tom Rini 0 siblings, 1 reply; 20+ messages in thread From: Heinrich Schuchardt @ 2022-09-26 13:33 UTC (permalink / raw) To: Tom Rini, Paul Barker; +Cc: u-boot, Ilias Apalodimas, Marek Vasut, Marek Vasut On 9/26/22 15:13, Tom Rini wrote: > On Mon, Sep 26, 2022 at 02:33:23PM +0300, Ilias Apalodimas wrote: >> Hi Tom >> >> On Wed, 21 Sept 2022 at 19:15, Tom Rini <trini@konsulko.com> wrote: >>> >>> On Wed, Sep 21, 2022 at 05:06:27PM +0100, Paul Barker wrote: >>>> Add properties to the Authenta SPI flash device node to enable access by >>>> a UEFI application using a fixed GUID. Also specify that this device is >>>> JEDEC compatible so that it is correctly initialized when running >>>> `sf probe`. >>>> >>>> Signed-off-by: Paul Barker <paul.barker@sancloud.com> >>>> --- >>>> arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 ++++++++-- >>>> 1 file changed, 8 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite.dts b/arch/arm/dts/am335x-sancloud-bbe-lite.dts >>>> index d6ef19311a91..f1ff9d6024cb 100644 >>>> --- a/arch/arm/dts/am335x-sancloud-bbe-lite.dts >>>> +++ b/arch/arm/dts/am335x-sancloud-bbe-lite.dts >>>> @@ -37,14 +37,20 @@ >>>> pinctrl-names = "default"; >>>> pinctrl-0 = <&bb_spi0_pins>; >>>> >>>> - channel@0 { >>>> + authenta-flash@0 { >>>> #address-cells = <1>; >>>> #size-cells = <0>; >>>> >>>> - compatible = "micron,spi-authenta"; >>>> + compatible = "micron,spi-authenta", "jedec,spi-nor"; >>>> >>>> reg = <0>; >>>> spi-max-frequency = <16000000>; >>>> spi-cpha; >>>> + >>>> + uefi-spi-vendor = "micron"; >>>> + uefi-spi-part-number = "mt25ql128abb"; >>>> + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ >>>> + uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 >>>> + b3 41 88 1f e1 8e 7f 7d]; >>>> }; >>>> }; >>> >>> Are we far enough along with part one of this series to talk about >>> getting these properties in the upstream binding document now? >> >> You mean those bindings being part of the DT spec? > > Yes, getting these added to > Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel. > There is a related patch [PATCH] dt-bindings: spi: Add YAML DT binding document for trivial devices https://lore.kernel.org/all/20220407194936.223041-1-marex@denx.de/ that didn't make it into the kernel yet. Best regards Heinrich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export 2022-09-26 13:33 ` Heinrich Schuchardt @ 2022-09-26 19:13 ` Tom Rini 0 siblings, 0 replies; 20+ messages in thread From: Tom Rini @ 2022-09-26 19:13 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Paul Barker, u-boot, Ilias Apalodimas, Marek Vasut, Marek Vasut [-- Attachment #1: Type: text/plain, Size: 2758 bytes --] On Mon, Sep 26, 2022 at 03:33:44PM +0200, Heinrich Schuchardt wrote: > On 9/26/22 15:13, Tom Rini wrote: > > On Mon, Sep 26, 2022 at 02:33:23PM +0300, Ilias Apalodimas wrote: > > > Hi Tom > > > > > > On Wed, 21 Sept 2022 at 19:15, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Wed, Sep 21, 2022 at 05:06:27PM +0100, Paul Barker wrote: > > > > > Add properties to the Authenta SPI flash device node to enable access by > > > > > a UEFI application using a fixed GUID. Also specify that this device is > > > > > JEDEC compatible so that it is correctly initialized when running > > > > > `sf probe`. > > > > > > > > > > Signed-off-by: Paul Barker <paul.barker@sancloud.com> > > > > > --- > > > > > arch/arm/dts/am335x-sancloud-bbe-lite.dts | 10 ++++++++-- > > > > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/arch/arm/dts/am335x-sancloud-bbe-lite.dts b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > > > > index d6ef19311a91..f1ff9d6024cb 100644 > > > > > --- a/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > > > > +++ b/arch/arm/dts/am335x-sancloud-bbe-lite.dts > > > > > @@ -37,14 +37,20 @@ > > > > > pinctrl-names = "default"; > > > > > pinctrl-0 = <&bb_spi0_pins>; > > > > > > > > > > - channel@0 { > > > > > + authenta-flash@0 { > > > > > #address-cells = <1>; > > > > > #size-cells = <0>; > > > > > > > > > > - compatible = "micron,spi-authenta"; > > > > > + compatible = "micron,spi-authenta", "jedec,spi-nor"; > > > > > > > > > > reg = <0>; > > > > > spi-max-frequency = <16000000>; > > > > > spi-cpha; > > > > > + > > > > > + uefi-spi-vendor = "micron"; > > > > > + uefi-spi-part-number = "mt25ql128abb"; > > > > > + /* GUID in UEFI format: 77126730-a4ca-4386-b341-881fe18e7f7d */ > > > > > + uefi-spi-io-guid = [30 67 12 77 ca a4 86 43 > > > > > + b3 41 88 1f e1 8e 7f 7d]; > > > > > }; > > > > > }; > > > > > > > > Are we far enough along with part one of this series to talk about > > > > getting these properties in the upstream binding document now? > > > > > > You mean those bindings being part of the DT spec? > > > > Yes, getting these added to > > Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel. > > > > There is a related patch > [PATCH] dt-bindings: spi: Add YAML DT binding document for trivial devices > https://lore.kernel.org/all/20220407194936.223041-1-marex@denx.de/ > that didn't make it into the kernel yet. OK, but would that be a better way forward for this? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support 2022-09-21 16:06 [PATCH v3 0/3] Support UEFI SPI I/O protocol Paul Barker 2022-09-21 16:06 ` [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support Paul Barker 2022-09-21 16:06 ` [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export Paul Barker @ 2022-09-21 16:06 ` Paul Barker 2022-09-26 13:21 ` Heinrich Schuchardt 2 siblings, 1 reply; 20+ messages in thread From: Paul Barker @ 2022-09-21 16:06 UTC (permalink / raw) To: u-boot, Heinrich Schuchardt, Tom Rini, Ilias Apalodimas; +Cc: Paul Barker Signed-off-by: Paul Barker <paul.barker@sancloud.com> --- configs/am335x_evm_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index f0fbe475b394..f73123e0b71d 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -92,6 +92,7 @@ CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y CONFIG_SYS_NAND_U_BOOT_OFFS=0xc0000 CONFIG_DM_SPI_FLASH=y CONFIG_SF_DEFAULT_SPEED=24000000 +CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_WINBOND=y CONFIG_PHY_ATHEROS=y CONFIG_PHY_SMSC=y -- 2.25.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support 2022-09-21 16:06 ` [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support Paul Barker @ 2022-09-26 13:21 ` Heinrich Schuchardt 2022-10-03 12:07 ` Paul Barker 0 siblings, 1 reply; 20+ messages in thread From: Heinrich Schuchardt @ 2022-09-26 13:21 UTC (permalink / raw) To: Paul Barker; +Cc: u-boot, Tom Rini, Ilias Apalodimas On 9/21/22 18:06, Paul Barker wrote: > Signed-off-by: Paul Barker <paul.barker@sancloud.com> > --- > configs/am335x_evm_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig > index f0fbe475b394..f73123e0b71d 100644 > --- a/configs/am335x_evm_defconfig > +++ b/configs/am335x_evm_defconfig > @@ -92,6 +92,7 @@ CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y > CONFIG_SYS_NAND_U_BOOT_OFFS=0xc0000 > CONFIG_DM_SPI_FLASH=y > CONFIG_SF_DEFAULT_SPEED=24000000 > +CONFIG_SPI_FLASH_STMICRO=y You don't enable CONFIG_EFI_SPI_PROTOCOL anywhere; not even on the sandbox. So the code is never compiled and never tested. This cannot be correct. Best regards Heinrich > CONFIG_SPI_FLASH_WINBOND=y > CONFIG_PHY_ATHEROS=y > CONFIG_PHY_SMSC=y ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support 2022-09-26 13:21 ` Heinrich Schuchardt @ 2022-10-03 12:07 ` Paul Barker 0 siblings, 0 replies; 20+ messages in thread From: Paul Barker @ 2022-10-03 12:07 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: u-boot, Tom Rini, Ilias Apalodimas [-- Attachment #1.1.1: Type: text/plain, Size: 1077 bytes --] On 26/09/2022 14:21, Heinrich Schuchardt wrote: > On 9/21/22 18:06, Paul Barker wrote: >> Signed-off-by: Paul Barker <paul.barker@sancloud.com> >> --- >> configs/am335x_evm_defconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig >> index f0fbe475b394..f73123e0b71d 100644 >> --- a/configs/am335x_evm_defconfig >> +++ b/configs/am335x_evm_defconfig >> @@ -92,6 +92,7 @@ CONFIG_SYS_NAND_U_BOOT_LOCATIONS=y >> CONFIG_SYS_NAND_U_BOOT_OFFS=0xc0000 >> CONFIG_DM_SPI_FLASH=y >> CONFIG_SF_DEFAULT_SPEED=24000000 >> +CONFIG_SPI_FLASH_STMICRO=y > > You don't enable CONFIG_EFI_SPI_PROTOCOL anywhere; not even on the > sandbox. So the code is never compiled and never tested. This cannot be > correct. I've left this config off by default - it can be turned on as needed. I've ran the self tests with this option enabled and everything passes. Thanks, -- Paul Barker Principal Software Engineer SanCloud Ltd e: paul.barker@sancloud.com w: https://sancloud.com/ [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 6865 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-10-04 17:26 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-21 16:06 [PATCH v3 0/3] Support UEFI SPI I/O protocol Paul Barker 2022-09-21 16:06 ` [PATCH v3 1/3] efi_loader: Add SPI I/O protocol support Paul Barker 2022-09-26 12:43 ` Ilias Apalodimas 2022-09-26 13:13 ` Heinrich Schuchardt 2022-09-26 13:40 ` Ilias Apalodimas 2022-10-03 12:05 ` Paul Barker 2022-10-03 12:00 ` Paul Barker 2022-09-26 13:52 ` Heinrich Schuchardt 2022-10-03 12:05 ` Paul Barker 2022-10-04 16:31 ` Paul Barker 2022-10-04 17:25 ` Heinrich Schuchardt 2022-09-21 16:06 ` [PATCH v3 2/3] arm: dts: am335x-sancloud-bbe-lite: UEFI SPI export Paul Barker 2022-09-21 16:15 ` Tom Rini 2022-09-26 11:33 ` Ilias Apalodimas 2022-09-26 13:13 ` Tom Rini 2022-09-26 13:33 ` Heinrich Schuchardt 2022-09-26 19:13 ` Tom Rini 2022-09-21 16:06 ` [PATCH v3 3/3] am335x_evm_defconfig: Enable Micron SPI flash support Paul Barker 2022-09-26 13:21 ` Heinrich Schuchardt 2022-10-03 12:07 ` Paul Barker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).