How to upload fpga firmware
diff mbox series

Message ID 20200422114432.GM1694@pengutronix.de
State New
Headers show
Series
  • How to upload fpga firmware
Related show

Commit Message

Sascha Hauer April 22, 2020, 11:44 a.m. UTC
Hi,

I wonder what can be done with the mainline state of drivers/fpga/. The
entry to the framework seems to be fpga_mgr_load(). The only user of
this function is fpga_region_program_fpga(). This in turn is only called
in response of applying a device tree overlay. A device tree overlay is
applied with of_overlay_fdt_apply() which has no users in the Kernel.

My current task is to load a firmware to a FPGA. The code all seems to
be there in the Kernel, it only lacks a way to trigger it. I am not very
interested in device tree overlays since the FPGA appears as a PCI
device (although applying a dtbo could enable the PCIe controller device
tree node). Is there some mainline way to upload FPGA firmware? At the
moment we are using the attached patch to trigger loading the firmware
from userspace. Would something like this be acceptable for mainline?

Sascha

---------------------------8<----------------------------------

From 71a5ea845dd673d4011391f9e57fdaf427767ed5 Mon Sep 17 00:00:00 2001
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: Tue, 2 Oct 2018 17:13:40 +0200
Subject: [PATCH] fpga: region: Add sysfs attribute for loading firmware

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/fpga/fpga-region.c | 50 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

Comments

Tom Rix April 22, 2020, 6:24 p.m. UTC | #1
I believe you need to use OPAE, user level interface to the kernel.

Here is a good starting point.

https://opae.github.io/latest/docs/fpga_tools/fpgaconf/fpgaconf.html

Tom

