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