From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 35E22C433FE for ; Wed, 23 Nov 2022 02:12:21 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7820E854F1; Wed, 23 Nov 2022 03:10:52 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="FPHLE1hj"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6BEA6854F7; Wed, 23 Nov 2022 03:10:33 +0100 (CET) Received: from mail-il1-x132.google.com (mail-il1-x132.google.com [IPv6:2607:f8b0:4864:20::132]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 84CEF85516 for ; Wed, 23 Nov 2022 03:10:28 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-il1-x132.google.com with SMTP id bp12so7934630ilb.9 for ; Tue, 22 Nov 2022 18:10:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=34N6HvpKtnFKYeeOy0lkbL9mCKePYL6/7kwCgLKy7YM=; b=FPHLE1hj7/L8o4PrSYrVN+fQfO7hq3lDUrbf++iDPQGLndsHVeH5jeFhEeS1YbpDHl kNm4PFOeVvfE5i1bq20LQxLCL8zLZMQcO7uOxeWViHw/zSJzagwtjPznHaByo0vYjMXw o6b4i8X3zMzbbyRhizx/MwMUCuCRO3dy3+he8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=34N6HvpKtnFKYeeOy0lkbL9mCKePYL6/7kwCgLKy7YM=; b=YJezdyc86Wx67FW3G/++8yrTT5HQwS7IB/3LOBWviE6ldUfF6MKh/mbhDtAXOK999e Eliny3Y63EHguChvm539f1KaqY9dQiLMERUCYY41upJwyuzOXEVogmzXTUnfW/sTbjQ1 MyNAA7ecViYa0dDSPpAuxshQVS5XmUzEwJOvZ6A02NZwEkoBfqnU+oeHbhWKmAFh1age u3xj9F1v7iV9THoiceqjaRLNGRDQyPnDgTLdE9QPJo2lYjIWfLbqTSVTrh4dhpEsoLQT eLp62wmaaI8W750QN7KNME2WLoazbqNBqtDHC6U/O4fHEVxGRCdCuVpdpryBwmzYl09N TZvw== X-Gm-Message-State: ANoB5pm7U/Lwj+gq6RRM9z1PHysFY1TaJklsUnrB31KCL5GtfoiAi4Al WKT+TA8L1B70rw6hNp/GwzNaVgy3Y53do7UxgHxh/wqmENJaGA== X-Google-Smtp-Source: AA0mqf6QfIpWt5MUOuwLMpNeoYtNgDrfJH6lsTXW0ZZV6n6TmxeZKQ4bmXKIehIV2C1anBL7MlHMGUS6mb33U1AM7HU= X-Received: by 2002:a92:1912:0:b0:302:5c57:c19d with SMTP id 18-20020a921912000000b003025c57c19dmr3966697ilz.226.1669169427746; Tue, 22 Nov 2022 18:10:27 -0800 (PST) MIME-Version: 1.0 References: <20221107192055.21669-1-abdellatif.elkhlifi@arm.com> <20221122131751.22747-1-abdellatif.elkhlifi@arm.com> <20221122131751.22747-4-abdellatif.elkhlifi@arm.com> In-Reply-To: <20221122131751.22747-4-abdellatif.elkhlifi@arm.com> From: Simon Glass Date: Tue, 22 Nov 2022 19:09:16 -0700 Message-ID: Subject: Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver To: Abdellatif El Khlifi Cc: Drew.Reed@arm.com, achin.gupta@arm.com, ilias.apalodimas@linaro.org, jens.wiklander@linaro.org, nd@arm.com, trini@konsulko.com, u-boot@lists.denx.de, vishnu.banavath@arm.com, xueliang.zhong@arm.com Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean should be called 'priov' and should beHi Abdellatif, On Tue, 22 Nov 2022 at 06:18, Abdellatif El Khlifi wrote: > > Add the core driver implementing Arm Firmware Framework for Armv8-A v1.0 > > The Firmware Framework for Arm A-profile processors (FF-A v1.0) [1] > describes interfaces (ABIs) that standardize communication > between the Secure World and Normal World leveraging TrustZone > technology. > > This driver uses 64-bit registers as per SMCCCv1.2 spec and comes > on top of the SMCCC layer. The driver provides the FF-A ABIs needed for > querying the FF-A framework from the secure world. > > The driver uses SMC32 calling convention which means using the first > 32-bit data of the Xn registers. > > All supported ABIs come with their 32-bit version except FFA_RXTX_MAP > which has 64-bit version supported. > > Both 32-bit and 64-bit direct messaging are supported which allows both > 32-bit and 64-bit clients to use the FF-A bus. > > In U-Boot FF-A design, FF-A is considered as a discoverable bus. > The Secure World is considered as one entity to communicate with > using the FF-A bus. FF-A communication is handled by one device and > one instance (the bus). This FF-A driver takes care of all the > interactions between Normal world and Secure World. > > The driver exports its operations to be used by upper layers. > > Exported operations: > > - partition_info_get > - sync_send_receive > - rxtx_unmap > > For more details please refer to the driver documentation [2]. > > [1]: https://developer.arm.com/documentation/den0077/latest/ > [2]: doc/arch/arm64.ffa.rst > > Signed-off-by: Abdellatif El Khlifi > Cc: Tom Rini > Cc: Simon Glass > Cc: Ilias Apalodimas > Cc: Jens Wiklander > > --- > > Changelog: > =============== > > v8: > > * make ffa_get_partitions_info() second argument to be an SP count in both > modes > * update ffa_bus_prvdata_get() to return a pointer rather than a pointer > address > * remove packing from ffa_partition_info and ffa_send_direct_data structures > * pass the FF-A bus device to the bus operations > > v7: > > * add support for 32-bit direct messaging > * rename be_uuid_str_to_le_bin() to uuid_str_to_le_bin() > * improve the declaration of error handling mapping > * stating in doc/arch/arm64.ffa.rst that EFI runtime is not supported > > v6: > > * drop use of EFI runtime support (We decided with Linaro to add this later) > * drop discovery from initcalls (discovery will be on demand by FF-A users) > * set the alignment of the RX/TX buffers to the larger translation granule size > * move FF-A RX/TX buffers unmapping at ExitBootServices() to a separate commit > * update the documentation and move it to doc/arch/arm64.ffa.rst > > v4: > > * add doc/README.ffa.drv > * moving the FF-A driver work to drivers/firmware/arm-ffa > * use less #ifdefs in lib/efi_loader/efi_boottime.c and replace > #if defined by #if CONFIG_IS_ENABLED > * improving error handling by mapping the FF-A errors to standard errors > and logs > * replacing panics with an error log and returning an error code > * improving features discovery in FFA_FEATURES by introducing > rxtx_min_pages private data field > * add ffa_remove and ffa_unbind functions > * improve how the driver behaves when bus discovery is done more than > once > > v3: > > * align the interfaces of the U-Boot FF-A driver with those in the linux > FF-A driver > * remove the FF-A helper layer > * make the U-Boot FF-A driver independent from EFI > * provide an optional config that enables copying the driver data to EFI > runtime section at ExitBootServices service > * use 64-bit version of FFA_RXTX_MAP, FFA_MSG_SEND_DIRECT_{REQ, RESP} > > v2: > > * make FF-A bus discoverable using device_{bind, probe} APIs > * remove device tree support > > v1: > > * introduce FF-A bus driver with device tree support > > MAINTAINERS | 7 + > doc/arch/arm64.ffa.rst | 218 ++++ > doc/arch/index.rst | 1 + > drivers/Kconfig | 2 + > drivers/Makefile | 1 + > drivers/firmware/arm-ffa/Kconfig | 30 + > drivers/firmware/arm-ffa/Makefile | 6 + > drivers/firmware/arm-ffa/arm-ffa-uclass.c | 16 + > drivers/firmware/arm-ffa/arm_ffa_prv.h | 200 ++++ > drivers/firmware/arm-ffa/core.c | 1315 +++++++++++++++++++++ > include/arm_ffa.h | 97 ++ > include/dm/uclass-id.h | 4 + > 12 files changed, 1897 insertions(+) > create mode 100644 doc/arch/arm64.ffa.rst > create mode 100644 drivers/firmware/arm-ffa/Kconfig > create mode 100644 drivers/firmware/arm-ffa/Makefile > create mode 100644 drivers/firmware/arm-ffa/arm-ffa-uclass.c > create mode 100644 drivers/firmware/arm-ffa/arm_ffa_prv.h > create mode 100644 drivers/firmware/arm-ffa/core.c > create mode 100644 include/arm_ffa.h This looks mostly OK to me. I have a few comments below. [..] > diff --git a/drivers/firmware/arm-ffa/arm_ffa_prv.h b/drivers/firmware/arm-ffa/arm_ffa_prv.h > new file mode 100644 > index 0000000000..4eea7dc036 > --- /dev/null > +++ b/drivers/firmware/arm-ffa/arm_ffa_prv.h > @@ -0,0 +1,200 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi > + */ > + > +#ifndef __ARM_FFA_PRV_H > +#define __ARM_FFA_PRV_H > + > +#include > +#include > +#include > +#include > + > +/* > + * This header is private. It is exclusively used by the FF-A driver > + */ /* ...*/ is the single-line comment style > + > +/* FF-A core driver name */ > +#define FFA_DRV_NAME "arm_ffa" > + > +/* FF-A driver version definitions */ > + > +#define MAJOR_VERSION_MASK GENMASK(30, 16) > +#define MINOR_VERSION_MASK GENMASK(15, 0) > +#define GET_FFA_MAJOR_VERSION(x) \ > + ((u16)(FIELD_GET(MAJOR_VERSION_MASK, (x)))) > +#define GET_FFA_MINOR_VERSION(x) \ > + ((u16)(FIELD_GET(MINOR_VERSION_MASK, (x)))) > +#define PACK_VERSION_INFO(major, minor) \ > + (FIELD_PREP(MAJOR_VERSION_MASK, (major)) | \ > + FIELD_PREP(MINOR_VERSION_MASK, (minor))) > + > +#define FFA_MAJOR_VERSION (1) > +#define FFA_MINOR_VERSION (0) > +#define FFA_VERSION_1_0 \ > + PACK_VERSION_INFO(FFA_MAJOR_VERSION, FFA_MINOR_VERSION) > + > +/* Endpoint ID mask (u-boot endpoint ID) */ > + > +#define GET_SELF_ENDPOINT_ID_MASK GENMASK(15, 0) > +#define GET_SELF_ENDPOINT_ID(x) \ > + ((u16)(FIELD_GET(GET_SELF_ENDPOINT_ID_MASK, (x)))) > + > +#define PREP_SELF_ENDPOINT_ID_MASK GENMASK(31, 16) > +#define PREP_SELF_ENDPOINT_ID(x) \ > + (FIELD_PREP(PREP_SELF_ENDPOINT_ID_MASK, (x))) > + > +/* Partition endpoint ID mask (partition with which u-boot communicates with) */ > + > +#define PREP_PART_ENDPOINT_ID_MASK GENMASK(15, 0) > +#define PREP_PART_ENDPOINT_ID(x) \ > + (FIELD_PREP(PREP_PART_ENDPOINT_ID_MASK, (x))) > + > +/* > + * Definitions of the Arm FF-A interfaces supported by the Arm FF-A driver > + */ > + > +#define FFA_SMC(calling_convention, func_num) \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, (calling_convention), \ > + ARM_SMCCC_OWNER_STANDARD, (func_num)) > + > +#define FFA_SMC_32(func_num) FFA_SMC(ARM_SMCCC_SMC_32, (func_num)) > +#define FFA_SMC_64(func_num) FFA_SMC(ARM_SMCCC_SMC_64, (func_num)) > + > +enum ffa_abis { > + FFA_ERROR = 0x60, > + FFA_SUCCESS = 0x61, > + FFA_INTERRUPT = 0x62, > + FFA_VERSION = 0x63, > + FFA_FEATURES = 0x64, > + FFA_RX_RELEASE = 0x65, > + FFA_RXTX_MAP = 0x66, > + FFA_RXTX_UNMAP = 0x67, > + FFA_PARTITION_INFO_GET = 0x68, > + FFA_ID_GET = 0x69, > + FFA_RUN = 0x6D, > + FFA_MSG_SEND_DIRECT_REQ = 0x6F, Can you use lower-case hex consistently? > + FFA_MSG_SEND_DIRECT_RESP = 0x70, > + > + /* to be updated when adding new FFA IDs */ > + FFA_FIRST_ID = FFA_ERROR, /* lowest number ID*/ > + FFA_LAST_ID = FFA_MSG_SEND_DIRECT_RESP, /* highest number ID*/ not: spaces before */ > +}; > + > +enum ffa_abi_errcode { > + NOT_SUPPORTED = 1, > + INVALID_PARAMETERS, > + NO_MEMORY, > + BUSY, > + INTERRUPTED, > + DENIED, > + RETRY, > + ABORTED, > + MAX_NUMBER_FFA_ERR > +}; > + > +/* container structure and helper macros to map between an FF-A error and relevant error log */ > +struct ffa_abi_errmap { > + char *err_str[MAX_NUMBER_FFA_ERR]; > +}; > + > +#define FFA_ERRMAP_COUNT (FFA_LAST_ID - FFA_FIRST_ID + 1) > +#define FFA_ID_TO_ERRMAP_ID(ffa_id) ((ffa_id) - FFA_FIRST_ID) > + > +/* The FF-A SMC function definitions */ > + > +typedef struct arm_smccc_1_2_regs ffa_value_t; > +typedef void (*invoke_ffa_fn_t)(ffa_value_t args, ffa_value_t *res); that needs a comment [..] > +/** > + * struct ffa_rxtxpair - structure hosting the RX/TX buffers virtual addresses > + * @rxbuf: virtual address of the RX buffer > + * @txbuf: virtual address of the TX buffer > + * @rxtx_min_pages: RX/TX buffers minimum size in pages > + * > + * Data structure hosting the virtual addresses of the mapped RX/TX buffers Just a nit but it catches my eye. We know this is a struct. You can just say "Hosts the ..." [..] > diff --git a/drivers/firmware/arm-ffa/core.c b/drivers/firmware/arm-ffa/core.c > new file mode 100644 > index 0000000000..0b1f8e6a07 > --- /dev/null > +++ b/drivers/firmware/arm-ffa/core.c > @@ -0,0 +1,1315 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi > + */ > + > +#include "arm_ffa_prv.h" > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include See this: https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html?highlight=style#include-files [..] > +/* > + * Driver core functions > + */ > + > +/** > + * ffa_remove_device - removes the arm_ffa device > + * @dev: the device to be removed > + * > + * This function makes sure the arm_ffa device is removed > + * No need to free the kmalloced data when the device is destroyed. > + * It's automatically done by devm management by > + * device_remove() -> device_free() -> devres_release_probe(). > + * > + * Return: > + * > + * 0 on success. Otherwise, failure > + */ > +int ffa_remove_device(struct udevice *dev) > +{ > + int ret; > + > + if (!dev) { > + ffa_err("no udevice found"); > + return -ENODEV; > + } > + > + ret = device_remove(dev, DM_REMOVE_NORMAL); > + if (ret) { > + ffa_err("unable to remove. err:%d\n", ret); > + return ret; > + } > + > + ffa_info("device removed and freed"); > + > + ret = device_unbind(dev); > + if (ret) { > + ffa_err("unable to unbind. err:%d\n", ret); > + return ret; > + } > + > + ffa_info("device unbound"); > + > + return 0; > +} Do we need this function? We should not be unbinding devices. Even removing them should be done elsewhere if needed. But why? > + > +/** > + * ffa_device_get - create, bind and probe the arm_ffa device > + * @pdev: the address of a device pointer (to be filled when the arm_ffa bus device is created > + * successfully) > + * > + * This function makes sure the arm_ffa device is > + * created, bound to this driver, probed and ready to use. > + * Arm FF-A transport is implemented through a single U-Boot > + * device managing the FF-A bus (arm_ffa). > + * > + * Return: > + * > + * 0 on success. Otherwise, failure > + */ > +int ffa_device_get(struct udevice **pdev) > +{ > + int ret; > + struct udevice *dev = NULL; > + > + ret = device_bind(dm_root(), DM_DRIVER_GET(arm_ffa), FFA_DRV_NAME, NULL, ofnode_null(), > + &dev); Please add a DT binding. Even if only temporary, we need something for this. [..] > +static int ffa_get_version(void) > +{ > + u16 major, minor; > + ffa_value_t res = {0}; > + int ffa_errno; > + > + ffa_priv_data->invoke_ffa_fn((ffa_value_t){ > + .a0 = FFA_SMC_32(FFA_VERSION), .a1 = FFA_VERSION_1_0, > + }, &res); > + > + ffa_errno = res.a0; > + if (ffa_errno < 0) { > + ffa_print_error_log(FFA_VERSION, ffa_errno); > + return ffa_to_std_errno(ffa_errno); > + } > + > + major = GET_FFA_MAJOR_VERSION(res.a0); > + minor = GET_FFA_MINOR_VERSION(res.a0); > + > + ffa_info("FF-A driver %d.%d\nFF-A framework %d.%d", > + FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); > + > + if ((major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION)) { nit: Drop extra brackets > + ffa_info("Versions are compatible "); > + > + ffa_priv_data->fwk_version = res.a0; > + > + return 0; > + } > + > + ffa_err("versions are incompatible\nExpected: %d.%d , Found: %d.%d\n", > + FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); > + > + return -EPROTONOSUPPORT; > +} > + > +/** > + * ffa_get_endpoint_id - FFA_ID_GET handler function I believe these should have () at the end, so: ffa_get_endpoint_id() - FFA_ID_GET handler function > + * > + * This function implements FFA_ID_GET FF-A function > + * to get from the secure world u-boot endpoint ID > + * > + * Return: > + * > + * 0 on success. Otherwise, failure > + */ [..] > +/** > + * ffa_free_rxtx_buffers - frees the RX/TX buffers > + * > + * This function frees the RX/TX buffers > + * > + */ > +static void ffa_free_rxtx_buffers(void) > +{ > + ffa_info("Freeing RX/TX buffers"); > + > + if (ffa_priv_data->pair.rxbuf) { > + free((void *)ffa_priv_data->pair.rxbuf); You can't cast an address to a pointer in this way. Should use map_sysmem() or store a pointer. > + ffa_priv_data->pair.rxbuf = 0; > + } > + > + if (ffa_priv_data->pair.txbuf) { > + free((void *)ffa_priv_data->pair.txbuf); > + ffa_priv_data->pair.txbuf = 0; > + } > +} > +[..] > + /* > + * make sure the buffers are cleared before use > + */ > + memset((void *)ffa_priv_data->pair.rxbuf, 0, bytes); > + memset((void *)ffa_priv_data->pair.txbuf, 0, bytes); Yes these should be pointers everywhere, since you are casting all the time. > + > + return 0; > +} > + [..] > +/** > + * ffa_read_partitions_info - reads the data queried by FFA_PARTITION_INFO_GET > + * and saves it in the private structure Can you fit all these titles on one line, like: ffa_read_partitions_info() - Read partition information > + * @count: The number of partitions queried > + * @part_uuid: Pointer to the partition(s) UUID > + * > + * This function reads the partitions information > + * returned by the FFA_PARTITION_INFO_GET and saves it in the private > + * data structure. > + * > + * Return: > + * > + * The private data structure is updated with the partition(s) information > + * 0 is returned on success. Otherwise, failure > + */ [..] [..] > +static int ffa_msg_send_direct_req(struct udevice *dev, u16 dst_part_id, > + struct ffa_send_direct_data *msg, bool is_smc64) > +{ > + ffa_value_t res = {0}; > + int ffa_errno; > + u64 req_mode, resp_mode; > + > + if (!ffa_priv_data || !ffa_priv_data->invoke_ffa_fn) > + return -EINVAL; ffa_priv_data should be called 'priv' and attached to the device with device_get_priv() [..] > +/** > + * ffa_probe - The driver probe function > + * @dev: the arm_ffa device > + * > + * Probing is done at boot time and triggered by the uclass device discovery. > + * At probe level the following actions are done: > + * - setting the conduit > + * - querying the FF-A framework version > + * - querying from secure world the u-boot endpoint ID > + * - querying from secure world the supported features of FFA_RXTX_MAP > + * - mapping the RX/TX buffers > + * - querying from secure world all the partitions information > + * > + * All data queried from secure world is saved in the resident private data structure. > + * > + * The probe will fail if either FF-A framework is not detected or the > + * FF-A requests are not behaving correctly. This ensures that the > + * driver is not installed and its operations are not exported to the clients. > + * > + * Return: > + * > + * 0 on success. Otherwise, failure > + */ > +static int ffa_probe(struct udevice *dev) > +{ > + int ret; > + > + ret = ffa_alloc_prvdata(dev); don't do this, see above. [..] > diff --git a/include/arm_ffa.h b/include/arm_ffa.h > new file mode 100644 > index 0000000000..74b16174c2 > --- /dev/null > +++ b/include/arm_ffa.h > @@ -0,0 +1,97 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * (C) Copyright 2022 ARM Limited > + * Abdellatif El Khlifi > + */ > + > +#ifndef __ARM_FFA_H > +#define __ARM_FFA_H > + > +#include > + > +/* > + * This header is public. It can be used by clients to access > + * data structures and definitions they need > + */ > + > +/* > + * Macros for displaying logs > + */ > + > +#define ffa_info(fmt, ...) pr_info("[FFA] " fmt "\n", ##__VA_ARGS__) > +#define ffa_err(fmt, ...) pr_err("[FFA] " fmt "\n", ##__VA_ARGS__) You could use log_info(), log_err() > + > +/* > + * struct ffa_partition_info - Partition information descriptor > + * @id: Partition ID > + * @exec_ctxt: Execution context count > + * @properties: Partition properties > + * > + * Data structure containing information about partitions instantiated in the system > + * This structure is filled with the data queried by FFA_PARTITION_INFO_GET > + */ > +struct ffa_partition_info { > + u16 id; > + u16 exec_ctxt; > +/* partition supports receipt of direct requests */ > +#define FFA_PARTITION_DIRECT_RECV BIT(0) > +/* partition can send direct requests. */ > +#define FFA_PARTITION_DIRECT_SEND BIT(1) > +/* partition can send and receive indirect messages. */ > +#define FFA_PARTITION_INDIRECT_MSG BIT(2) > + u32 properties; > +}; > + > +/* > + * struct ffa_send_direct_data - Data structure hosting the data > + * used by FFA_MSG_SEND_DIRECT_{REQ,RESP} > + * @data0-4: Data read/written from/to x3-x7 registers > + * > + * Data structure containing the data to be sent by FFA_MSG_SEND_DIRECT_REQ > + * or read from FFA_MSG_SEND_DIRECT_RESP > + */ > + > +/* For use with FFA_MSG_SEND_DIRECT_{REQ,RESP} which pass data via registers */ > +struct ffa_send_direct_data { > + unsigned long data0; /* w3/x3 */ > + unsigned long data1; /* w4/x4 */ > + unsigned long data2; /* w5/x5 */ > + unsigned long data3; /* w6/x6 */ > + unsigned long data4; /* w7/x7 */ > +}; > + > +struct udevice; > + > +/** > + * struct ffa_bus_ops - The driver operations structure > + * @partition_info_get: callback for the FFA_PARTITION_INFO_GET > + * @sync_send_receive: callback for the FFA_MSG_SEND_DIRECT_REQ > + * @rxtx_unmap: callback for the FFA_RXTX_UNMAP > + * > + * The data structure providing all the operations supported by the driver. > + * This structure is EFI runtime resident. > + */ > +struct ffa_bus_ops { > + int (*partition_info_get)(struct udevice *dev, const char *uuid_str, > + u32 *sp_count, struct ffa_partition_info *buffer); > + int (*sync_send_receive)(struct udevice *dev, u16 dst_part_id, > + struct ffa_send_direct_data *msg, > + bool is_smc64); > + int (*rxtx_unmap)(struct udevice *dev); > +}; Shouldn't this be the .ops member in your driver? > + > +/** > + * The device driver and the Uclass driver public functions > + */ > + > +/** > + * ffa_bus_ops_get - driver operations getter > + */ > +const struct ffa_bus_ops *ffa_bus_ops_get(void); See how this is done in other uclasses, e.g. spi_get_ops() Regards, SImon