Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Mon, Mar 12, 2018 at 02:36:48PM -0700, matthew.gerl...@linux.intel.com wrote: > > > On Mon, 12 Mar 2018, Wu Hao wrote: > > Hi Hao, > > Please see my two comments inline. > > Thanks, > Matthew Gerlach > > >On Sun, Mar 11, 2018 at 01:09:31PM -0700, matthew.gerl...@linux.intel.com > >wrote: > >> > >> > >>On Mon, 5 Mar 2018, Alan Tull wrote: > >> > >> > >>Hi Hao, > >> > >>I do think we should consider different hw implementations with this code > >>because it does look like most of it is generic. Specifically, I think > >>we should consider DFH based fpga images that have been shipped already, > >>and I think we need to consider new hardware implementations as well. > >>Full disclosure, I am particularly interested in porting to a new hw > >>implementation for partial reconfiguration. > > > >Hi Matthew, > > > >This dfl-fme-pr.c driver is developed for the PR sub feature (feature id > >= 0x5), but we can reuse it for any cases if possible. > > > >> > >>Please see some comments below. > > > >Thanks for the comments, please see my comments inline. > > > >> > >>Matthew Gerlach > >> > >>>On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote: > >>> > >>>Hi Hao, > >>> > >>>We are going to want to be able use different FPGA managers with this > >>>framework. The different manager may be part of a different FME in > >>>fabric or it may be a hardware FPGA manager. Fortunately, at this > >>>point now the changes, noted below, to get there are pretty small. > >>> > From: Kang Luwei > > Partial Reconfiguration (PR) is the most important function for FME. It > allows reconfiguration for given Port/Accelerated Function Unit (AFU). > > It creates platform devices for fpga-mgr, fpga-regions and fpga-bridges, > and invokes fpga-region's interface (fpga_region_program_fpga) for PR > operation once PR request received via ioctl. Below user space interface > is exposed by this sub feature. > > Ioctl interface: > * FPGA_FME_PORT_PR > Do partial reconfiguration per information from userspace, including > target port(AFU), buffer size and address info. It returns error code > to userspace if failed. For detailed PR error information, user needs > to read fpga-mgr's status sysfs interface. > > Signed-off-by: Tim Whisonant > Signed-off-by: Enno Luebbers > Signed-off-by: Shiva Rao > Signed-off-by: Christopher Rauer > Signed-off-by: Kang Luwei > Signed-off-by: Xiao Guangrong > Signed-off-by: Wu Hao > --- > v2: moved the code to drivers/fpga folder as suggested by Alan Tull. > switched to GPLv2 license. > removed status from FPGA_FME_PORT_PR ioctl data structure. > added platform devices creation for fpga-mgr/fpga-region/fpga-bridge. > switched to fpga-region interface fpga_region_program_fpga for PR. > fixed comments from Alan Tull on FPGA_MGR_PARTIAL_RECONFIG flag usage. > fixed kbuild warnings. > v3: rename driver files to dfl-fme-*. > rebase due to fpga APIs change. > replace bitfields. > switch to fpga_cdev_find_port to find port device. > v4: rebase and correct comments for some function. > fix SPDX license issue. > remove unnecessary input parameter for destroy_bridge/region function. > add dfl-fme-pr.h for PR sub feature data structure and registers. > --- > drivers/fpga/Makefile | 2 +- > drivers/fpga/dfl-fme-main.c | 45 +++- > drivers/fpga/dfl-fme-pr.c | 497 > ++ > drivers/fpga/dfl-fme-pr.h | 113 ++ > drivers/fpga/dfl-fme.h| 38 > include/uapi/linux/fpga-dfl.h | 27 +++ > 6 files changed, 720 insertions(+), 2 deletions(-) > create mode 100644 drivers/fpga/dfl-fme-pr.c > create mode 100644 drivers/fpga/dfl-fme-pr.h > create mode 100644 drivers/fpga/dfl-fme.h > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index fbd1c85..3c44fc9 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -32,7 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += > of-fpga-region.o > obj-$(CONFIG_FPGA_DFL) += dfl.o > obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o > > -dfl-fme-objs := dfl-fme-main.o > +dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o > > # Drivers for FPGAs which implement DFL > obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o > diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c > index 1a9929c..967a44c 100644 > --- a/drivers/fpga/dfl-fme-main.c > +++ b/drivers/fpga/dfl-fme-main.c > @@ -19,6 +19,7 @@ > #include > > #include "dfl.h" > +#include "dfl-fme.h" > > static ssize_t ports_num_show(struct device *dev, > struct device_attribute *attr, char *buf) > @@ -111,6 +112,10 @@ static s
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Mon, 12 Mar 2018, Wu Hao wrote:
Hi Hao,
Please see my two comments inline.
Thanks,
Matthew Gerlach
On Sun, Mar 11, 2018 at 01:09:31PM -0700, matthew.gerl...@linux.intel.com wrote:
On Mon, 5 Mar 2018, Alan Tull wrote:
Hi Hao,
I do think we should consider different hw implementations with this code
because it does look like most of it is generic. Specifically, I think
we should consider DFH based fpga images that have been shipped already,
and I think we need to consider new hardware implementations as well.
Full disclosure, I am particularly interested in porting to a new hw
implementation for partial reconfiguration.
Hi Matthew,
This dfl-fme-pr.c driver is developed for the PR sub feature (feature id
= 0x5), but we can reuse it for any cases if possible.
Please see some comments below.
Thanks for the comments, please see my comments inline.
Matthew Gerlach
On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
Hi Hao,
We are going to want to be able use different FPGA managers with this
framework. The different manager may be part of a different FME in
fabric or it may be a hardware FPGA manager. Fortunately, at this
point now the changes, noted below, to get there are pretty small.
From: Kang Luwei
Partial Reconfiguration (PR) is the most important function for FME. It
allows reconfiguration for given Port/Accelerated Function Unit (AFU).
It creates platform devices for fpga-mgr, fpga-regions and fpga-bridges,
and invokes fpga-region's interface (fpga_region_program_fpga) for PR
operation once PR request received via ioctl. Below user space interface
is exposed by this sub feature.
Ioctl interface:
* FPGA_FME_PORT_PR
Do partial reconfiguration per information from userspace, including
target port(AFU), buffer size and address info. It returns error code
to userspace if failed. For detailed PR error information, user needs
to read fpga-mgr's status sysfs interface.
Signed-off-by: Tim Whisonant
Signed-off-by: Enno Luebbers
Signed-off-by: Shiva Rao
Signed-off-by: Christopher Rauer
Signed-off-by: Kang Luwei
Signed-off-by: Xiao Guangrong
Signed-off-by: Wu Hao
---
v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
switched to GPLv2 license.
removed status from FPGA_FME_PORT_PR ioctl data structure.
added platform devices creation for fpga-mgr/fpga-region/fpga-bridge.
switched to fpga-region interface fpga_region_program_fpga for PR.
fixed comments from Alan Tull on FPGA_MGR_PARTIAL_RECONFIG flag usage.
fixed kbuild warnings.
v3: rename driver files to dfl-fme-*.
rebase due to fpga APIs change.
replace bitfields.
switch to fpga_cdev_find_port to find port device.
v4: rebase and correct comments for some function.
fix SPDX license issue.
remove unnecessary input parameter for destroy_bridge/region function.
add dfl-fme-pr.h for PR sub feature data structure and registers.
---
drivers/fpga/Makefile | 2 +-
drivers/fpga/dfl-fme-main.c | 45 +++-
drivers/fpga/dfl-fme-pr.c | 497 ++
drivers/fpga/dfl-fme-pr.h | 113 ++
drivers/fpga/dfl-fme.h| 38
include/uapi/linux/fpga-dfl.h | 27 +++
6 files changed, 720 insertions(+), 2 deletions(-)
create mode 100644 drivers/fpga/dfl-fme-pr.c
create mode 100644 drivers/fpga/dfl-fme-pr.h
create mode 100644 drivers/fpga/dfl-fme.h
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index fbd1c85..3c44fc9 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
obj-$(CONFIG_FPGA_DFL) += dfl.o
obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
-dfl-fme-objs := dfl-fme-main.o
+dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
# Drivers for FPGAs which implement DFL
obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index 1a9929c..967a44c 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -19,6 +19,7 @@
#include
#include "dfl.h"
+#include "dfl-fme.h"
static ssize_t ports_num_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -111,6 +112,10 @@ static struct feature_driver fme_feature_drvs[] = {
.ops = &fme_hdr_ops,
},
{
+ .id = FME_FEATURE_ID_PR_MGMT,
+ .ops = &pr_mgmt_ops,
+ },
+ {
.ops = NULL,
},
};
@@ -194,14 +199,49 @@ static const struct file_operations fme_fops = {
.unlocked_ioctl = fme_ioctl,
};
+static int fme_dev_init(struct platform_device *pdev)
+{
+ struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct fpga_fme *fme;
+
+ fme = devm_kzalloc(&pdev->dev, sizeof(*fme), GFP_KERNEL);
+ if (!fme)
+ return -ENOMEM;
+
+ fme->pdata = pdata;
+
+ mutex_lock(&pdata->lock);
+ fpga_pdata_set_p
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Sun, Mar 11, 2018 at 11:29 PM, Wu Hao wrote: > On Sun, Mar 11, 2018 at 01:09:31PM -0700, matthew.gerl...@linux.intel.com > wrote: >> >> Hi Hao, >> >> I do think we should consider different hw implementations with this code >> because it does look like most of it is generic. Specifically, I think >> we should consider DFH based fpga images that have been shipped already, >> and I think we need to consider new hardware implementations as well. >> Full disclosure, I am particularly interested in porting to a new hw >> implementation for partial reconfiguration. Hi Matthew, The manager may not be the only thing that has to change for a new implementation, i.e. will your 'port' be able to work with this patchset? In the current implementation, the port is part of the dfl enumeration code (dfl.c and dfl.h) rather than being part of the bridge for some reason. We discussed the possibility of putting the port enable/disable code into the bridge driver [1], but that didn't seem feasible at least last December. I still would feel more confident if port were part of the bridge instead of part of dfl. Alan [1] https://lkml.org/lkml/2017/12/21/62
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Sun, Mar 11, 2018 at 01:09:31PM -0700, matthew.gerl...@linux.intel.com wrote:
>
>
> On Mon, 5 Mar 2018, Alan Tull wrote:
>
>
> Hi Hao,
>
> I do think we should consider different hw implementations with this code
> because it does look like most of it is generic. Specifically, I think
> we should consider DFH based fpga images that have been shipped already,
> and I think we need to consider new hardware implementations as well.
> Full disclosure, I am particularly interested in porting to a new hw
> implementation for partial reconfiguration.
Hi Matthew,
This dfl-fme-pr.c driver is developed for the PR sub feature (feature id
= 0x5), but we can reuse it for any cases if possible.
>
> Please see some comments below.
Thanks for the comments, please see my comments inline.
>
> Matthew Gerlach
>
> >On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
> >
> >Hi Hao,
> >
> >We are going to want to be able use different FPGA managers with this
> >framework. The different manager may be part of a different FME in
> >fabric or it may be a hardware FPGA manager. Fortunately, at this
> >point now the changes, noted below, to get there are pretty small.
> >
> >>From: Kang Luwei
> >>
> >>Partial Reconfiguration (PR) is the most important function for FME. It
> >>allows reconfiguration for given Port/Accelerated Function Unit (AFU).
> >>
> >>It creates platform devices for fpga-mgr, fpga-regions and fpga-bridges,
> >>and invokes fpga-region's interface (fpga_region_program_fpga) for PR
> >>operation once PR request received via ioctl. Below user space interface
> >>is exposed by this sub feature.
> >>
> >>Ioctl interface:
> >>* FPGA_FME_PORT_PR
> >> Do partial reconfiguration per information from userspace, including
> >> target port(AFU), buffer size and address info. It returns error code
> >> to userspace if failed. For detailed PR error information, user needs
> >> to read fpga-mgr's status sysfs interface.
> >>
> >>Signed-off-by: Tim Whisonant
> >>Signed-off-by: Enno Luebbers
> >>Signed-off-by: Shiva Rao
> >>Signed-off-by: Christopher Rauer
> >>Signed-off-by: Kang Luwei
> >>Signed-off-by: Xiao Guangrong
> >>Signed-off-by: Wu Hao
> >>---
> >>v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
> >>switched to GPLv2 license.
> >>removed status from FPGA_FME_PORT_PR ioctl data structure.
> >>added platform devices creation for fpga-mgr/fpga-region/fpga-bridge.
> >>switched to fpga-region interface fpga_region_program_fpga for PR.
> >>fixed comments from Alan Tull on FPGA_MGR_PARTIAL_RECONFIG flag usage.
> >>fixed kbuild warnings.
> >>v3: rename driver files to dfl-fme-*.
> >>rebase due to fpga APIs change.
> >>replace bitfields.
> >>switch to fpga_cdev_find_port to find port device.
> >>v4: rebase and correct comments for some function.
> >>fix SPDX license issue.
> >>remove unnecessary input parameter for destroy_bridge/region function.
> >>add dfl-fme-pr.h for PR sub feature data structure and registers.
> >>---
> >> drivers/fpga/Makefile | 2 +-
> >> drivers/fpga/dfl-fme-main.c | 45 +++-
> >> drivers/fpga/dfl-fme-pr.c | 497
> >> ++
> >> drivers/fpga/dfl-fme-pr.h | 113 ++
> >> drivers/fpga/dfl-fme.h| 38
> >> include/uapi/linux/fpga-dfl.h | 27 +++
> >> 6 files changed, 720 insertions(+), 2 deletions(-)
> >> create mode 100644 drivers/fpga/dfl-fme-pr.c
> >> create mode 100644 drivers/fpga/dfl-fme-pr.h
> >> create mode 100644 drivers/fpga/dfl-fme.h
> >>
> >>diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> >>index fbd1c85..3c44fc9 100644
> >>--- a/drivers/fpga/Makefile
> >>+++ b/drivers/fpga/Makefile
> >>@@ -32,7 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
> >> obj-$(CONFIG_FPGA_DFL) += dfl.o
> >> obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
> >>
> >>-dfl-fme-objs := dfl-fme-main.o
> >>+dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
> >>
> >> # Drivers for FPGAs which implement DFL
> >> obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
> >>diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> >>index 1a9929c..967a44c 100644
> >>--- a/drivers/fpga/dfl-fme-main.c
> >>+++ b/drivers/fpga/dfl-fme-main.c
> >>@@ -19,6 +19,7 @@
> >> #include
> >>
> >> #include "dfl.h"
> >>+#include "dfl-fme.h"
> >>
> >> static ssize_t ports_num_show(struct device *dev,
> >> struct device_attribute *attr, char *buf)
> >>@@ -111,6 +112,10 @@ static struct feature_driver fme_feature_drvs[] = {
> >>.ops = &fme_hdr_ops,
> >>},
> >>{
> >>+ .id = FME_FEATURE_ID_PR_MGMT,
> >>+ .ops = &pr_mgmt_ops,
> >>+ },
> >>+ {
> >>.ops = NULL,
> >>},
> >> };
> >>@@ -194,14 +199,49 @@ static const struct file_operations fme_fops = {
> >>.unlocked_ioctl = fme_ioctl,
> >> };
> >>
> >>+stati
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Mon, 5 Mar 2018, Alan Tull wrote:
Hi Hao,
I do think we should consider different hw implementations with this code
because it does look like most of it is generic. Specifically, I think
we should consider DFH based fpga images that have been shipped already,
and I think we need to consider new hardware implementations as well.
Full disclosure, I am particularly interested in porting to a new hw
implementation for partial reconfiguration.
Please see some comments below.
Matthew Gerlach
On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
Hi Hao,
We are going to want to be able use different FPGA managers with this
framework. The different manager may be part of a different FME in
fabric or it may be a hardware FPGA manager. Fortunately, at this
point now the changes, noted below, to get there are pretty small.
From: Kang Luwei
Partial Reconfiguration (PR) is the most important function for FME. It
allows reconfiguration for given Port/Accelerated Function Unit (AFU).
It creates platform devices for fpga-mgr, fpga-regions and fpga-bridges,
and invokes fpga-region's interface (fpga_region_program_fpga) for PR
operation once PR request received via ioctl. Below user space interface
is exposed by this sub feature.
Ioctl interface:
* FPGA_FME_PORT_PR
Do partial reconfiguration per information from userspace, including
target port(AFU), buffer size and address info. It returns error code
to userspace if failed. For detailed PR error information, user needs
to read fpga-mgr's status sysfs interface.
Signed-off-by: Tim Whisonant
Signed-off-by: Enno Luebbers
Signed-off-by: Shiva Rao
Signed-off-by: Christopher Rauer
Signed-off-by: Kang Luwei
Signed-off-by: Xiao Guangrong
Signed-off-by: Wu Hao
---
v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
switched to GPLv2 license.
removed status from FPGA_FME_PORT_PR ioctl data structure.
added platform devices creation for fpga-mgr/fpga-region/fpga-bridge.
switched to fpga-region interface fpga_region_program_fpga for PR.
fixed comments from Alan Tull on FPGA_MGR_PARTIAL_RECONFIG flag usage.
fixed kbuild warnings.
v3: rename driver files to dfl-fme-*.
rebase due to fpga APIs change.
replace bitfields.
switch to fpga_cdev_find_port to find port device.
v4: rebase and correct comments for some function.
fix SPDX license issue.
remove unnecessary input parameter for destroy_bridge/region function.
add dfl-fme-pr.h for PR sub feature data structure and registers.
---
drivers/fpga/Makefile | 2 +-
drivers/fpga/dfl-fme-main.c | 45 +++-
drivers/fpga/dfl-fme-pr.c | 497 ++
drivers/fpga/dfl-fme-pr.h | 113 ++
drivers/fpga/dfl-fme.h| 38
include/uapi/linux/fpga-dfl.h | 27 +++
6 files changed, 720 insertions(+), 2 deletions(-)
create mode 100644 drivers/fpga/dfl-fme-pr.c
create mode 100644 drivers/fpga/dfl-fme-pr.h
create mode 100644 drivers/fpga/dfl-fme.h
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index fbd1c85..3c44fc9 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
obj-$(CONFIG_FPGA_DFL) += dfl.o
obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
-dfl-fme-objs := dfl-fme-main.o
+dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
# Drivers for FPGAs which implement DFL
obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index 1a9929c..967a44c 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -19,6 +19,7 @@
#include
#include "dfl.h"
+#include "dfl-fme.h"
static ssize_t ports_num_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -111,6 +112,10 @@ static struct feature_driver fme_feature_drvs[] = {
.ops = &fme_hdr_ops,
},
{
+ .id = FME_FEATURE_ID_PR_MGMT,
+ .ops = &pr_mgmt_ops,
+ },
+ {
.ops = NULL,
},
};
@@ -194,14 +199,49 @@ static const struct file_operations fme_fops = {
.unlocked_ioctl = fme_ioctl,
};
+static int fme_dev_init(struct platform_device *pdev)
+{
+ struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct fpga_fme *fme;
+
+ fme = devm_kzalloc(&pdev->dev, sizeof(*fme), GFP_KERNEL);
+ if (!fme)
+ return -ENOMEM;
+
+ fme->pdata = pdata;
+
+ mutex_lock(&pdata->lock);
+ fpga_pdata_set_private(pdata, fme);
+ mutex_unlock(&pdata->lock);
+
+ return 0;
+}
+
+static void fme_dev_destroy(struct platform_device *pdev)
+{
+ struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
+ struct fpga_fme *fme;
+
+ mutex_lock(&pdata->lock);
+ fme = fpga_pdata_get_private(pdata);
+ fpga_pdata_set_p
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Tue, Mar 06, 2018 at 12:29:35PM -0600, Alan Tull wrote:
> On Mon, Mar 5, 2018 at 8:08 PM, Wu Hao wrote:
> > On Mon, Mar 05, 2018 at 04:46:02PM -0600, Alan Tull wrote:
> >> On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
> >>
> >> Hi Hao,
> >
> > Hi Alan,
> >
> > Thanks for the comments.
> >
> >>
> >> We are going to want to be able use different FPGA managers with this
> >> framework. The different manager may be part of a different FME in
> >> fabric or it may be a hardware FPGA manager. Fortunately, at this
> >> point now the changes, noted below, to get there are pretty small.
> >
> > Yes, understand these could be some cases that FME having different PR
> > hardware.
> >
>
> Or supporting reduced FME plus hardware-based FPGA manager.
>
> Just to re-emphasize, the basic intent of the FPGA manager subsystem
> in the first place is to have FPGA managers separate from higher level
> frameworks so that the higher level frameworks will be able to able to
> use different FPGAs.
>
> >> > +/**
> >> > + * fpga_fme_create_mgr - create fpga mgr platform device as child device
> >> > + *
> >> > + * @pdata: fme platform_device's pdata
> >> > + *
> >> > + * Return: mgr platform device if successful, and error code otherwise.
> >> > + */
> >> > +static struct platform_device *
> >> > +fpga_fme_create_mgr(struct feature_platform_data *pdata)
> >> > +{
> >> > + struct platform_device *mgr, *fme = pdata->dev;
> >> > + struct feature *feature;
> >> > + struct resource res;
> >> > + struct resource *pres;
> >> > + int ret = -ENOMEM;
> >> > +
> >> > + feature = get_feature_by_id(&pdata->dev->dev,
> >> > FME_FEATURE_ID_PR_MGMT);
> >> > + if (!feature)
> >> > + return ERR_PTR(-ENODEV);
> >> > +
> >> > + /*
> >> > +* Each FME has only one fpga-mgr, so allocate platform device
> >> > using
> >> > +* the same FME platform device id.
> >> > +*/
> >> > + mgr = platform_device_alloc(FPGA_DFL_FME_MGR, fme->id);
> >>
> >> At this point, the framework is assuming all FME's include the same
> >> FPGA manager device which would use the driver in dfl-fme-mgr.c.
> >>
> >> I'm thinking of two cases where the manager isn't the same as a
> >> dfl-fme-mgr.c manager are a bit different:
> >>
> >> (1) a FME-based FPGA manager, but different implementation, different
> >> registers. The constraint is that the port implementation has to be
> >> similar enough to use the rest of the base FME code. I am wondering
> >> if the FPGA manager can be added to the DFL. At this point, the DFL
> >> would drive which FPGA manager is alloc'd. That way the user gets to
> >> use all this code in dfl-fme-pr.c but with their FPGA manager.
> >
> > Actually I'm not sure how this will be implemented in the hardware in the
> > future, but from my understanding, there may be several methods to add this
> > support (a different PR hardware) to FME.
> >
> > 1) introduce a new PR sub feature to FME.
> >driver knows it by different feature id, and create different fpga
> >manager platform device, but may not be able to reuse dfl-fme-pr.c.
>
> What would prevent reusing dfl-fme-pr.c? It looks like this is 98% of
> the way there and only needs a way of knowing which FPGA manager
> driver to alloc here. I am hoping that a new PR sub feature could be
> added and that dfl-fme-pr.c can be reused.
It depeneds on how hardware implements it. : )
I agree that if it follows the same way of current PR sub feature, then we
could reuse the dfl-fme-pr.c for sure.
>
> >
> > 2) add a different PR hardware support to current PR sub feature.
> >It requires hardware to add registers to indicate this is a different
> >PR hardware than dfl-fme-mgr.c, and its register region information
> >(e.g location and size). Then this dfl-fme-pr.c driver could read
> >these information from hardware and create a different fpga manager
> >platform device with correct resources.
>
> So this dfl-fme-pr.c would have to know where some ID registers are
> and the enumeration gets messier. Some of the enumeration would be
> DFL and some would be from information that is not in the DFL headers.
> The DFL should contain the knowledge of which mgr to use.
Actually I don't know how hardware will implement this in the future, but I
just listed my ideas here. Per my understanding, driver (reuse dfl-fme-pr.c)
needs some more information to decide which platform device to create (for
fpga manager).
1) introduce a new PR sub feature. Then it has a different private feature
id in DFH (Device Feature Header). driver could use this id to create a
different platform device.
2) introduce some registers inside the current PR sub feature. Then driver
could read these registers to know which platform device to create for fpga
manager.
I think either 1) or 2) will only require small changes to current driver
code, I don't have any concern on supporting different PR hardware. :)
I u
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Mon, Mar 5, 2018 at 8:08 PM, Wu Hao wrote:
> On Mon, Mar 05, 2018 at 04:46:02PM -0600, Alan Tull wrote:
>> On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
>>
>> Hi Hao,
>
> Hi Alan,
>
> Thanks for the comments.
>
>>
>> We are going to want to be able use different FPGA managers with this
>> framework. The different manager may be part of a different FME in
>> fabric or it may be a hardware FPGA manager. Fortunately, at this
>> point now the changes, noted below, to get there are pretty small.
>
> Yes, understand these could be some cases that FME having different PR
> hardware.
>
Or supporting reduced FME plus hardware-based FPGA manager.
Just to re-emphasize, the basic intent of the FPGA manager subsystem
in the first place is to have FPGA managers separate from higher level
frameworks so that the higher level frameworks will be able to able to
use different FPGAs.
>> > +/**
>> > + * fpga_fme_create_mgr - create fpga mgr platform device as child device
>> > + *
>> > + * @pdata: fme platform_device's pdata
>> > + *
>> > + * Return: mgr platform device if successful, and error code otherwise.
>> > + */
>> > +static struct platform_device *
>> > +fpga_fme_create_mgr(struct feature_platform_data *pdata)
>> > +{
>> > + struct platform_device *mgr, *fme = pdata->dev;
>> > + struct feature *feature;
>> > + struct resource res;
>> > + struct resource *pres;
>> > + int ret = -ENOMEM;
>> > +
>> > + feature = get_feature_by_id(&pdata->dev->dev,
>> > FME_FEATURE_ID_PR_MGMT);
>> > + if (!feature)
>> > + return ERR_PTR(-ENODEV);
>> > +
>> > + /*
>> > +* Each FME has only one fpga-mgr, so allocate platform device
>> > using
>> > +* the same FME platform device id.
>> > +*/
>> > + mgr = platform_device_alloc(FPGA_DFL_FME_MGR, fme->id);
>>
>> At this point, the framework is assuming all FME's include the same
>> FPGA manager device which would use the driver in dfl-fme-mgr.c.
>>
>> I'm thinking of two cases where the manager isn't the same as a
>> dfl-fme-mgr.c manager are a bit different:
>>
>> (1) a FME-based FPGA manager, but different implementation, different
>> registers. The constraint is that the port implementation has to be
>> similar enough to use the rest of the base FME code. I am wondering
>> if the FPGA manager can be added to the DFL. At this point, the DFL
>> would drive which FPGA manager is alloc'd. That way the user gets to
>> use all this code in dfl-fme-pr.c but with their FPGA manager.
>
> Actually I'm not sure how this will be implemented in the hardware in the
> future, but from my understanding, there may be several methods to add this
> support (a different PR hardware) to FME.
>
> 1) introduce a new PR sub feature to FME.
>driver knows it by different feature id, and create different fpga
>manager platform device, but may not be able to reuse dfl-fme-pr.c.
What would prevent reusing dfl-fme-pr.c? It looks like this is 98% of
the way there and only needs a way of knowing which FPGA manager
driver to alloc here. I am hoping that a new PR sub feature could be
added and that dfl-fme-pr.c can be reused.
>
> 2) add a different PR hardware support to current PR sub feature.
>It requires hardware to add registers to indicate this is a different
>PR hardware than dfl-fme-mgr.c, and its register region information
>(e.g location and size). Then this dfl-fme-pr.c driver could read
>these information from hardware and create a different fpga manager
>platform device with correct resources.
So this dfl-fme-pr.c would have to know where some ID registers are
and the enumeration gets messier. Some of the enumeration would be
DFL and some would be from information that is not in the DFL headers.
The DFL should contain the knowledge of which mgr to use.
>
> I think current driver framework could support both cases above for sure.
>
>>
>> (2) a FPGA manager that can be added by device tree in the case of a
>> platform that is using device tree. I think this will be pretty
>> simple and can be done later when someone is actually bringing this
>> framework up on a FPGA running under device tree. I'm thinking that
>> the base DFL device that reads the dfl data from hardware can have a
>> DT property that points to the FPGA manager. That manager can be
>> saved somewhere handy like the pdata and passed down to this code,
>> which realizes it can use that existing device and doesn't need to
>> alloc a platform device. But again, that's probably best done later.
>
> Sure, we can discuss further when we really need it. Actually per my
> understanding, if hardware describes itself clearly, we may not have to
> use DT for fpga manager, as driver could create it automatically based
> on information read from hardware. :)
DT exists for busses that don't have that kind of discovery. For a
concrete example, consider how the Arria10 driver (socfpga-a10.c)
probe function is gett
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Mon, Mar 05, 2018 at 04:46:02PM -0600, Alan Tull wrote:
> On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
>
> Hi Hao,
Hi Alan,
Thanks for the comments.
>
> We are going to want to be able use different FPGA managers with this
> framework. The different manager may be part of a different FME in
> fabric or it may be a hardware FPGA manager. Fortunately, at this
> point now the changes, noted below, to get there are pretty small.
Yes, understand these could be some cases that FME having different PR
hardware.
>
> > From: Kang Luwei
> >
> > Partial Reconfiguration (PR) is the most important function for FME. It
> > allows reconfiguration for given Port/Accelerated Function Unit (AFU).
> >
> > It creates platform devices for fpga-mgr, fpga-regions and fpga-bridges,
> > and invokes fpga-region's interface (fpga_region_program_fpga) for PR
> > operation once PR request received via ioctl. Below user space interface
> > is exposed by this sub feature.
> >
> > Ioctl interface:
> > * FPGA_FME_PORT_PR
> > Do partial reconfiguration per information from userspace, including
> > target port(AFU), buffer size and address info. It returns error code
> > to userspace if failed. For detailed PR error information, user needs
> > to read fpga-mgr's status sysfs interface.
> >
> > Signed-off-by: Tim Whisonant
> > Signed-off-by: Enno Luebbers
> > Signed-off-by: Shiva Rao
> > Signed-off-by: Christopher Rauer
> > Signed-off-by: Kang Luwei
> > Signed-off-by: Xiao Guangrong
> > Signed-off-by: Wu Hao
> > ---
> > v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
> > switched to GPLv2 license.
> > removed status from FPGA_FME_PORT_PR ioctl data structure.
> > added platform devices creation for fpga-mgr/fpga-region/fpga-bridge.
> > switched to fpga-region interface fpga_region_program_fpga for PR.
> > fixed comments from Alan Tull on FPGA_MGR_PARTIAL_RECONFIG flag usage.
> > fixed kbuild warnings.
> > v3: rename driver files to dfl-fme-*.
> > rebase due to fpga APIs change.
> > replace bitfields.
> > switch to fpga_cdev_find_port to find port device.
> > v4: rebase and correct comments for some function.
> > fix SPDX license issue.
> > remove unnecessary input parameter for destroy_bridge/region function.
> > add dfl-fme-pr.h for PR sub feature data structure and registers.
> > ---
> > drivers/fpga/Makefile | 2 +-
> > drivers/fpga/dfl-fme-main.c | 45 +++-
> > drivers/fpga/dfl-fme-pr.c | 497
> > ++
> > drivers/fpga/dfl-fme-pr.h | 113 ++
> > drivers/fpga/dfl-fme.h| 38
> > include/uapi/linux/fpga-dfl.h | 27 +++
> > 6 files changed, 720 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/fpga/dfl-fme-pr.c
> > create mode 100644 drivers/fpga/dfl-fme-pr.h
> > create mode 100644 drivers/fpga/dfl-fme.h
> >
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index fbd1c85..3c44fc9 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -32,7 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
> > obj-$(CONFIG_FPGA_DFL) += dfl.o
> > obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
> >
> > -dfl-fme-objs := dfl-fme-main.o
> > +dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
> >
> > # Drivers for FPGAs which implement DFL
> > obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
> > diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> > index 1a9929c..967a44c 100644
> > --- a/drivers/fpga/dfl-fme-main.c
> > +++ b/drivers/fpga/dfl-fme-main.c
> > @@ -19,6 +19,7 @@
> > #include
> >
> > #include "dfl.h"
> > +#include "dfl-fme.h"
> >
> > static ssize_t ports_num_show(struct device *dev,
> > struct device_attribute *attr, char *buf)
> > @@ -111,6 +112,10 @@ static struct feature_driver fme_feature_drvs[] = {
> > .ops = &fme_hdr_ops,
> > },
> > {
> > + .id = FME_FEATURE_ID_PR_MGMT,
> > + .ops = &pr_mgmt_ops,
> > + },
> > + {
> > .ops = NULL,
> > },
> > };
> > @@ -194,14 +199,49 @@ static const struct file_operations fme_fops = {
> > .unlocked_ioctl = fme_ioctl,
> > };
> >
> > +static int fme_dev_init(struct platform_device *pdev)
> > +{
> > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> > + struct fpga_fme *fme;
> > +
> > + fme = devm_kzalloc(&pdev->dev, sizeof(*fme), GFP_KERNEL);
> > + if (!fme)
> > + return -ENOMEM;
> > +
> > + fme->pdata = pdata;
> > +
> > + mutex_lock(&pdata->lock);
> > + fpga_pdata_set_private(pdata, fme);
> > + mutex_unlock(&pdata->lock);
> > +
> > + return 0;
> > +}
> > +
> > +static void fme_dev_destroy(struct platform_device *pdev)
> > +{
> > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev
Re: [PATCH v4 14/24] fpga: dfl: fme: add partial reconfiguration sub feature support
On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote:
Hi Hao,
We are going to want to be able use different FPGA managers with this
framework. The different manager may be part of a different FME in
fabric or it may be a hardware FPGA manager. Fortunately, at this
point now the changes, noted below, to get there are pretty small.
> From: Kang Luwei
>
> Partial Reconfiguration (PR) is the most important function for FME. It
> allows reconfiguration for given Port/Accelerated Function Unit (AFU).
>
> It creates platform devices for fpga-mgr, fpga-regions and fpga-bridges,
> and invokes fpga-region's interface (fpga_region_program_fpga) for PR
> operation once PR request received via ioctl. Below user space interface
> is exposed by this sub feature.
>
> Ioctl interface:
> * FPGA_FME_PORT_PR
> Do partial reconfiguration per information from userspace, including
> target port(AFU), buffer size and address info. It returns error code
> to userspace if failed. For detailed PR error information, user needs
> to read fpga-mgr's status sysfs interface.
>
> Signed-off-by: Tim Whisonant
> Signed-off-by: Enno Luebbers
> Signed-off-by: Shiva Rao
> Signed-off-by: Christopher Rauer
> Signed-off-by: Kang Luwei
> Signed-off-by: Xiao Guangrong
> Signed-off-by: Wu Hao
> ---
> v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
> switched to GPLv2 license.
> removed status from FPGA_FME_PORT_PR ioctl data structure.
> added platform devices creation for fpga-mgr/fpga-region/fpga-bridge.
> switched to fpga-region interface fpga_region_program_fpga for PR.
> fixed comments from Alan Tull on FPGA_MGR_PARTIAL_RECONFIG flag usage.
> fixed kbuild warnings.
> v3: rename driver files to dfl-fme-*.
> rebase due to fpga APIs change.
> replace bitfields.
> switch to fpga_cdev_find_port to find port device.
> v4: rebase and correct comments for some function.
> fix SPDX license issue.
> remove unnecessary input parameter for destroy_bridge/region function.
> add dfl-fme-pr.h for PR sub feature data structure and registers.
> ---
> drivers/fpga/Makefile | 2 +-
> drivers/fpga/dfl-fme-main.c | 45 +++-
> drivers/fpga/dfl-fme-pr.c | 497
> ++
> drivers/fpga/dfl-fme-pr.h | 113 ++
> drivers/fpga/dfl-fme.h| 38
> include/uapi/linux/fpga-dfl.h | 27 +++
> 6 files changed, 720 insertions(+), 2 deletions(-)
> create mode 100644 drivers/fpga/dfl-fme-pr.c
> create mode 100644 drivers/fpga/dfl-fme-pr.h
> create mode 100644 drivers/fpga/dfl-fme.h
>
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index fbd1c85..3c44fc9 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -32,7 +32,7 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o
> obj-$(CONFIG_FPGA_DFL) += dfl.o
> obj-$(CONFIG_FPGA_DFL_FME) += dfl-fme.o
>
> -dfl-fme-objs := dfl-fme-main.o
> +dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
>
> # Drivers for FPGAs which implement DFL
> obj-$(CONFIG_FPGA_DFL_PCI) += dfl-pci.o
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index 1a9929c..967a44c 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -19,6 +19,7 @@
> #include
>
> #include "dfl.h"
> +#include "dfl-fme.h"
>
> static ssize_t ports_num_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> @@ -111,6 +112,10 @@ static struct feature_driver fme_feature_drvs[] = {
> .ops = &fme_hdr_ops,
> },
> {
> + .id = FME_FEATURE_ID_PR_MGMT,
> + .ops = &pr_mgmt_ops,
> + },
> + {
> .ops = NULL,
> },
> };
> @@ -194,14 +199,49 @@ static const struct file_operations fme_fops = {
> .unlocked_ioctl = fme_ioctl,
> };
>
> +static int fme_dev_init(struct platform_device *pdev)
> +{
> + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + struct fpga_fme *fme;
> +
> + fme = devm_kzalloc(&pdev->dev, sizeof(*fme), GFP_KERNEL);
> + if (!fme)
> + return -ENOMEM;
> +
> + fme->pdata = pdata;
> +
> + mutex_lock(&pdata->lock);
> + fpga_pdata_set_private(pdata, fme);
> + mutex_unlock(&pdata->lock);
> +
> + return 0;
> +}
> +
> +static void fme_dev_destroy(struct platform_device *pdev)
> +{
> + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + struct fpga_fme *fme;
> +
> + mutex_lock(&pdata->lock);
> + fme = fpga_pdata_get_private(pdata);
> + fpga_pdata_set_private(pdata, NULL);
> + mutex_unlock(&pdata->lock);
> +
> + devm_kfree(&pdev->dev, fme);
Is this needed? This is called when the device is being destroyed anyway.
> +}
> +
> static int fme_probe(struct platform_device *pdev)
> {
> int ret;
>
> -