On 4/22/20 4:44 AM, Sascha Hauer wrote:
> Hi,
>
> I wonder what can be done with the mainline state of drivers/fpga/. The
> entry to the framework seems to be fpga_mgr_load(). The only user of
> this function is fpga_region_program_fpga(). This in turn is only called
> in response of applying a device tree overlay. A device tree overlay is
> applied with of_overlay_fdt_apply() which has no users in the Kernel.
>
> My current task is to load a firmware to a FPGA. The code all seems to
> be there in the Kernel, it only lacks a way to trigger it. I am not very
> interested in device tree overlays since the FPGA appears as a PCI
> device (although applying a dtbo could enable the PCIe controller device
> tree node). Is there some mainline way to upload FPGA firmware? At the
> moment we are using the attached patch to trigger loading the firmware
> from userspace. Would something like this be acceptable for mainline?
>
> Sascha
>
> ---------------------------8<----------------------------------
>
> From 71a5ea845dd673d4011391f9e57fdaf427767ed5 Mon Sep 17 00:00:00 2001
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Date: Tue, 2 Oct 2018 17:13:40 +0200
> Subject: [PATCH] fpga: region: Add sysfs attribute for loading firmware
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/fpga/fpga-region.c | 50 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index bde5a9d460c5..ca6dc830fadf 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -5,6 +5,7 @@
>   *  Copyright (C) 2013-2016 Altera Corporation
>   *  Copyright (C) 2017 Intel Corporation
>   */
> +#include <linux/device.h>
>  #include <linux/fpga/fpga-bridge.h>
>  #include <linux/fpga/fpga-mgr.h>
>  #include <linux/fpga/fpga-region.h>
> @@ -170,11 +171,58 @@ static ssize_t compat_id_show(struct device *dev,
>  		       (unsigned long long)region->compat_id->id_h,
>  		       (unsigned long long)region->compat_id->id_l);
>  }
> -
>  static DEVICE_ATTR_RO(compat_id);
>  
> +static ssize_t firmware_name_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct fpga_region *region = to_fpga_region(dev);
> +
> +	if (!region->info || !region->info->firmware_name)
> +		return 0;
> +
> +	return sprintf(buf, "%s\n", region->info->firmware_name);
> +}
> +
> +static ssize_t firmware_name_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *firmware_name, size_t count)
> +{
> +	struct fpga_region *region = to_fpga_region(dev);
> +	struct fpga_image_info *info = region->info;
> +	int error;
> +
> +	if (!info) {
> +		info = fpga_image_info_alloc(dev);
> +		if (!info)
> +			return -ENOMEM;
> +	} else if (info->firmware_name) {
> +		devm_kfree(dev, info->firmware_name);
> +	}
> +
> +	info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
> +	if (!info->firmware_name)
> +		return -ENOMEM;
> +
> +	if (count >  0 && info->firmware_name[count - 1] == '\n')
> +		info->firmware_name[count - 1] = '\0';
> +
> +	region->info = info;
> +	error = fpga_region_program_fpga(region);
> +	if (error) {
> +		devm_kfree(dev, info->firmware_name);
> +		info->firmware_name = NULL;
> +	}
> +
> +	return error ? error : count;
> +}
> +
> +static DEVICE_ATTR_RW(firmware_name);
> +
>  static struct attribute *fpga_region_attrs[] = {
>  	&dev_attr_compat_id.attr,
> +	&dev_attr_firmware_name.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(fpga_region);
Moritz Fischer April 23, 2020, 1:36 a.m. UTC | #2
Hi Sascha,

On Wed, Apr 22, 2020 at 01:44:32PM +0200, Sascha Hauer wrote:
> Hi,
> 
> I wonder what can be done with the mainline state of drivers/fpga/. The
> entry to the framework seems to be fpga_mgr_load(). The only user of
> this function is fpga_region_program_fpga(). This in turn is only called
> in response of applying a device tree overlay. A device tree overlay is
> applied with of_overlay_fdt_apply() which has no users in the Kernel.

Yes. It is waiting for dt_overlays one way or another. I personally
don't currently have the bandwidth to work actively on this.

> My current task is to load a firmware to a FPGA. The code all seems to
> be there in the Kernel, it only lacks a way to trigger it. I am not very
> interested in device tree overlays since the FPGA appears as a PCI
> device (although applying a dtbo could enable the PCIe controller device
> tree node). Is there some mainline way to upload FPGA firmware? At the
> moment we are using the attached patch to trigger loading the firmware
> from userspace. Would something like this be acceptable for mainline?

We've looked into this sort of patches over the years and never came to
a general interface that really works.

The OPAE folks (and other users I know of) usually use FPGA Manager with
a higher layer on top of it that moves the bitstream into the kernel via
an ioctl().

One concept I had toyed with mentally, but haven't really gotten around
to implement is a 'discoverable' region, that would deal with the
necessary re-enumeration via a callback and have a sysfs interface
similar to what the patch below has.
This would essentially cover use-cases where you have a discoverable
device implemented in FPGA logic, such as say an FPGA hanging off of
PCIe bus that can get loaded over USB, a CPLD or some other side-band
mechanism. After loading the image you'd have to rescan the PCIe bus -
which - imho is the kernel's job.

What I really wanna avoid is creating another /dev/fpga0 / /dev/xdevcfg
that completely leaves the kernel in the dark about the fact that it
reconfigures a bit of hardware hanging off the bus.

In my ideal world you'd create a pci driver that binds to your device,
and creates mfd style subdevices for whatever you'd want your design to
do. One of these devices would be an FPGA and a FPGA region attached to
that FPGA manager. Your top level driver would co-ordinate the fact that
you are re-programming parts of the FPGA and create / destroy devices as
needed for the hardware contained in the bitstream.

[..]
> +static ssize_t firmware_name_show(struct device *dev,
> +				  struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct fpga_region *region = to_fpga_region(dev);
> +
> +	if (!region->info || !region->info->firmware_name)
> +		return 0;
> +
> +	return sprintf(buf, "%s\n", region->info->firmware_name);
> +}
> +
> +static ssize_t firmware_name_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *firmware_name, size_t count)
> +{
> +	struct fpga_region *region = to_fpga_region(dev);
> +	struct fpga_image_info *info = region->info;
> +	int error;
> +
> +	if (!info) {
> +		info = fpga_image_info_alloc(dev);
> +		if (!info)
> +			return -ENOMEM;
> +	} else if (info->firmware_name) {
> +		devm_kfree(dev, info->firmware_name);
> +	}
> +
> +	info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
> +	if (!info->firmware_name)
> +		return -ENOMEM;
> +
> +	if (count >  0 && info->firmware_name[count - 1] == '\n')
> +		info->firmware_name[count - 1] = '\0';
> +
> +	region->info = info;
> +	error = fpga_region_program_fpga(region);
> +	if (error) {
> +		devm_kfree(dev, info->firmware_name);
> +		info->firmware_name = NULL;
> +	}
> +
> +	return error ? error : count;
> +}

