Re: [PATCH v10 04/10] arm_ffa: introduce Arm FF-A support

Simon Glass
2023-04-02T02:42:41Z

Hi Abdellatif,

On Wed, 29 Mar 2023 at 05:12, Abdellatif El Khlifi <
abdellatif.elkhlifi@arm.com> wrote:
>
> Add Arm FF-A support 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.
>
> FF-A is a discoverable bus and similar to architecture features.
> FF-A bus is discovered using ARM_SMCCC_FEATURES mechanism performed
> by the PSCI driver.
>
> Clients are able to probe then use the FF-A bus by calling the DM class
> searching APIs (e.g: uclass_first_device).
>
> 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:
>
> - ffa_partition_info_get
> - ffa_sync_send_receive
> - ffa_rxtx_unmap
>
> Generic FF-A methods are implemented in the Uclass (arm-ffa-uclass.c).
> Arm specific methods are implemented in the Arm driver (arm-ffa.c).
>
> 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 <abdellatif.elkhlifi@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> ---
> Changelog:
> ===============
>
> v10:
>
> * provide the driver operations through the Uclass
> * move the generic FF-A methods to the Uclass
> * keep Arm specific methods in the Arm driver (arm-ffa.c)
> * rename core.c to arm-ffa.c
> * address nits

The functions at the bottom like ffa_do_remove() should be changed to
uclass methods.

E.g.

.pre_remove = ffa_do_remove,
.pre_probe = ffa_do_probe
.post_bind = ffa_do_unbind

You can make them static and they don't need to called from elsewhere. It
is automatic.

- no need to check for !dev since this is the caller's responsibiliy
- no need to check for !priv since this cannot happen

It looks like you are using 'priv' as per-device uclass data. So instead,
please add something like:

per_device_auto = sizeof(struct ffa_priv)

in your uclass driver, then use dev_get_uclass_priv() instead of
dev_get_priv().

You should use the var name uc_priv instead of priv, so it is clear that
this relates to the uclass (i.e. is common across all devices in that
uclass)

You should almost never need to set the priv data, since driver model
handles this.

Regards,
Simon