Hi,
On Wed, 16 Nov 2022 at 06:03, Abdellatif El Khlifi
<abdellatif.elkhl...@arm.com> wrote:
>
> On Tue, Nov 15, 2022 at 08:24:24AM -0700, Simon Glass wrote:
> > Hi,
> >
> > On Mon, 1 Aug 2022 at 11:21, Abdellatif El Khlifi
> > <abdellatif.elkhl...@arm.com> wrote:
> > >
> > > Add the driver implementing Arm Firmware Framework for Armv8-A v1.0
> > >
> > > The Firmware Framework for Arm A-profile processors (FF-A)
> > > 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.
> > >
> > > 32-bit version of the ABIs is supported and 64-bit version of FFA_RXTX_MAP
> > > and FFA_MSG_SEND_DIRECT_{REQ, RESP}.
> > >
> > > 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
> > >
> > > This implementation provides an optional feature to copy the driver data
> > > to EFI runtime area.
> > >
> > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhl...@arm.com>
> > > Cc: Tom Rini <tr...@konsulko.com>
> > > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > > Cc: Jens Wiklander <jens.wiklan...@linaro.org>
> > > ---
> > > MAINTAINERS | 6 +
> > > common/board_r.c | 7 +
> > > drivers/Kconfig | 2 +
> > > drivers/Makefile | 1 +
> > > drivers/arm-ffa/Kconfig | 33 +
> > > drivers/arm-ffa/Makefile | 7 +
> > > drivers/arm-ffa/arm-ffa-uclass.c | 16 +
> > > drivers/arm-ffa/arm_ffa_prv.h | 219 ++++
> > > drivers/arm-ffa/core.c | 1338 ++++++++++++++++++++
> > > drivers/arm-ffa/efi_ffa_runtime_data_mgr.c | 94 ++
> > > include/arm_ffa.h | 132 ++
> > > include/dm/uclass-id.h | 1 +
> > > include/uuid.h | 8 +
> > > lib/efi_loader/efi_boottime.c | 17 +
> > > lib/uuid.c | 65 +
> > > 15 files changed, 1946 insertions(+)
> > > create mode 100644 drivers/arm-ffa/Kconfig
> > > create mode 100644 drivers/arm-ffa/Makefile
> > > create mode 100644 drivers/arm-ffa/arm-ffa-uclass.c
> > > create mode 100644 drivers/arm-ffa/arm_ffa_prv.h
> > > create mode 100644 drivers/arm-ffa/core.c
> > > create mode 100644 drivers/arm-ffa/.c
> > > create mode 100644 include/arm_ffa.h
> >
> > Please add something to doc/ so people know what this is.
> >
> > Since you are adding a new uclass you need a sandbox driver and tests.
> >
> > The driver appears to have no operations, but there is a bus_ops. The
> > ops should go in the driver, I suspect, and should pass the device as
> > the first arg.
> >
> > Can FFA_ERR_STAT_SUCCESS be 0 so you don't have to sprinkle the code with
> > it?
> >
> > Why is it using EFI things? Can this driver only be used with UEFI? I
> > hope not, if it is an official way of updating firmware.
> >
> > Please don't add more things to board_r.c - we are trying to remove
> > this init over time. If it is a device it should be probed as needed.
> >
> > Is there a device tree binding?
> >
> > Also should this go in drivers/misc instead of creating a whole new subdir?
>
> Hi Simon, thanks for reviewing.
>
> All the above comments have already been addressed in the new versions
> of the patchset. Please refer to the latest version v7 [1].
>
> By the way I'd like to highlight the following:
>
> - The FF-A driver documentation is at doc/arch/arm64.ffa.rst, please
> refer to it since it provides helpful details about the FF-A support
> in U-Boot
OK
> - The patchset comes with Sandbox driver and tests [2]
OK I suppose I saw that previously and forgot.
> - The driver has operations defined in struct ffa_bus_ops (include/arm_ffa.h).
> ffa_bus_ops_get() gets the ops. All these are in the driver
> (drivers/firmware/arm-ffa/core.c)
Can you please push a tree somewhere so I can look?
> - The FF-A bus has only 1 device. No multiple instances. So passing the
> device doesn't make sense in our case
It must still pass the device.
> - FFA_ERR_STAT_SUCCESS has been removed and replaced with 0
OK
> - The driver is independent from EFI and can be compiled without EFI
Oh, so what is ffa_copy_runtime_data() for?
> - FF-A bus discovery has been removed from the initcall level (board_r.c)
Good.
> Discovery is done on demand. Clients can call ffa_bus_discover() when
> they want to use the FF-A bus. As an example of how clients initiate
> discovery please refer to the FF-A MM comms client [3].
Is there a command to do this?
> - As done in the Linux kernel, the FF-A bus doesn't have a device tree
> binding since there is no peripheral associated with FF-A. At the
> early stages of this patchset, we double checked with the device tree
> maintainer and the decision was no device tree for FF-A
Sorry, but you must add one.
> - The links below are from the U-Boot mailing list mirror in lore.kernel.org
Regards,
Simon
>
> Cheers.
>
> [1]:
> https://lore.kernel.org/all/20221107192055.21669-1-abdellatif.elkhl...@arm.com/
> [2]:
> https://lore.kernel.org/all/20221107192055.21669-7-abdellatif.elkhl...@arm.com/
>
> https://lore.kernel.org/all/20221107192055.21669-8-abdellatif.elkhl...@arm.com/
>
> https://lore.kernel.org/all/20221107192055.21669-9-abdellatif.elkhl...@arm.com/
> [3]:
> https://lore.kernel.org/all/20221107192055.21669-10-abdellatif.elkhl...@arm.com/
> >
> > Regards,
> > Simon