Cheers,
Moritz
Xu Yilun April 23, 2020, 5:09 a.m. UTC | #3
Hi Moritz:

On Wed, Apr 22, 2020 at 06:36:48PM -0700, Moritz Fischer wrote:
> Hi Sascha,
> 
> On Wed, Apr 22, 2020 at 01:44:32PM +0200, Sascha Hauer wrote:
> > Hi,
> > 
> > I wonder what can be done with the mainline state of drivers/fpga/. The
> > entry to the framework seems to be fpga_mgr_load(). The only user of
> > this function is fpga_region_program_fpga(). This in turn is only called
> > in response of applying a device tree overlay. A device tree overlay is
> > applied with of_overlay_fdt_apply() which has no users in the Kernel.
> 
> Yes. It is waiting for dt_overlays one way or another. I personally
> don't currently have the bandwidth to work actively on this.
> 
> > My current task is to load a firmware to a FPGA. The code all seems to
> > be there in the Kernel, it only lacks a way to trigger it. I am not very
> > interested in device tree overlays since the FPGA appears as a PCI
> > device (although applying a dtbo could enable the PCIe controller device
> > tree node). Is there some mainline way to upload FPGA firmware? At the
> > moment we are using the attached patch to trigger loading the firmware
> > from userspace. Would something like this be acceptable for mainline?
> 
> We've looked into this sort of patches over the years and never came to
> a general interface that really works.
> 
> The OPAE folks (and other users I know of) usually use FPGA Manager with
> a higher layer on top of it that moves the bitstream into the kernel via
> an ioctl().
> 
> One concept I had toyed with mentally, but haven't really gotten around
> to implement is a 'discoverable' region, that would deal with the
> necessary re-enumeration via a callback and have a sysfs interface
> similar to what the patch below has.
> This would essentially cover use-cases where you have a discoverable
> device implemented in FPGA logic, such as say an FPGA hanging off of
> PCIe bus that can get loaded over USB, a CPLD or some other side-band
> mechanism. After loading the image you'd have to rescan the PCIe bus -
> which - imho is the kernel's job.

Seems you mentioned the static region update. I have the pcie based FPGA
card, and the pcie endpoint is implemented in FPGA static region. So after
I have written image into Flash (over USB or other ways), and trying to
load it to FPGA, the pcie endpoint is also disfunctional, the pcie device
is actually lost. We need to rescan the pcie bus after loading is finished.

The concern is that when the pcie device lost, the 'discoverable' region
created by the pcie driver is also destroyed. When you rescaned the pcie
device back, it is like everything is start over again (pci probe, fpga
region creation, subdev enumeration ...) rather than just a re-enumeration
callback for discoverable region.

How do you think about it?

> 
> What I really wanna avoid is creating another /dev/fpga0 / /dev/xdevcfg
> that completely leaves the kernel in the dark about the fact that it
> reconfigures a bit of hardware hanging off the bus.

I have the FPGA board whose image binary for static region is stored in Flash.
I was able to enumerate the Flash as MTD device and update it directly, (now
switch to another interface for security support). 
Do you mean I should avoid doing that cause FPGA region device is unaware of
the update? Some interface in FPGA region would be better choice?

Thanks!

