[PATCH 0/3] firmware: arm_ffa: Add DT support

[PATCH 0/3] firmware: arm_ffa: Add DT support

[Viresh Kumar]

Hello,

This series proposes basic DT bindings for FFA devices, and sets up the of_node
and dma_configure() callback for the devices.

Viresh Kumar (3):
  dt-bindings: firmware: Add bindings for ARM FFA
  firmware: arm_ffa: Setup of_node for ffa devices
  firmware: arm_ffa: Provide .dma_configure()

 .../devicetree/bindings/firmware/arm,ffa.yaml | 75 +++++++++++++++++++
 drivers/firmware/arm_ffa/bus.c                | 39 ++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/arm,ffa.yaml

-- 
2.31.1.272.g89b43f80a514

[PATCH 2/3] firmware: arm_ffa: Setup of_node for ffa devices

[Viresh Kumar]

Match and bind the of_node for the FFA device.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/firmware/arm_ffa/bus.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index dea3eb741d95..ca526e438e71 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -7,6 +7,8 @@
 
 #include <linux/arm_ffa.h>
 #include <linux/device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/fs.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -187,6 +189,32 @@ bool ffa_device_is_valid(struct ffa_device *ffa_dev)
 	return valid;
 }
 
+static void ffa_device_of_setup(struct ffa_device *ffa_dev)
+{
+	struct device_node *np;
+	uuid_t uuid;
+	u32 val;
+
+	/* vm-id and UUID must match */
+	for_each_compatible_node(np, NULL, "arm,ffa") {
+		if (of_property_read_u32(np, "vm-id", &val))
+			continue;
+
+		if (ffa_dev->vm_id != val)
+			continue;
+
+		if (of_property_read_u32_array(np, "uuid", (u32 *)&uuid,
+					       sizeof(uuid) / sizeof(u32)))
+			continue;
+
+		if (!uuid_equal(&ffa_dev->uuid, &uuid))
+			continue;
+
+		device_set_node(&ffa_dev->dev, of_fwnode_handle(np));
+		break;
+	}
+}
+
 struct ffa_device *
 ffa_device_register(const struct ffa_partition_info *part_info,
 		    const struct ffa_ops *ops)