> 
> In my ideal world you'd create a pci driver that binds to your device,
> and creates mfd style subdevices for whatever you'd want your design to
> do. One of these devices would be an FPGA and a FPGA region attached to
> that FPGA manager. Your top level driver would co-ordinate the fact that
> you are re-programming parts of the FPGA and create / destroy devices as
> needed for the hardware contained in the bitstream.
> 
> [..]
> > +static ssize_t firmware_name_show(struct device *dev,
> > +				  struct device_attribute *attr,
> > +				  char *buf)
> > +{
> > +	struct fpga_region *region = to_fpga_region(dev);
> > +
> > +	if (!region->info || !region->info->firmware_name)
> > +		return 0;
> > +
> > +	return sprintf(buf, "%s\n", region->info->firmware_name);
> > +}
> > +
> > +static ssize_t firmware_name_store(struct device *dev,
> > +				   struct device_attribute *attr,
> > +				   const char *firmware_name, size_t count)
> > +{
> > +	struct fpga_region *region = to_fpga_region(dev);
> > +	struct fpga_image_info *info = region->info;
> > +	int error;
> > +
> > +	if (!info) {
> > +		info = fpga_image_info_alloc(dev);
> > +		if (!info)
> > +			return -ENOMEM;
> > +	} else if (info->firmware_name) {
> > +		devm_kfree(dev, info->firmware_name);
> > +	}
> > +
> > +	info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
> > +	if (!info->firmware_name)
> > +		return -ENOMEM;
> > +
> > +	if (count >  0 && info->firmware_name[count - 1] == '\n')
> > +		info->firmware_name[count - 1] = '\0';
> > +
> > +	region->info = info;
> > +	error = fpga_region_program_fpga(region);
> > +	if (error) {
> > +		devm_kfree(dev, info->firmware_name);
> > +		info->firmware_name = NULL;
> > +	}
> > +
> > +	return error ? error : count;
> > +}
> 
> Cheers,
> Moritz
Sascha Hauer April 23, 2020, 6:23 a.m. UTC | #4
Hi Moritz,

On Wed, Apr 22, 2020 at 06:36:48PM -0700, Moritz Fischer wrote:
> Hi Sascha,
> 
> On Wed, Apr 22, 2020 at 01:44:32PM +0200, Sascha Hauer wrote:
> > Hi,
> > 
> > I wonder what can be done with the mainline state of drivers/fpga/. The
> > entry to the framework seems to be fpga_mgr_load(). The only user of
> > this function is fpga_region_program_fpga(). This in turn is only called
> > in response of applying a device tree overlay. A device tree overlay is
> > applied with of_overlay_fdt_apply() which has no users in the Kernel.
> 
> Yes. It is waiting for dt_overlays one way or another. I personally
> don't currently have the bandwidth to work actively on this.
> 
> > My current task is to load a firmware to a FPGA. The code all seems to
> > be there in the Kernel, it only lacks a way to trigger it. I am not very
> > interested in device tree overlays since the FPGA appears as a PCI
> > device (although applying a dtbo could enable the PCIe controller device
> > tree node). Is there some mainline way to upload FPGA firmware? At the
> > moment we are using the attached patch to trigger loading the firmware
> > from userspace. Would something like this be acceptable for mainline?
> 
> We've looked into this sort of patches over the years and never came to
> a general interface that really works.
> 
> The OPAE folks (and other users I know of) usually use FPGA Manager with
> a higher layer on top of it that moves the bitstream into the kernel via
> an ioctl().
> 
> One concept I had toyed with mentally, but haven't really gotten around
> to implement is a 'discoverable' region, that would deal with the
> necessary re-enumeration via a callback and have a sysfs interface
> similar to what the patch below has.
> This would essentially cover use-cases where you have a discoverable
> device implemented in FPGA logic, such as say an FPGA hanging off of
> PCIe bus that can get loaded over USB, a CPLD or some other side-band
> mechanism. After loading the image you'd have to rescan the PCIe bus -
> which - imho is the kernel's job.
> 
> What I really wanna avoid is creating another /dev/fpga0 / /dev/xdevcfg
> that completely leaves the kernel in the dark about the fact that it
> reconfigures a bit of hardware hanging off the bus.

Yes, makes sense. While this would suffice my needs at the moment it
really sounds like a dead end.

> 
> In my ideal world you'd create a pci driver that binds to your device,
> and creates mfd style subdevices for whatever you'd want your design to
> do. One of these devices would be an FPGA and a FPGA region attached to
> that FPGA manager. Your top level driver would co-ordinate the fact that
> you are re-programming parts of the FPGA and create / destroy devices as
> needed for the hardware contained in the bitstream.

In my case there is no pci device visible before loading the firmware,
so creating a pci driver is not an option. Maybe pci host controllers
could register themselves as fpga-bridges. With this we could put the
pci host controller (or the pci device, AFAIK there is a PCI device tree
binding) where the fpga is connected into the fpga-bridges phandles list
of the fpga-region.

Regards,
  Sascha
Moritz Fischer April 25, 2020, 3:59 a.m. UTC | #5
Hi Sascha,

On Thu, Apr 23, 2020 at 08:23:31AM +0200, Sascha Hauer wrote:
> Hi Moritz,
> 
> On Wed, Apr 22, 2020 at 06:36:48PM -0700, Moritz Fischer wrote:
> > Hi Sascha,
> > 
> > On Wed, Apr 22, 2020 at 01:44:32PM +0200, Sascha Hauer wrote:
> > > Hi,
> > > 
> > > I wonder what can be done with the mainline state of drivers/fpga/. The
> > > entry to the framework seems to be fpga_mgr_load(). The only user of
> > > this function is fpga_region_program_fpga(). This in turn is only called
> > > in response of applying a device tree overlay. A device tree overlay is
> > > applied with of_overlay_fdt_apply() which has no users in the Kernel.
> > 
> > Yes. It is waiting for dt_overlays one way or another. I personally
> > don't currently have the bandwidth to work actively on this.
> > 
> > > My current task is to load a firmware to a FPGA. The code all seems to
> > > be there in the Kernel, it only lacks a way to trigger it. I am not very
> > > interested in device tree overlays since the FPGA appears as a PCI
> > > device (although applying a dtbo could enable the PCIe controller device
> > > tree node). Is there some mainline way to upload FPGA firmware? At the
> > > moment we are using the attached patch to trigger loading the firmware
> > > from userspace. Would something like this be acceptable for mainline?
> > 
> > We've looked into this sort of patches over the years and never came to
> > a general interface that really works.
> > 
> > The OPAE folks (and other users I know of) usually use FPGA Manager with
> > a higher layer on top of it that moves the bitstream into the kernel via
> > an ioctl().
> > 
> > One concept I had toyed with mentally, but haven't really gotten around
> > to implement is a 'discoverable' region, that would deal with the
> > necessary re-enumeration via a callback and have a sysfs interface
> > similar to what the patch below has.
> > This would essentially cover use-cases where you have a discoverable
> > device implemented in FPGA logic, such as say an FPGA hanging off of
> > PCIe bus that can get loaded over USB, a CPLD or some other side-band
> > mechanism. After loading the image you'd have to rescan the PCIe bus -
> > which - imho is the kernel's job.
> > 
> > What I really wanna avoid is creating another /dev/fpga0 / /dev/xdevcfg
> > that completely leaves the kernel in the dark about the fact that it
> > reconfigures a bit of hardware hanging off the bus.
> 
> Yes, makes sense. While this would suffice my needs at the moment it
> really sounds like a dead end.
> 
> > 
> > In my ideal world you'd create a pci driver that binds to your device,
> > and creates mfd style subdevices for whatever you'd want your design to
> > do. One of these devices would be an FPGA and a FPGA region attached to
> > that FPGA manager. Your top level driver would co-ordinate the fact that
> > you are re-programming parts of the FPGA and create / destroy devices as
> > needed for the hardware contained in the bitstream.
> 
> In my case there is no pci device visible before loading the firmware,
> so creating a pci driver is not an option. Maybe pci host controllers
> could register themselves as fpga-bridges. With this we could put the
> pci host controller (or the pci device, AFAIK there is a PCI device tree
> binding) where the fpga is connected into the fpga-bridges phandles list
> of the fpga-region.

Can you talk a bit more about the system you're working with? Is there
some sort of sideband mechanism to load the image? What exposes the
image loading capaibility? A CPLD, an ASIC, USB device, JTAG?

What does the topology look like?

Sideband  -----> FPGA
PCIe      -----> FPGA

It also depends a bit on the use-case: After you program the bitstream,
do you need to runtime re-program it? Does it do partial
reconfiguration? Are there subdevices inside your device, or is the
whole thing a discoverable PCI device afterwards?