@@ -222,6 +250,8 @@ ffa_device_register(const struct ffa_partition_info *part_info,
 	import_uuid(&uuid, (u8 *)part_info->uuid);
 	uuid_copy(&ffa_dev->uuid, &uuid);
 
+	ffa_device_of_setup(ffa_dev);
+
 	ret = device_register(&ffa_dev->dev);
 	if (ret) {
 		dev_err(dev, "unable to register device %s err=%d\n",
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH 2/3] firmware: arm_ffa: Setup of_node for ffa devices

[Sudeep Holla]

On Tue, Jan 21, 2025 at 12:26:39PM +0530, Viresh Kumar wrote:
> Match and bind the of_node for the FFA device.

I am bit confused and need more details than the above line 😄, because

1. We don't have any binding for FF-A and compatible "arm,ffa" is not yet
   defined
2. Even if we have binding, we will not have vm_id and UUID which are
   discoverable from the firmware
3. DT maintainers rejected the bindings when it was initially proposed
   as it is discoverable. If we need it for a valid reason, we need more
   info, this patch doesn't provide full picture to change their stance.

I would like to add minimal binding not for vmid or UUID but for interrupts
only. It is still WIP but I haven't made up my mind on how that should look.

--
Regards,
Sudeep

Re: [PATCH 2/3] firmware: arm_ffa: Setup of_node for ffa devices

[Viresh Kumar]

On 21-01-25, 09:59, Sudeep Holla wrote:
> On Tue, Jan 21, 2025 at 12:26:39PM +0530, Viresh Kumar wrote:
> > Match and bind the of_node for the FFA device.
> 
> I am bit confused and need more details than the above line 😄, because
> 
> 1. We don't have any binding for FF-A and compatible "arm,ffa" is not yet
>    defined

Realized now that you weren't cc'd on 1/3. Bounced it to your email id
now.

> 2. Even if we have binding, we will not have vm_id and UUID which are
>    discoverable from the firmware

Hmm..

> 3. DT maintainers rejected the bindings when it was initially proposed
>    as it is discoverable. If we need it for a valid reason, we need more
>    info, this patch doesn't provide full picture to change their stance.

We are working on FFA based Virtio bus/devices. The
dma_alloc_coherent() helpers for the virtqueue (and other DMA-ble
memory helpers) end up calling the dma-core for the parent device,
which is FFA device in our case.

We want to avoid separate mapping for each buffer sent by the guest
and keep a reserved mem (via DT) for the specific FFA device. With
that, the memory can be mapped at once at the host (shared over FFA)
and all the following DMA allocations can happen from that
memory-region.

This needs to come via DT and this was my first attempt to get the
discussion going.

> I would like to add minimal binding not for vmid or UUID but for interrupts
> only. It is still WIP but I haven't made up my mind on how that should look.

These were required to identify the exact FFA device from DT, what
else can we do ?

-- 
viresh

Re: [PATCH 2/3] firmware: arm_ffa: Setup of_node for ffa devices

[Rob Herring]

On Tue, Jan 21, 2025 at 03:39:46PM +0530, Viresh Kumar wrote:
> On 21-01-25, 09:59, Sudeep Holla wrote:
> > On Tue, Jan 21, 2025 at 12:26:39PM +0530, Viresh Kumar wrote:
> > > Match and bind the of_node for the FFA device.
> > 
> > I am bit confused and need more details than the above line 😄, because
> > 
> > 1. We don't have any binding for FF-A and compatible "arm,ffa" is not yet
> >    defined
> 
> Realized now that you weren't cc'd on 1/3. Bounced it to your email id
> now.
> 
> > 2. Even if we have binding, we will not have vm_id and UUID which are
> >    discoverable from the firmware
> 
> Hmm..
> 
> > 3. DT maintainers rejected the bindings when it was initially proposed
> >    as it is discoverable. If we need it for a valid reason, we need more
> >    info, this patch doesn't provide full picture to change their stance.

For background, go read this thread[1]. :)

> We are working on FFA based Virtio bus/devices. The
> dma_alloc_coherent() helpers for the virtqueue (and other DMA-ble
> memory helpers) end up calling the dma-core for the parent device,
> which is FFA device in our case.
> 
> We want to avoid separate mapping for each buffer sent by the guest
> and keep a reserved mem (via DT) for the specific FFA device. With
> that, the memory can be mapped at once at the host (shared over FFA)
> and all the following DMA allocations can happen from that
> memory-region.

Well, that's a better explanation than the patches have as they just 
appear to ignore all the history here.

But I still don't really understand. I know next to 0 about FF-A. Sounds 
like OS specific problems to me though.

Rob

[1] https://lore.kernel.org/all/20220329151659.16894-1-abdellatif.elkhlifi@arm.com/#r

[PATCH 3/3] firmware: arm_ffa: Provide .dma_configure()

[Viresh Kumar]

Provide .dma_configure() callback to make reserved-memory work for FFA
devices.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/firmware/arm_ffa/bus.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index ca526e438e71..e7fd25ba71a4 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -110,12 +110,21 @@ static struct attribute *ffa_device_attributes_attrs[] = {
 };
 ATTRIBUTE_GROUPS(ffa_device_attributes);
 
+static int ffa_Device_dma_configure(struct device *dev)
+{
+	if (dev->of_node)
+		return of_dma_configure(dev, dev->of_node, true);
+
+	return 0;
+}
+
 const struct bus_type ffa_bus_type = {
 	.name		= "arm_ffa",
 	.match		= ffa_device_match,
 	.probe		= ffa_device_probe,
 	.remove		= ffa_device_remove,
 	.uevent		= ffa_device_uevent,
+	.dma_configure	= ffa_Device_dma_configure,
 	.dev_groups	= ffa_device_attributes_groups,
 };
 EXPORT_SYMBOL_GPL(ffa_bus_type);
-- 
2.31.1.272.g89b43f80a514