Cheers,
Moritz
Sascha Hauer April 27, 2020, 6:44 a.m. UTC | #6
On Fri, Apr 24, 2020 at 08:59:49PM -0700, Moritz Fischer wrote:
> Hi Sascha,
> 
> On Thu, Apr 23, 2020 at 08:23:31AM +0200, Sascha Hauer wrote:
> > Hi Moritz,
> > 
> > On Wed, Apr 22, 2020 at 06:36:48PM -0700, Moritz Fischer wrote:
> > > Hi Sascha,
> > > 
> > > On Wed, Apr 22, 2020 at 01:44:32PM +0200, Sascha Hauer wrote:
> > > > Hi,
> > > > 
> > > > I wonder what can be done with the mainline state of drivers/fpga/. The
> > > > entry to the framework seems to be fpga_mgr_load(). The only user of
> > > > this function is fpga_region_program_fpga(). This in turn is only called
> > > > in response of applying a device tree overlay. A device tree overlay is
> > > > applied with of_overlay_fdt_apply() which has no users in the Kernel.
> > > 
> > > Yes. It is waiting for dt_overlays one way or another. I personally
> > > don't currently have the bandwidth to work actively on this.
> > > 
> > > > My current task is to load a firmware to a FPGA. The code all seems to
> > > > be there in the Kernel, it only lacks a way to trigger it. I am not very
> > > > interested in device tree overlays since the FPGA appears as a PCI
> > > > device (although applying a dtbo could enable the PCIe controller device
> > > > tree node). Is there some mainline way to upload FPGA firmware? At the
> > > > moment we are using the attached patch to trigger loading the firmware
> > > > from userspace. Would something like this be acceptable for mainline?
> > > 
> > > We've looked into this sort of patches over the years and never came to
> > > a general interface that really works.
> > > 
> > > The OPAE folks (and other users I know of) usually use FPGA Manager with
> > > a higher layer on top of it that moves the bitstream into the kernel via
> > > an ioctl().
> > > 
> > > One concept I had toyed with mentally, but haven't really gotten around
> > > to implement is a 'discoverable' region, that would deal with the
> > > necessary re-enumeration via a callback and have a sysfs interface
> > > similar to what the patch below has.
> > > This would essentially cover use-cases where you have a discoverable
> > > device implemented in FPGA logic, such as say an FPGA hanging off of
> > > PCIe bus that can get loaded over USB, a CPLD or some other side-band
> > > mechanism. After loading the image you'd have to rescan the PCIe bus -
> > > which - imho is the kernel's job.
> > > 
> > > What I really wanna avoid is creating another /dev/fpga0 / /dev/xdevcfg
> > > that completely leaves the kernel in the dark about the fact that it
> > > reconfigures a bit of hardware hanging off the bus.
> > 
> > Yes, makes sense. While this would suffice my needs at the moment it
> > really sounds like a dead end.
> > 
> > > 
> > > In my ideal world you'd create a pci driver that binds to your device,
> > > and creates mfd style subdevices for whatever you'd want your design to
> > > do. One of these devices would be an FPGA and a FPGA region attached to
> > > that FPGA manager. Your top level driver would co-ordinate the fact that
> > > you are re-programming parts of the FPGA and create / destroy devices as
> > > needed for the hardware contained in the bitstream.
> > 
> > In my case there is no pci device visible before loading the firmware,
> > so creating a pci driver is not an option. Maybe pci host controllers
> > could register themselves as fpga-bridges. With this we could put the
> > pci host controller (or the pci device, AFAIK there is a PCI device tree
> > binding) where the fpga is connected into the fpga-bridges phandles list
> > of the fpga-region.
> 
> Can you talk a bit more about the system you're working with? Is there
> some sort of sideband mechanism to load the image? What exposes the
> image loading capaibility? A CPLD, an ASIC, USB device, JTAG?

We have two systems. One uploads the FPGA firmware via SPI passive
serial mode (compatible fpga-arria10-passive-serial). The other system
has a ftdi FT232H USB/serial converter chip working in synchronous FIFO
mode, connected to the FPGA via a CPLD. The series from Anatolij Gustchin
here https://patchwork.kernel.org/cover/10824743/ would have been useful
for us, although of course our CPLD has a different protocol. On this
system we currently ended up with a small custom driver which bypasses
FPGA manager and only uploads the firmware at USB connect time.

> 
> What does the topology look like?
> 
> Sideband  -----> FPGA
> PCIe      -----> FPGA
> 
> It also depends a bit on the use-case: After you program the bitstream,
> do you need to runtime re-program it? Does it do partial
> reconfiguration? Are there subdevices inside your device, or is the
> whole thing a discoverable PCI device afterwards?

On both systems It's one PCI device only. Currently we don't need runtime
re-programming, although this might be useful in the future. Also there's
no partial reconfiguration.

Sascha
Moritz Fischer April 30, 2020, 3:26 a.m. UTC | #7
On Mon, Apr 27, 2020 at 08:44:33AM +0200, Sascha Hauer wrote:
> On Fri, Apr 24, 2020 at 08:59:49PM -0700, Moritz Fischer wrote:
> > Hi Sascha,
> > 
> > On Thu, Apr 23, 2020 at 08:23:31AM +0200, Sascha Hauer wrote:
> > > Hi Moritz,
> > > 
> > > On Wed, Apr 22, 2020 at 06:36:48PM -0700, Moritz Fischer wrote:
> > > > Hi Sascha,
> > > > 
> > > > On Wed, Apr 22, 2020 at 01:44:32PM +0200, Sascha Hauer wrote:
> > > > > Hi,
> > > > > 
> > > > > I wonder what can be done with the mainline state of drivers/fpga/. The
> > > > > entry to the framework seems to be fpga_mgr_load(). The only user of
> > > > > this function is fpga_region_program_fpga(). This in turn is only called
> > > > > in response of applying a device tree overlay. A device tree overlay is
> > > > > applied with of_overlay_fdt_apply() which has no users in the Kernel.
> > > > 
> > > > Yes. It is waiting for dt_overlays one way or another. I personally
> > > > don't currently have the bandwidth to work actively on this.
> > > > 
> > > > > My current task is to load a firmware to a FPGA. The code all seems to
> > > > > be there in the Kernel, it only lacks a way to trigger it. I am not very
> > > > > interested in device tree overlays since the FPGA appears as a PCI
> > > > > device (although applying a dtbo could enable the PCIe controller device
> > > > > tree node). Is there some mainline way to upload FPGA firmware? At the
> > > > > moment we are using the attached patch to trigger loading the firmware
> > > > > from userspace. Would something like this be acceptable for mainline?
> > > > 
> > > > We've looked into this sort of patches over the years and never came to
> > > > a general interface that really works.
> > > > 
> > > > The OPAE folks (and other users I know of) usually use FPGA Manager with
> > > > a higher layer on top of it that moves the bitstream into the kernel via
> > > > an ioctl().
> > > > 
> > > > One concept I had toyed with mentally, but haven't really gotten around
> > > > to implement is a 'discoverable' region, that would deal with the
> > > > necessary re-enumeration via a callback and have a sysfs interface
> > > > similar to what the patch below has.
> > > > This would essentially cover use-cases where you have a discoverable
> > > > device implemented in FPGA logic, such as say an FPGA hanging off of
> > > > PCIe bus that can get loaded over USB, a CPLD or some other side-band
> > > > mechanism. After loading the image you'd have to rescan the PCIe bus -
> > > > which - imho is the kernel's job.
> > > > 
> > > > What I really wanna avoid is creating another /dev/fpga0 / /dev/xdevcfg
> > > > that completely leaves the kernel in the dark about the fact that it
> > > > reconfigures a bit of hardware hanging off the bus.
> > > 
> > > Yes, makes sense. While this would suffice my needs at the moment it
> > > really sounds like a dead end.
> > > 
> > > > 
> > > > In my ideal world you'd create a pci driver that binds to your device,
> > > > and creates mfd style subdevices for whatever you'd want your design to
> > > > do. One of these devices would be an FPGA and a FPGA region attached to
> > > > that FPGA manager. Your top level driver would co-ordinate the fact that
> > > > you are re-programming parts of the FPGA and create / destroy devices as
> > > > needed for the hardware contained in the bitstream.
> > > 
> > > In my case there is no pci device visible before loading the firmware,
> > > so creating a pci driver is not an option. Maybe pci host controllers
> > > could register themselves as fpga-bridges. With this we could put the
> > > pci host controller (or the pci device, AFAIK there is a PCI device tree
> > > binding) where the fpga is connected into the fpga-bridges phandles list
> > > of the fpga-region.
> > 
> > Can you talk a bit more about the system you're working with? Is there
> > some sort of sideband mechanism to load the image? What exposes the
> > image loading capaibility? A CPLD, an ASIC, USB device, JTAG?
> 
> We have two systems. One uploads the FPGA firmware via SPI passive
> serial mode (compatible fpga-arria10-passive-serial). The other system
> has a ftdi FT232H USB/serial converter chip working in synchronous FIFO
> mode, connected to the FPGA via a CPLD. The series from Anatolij Gustchin
> here https://patchwork.kernel.org/cover/10824743/ would have been useful
> for us, although of course our CPLD has a different protocol. On this
> system we currently ended up with a small custom driver which bypasses
> FPGA manager and only uploads the firmware at USB connect time.

Yeah this seems imho the only legit use-case for a something like the
sysfs entry for the fpga-region. That being said once we add that, it's
going to become ABI and everyone is gonna use that instead of
implementing a proper driver using FPGA Regions to deal with
re-enumeration.

We don't really have a good way of modelling systems where the bus is
100% discoverable (like a PCI device), but only after we load firmware.

I mean you could implement a FPGA manager for your CPLD that also
instantiates a region that you then target when you want to reprogram
the FPGA region. How you'd tie that together though with the PCI device
to get the bus to be re-enumerated as part of reprogramming the region,
I'm not 100% sure :)

> > It also depends a bit on the use-case: After you program the bitstream,
> > do you need to runtime re-program it? Does it do partial
> > reconfiguration? Are there subdevices inside your device, or is the
> > whole thing a discoverable PCI device afterwards?
> 
> On both systems It's one PCI device only. Currently we don't need runtime
> re-programming, although this might be useful in the future. Also there's
> no partial reconfiguration.
> 

Sorry for the late response, things are busy at the moment.

Cheers,
Moritz

Patch
diff mbox series

diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
index bde5a9d460c5..ca6dc830fadf 100644
--- a/drivers/fpga/fpga-region.c
+++ b/drivers/fpga/fpga-region.c
@@ -5,6 +5,7 @@ 
  *  Copyright (C) 2013-2016 Altera Corporation
  *  Copyright (C) 2017 Intel Corporation
  */
+#include <linux/device.h>
 #include <linux/fpga/fpga-bridge.h>
 #include <linux/fpga/fpga-mgr.h>
 #include <linux/fpga/fpga-region.h>
@@ -170,11 +171,58 @@  static ssize_t compat_id_show(struct device *dev,
 		       (unsigned long long)region->compat_id->id_h,
 		       (unsigned long long)region->compat_id->id_l);
 }
-
 static DEVICE_ATTR_RO(compat_id);
 
+static ssize_t firmware_name_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+
+	if (!region->info || !region->info->firmware_name)
+		return 0;
+
+	return sprintf(buf, "%s\n", region->info->firmware_name);
+}
+
+static ssize_t firmware_name_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *firmware_name, size_t count)
+{
+	struct fpga_region *region = to_fpga_region(dev);
+	struct fpga_image_info *info = region->info;
+	int error;
+
+	if (!info) {
+		info = fpga_image_info_alloc(dev);
+		if (!info)
+			return -ENOMEM;
+	} else if (info->firmware_name) {
+		devm_kfree(dev, info->firmware_name);
+	}
+
+	info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
+	if (!info->firmware_name)
+		return -ENOMEM;
+
+	if (count >  0 && info->firmware_name[count - 1] == '\n')
+		info->firmware_name[count - 1] = '\0';
+
+	region->info = info;
+	error = fpga_region_program_fpga(region);
+	if (error) {
+		devm_kfree(dev, info->firmware_name);
+		info->firmware_name = NULL;
+	}
+
+	return error ? error : count;
+}
+
+static DEVICE_ATTR_RW(firmware_name);
+
 static struct attribute *fpga_region_attrs[] = {
 	&dev_attr_compat_id.attr,
+	&dev_attr_firmware_name.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(fpga_region);