Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-07 Thread Alan Tull
On Tue, Feb 6, 2018 at 10:52 PM, Wu Hao  wrote:
> On Tue, Feb 06, 2018 at 12:53:44PM -0600, Alan Tull wrote:
>> On Tue, Feb 6, 2018 at 12:47 AM, Wu Hao  wrote:
>> > On Mon, Feb 05, 2018 at 10:25:54PM -0600, Alan Tull wrote:
>> >> On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao  wrote:
>> >> > On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:
>> >> >> Hi Hao,
>> >> >>
>> >> >> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
>> >> >> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
>> >> >> > > Hi Hao, Alan,
>> >> >> > >
>> >> >> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
>> >> >> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
>> >> >> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  
>> >> >> > > > > wrote:
>> >> >> > > > >
>> >> >> > > > > Hi Hao,
>> >> >> > > > >
>> >> >> > > > > A few comments below.   Besides that, looks good.
>> >> >> > > > >
>> >> >> > > > > > This patch adds fpga manager driver for FPGA Management 
>> >> >> > > > > > Engine (FME). It
>> >> >> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
>> >> >> > > > > > function.
>> >> >> > > > > >
>> >> >> > > > > > 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 
>> >> >> > > > > > 
>> >> >> > > > > > v3: rename driver to dfl-fpga-fme-mgr
>> >> >> > > > > > implemented status callback for fpga manager
>> >> >> > > > > > rebased due to fpga api changes
>> >> >> > > > > > ---
>> >> >> > > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
>> >> >> > > > > >  drivers/fpga/Kconfig   |   6 +
>> >> >> > > > > >  drivers/fpga/Makefile  |   1 +
>> >> >> > > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
>> >> >> > > > > > +
>> >> >> > > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
>> >> >> > > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
>> >> >> > > > > >  create mode 100644 
>> >> >> > > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> >> > > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
>> >> >> > > > > >
>> >> >> > > > > > diff --git 
>> >> >> > > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
>> >> >> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> >> > > > > > new file mode 100644
>> >> >> > > > > > index 000..2d4f917
>> >> >> > > > > > --- /dev/null
>> >> >> > > > > > +++ 
>> >> >> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> >> > > > > > @@ -0,0 +1,8 @@
>> >> >> > > > > > +What:  
>> >> >> > > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
>> >> >> > > > > > +Date:  November 2017
>> >> >> > > > > > +KernelVersion:  4.15
>> >> >> > > > > > +Contact:   Wu Hao 
>> >> >> > > > > > +Description:   Read-only. It returns interface id of 
>> >> >> > > > > > partial reconfiguration
>> >> >> > > > > > +   hardware. Userspace could use this 
>> >> >> > > > > > information to check if
>> >> >> > > > > > +   current hardware is compatible with given 
>> >> >> > > > > > image before FPGA
>> >> >> > > > > > +   programming.
>> >> >> > > > >
>> >> >> > > > > I'm a little confused by this.  I can understand that the PR 
>> >> >> > > > > bitstream
>> >> >> > > > > has a dependency on the FPGA's static image, but I don't 
>> >> >> > > > > understand
>> >> >> > > > > the dependency of the bistream on the hardware that is used to 
>> >> >> > > > > program
>> >> >> > > > > the bitstream to the FPGA.
>> >> >> > > >
>> >> >> > > > Sorry for the confusion, the interface_id is used to indicate 
>> >> >> > > > the version of
>> >> >> > > > the hardware for partial reconfiguration (it's part of the 
>> >> >> > > > static image of
>> >> >> > > > the FPGA device). Will improve the description on this.
>> >> >> > > >
>> >> >> > >
>> >> >> > > The interface_id expresses the compatibility of the static region 
>> >> >> > > with PR
>> >> >> > > bitstreams generated for it. It changes every time a new static 
>> >> >> > > region is
>> >> >> > > generated.
>> >> >> > >
>> >> >> > > Would it make more sense to have the interface_id exposed as part 
>> >> >> > > of the FME
>> >> >> > > device (which represents the static region)? I'm not sure - it 
>> >> >> > > kind of also
>> >> >> > > makes sense here, where you would have all the information in one 
>> >> >> > > place (if the
>> >> >> > > interface_id matches, I can use this component to program a 
>> >> >> > > bitstream).
>> >> >> >
>> >> >> > Hi Enno
>> >> >> >
>> >> >> > Yes, this interface is unde

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-06 Thread Wu Hao
On Tue, Feb 06, 2018 at 12:53:44PM -0600, Alan Tull wrote:
> On Tue, Feb 6, 2018 at 12:47 AM, Wu Hao  wrote:
> > On Mon, Feb 05, 2018 at 10:25:54PM -0600, Alan Tull wrote:
> >> On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao  wrote:
> >> > On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:
> >> >> Hi Hao,
> >> >>
> >> >> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
> >> >> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> >> >> > > Hi Hao, Alan,
> >> >> > >
> >> >> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> >> >> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> >> >> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  
> >> >> > > > > wrote:
> >> >> > > > >
> >> >> > > > > Hi Hao,
> >> >> > > > >
> >> >> > > > > A few comments below.   Besides that, looks good.
> >> >> > > > >
> >> >> > > > > > This patch adds fpga manager driver for FPGA Management 
> >> >> > > > > > Engine (FME). It
> >> >> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
> >> >> > > > > > function.
> >> >> > > > > >
> >> >> > > > > > 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 
> >> >> > > > > > 
> >> >> > > > > > v3: rename driver to dfl-fpga-fme-mgr
> >> >> > > > > > implemented status callback for fpga manager
> >> >> > > > > > rebased due to fpga api changes
> >> >> > > > > > ---
> >> >> > > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> >> >> > > > > >  drivers/fpga/Kconfig   |   6 +
> >> >> > > > > >  drivers/fpga/Makefile  |   1 +
> >> >> > > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> >> >> > > > > > +
> >> >> > > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> >> >> > > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> >> >> > > > > >  create mode 100644 
> >> >> > > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> >> >> > > > > >
> >> >> > > > > > diff --git 
> >> >> > > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> >> >> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > > new file mode 100644
> >> >> > > > > > index 000..2d4f917
> >> >> > > > > > --- /dev/null
> >> >> > > > > > +++ 
> >> >> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > > @@ -0,0 +1,8 @@
> >> >> > > > > > +What:  
> >> >> > > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> >> >> > > > > > +Date:  November 2017
> >> >> > > > > > +KernelVersion:  4.15
> >> >> > > > > > +Contact:   Wu Hao 
> >> >> > > > > > +Description:   Read-only. It returns interface id of partial 
> >> >> > > > > > reconfiguration
> >> >> > > > > > +   hardware. Userspace could use this 
> >> >> > > > > > information to check if
> >> >> > > > > > +   current hardware is compatible with given 
> >> >> > > > > > image before FPGA
> >> >> > > > > > +   programming.
> >> >> > > > >
> >> >> > > > > I'm a little confused by this.  I can understand that the PR 
> >> >> > > > > bitstream
> >> >> > > > > has a dependency on the FPGA's static image, but I don't 
> >> >> > > > > understand
> >> >> > > > > the dependency of the bistream on the hardware that is used to 
> >> >> > > > > program
> >> >> > > > > the bitstream to the FPGA.
> >> >> > > >
> >> >> > > > Sorry for the confusion, the interface_id is used to indicate the 
> >> >> > > > version of
> >> >> > > > the hardware for partial reconfiguration (it's part of the static 
> >> >> > > > image of
> >> >> > > > the FPGA device). Will improve the description on this.
> >> >> > > >
> >> >> > >
> >> >> > > The interface_id expresses the compatibility of the static region 
> >> >> > > with PR
> >> >> > > bitstreams generated for it. It changes every time a new static 
> >> >> > > region is
> >> >> > > generated.
> >> >> > >
> >> >> > > Would it make more sense to have the interface_id exposed as part 
> >> >> > > of the FME
> >> >> > > device (which represents the static region)? I'm not sure - it kind 
> >> >> > > of also
> >> >> > > makes sense here, where you would have all the information in one 
> >> >> > > place (if the
> >> >> > > interface_id matches, I can use this component to program a 
> >> >> > > bitstream).
> >> >> >
> >> >> > Hi Enno
> >> >> >
> >> >> > Yes, this interface is under fpga-dfl-fme-mgr.0, and 
> >> >> > fpga-dfl-fme-mgr.0 is
> >> >> > under fpga-dfl-fme.0. It's part of the FME device for sure. From 
> >> >> > another
> >> >> > point of view, it means if an

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-06 Thread Alan Tull
On Tue, Feb 6, 2018 at 12:47 AM, Wu Hao  wrote:
> On Mon, Feb 05, 2018 at 10:25:54PM -0600, Alan Tull wrote:
>> On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao  wrote:
>> > On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:
>> >> Hi Hao,
>> >>
>> >> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
>> >> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
>> >> > > Hi Hao, Alan,
>> >> > >
>> >> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
>> >> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
>> >> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
>> >> > > > >
>> >> > > > > Hi Hao,
>> >> > > > >
>> >> > > > > A few comments below.   Besides that, looks good.
>> >> > > > >
>> >> > > > > > This patch adds fpga manager driver for FPGA Management Engine 
>> >> > > > > > (FME). It
>> >> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
>> >> > > > > > function.
>> >> > > > > >
>> >> > > > > > 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 
>> >> > > > > > 
>> >> > > > > > v3: rename driver to dfl-fpga-fme-mgr
>> >> > > > > > implemented status callback for fpga manager
>> >> > > > > > rebased due to fpga api changes
>> >> > > > > > ---
>> >> > > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
>> >> > > > > >  drivers/fpga/Kconfig   |   6 +
>> >> > > > > >  drivers/fpga/Makefile  |   1 +
>> >> > > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
>> >> > > > > > +
>> >> > > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
>> >> > > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
>> >> > > > > >  create mode 100644 
>> >> > > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> > > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
>> >> > > > > >
>> >> > > > > > diff --git 
>> >> > > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
>> >> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> > > > > > new file mode 100644
>> >> > > > > > index 000..2d4f917
>> >> > > > > > --- /dev/null
>> >> > > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> > > > > > @@ -0,0 +1,8 @@
>> >> > > > > > +What:  
>> >> > > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
>> >> > > > > > +Date:  November 2017
>> >> > > > > > +KernelVersion:  4.15
>> >> > > > > > +Contact:   Wu Hao 
>> >> > > > > > +Description:   Read-only. It returns interface id of partial 
>> >> > > > > > reconfiguration
>> >> > > > > > +   hardware. Userspace could use this information 
>> >> > > > > > to check if
>> >> > > > > > +   current hardware is compatible with given image 
>> >> > > > > > before FPGA
>> >> > > > > > +   programming.
>> >> > > > >
>> >> > > > > I'm a little confused by this.  I can understand that the PR 
>> >> > > > > bitstream
>> >> > > > > has a dependency on the FPGA's static image, but I don't 
>> >> > > > > understand
>> >> > > > > the dependency of the bistream on the hardware that is used to 
>> >> > > > > program
>> >> > > > > the bitstream to the FPGA.
>> >> > > >
>> >> > > > Sorry for the confusion, the interface_id is used to indicate the 
>> >> > > > version of
>> >> > > > the hardware for partial reconfiguration (it's part of the static 
>> >> > > > image of
>> >> > > > the FPGA device). Will improve the description on this.
>> >> > > >
>> >> > >
>> >> > > The interface_id expresses the compatibility of the static region 
>> >> > > with PR
>> >> > > bitstreams generated for it. It changes every time a new static 
>> >> > > region is
>> >> > > generated.
>> >> > >
>> >> > > Would it make more sense to have the interface_id exposed as part of 
>> >> > > the FME
>> >> > > device (which represents the static region)? I'm not sure - it kind 
>> >> > > of also
>> >> > > makes sense here, where you would have all the information in one 
>> >> > > place (if the
>> >> > > interface_id matches, I can use this component to program a 
>> >> > > bitstream).
>> >> >
>> >> > Hi Enno
>> >> >
>> >> > Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 
>> >> > is
>> >> > under fpga-dfl-fme.0. It's part of the FME device for sure. From another
>> >> > point of view, it means if anyone wants to do PR on this Intel FPGA 
>> >> > device,
>> >> > he needs to find the FME device firstly, and then check if any fpga 
>> >> > manager
>> >> > created under this FME device, if yes, check the interface_id before PR 
>> >> > via
>> >> > the FME device node ioctl.
>> >>
>> >> That sounds good, thank

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Wu Hao
On Mon, Feb 05, 2018 at 10:25:54PM -0600, Alan Tull wrote:
> On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao  wrote:
> > On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:
> >> Hi Hao,
> >>
> >> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
> >> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> >> > > Hi Hao, Alan,
> >> > >
> >> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> >> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> >> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> >> > > > >
> >> > > > > Hi Hao,
> >> > > > >
> >> > > > > A few comments below.   Besides that, looks good.
> >> > > > >
> >> > > > > > This patch adds fpga manager driver for FPGA Management Engine 
> >> > > > > > (FME). It
> >> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
> >> > > > > > function.
> >> > > > > >
> >> > > > > > 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 
> >> > > > > > 
> >> > > > > > v3: rename driver to dfl-fpga-fme-mgr
> >> > > > > > implemented status callback for fpga manager
> >> > > > > > rebased due to fpga api changes
> >> > > > > > ---
> >> > > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> >> > > > > >  drivers/fpga/Kconfig   |   6 +
> >> > > > > >  drivers/fpga/Makefile  |   1 +
> >> > > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> >> > > > > > +
> >> > > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> >> > > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> >> > > > > >  create mode 100644 
> >> > > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> > > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> >> > > > > >
> >> > > > > > diff --git 
> >> > > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> >> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> > > > > > new file mode 100644
> >> > > > > > index 000..2d4f917
> >> > > > > > --- /dev/null
> >> > > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> > > > > > @@ -0,0 +1,8 @@
> >> > > > > > +What:  
> >> > > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> >> > > > > > +Date:  November 2017
> >> > > > > > +KernelVersion:  4.15
> >> > > > > > +Contact:   Wu Hao 
> >> > > > > > +Description:   Read-only. It returns interface id of partial 
> >> > > > > > reconfiguration
> >> > > > > > +   hardware. Userspace could use this information 
> >> > > > > > to check if
> >> > > > > > +   current hardware is compatible with given image 
> >> > > > > > before FPGA
> >> > > > > > +   programming.
> >> > > > >
> >> > > > > I'm a little confused by this.  I can understand that the PR 
> >> > > > > bitstream
> >> > > > > has a dependency on the FPGA's static image, but I don't understand
> >> > > > > the dependency of the bistream on the hardware that is used to 
> >> > > > > program
> >> > > > > the bitstream to the FPGA.
> >> > > >
> >> > > > Sorry for the confusion, the interface_id is used to indicate the 
> >> > > > version of
> >> > > > the hardware for partial reconfiguration (it's part of the static 
> >> > > > image of
> >> > > > the FPGA device). Will improve the description on this.
> >> > > >
> >> > >
> >> > > The interface_id expresses the compatibility of the static region with 
> >> > > PR
> >> > > bitstreams generated for it. It changes every time a new static region 
> >> > > is
> >> > > generated.
> >> > >
> >> > > Would it make more sense to have the interface_id exposed as part of 
> >> > > the FME
> >> > > device (which represents the static region)? I'm not sure - it kind of 
> >> > > also
> >> > > makes sense here, where you would have all the information in one 
> >> > > place (if the
> >> > > interface_id matches, I can use this component to program a bitstream).
> >> >
> >> > Hi Enno
> >> >
> >> > Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 
> >> > is
> >> > under fpga-dfl-fme.0. It's part of the FME device for sure. From another
> >> > point of view, it means if anyone wants to do PR on this Intel FPGA 
> >> > device,
> >> > he needs to find the FME device firstly, and then check if any fpga 
> >> > manager
> >> > created under this FME device, if yes, check the interface_id before PR 
> >> > via
> >> > the FME device node ioctl.
> >>
> >> That sounds good, thank you!
> >>
> >> >
> >> > >
> >> > > Sorry for my limited understanding of the infrastructure - would this 
> >> > > same
> >> > > "fpga-dfl-fme-mgr.0" be used for PR if we had multipl

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Moritz Fischer
On Mon, Feb 05, 2018 at 10:25:27PM -0600, Alan Tull wrote:
> On Mon, Feb 5, 2018 at 8:17 PM, Wu Hao  wrote:
> > On Mon, Feb 05, 2018 at 11:21:52AM -0600, Alan Tull wrote:
> >> On Sun, Feb 4, 2018 at 4:05 AM, Wu Hao  wrote:
> >> > On Sat, Feb 03, 2018 at 11:41:24AM +0100, Moritz Fischer wrote:
> >> >> Hi Hao,
> >> >>
> >> >> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> >> >> > Hi Hao, Alan,
> >> >> >
> >> >> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> >> >> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> >> >> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> >> >> > > >
> >> >> > > > Hi Hao,
> >> >> > > >
> >> >> > > > A few comments below.   Besides that, looks good.
> >> >> > > >
> >> >> > > > > This patch adds fpga manager driver for FPGA Management Engine 
> >> >> > > > > (FME). It
> >> >> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
> >> >> > > > > function.
> >> >> > > > >
> >> >> > > > > 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 
> >> >> > > > > 
> >> >> > > > > v3: rename driver to dfl-fpga-fme-mgr
> >> >> > > > > implemented status callback for fpga manager
> >> >> > > > > rebased due to fpga api changes
> >> >> > > > > ---
> >> >> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> >> >> > > > >  drivers/fpga/Kconfig   |   6 +
> >> >> > > > >  drivers/fpga/Makefile  |   1 +
> >> >> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> >> >> > > > > +
> >> >> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> >> >> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> >> >> > > > >  create mode 100644 
> >> >> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> >> >> > > > >
> >> >> > > > > diff --git 
> >> >> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> >> >> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > new file mode 100644
> >> >> > > > > index 000..2d4f917
> >> >> > > > > --- /dev/null
> >> >> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > @@ -0,0 +1,8 @@
> >> >> > > > > +What:  
> >> >> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> >> >> > > > > +Date:  November 2017
> >> >> > > > > +KernelVersion:  4.15
> >> >> > > > > +Contact:   Wu Hao 
> >> >> > > > > +Description:   Read-only. It returns interface id of partial 
> >> >> > > > > reconfiguration
> >> >> > > > > +   hardware. Userspace could use this information 
> >> >> > > > > to check if
> >> >> > > > > +   current hardware is compatible with given image 
> >> >> > > > > before FPGA
> >> >> > > > > +   programming.
> >> >> > > >
> >> >> > > > I'm a little confused by this.  I can understand that the PR 
> >> >> > > > bitstream
> >> >> > > > has a dependency on the FPGA's static image, but I don't 
> >> >> > > > understand
> >> >> > > > the dependency of the bistream on the hardware that is used to 
> >> >> > > > program
> >> >> > > > the bitstream to the FPGA.
> >> >> > >
> >> >> > > Sorry for the confusion, the interface_id is used to indicate the 
> >> >> > > version of
> >> >> > > the hardware for partial reconfiguration (it's part of the static 
> >> >> > > image of
> >> >> > > the FPGA device). Will improve the description on this.
> >> >>
> >> >> I'm not sure userland should be making the call on whether what you're
> >> >> trying to load is compatible or not.

I guess I was worried that you might take down a PR region that you
could've avoided. On second thought unless the kernel could extract that
info somehow from the bitstream before programming this is probably
anyway not possible, since the only one that knows the id -> bitstream
mapping is userland.

Hao's comments further down outline well enough that the hardware can
deal with incompatible bitstreams. So ignore my comments ;-)

> >>
> >> Could you explain more about what your concern was about this (unless
> >> Hao has covered it below)?
> >>
> >> It makes sense to me in this use case at least since userspace has a
> >> pile of images and is choosing which one to load.
> >>
> >> >> Isn't there a way to check this in
> >> >> your PR reconfiguration handler in-kernel?
> >> >
> >> > Hi Moritz
> >> >
> >> > Actually with current driver interface, doing a partial reconfiguration 
> >> > with an
> >> > incompatible image, then driver will report PR failure with error code
> >> > FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR as hardware chec

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Wu Hao
On Mon, Feb 05, 2018 at 10:25:27PM -0600, Alan Tull wrote:
> On Mon, Feb 5, 2018 at 8:17 PM, Wu Hao  wrote:
> > On Mon, Feb 05, 2018 at 11:21:52AM -0600, Alan Tull wrote:
> >> On Sun, Feb 4, 2018 at 4:05 AM, Wu Hao  wrote:
> >> > On Sat, Feb 03, 2018 at 11:41:24AM +0100, Moritz Fischer wrote:
> >> >> Hi Hao,
> >> >>
> >> >> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> >> >> > Hi Hao, Alan,
> >> >> >
> >> >> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> >> >> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> >> >> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> >> >> > > >
> >> >> > > > Hi Hao,
> >> >> > > >
> >> >> > > > A few comments below.   Besides that, looks good.
> >> >> > > >
> >> >> > > > > This patch adds fpga manager driver for FPGA Management Engine 
> >> >> > > > > (FME). It
> >> >> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
> >> >> > > > > function.
> >> >> > > > >
> >> >> > > > > 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 
> >> >> > > > > 
> >> >> > > > > v3: rename driver to dfl-fpga-fme-mgr
> >> >> > > > > implemented status callback for fpga manager
> >> >> > > > > rebased due to fpga api changes
> >> >> > > > > ---
> >> >> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> >> >> > > > >  drivers/fpga/Kconfig   |   6 +
> >> >> > > > >  drivers/fpga/Makefile  |   1 +
> >> >> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> >> >> > > > > +
> >> >> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> >> >> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> >> >> > > > >  create mode 100644 
> >> >> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> >> >> > > > >
> >> >> > > > > diff --git 
> >> >> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> >> >> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > new file mode 100644
> >> >> > > > > index 000..2d4f917
> >> >> > > > > --- /dev/null
> >> >> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> >> > > > > @@ -0,0 +1,8 @@
> >> >> > > > > +What:  
> >> >> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> >> >> > > > > +Date:  November 2017
> >> >> > > > > +KernelVersion:  4.15
> >> >> > > > > +Contact:   Wu Hao 
> >> >> > > > > +Description:   Read-only. It returns interface id of partial 
> >> >> > > > > reconfiguration
> >> >> > > > > +   hardware. Userspace could use this information 
> >> >> > > > > to check if
> >> >> > > > > +   current hardware is compatible with given image 
> >> >> > > > > before FPGA
> >> >> > > > > +   programming.
> >> >> > > >
> >> >> > > > I'm a little confused by this.  I can understand that the PR 
> >> >> > > > bitstream
> >> >> > > > has a dependency on the FPGA's static image, but I don't 
> >> >> > > > understand
> >> >> > > > the dependency of the bistream on the hardware that is used to 
> >> >> > > > program
> >> >> > > > the bitstream to the FPGA.
> >> >> > >
> >> >> > > Sorry for the confusion, the interface_id is used to indicate the 
> >> >> > > version of
> >> >> > > the hardware for partial reconfiguration (it's part of the static 
> >> >> > > image of
> >> >> > > the FPGA device). Will improve the description on this.
> >> >>
> >> >> I'm not sure userland should be making the call on whether what you're
> >> >> trying to load is compatible or not.
> >>
> >> Could you explain more about what your concern was about this (unless
> >> Hao has covered it below)?
> >>
> >> It makes sense to me in this use case at least since userspace has a
> >> pile of images and is choosing which one to load.
> >>
> >> >> Isn't there a way to check this in
> >> >> your PR reconfiguration handler in-kernel?
> >> >
> >> > Hi Moritz
> >> >
> >> > Actually with current driver interface, doing a partial reconfiguration 
> >> > with an
> >> > incompatible image, then driver will report PR failure with error code
> >> > FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR as hardware checks it, but anyway 
> >> > user
> >> > needs to know hardware interface_id information to find or re-generated 
> >> > correct
> >> > images. I think it's more flexible to leave it to userspace on using this
> >> > information exposed by driver. : )
> >> >
> >> > Thanks
> >> > Hao
> >> >
> >> >>
> >> >> > >
> >> >> >
> >> >> > The interface_id expresses the compatibility of the static region 
> >> >> > with PR
> >> >> > bitstreams g

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Alan Tull
On Mon, Feb 5, 2018 at 7:47 PM, Wu Hao  wrote:
> On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:
>> Hi Hao,
>>
>> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
>> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
>> > > Hi Hao, Alan,
>> > >
>> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
>> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
>> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
>> > > > >
>> > > > > Hi Hao,
>> > > > >
>> > > > > A few comments below.   Besides that, looks good.
>> > > > >
>> > > > > > This patch adds fpga manager driver for FPGA Management Engine 
>> > > > > > (FME). It
>> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
>> > > > > > function.
>> > > > > >
>> > > > > > 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 
>> > > > > > 
>> > > > > > v3: rename driver to dfl-fpga-fme-mgr
>> > > > > > implemented status callback for fpga manager
>> > > > > > rebased due to fpga api changes
>> > > > > > ---
>> > > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
>> > > > > >  drivers/fpga/Kconfig   |   6 +
>> > > > > >  drivers/fpga/Makefile  |   1 +
>> > > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
>> > > > > > +
>> > > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
>> > > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
>> > > > > >  create mode 100644 
>> > > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> > > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
>> > > > > >
>> > > > > > diff --git 
>> > > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
>> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> > > > > > new file mode 100644
>> > > > > > index 000..2d4f917
>> > > > > > --- /dev/null
>> > > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> > > > > > @@ -0,0 +1,8 @@
>> > > > > > +What:  
>> > > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
>> > > > > > +Date:  November 2017
>> > > > > > +KernelVersion:  4.15
>> > > > > > +Contact:   Wu Hao 
>> > > > > > +Description:   Read-only. It returns interface id of partial 
>> > > > > > reconfiguration
>> > > > > > +   hardware. Userspace could use this information to 
>> > > > > > check if
>> > > > > > +   current hardware is compatible with given image 
>> > > > > > before FPGA
>> > > > > > +   programming.
>> > > > >
>> > > > > I'm a little confused by this.  I can understand that the PR 
>> > > > > bitstream
>> > > > > has a dependency on the FPGA's static image, but I don't understand
>> > > > > the dependency of the bistream on the hardware that is used to 
>> > > > > program
>> > > > > the bitstream to the FPGA.
>> > > >
>> > > > Sorry for the confusion, the interface_id is used to indicate the 
>> > > > version of
>> > > > the hardware for partial reconfiguration (it's part of the static 
>> > > > image of
>> > > > the FPGA device). Will improve the description on this.
>> > > >
>> > >
>> > > The interface_id expresses the compatibility of the static region with PR
>> > > bitstreams generated for it. It changes every time a new static region is
>> > > generated.
>> > >
>> > > Would it make more sense to have the interface_id exposed as part of the 
>> > > FME
>> > > device (which represents the static region)? I'm not sure - it kind of 
>> > > also
>> > > makes sense here, where you would have all the information in one place 
>> > > (if the
>> > > interface_id matches, I can use this component to program a bitstream).
>> >
>> > Hi Enno
>> >
>> > Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 is
>> > under fpga-dfl-fme.0. It's part of the FME device for sure. From another
>> > point of view, it means if anyone wants to do PR on this Intel FPGA device,
>> > he needs to find the FME device firstly, and then check if any fpga manager
>> > created under this FME device, if yes, check the interface_id before PR via
>> > the FME device node ioctl.
>>
>> That sounds good, thank you!
>>
>> >
>> > >
>> > > Sorry for my limited understanding of the infrastructure - would this 
>> > > same
>> > > "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In 
>> > > that case
>> > > it would need to expose multiple interface_ids (or we'd have to track 
>> > > both
>> > > interface IDs and an identifier for the target PR region).
>> >
>> > Yes, the fpga manager could be shared with different PR regions.
>> >
>> > Sorry, I'm not sure where we need to expose multi

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Alan Tull
On Mon, Feb 5, 2018 at 8:17 PM, Wu Hao  wrote:
> On Mon, Feb 05, 2018 at 11:21:52AM -0600, Alan Tull wrote:
>> On Sun, Feb 4, 2018 at 4:05 AM, Wu Hao  wrote:
>> > On Sat, Feb 03, 2018 at 11:41:24AM +0100, Moritz Fischer wrote:
>> >> Hi Hao,
>> >>
>> >> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
>> >> > Hi Hao, Alan,
>> >> >
>> >> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
>> >> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
>> >> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
>> >> > > >
>> >> > > > Hi Hao,
>> >> > > >
>> >> > > > A few comments below.   Besides that, looks good.
>> >> > > >
>> >> > > > > This patch adds fpga manager driver for FPGA Management Engine 
>> >> > > > > (FME). It
>> >> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
>> >> > > > > function.
>> >> > > > >
>> >> > > > > 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 
>> >> > > > > 
>> >> > > > > v3: rename driver to dfl-fpga-fme-mgr
>> >> > > > > implemented status callback for fpga manager
>> >> > > > > rebased due to fpga api changes
>> >> > > > > ---
>> >> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
>> >> > > > >  drivers/fpga/Kconfig   |   6 +
>> >> > > > >  drivers/fpga/Makefile  |   1 +
>> >> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
>> >> > > > > +
>> >> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
>> >> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
>> >> > > > >  create mode 100644 
>> >> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
>> >> > > > >
>> >> > > > > diff --git 
>> >> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
>> >> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> > > > > new file mode 100644
>> >> > > > > index 000..2d4f917
>> >> > > > > --- /dev/null
>> >> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> >> > > > > @@ -0,0 +1,8 @@
>> >> > > > > +What:  
>> >> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
>> >> > > > > +Date:  November 2017
>> >> > > > > +KernelVersion:  4.15
>> >> > > > > +Contact:   Wu Hao 
>> >> > > > > +Description:   Read-only. It returns interface id of partial 
>> >> > > > > reconfiguration
>> >> > > > > +   hardware. Userspace could use this information to 
>> >> > > > > check if
>> >> > > > > +   current hardware is compatible with given image 
>> >> > > > > before FPGA
>> >> > > > > +   programming.
>> >> > > >
>> >> > > > I'm a little confused by this.  I can understand that the PR 
>> >> > > > bitstream
>> >> > > > has a dependency on the FPGA's static image, but I don't understand
>> >> > > > the dependency of the bistream on the hardware that is used to 
>> >> > > > program
>> >> > > > the bitstream to the FPGA.
>> >> > >
>> >> > > Sorry for the confusion, the interface_id is used to indicate the 
>> >> > > version of
>> >> > > the hardware for partial reconfiguration (it's part of the static 
>> >> > > image of
>> >> > > the FPGA device). Will improve the description on this.
>> >>
>> >> I'm not sure userland should be making the call on whether what you're
>> >> trying to load is compatible or not.
>>
>> Could you explain more about what your concern was about this (unless
>> Hao has covered it below)?
>>
>> It makes sense to me in this use case at least since userspace has a
>> pile of images and is choosing which one to load.
>>
>> >> Isn't there a way to check this in
>> >> your PR reconfiguration handler in-kernel?
>> >
>> > Hi Moritz
>> >
>> > Actually with current driver interface, doing a partial reconfiguration 
>> > with an
>> > incompatible image, then driver will report PR failure with error code
>> > FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR as hardware checks it, but anyway 
>> > user
>> > needs to know hardware interface_id information to find or re-generated 
>> > correct
>> > images. I think it's more flexible to leave it to userspace on using this
>> > information exposed by driver. : )
>> >
>> > Thanks
>> > Hao
>> >
>> >>
>> >> > >
>> >> >
>> >> > The interface_id expresses the compatibility of the static region with 
>> >> > PR
>> >> > bitstreams generated for it. It changes every time a new static region 
>> >> > is
>> >> > generated.
>>
>> In the near future the DFL framework will be used with SoC's that have
>> a hard FPGA PR manager (that's not part of the static region).  The
>> hard FPGA manager driver won't know anything about the st

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Wu Hao
On Mon, Feb 05, 2018 at 11:21:52AM -0600, Alan Tull wrote:
> On Sun, Feb 4, 2018 at 4:05 AM, Wu Hao  wrote:
> > On Sat, Feb 03, 2018 at 11:41:24AM +0100, Moritz Fischer wrote:
> >> Hi Hao,
> >>
> >> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> >> > Hi Hao, Alan,
> >> >
> >> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> >> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> >> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> >> > > >
> >> > > > Hi Hao,
> >> > > >
> >> > > > A few comments below.   Besides that, looks good.
> >> > > >
> >> > > > > This patch adds fpga manager driver for FPGA Management Engine 
> >> > > > > (FME). It
> >> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
> >> > > > > function.
> >> > > > >
> >> > > > > 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 
> >> > > > > 
> >> > > > > v3: rename driver to dfl-fpga-fme-mgr
> >> > > > > implemented status callback for fpga manager
> >> > > > > rebased due to fpga api changes
> >> > > > > ---
> >> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> >> > > > >  drivers/fpga/Kconfig   |   6 +
> >> > > > >  drivers/fpga/Makefile  |   1 +
> >> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> >> > > > > +
> >> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> >> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> >> > > > >  create mode 100644 
> >> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> >> > > > >
> >> > > > > diff --git 
> >> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> >> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> > > > > new file mode 100644
> >> > > > > index 000..2d4f917
> >> > > > > --- /dev/null
> >> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >> > > > > @@ -0,0 +1,8 @@
> >> > > > > +What:  
> >> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> >> > > > > +Date:  November 2017
> >> > > > > +KernelVersion:  4.15
> >> > > > > +Contact:   Wu Hao 
> >> > > > > +Description:   Read-only. It returns interface id of partial 
> >> > > > > reconfiguration
> >> > > > > +   hardware. Userspace could use this information to 
> >> > > > > check if
> >> > > > > +   current hardware is compatible with given image 
> >> > > > > before FPGA
> >> > > > > +   programming.
> >> > > >
> >> > > > I'm a little confused by this.  I can understand that the PR 
> >> > > > bitstream
> >> > > > has a dependency on the FPGA's static image, but I don't understand
> >> > > > the dependency of the bistream on the hardware that is used to 
> >> > > > program
> >> > > > the bitstream to the FPGA.
> >> > >
> >> > > Sorry for the confusion, the interface_id is used to indicate the 
> >> > > version of
> >> > > the hardware for partial reconfiguration (it's part of the static 
> >> > > image of
> >> > > the FPGA device). Will improve the description on this.
> >>
> >> I'm not sure userland should be making the call on whether what you're
> >> trying to load is compatible or not.
> 
> Could you explain more about what your concern was about this (unless
> Hao has covered it below)?
> 
> It makes sense to me in this use case at least since userspace has a
> pile of images and is choosing which one to load.
> 
> >> Isn't there a way to check this in
> >> your PR reconfiguration handler in-kernel?
> >
> > Hi Moritz
> >
> > Actually with current driver interface, doing a partial reconfiguration 
> > with an
> > incompatible image, then driver will report PR failure with error code
> > FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR as hardware checks it, but anyway 
> > user
> > needs to know hardware interface_id information to find or re-generated 
> > correct
> > images. I think it's more flexible to leave it to userspace on using this
> > information exposed by driver. : )
> >
> > Thanks
> > Hao
> >
> >>
> >> > >
> >> >
> >> > The interface_id expresses the compatibility of the static region with PR
> >> > bitstreams generated for it. It changes every time a new static region is
> >> > generated.
> 
> In the near future the DFL framework will be used with SoC's that have
> a hard FPGA PR manager (that's not part of the static region).  The
> hard FPGA manager driver won't know anything about the static region.
> 
> >> >
> >> > Would it make more sense to have the interface_id exposed as part of the 
> >> > FME
> >> > device (which represents the static region)? I'm not su

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Wu Hao
On Mon, Feb 05, 2018 at 10:36:45AM -0800, Luebbers, Enno wrote:
> Hi Hao,
> 
> On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
> > On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> > > Hi Hao, Alan,
> > > 
> > > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> > > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> > > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> > > > > 
> > > > > Hi Hao,
> > > > > 
> > > > > A few comments below.   Besides that, looks good.
> > > > > 
> > > > > > This patch adds fpga manager driver for FPGA Management Engine 
> > > > > > (FME). It
> > > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
> > > > > > function.
> > > > > >
> > > > > > 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 
> > > > > > 
> > > > > > v3: rename driver to dfl-fpga-fme-mgr
> > > > > > implemented status callback for fpga manager
> > > > > > rebased due to fpga api changes
> > > > > > ---
> > > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> > > > > >  drivers/fpga/Kconfig   |   6 +
> > > > > >  drivers/fpga/Makefile  |   1 +
> > > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > > > > > +
> > > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> > > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> > > > > >  create mode 100644 
> > > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> > > > > >
> > > > > > diff --git 
> > > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > > new file mode 100644
> > > > > > index 000..2d4f917
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > > @@ -0,0 +1,8 @@
> > > > > > +What:  
> > > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > > > > > +Date:  November 2017
> > > > > > +KernelVersion:  4.15
> > > > > > +Contact:   Wu Hao 
> > > > > > +Description:   Read-only. It returns interface id of partial 
> > > > > > reconfiguration
> > > > > > +   hardware. Userspace could use this information to 
> > > > > > check if
> > > > > > +   current hardware is compatible with given image 
> > > > > > before FPGA
> > > > > > +   programming.
> > > > > 
> > > > > I'm a little confused by this.  I can understand that the PR bitstream
> > > > > has a dependency on the FPGA's static image, but I don't understand
> > > > > the dependency of the bistream on the hardware that is used to program
> > > > > the bitstream to the FPGA.
> > > > 
> > > > Sorry for the confusion, the interface_id is used to indicate the 
> > > > version of
> > > > the hardware for partial reconfiguration (it's part of the static image 
> > > > of
> > > > the FPGA device). Will improve the description on this.
> > > > 
> > > 
> > > The interface_id expresses the compatibility of the static region with PR
> > > bitstreams generated for it. It changes every time a new static region is
> > > generated.
> > > 
> > > Would it make more sense to have the interface_id exposed as part of the 
> > > FME
> > > device (which represents the static region)? I'm not sure - it kind of 
> > > also
> > > makes sense here, where you would have all the information in one place 
> > > (if the
> > > interface_id matches, I can use this component to program a bitstream).
> > 
> > Hi Enno
> > 
> > Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 is
> > under fpga-dfl-fme.0. It's part of the FME device for sure. From another
> > point of view, it means if anyone wants to do PR on this Intel FPGA device,
> > he needs to find the FME device firstly, and then check if any fpga manager
> > created under this FME device, if yes, check the interface_id before PR via
> > the FME device node ioctl.
> 
> That sounds good, thank you!
> 
> > 
> > > 
> > > Sorry for my limited understanding of the infrastructure - would this same
> > > "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In 
> > > that case
> > > it would need to expose multiple interface_ids (or we'd have to track both
> > > interface IDs and an identifier for the target PR region).
> > 
> > Yes, the fpga manager could be shared with different PR regions.
> > 
> > Sorry, I'm not sure where we need to expose multiple interface_ids and why.
> 
> It's basically a question of how to determine bitstream compatibility - 
> either,
> there's a separate interface_id per reconfigurable region, or t

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Luebbers, Enno
Hi Hao,

On Sun, Feb 04, 2018 at 05:37:06PM +0800, Wu Hao wrote:
> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> > Hi Hao, Alan,
> > 
> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> > > > 
> > > > Hi Hao,
> > > > 
> > > > A few comments below.   Besides that, looks good.
> > > > 
> > > > > This patch adds fpga manager driver for FPGA Management Engine (FME). 
> > > > > It
> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> > > > >
> > > > > 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 
> > > > > 
> > > > > v3: rename driver to dfl-fpga-fme-mgr
> > > > > implemented status callback for fpga manager
> > > > > rebased due to fpga api changes
> > > > > ---
> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> > > > >  drivers/fpga/Kconfig   |   6 +
> > > > >  drivers/fpga/Makefile  |   1 +
> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > > > > +
> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 
> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> > > > >
> > > > > diff --git 
> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > new file mode 100644
> > > > > index 000..2d4f917
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > @@ -0,0 +1,8 @@
> > > > > +What:  
> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > > > > +Date:  November 2017
> > > > > +KernelVersion:  4.15
> > > > > +Contact:   Wu Hao 
> > > > > +Description:   Read-only. It returns interface id of partial 
> > > > > reconfiguration
> > > > > +   hardware. Userspace could use this information to 
> > > > > check if
> > > > > +   current hardware is compatible with given image 
> > > > > before FPGA
> > > > > +   programming.
> > > > 
> > > > I'm a little confused by this.  I can understand that the PR bitstream
> > > > has a dependency on the FPGA's static image, but I don't understand
> > > > the dependency of the bistream on the hardware that is used to program
> > > > the bitstream to the FPGA.
> > > 
> > > Sorry for the confusion, the interface_id is used to indicate the version 
> > > of
> > > the hardware for partial reconfiguration (it's part of the static image of
> > > the FPGA device). Will improve the description on this.
> > > 
> > 
> > The interface_id expresses the compatibility of the static region with PR
> > bitstreams generated for it. It changes every time a new static region is
> > generated.
> > 
> > Would it make more sense to have the interface_id exposed as part of the FME
> > device (which represents the static region)? I'm not sure - it kind of also
> > makes sense here, where you would have all the information in one place (if 
> > the
> > interface_id matches, I can use this component to program a bitstream).
> 
> Hi Enno
> 
> Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 is
> under fpga-dfl-fme.0. It's part of the FME device for sure. From another
> point of view, it means if anyone wants to do PR on this Intel FPGA device,
> he needs to find the FME device firstly, and then check if any fpga manager
> created under this FME device, if yes, check the interface_id before PR via
> the FME device node ioctl.

That sounds good, thank you!

> 
> > 
> > Sorry for my limited understanding of the infrastructure - would this same
> > "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that 
> > case
> > it would need to expose multiple interface_ids (or we'd have to track both
> > interface IDs and an identifier for the target PR region).
> 
> Yes, the fpga manager could be shared with different PR regions.
> 
> Sorry, I'm not sure where we need to expose multiple interface_ids and why.

It's basically a question of how to determine bitstream compatibility - either,
there's a separate interface_id per reconfigurable region, or there is a single
interface_id for the entire device. Both make sense from a certain perspective.

If there are multiple interface_ids per device (one per region), the driver
would need to expose all of them. If there's only a single one, the driver only
exposes that one ID - compatibility would be determined by lookin

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-05 Thread Alan Tull
On Sun, Feb 4, 2018 at 4:05 AM, Wu Hao  wrote:
> On Sat, Feb 03, 2018 at 11:41:24AM +0100, Moritz Fischer wrote:
>> Hi Hao,
>>
>> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
>> > Hi Hao, Alan,
>> >
>> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
>> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
>> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
>> > > >
>> > > > Hi Hao,
>> > > >
>> > > > A few comments below.   Besides that, looks good.
>> > > >
>> > > > > This patch adds fpga manager driver for FPGA Management Engine 
>> > > > > (FME). It
>> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration 
>> > > > > function.
>> > > > >
>> > > > > 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 
>> > > > > 
>> > > > > v3: rename driver to dfl-fpga-fme-mgr
>> > > > > implemented status callback for fpga manager
>> > > > > rebased due to fpga api changes
>> > > > > ---
>> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
>> > > > >  drivers/fpga/Kconfig   |   6 +
>> > > > >  drivers/fpga/Makefile  |   1 +
>> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
>> > > > > +
>> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
>> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
>> > > > >  create mode 100644 
>> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
>> > > > >
>> > > > > diff --git 
>> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
>> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> > > > > new file mode 100644
>> > > > > index 000..2d4f917
>> > > > > --- /dev/null
>> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>> > > > > @@ -0,0 +1,8 @@
>> > > > > +What:  
>> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
>> > > > > +Date:  November 2017
>> > > > > +KernelVersion:  4.15
>> > > > > +Contact:   Wu Hao 
>> > > > > +Description:   Read-only. It returns interface id of partial 
>> > > > > reconfiguration
>> > > > > +   hardware. Userspace could use this information to 
>> > > > > check if
>> > > > > +   current hardware is compatible with given image 
>> > > > > before FPGA
>> > > > > +   programming.
>> > > >
>> > > > I'm a little confused by this.  I can understand that the PR bitstream
>> > > > has a dependency on the FPGA's static image, but I don't understand
>> > > > the dependency of the bistream on the hardware that is used to program
>> > > > the bitstream to the FPGA.
>> > >
>> > > Sorry for the confusion, the interface_id is used to indicate the 
>> > > version of
>> > > the hardware for partial reconfiguration (it's part of the static image 
>> > > of
>> > > the FPGA device). Will improve the description on this.
>>
>> I'm not sure userland should be making the call on whether what you're
>> trying to load is compatible or not.

Could you explain more about what your concern was about this (unless
Hao has covered it below)?

It makes sense to me in this use case at least since userspace has a
pile of images and is choosing which one to load.

>> Isn't there a way to check this in
>> your PR reconfiguration handler in-kernel?
>
> Hi Moritz
>
> Actually with current driver interface, doing a partial reconfiguration with 
> an
> incompatible image, then driver will report PR failure with error code
> FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR as hardware checks it, but anyway user
> needs to know hardware interface_id information to find or re-generated 
> correct
> images. I think it's more flexible to leave it to userspace on using this
> information exposed by driver. : )
>
> Thanks
> Hao
>
>>
>> > >
>> >
>> > The interface_id expresses the compatibility of the static region with PR
>> > bitstreams generated for it. It changes every time a new static region is
>> > generated.

In the near future the DFL framework will be used with SoC's that have
a hard FPGA PR manager (that's not part of the static region).  The
hard FPGA manager driver won't know anything about the static region.

>> >
>> > Would it make more sense to have the interface_id exposed as part of the 
>> > FME
>> > device (which represents the static region)? I'm not sure - it kind of also
>> > makes sense here, where you would have all the information in one place 
>> > (if the
>> > interface_id matches, I can use this component to program a bitstream).

According to the intel-fpga.txt document, the identifier for the
static image is at

/sys/class/fpga_region/regionX/fpga-dfl-fme.n

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-04 Thread Wu Hao
On Sat, Feb 03, 2018 at 11:41:24AM +0100, Moritz Fischer wrote:
> Hi Hao,
> 
> On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> > Hi Hao, Alan,
> > 
> > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> > > > 
> > > > Hi Hao,
> > > > 
> > > > A few comments below.   Besides that, looks good.
> > > > 
> > > > > This patch adds fpga manager driver for FPGA Management Engine (FME). 
> > > > > It
> > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> > > > >
> > > > > 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 
> > > > > 
> > > > > v3: rename driver to dfl-fpga-fme-mgr
> > > > > implemented status callback for fpga manager
> > > > > rebased due to fpga api changes
> > > > > ---
> > > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> > > > >  drivers/fpga/Kconfig   |   6 +
> > > > >  drivers/fpga/Makefile  |   1 +
> > > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > > > > +
> > > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> > > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 
> > > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> > > > >
> > > > > diff --git 
> > > > > a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > new file mode 100644
> > > > > index 000..2d4f917
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > > @@ -0,0 +1,8 @@
> > > > > +What:  
> > > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > > > > +Date:  November 2017
> > > > > +KernelVersion:  4.15
> > > > > +Contact:   Wu Hao 
> > > > > +Description:   Read-only. It returns interface id of partial 
> > > > > reconfiguration
> > > > > +   hardware. Userspace could use this information to 
> > > > > check if
> > > > > +   current hardware is compatible with given image 
> > > > > before FPGA
> > > > > +   programming.
> > > > 
> > > > I'm a little confused by this.  I can understand that the PR bitstream
> > > > has a dependency on the FPGA's static image, but I don't understand
> > > > the dependency of the bistream on the hardware that is used to program
> > > > the bitstream to the FPGA.
> > > 
> > > Sorry for the confusion, the interface_id is used to indicate the version 
> > > of
> > > the hardware for partial reconfiguration (it's part of the static image of
> > > the FPGA device). Will improve the description on this.
> 
> I'm not sure userland should be making the call on whether what you're
> trying to load is compatible or not. Isn't there a way to check this in
> your PR reconfiguration handler in-kernel?

Hi Moritz

Actually with current driver interface, doing a partial reconfiguration with an
incompatible image, then driver will report PR failure with error code
FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR as hardware checks it, but anyway user
needs to know hardware interface_id information to find or re-generated correct
images. I think it's more flexible to leave it to userspace on using this
information exposed by driver. : )

Thanks
Hao

> 
> > > 
> > 
> > The interface_id expresses the compatibility of the static region with PR
> > bitstreams generated for it. It changes every time a new static region is
> > generated.
> > 
> > Would it make more sense to have the interface_id exposed as part of the FME
> > device (which represents the static region)? I'm not sure - it kind of also
> > makes sense here, where you would have all the information in one place (if 
> > the
> > interface_id matches, I can use this component to program a bitstream).
> > 
> > Sorry for my limited understanding of the infrastructure - would this same
> > "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that 
> > case
> > it would need to expose multiple interface_ids (or we'd have to track both
> > interface IDs and an identifier for the target PR region).
> > 
> > > > 
> > > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > > index 57da904..0171ecb 100644
> > > > > --- a/drivers/fpga/Kconfig
> > > > > +++ b/drivers/fpga/Kconfig
> > > > > @@ -150,6 +150,12 @@ config FPGA_DFL_FME
> > > > >   FPGA platform level management features. There shall be 1 
> > > > > FME
> > > > >   per DFL based FPGA device.
> > > > >
> > > > > +config FPGA_DFL_FME

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-04 Thread Wu Hao
On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> Hi Hao, Alan,
> 
> On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> > > 
> > > Hi Hao,
> > > 
> > > A few comments below.   Besides that, looks good.
> > > 
> > > > This patch adds fpga manager driver for FPGA Management Engine (FME). It
> > > > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> > > >
> > > > 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 
> > > > 
> > > > v3: rename driver to dfl-fpga-fme-mgr
> > > > implemented status callback for fpga manager
> > > > rebased due to fpga api changes
> > > > ---
> > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> > > >  drivers/fpga/Kconfig   |   6 +
> > > >  drivers/fpga/Makefile  |   1 +
> > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > > > +
> > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> > > >  create mode 100644 
> > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > new file mode 100644
> > > > index 000..2d4f917
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > @@ -0,0 +1,8 @@
> > > > +What:  
> > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > > > +Date:  November 2017
> > > > +KernelVersion:  4.15
> > > > +Contact:   Wu Hao 
> > > > +Description:   Read-only. It returns interface id of partial 
> > > > reconfiguration
> > > > +   hardware. Userspace could use this information to check 
> > > > if
> > > > +   current hardware is compatible with given image before 
> > > > FPGA
> > > > +   programming.
> > > 
> > > I'm a little confused by this.  I can understand that the PR bitstream
> > > has a dependency on the FPGA's static image, but I don't understand
> > > the dependency of the bistream on the hardware that is used to program
> > > the bitstream to the FPGA.
> > 
> > Sorry for the confusion, the interface_id is used to indicate the version of
> > the hardware for partial reconfiguration (it's part of the static image of
> > the FPGA device). Will improve the description on this.
> > 
> 
> The interface_id expresses the compatibility of the static region with PR
> bitstreams generated for it. It changes every time a new static region is
> generated.
> 
> Would it make more sense to have the interface_id exposed as part of the FME
> device (which represents the static region)? I'm not sure - it kind of also
> makes sense here, where you would have all the information in one place (if 
> the
> interface_id matches, I can use this component to program a bitstream).

Hi Enno

Yes, this interface is under fpga-dfl-fme-mgr.0, and fpga-dfl-fme-mgr.0 is
under fpga-dfl-fme.0. It's part of the FME device for sure. From another
point of view, it means if anyone wants to do PR on this Intel FPGA device,
he needs to find the FME device firstly, and then check if any fpga manager
created under this FME device, if yes, check the interface_id before PR via
the FME device node ioctl.

> 
> Sorry for my limited understanding of the infrastructure - would this same
> "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that 
> case
> it would need to expose multiple interface_ids (or we'd have to track both
> interface IDs and an identifier for the target PR region).

Yes, the fpga manager could be shared with different PR regions.

Sorry, I'm not sure where we need to expose multiple interface_ids and why.

Thanks
Hao

> 
> > > 
> > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > index 57da904..0171ecb 100644
> > > > --- a/drivers/fpga/Kconfig
> > > > +++ b/drivers/fpga/Kconfig
> > > > @@ -150,6 +150,12 @@ config FPGA_DFL_FME
> > > >   FPGA platform level management features. There shall be 1 FME
> > > >   per DFL based FPGA device.
> > > >
> > > > +config FPGA_DFL_FME_MGR
> > > > +   tristate "FPGA DFL FME Manager Driver"
> > > > +   depends on FPGA_DFL_FME
> > > > +   help
> > > > + Say Y to enable FPGA Manager driver for FPGA Management 
> > > > Engine.
> > > > +
> > > >  config INTEL_FPGA_DFL_PCI
> > > > tristate "Intel FPGA DFL PCIe Device Driver"
> > > > depends on PCI && FPGA_DFL
> > > > diff 

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-03 Thread Moritz Fischer
Hi Hao,

On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote:
> Hi Hao, Alan,
> 
> On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> > > 
> > > Hi Hao,
> > > 
> > > A few comments below.   Besides that, looks good.
> > > 
> > > > This patch adds fpga manager driver for FPGA Management Engine (FME). It
> > > > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> > > >
> > > > 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 
> > > > 
> > > > v3: rename driver to dfl-fpga-fme-mgr
> > > > implemented status callback for fpga manager
> > > > rebased due to fpga api changes
> > > > ---
> > > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> > > >  drivers/fpga/Kconfig   |   6 +
> > > >  drivers/fpga/Makefile  |   1 +
> > > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > > > +
> > > >  drivers/fpga/fpga-dfl.h|  39 ++-
> > > >  5 files changed, 371 insertions(+), 1 deletion(-)
> > > >  create mode 100644 
> > > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > new file mode 100644
> > > > index 000..2d4f917
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > > @@ -0,0 +1,8 @@
> > > > +What:  
> > > > /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > > > +Date:  November 2017
> > > > +KernelVersion:  4.15
> > > > +Contact:   Wu Hao 
> > > > +Description:   Read-only. It returns interface id of partial 
> > > > reconfiguration
> > > > +   hardware. Userspace could use this information to check 
> > > > if
> > > > +   current hardware is compatible with given image before 
> > > > FPGA
> > > > +   programming.
> > > 
> > > I'm a little confused by this.  I can understand that the PR bitstream
> > > has a dependency on the FPGA's static image, but I don't understand
> > > the dependency of the bistream on the hardware that is used to program
> > > the bitstream to the FPGA.
> > 
> > Sorry for the confusion, the interface_id is used to indicate the version of
> > the hardware for partial reconfiguration (it's part of the static image of
> > the FPGA device). Will improve the description on this.

I'm not sure userland should be making the call on whether what you're
trying to load is compatible or not. Isn't there a way to check this in
your PR reconfiguration handler in-kernel?

> > 
> 
> The interface_id expresses the compatibility of the static region with PR
> bitstreams generated for it. It changes every time a new static region is
> generated.
> 
> Would it make more sense to have the interface_id exposed as part of the FME
> device (which represents the static region)? I'm not sure - it kind of also
> makes sense here, where you would have all the information in one place (if 
> the
> interface_id matches, I can use this component to program a bitstream).
> 
> Sorry for my limited understanding of the infrastructure - would this same
> "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that 
> case
> it would need to expose multiple interface_ids (or we'd have to track both
> interface IDs and an identifier for the target PR region).
> 
> > > 
> > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > index 57da904..0171ecb 100644
> > > > --- a/drivers/fpga/Kconfig
> > > > +++ b/drivers/fpga/Kconfig
> > > > @@ -150,6 +150,12 @@ config FPGA_DFL_FME
> > > >   FPGA platform level management features. There shall be 1 FME
> > > >   per DFL based FPGA device.
> > > >
> > > > +config FPGA_DFL_FME_MGR
> > > > +   tristate "FPGA DFL FME Manager Driver"
> > > > +   depends on FPGA_DFL_FME
> > > > +   help
> > > > + Say Y to enable FPGA Manager driver for FPGA Management 
> > > > Engine.
> > > > +
> > > >  config INTEL_FPGA_DFL_PCI
> > > > tristate "Intel FPGA DFL PCIe Device Driver"
> > > > depends on PCI && FPGA_DFL
> > > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > > index cc75bb3..6378580 100644
> > > > --- a/drivers/fpga/Makefile
> > > > +++ b/drivers/fpga/Makefile
> > > > @@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION)  += 
> > > > of-fpga-region.o
> > > >  # FPGA Device Feature List Support
> > > >  obj-$(CONFIG_FPGA_DFL) += fpga-dfl.o
> > > >  o

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-02 Thread Luebbers, Enno
Hi Hao, Alan,

On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote:
> On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> > 
> > Hi Hao,
> > 
> > A few comments below.   Besides that, looks good.
> > 
> > > This patch adds fpga manager driver for FPGA Management Engine (FME). It
> > > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> > >
> > > 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 
> > > 
> > > v3: rename driver to dfl-fpga-fme-mgr
> > > implemented status callback for fpga manager
> > > rebased due to fpga api changes
> > > ---
> > >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> > >  drivers/fpga/Kconfig   |   6 +
> > >  drivers/fpga/Makefile  |   1 +
> > >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > > +
> > >  drivers/fpga/fpga-dfl.h|  39 ++-
> > >  5 files changed, 371 insertions(+), 1 deletion(-)
> > >  create mode 100644 
> > > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > new file mode 100644
> > > index 000..2d4f917
> > > --- /dev/null
> > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > > @@ -0,0 +1,8 @@
> > > +What:  /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > > +Date:  November 2017
> > > +KernelVersion:  4.15
> > > +Contact:   Wu Hao 
> > > +Description:   Read-only. It returns interface id of partial 
> > > reconfiguration
> > > +   hardware. Userspace could use this information to check if
> > > +   current hardware is compatible with given image before 
> > > FPGA
> > > +   programming.
> > 
> > I'm a little confused by this.  I can understand that the PR bitstream
> > has a dependency on the FPGA's static image, but I don't understand
> > the dependency of the bistream on the hardware that is used to program
> > the bitstream to the FPGA.
> 
> Sorry for the confusion, the interface_id is used to indicate the version of
> the hardware for partial reconfiguration (it's part of the static image of
> the FPGA device). Will improve the description on this.
> 

The interface_id expresses the compatibility of the static region with PR
bitstreams generated for it. It changes every time a new static region is
generated.

Would it make more sense to have the interface_id exposed as part of the FME
device (which represents the static region)? I'm not sure - it kind of also
makes sense here, where you would have all the information in one place (if the
interface_id matches, I can use this component to program a bitstream).

Sorry for my limited understanding of the infrastructure - would this same
"fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In that case
it would need to expose multiple interface_ids (or we'd have to track both
interface IDs and an identifier for the target PR region).

> > 
> > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > index 57da904..0171ecb 100644
> > > --- a/drivers/fpga/Kconfig
> > > +++ b/drivers/fpga/Kconfig
> > > @@ -150,6 +150,12 @@ config FPGA_DFL_FME
> > >   FPGA platform level management features. There shall be 1 FME
> > >   per DFL based FPGA device.
> > >
> > > +config FPGA_DFL_FME_MGR
> > > +   tristate "FPGA DFL FME Manager Driver"
> > > +   depends on FPGA_DFL_FME
> > > +   help
> > > + Say Y to enable FPGA Manager driver for FPGA Management Engine.
> > > +
> > >  config INTEL_FPGA_DFL_PCI
> > > tristate "Intel FPGA DFL PCIe Device Driver"
> > > depends on PCI && FPGA_DFL
> > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > index cc75bb3..6378580 100644
> > > --- a/drivers/fpga/Makefile
> > > +++ b/drivers/fpga/Makefile
> > > @@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION)  += 
> > > of-fpga-region.o
> > >  # FPGA Device Feature List Support
> > >  obj-$(CONFIG_FPGA_DFL) += fpga-dfl.o
> > >  obj-$(CONFIG_FPGA_DFL_FME) += fpga-dfl-fme.o
> > > +obj-$(CONFIG_FPGA_DFL_FME_MGR) += fpga-dfl-fme-mgr.o
> > >
> > >  fpga-dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
> > >
> > > diff --git a/drivers/fpga/fpga-dfl-fme-mgr.c 
> > > b/drivers/fpga/fpga-dfl-fme-mgr.c
> > > new file mode 100644
> > > index 000..70356ce
> > > --- /dev/null
> > > +++ b/drivers/fpga/fpga-dfl-fme-mgr.c
> > > @@ -0,0 +1,318 @@
> > > +/*
> > > + * FPGA Manager Driver for FPGA Management Engine (FME)
> >

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-02 Thread Wu Hao
On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote:
> On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:
> 
> Hi Hao,
> 
> A few comments below.   Besides that, looks good.
> 
> > This patch adds fpga manager driver for FPGA Management Engine (FME). It
> > implements fpga_manager_ops for FPGA Partial Reconfiguration function.
> >
> > 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 
> > 
> > v3: rename driver to dfl-fpga-fme-mgr
> > implemented status callback for fpga manager
> > rebased due to fpga api changes
> > ---
> >  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
> >  drivers/fpga/Kconfig   |   6 +
> >  drivers/fpga/Makefile  |   1 +
> >  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> > +
> >  drivers/fpga/fpga-dfl.h|  39 ++-
> >  5 files changed, 371 insertions(+), 1 deletion(-)
> >  create mode 100644 
> > Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> >  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> > b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > new file mode 100644
> > index 000..2d4f917
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> > @@ -0,0 +1,8 @@
> > +What:  /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> > +Date:  November 2017
> > +KernelVersion:  4.15
> > +Contact:   Wu Hao 
> > +Description:   Read-only. It returns interface id of partial 
> > reconfiguration
> > +   hardware. Userspace could use this information to check if
> > +   current hardware is compatible with given image before FPGA
> > +   programming.
> 
> I'm a little confused by this.  I can understand that the PR bitstream
> has a dependency on the FPGA's static image, but I don't understand
> the dependency of the bistream on the hardware that is used to program
> the bitstream to the FPGA.

Sorry for the confusion, the interface_id is used to indicate the version of
the hardware for partial reconfiguration (it's part of the static image of
the FPGA device). Will improve the description on this.

> 
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index 57da904..0171ecb 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -150,6 +150,12 @@ config FPGA_DFL_FME
> >   FPGA platform level management features. There shall be 1 FME
> >   per DFL based FPGA device.
> >
> > +config FPGA_DFL_FME_MGR
> > +   tristate "FPGA DFL FME Manager Driver"
> > +   depends on FPGA_DFL_FME
> > +   help
> > + Say Y to enable FPGA Manager driver for FPGA Management Engine.
> > +
> >  config INTEL_FPGA_DFL_PCI
> > tristate "Intel FPGA DFL PCIe Device Driver"
> > depends on PCI && FPGA_DFL
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index cc75bb3..6378580 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION)  += of-fpga-region.o
> >  # FPGA Device Feature List Support
> >  obj-$(CONFIG_FPGA_DFL) += fpga-dfl.o
> >  obj-$(CONFIG_FPGA_DFL_FME) += fpga-dfl-fme.o
> > +obj-$(CONFIG_FPGA_DFL_FME_MGR) += fpga-dfl-fme-mgr.o
> >
> >  fpga-dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
> >
> > diff --git a/drivers/fpga/fpga-dfl-fme-mgr.c 
> > b/drivers/fpga/fpga-dfl-fme-mgr.c
> > new file mode 100644
> > index 000..70356ce
> > --- /dev/null
> > +++ b/drivers/fpga/fpga-dfl-fme-mgr.c
> > @@ -0,0 +1,318 @@
> > +/*
> > + * FPGA Manager Driver for FPGA Management Engine (FME)
> > + *
> > + * Copyright (C) 2017 Intel Corporation, Inc.
> > + *
> > + * Authors:
> > + *   Kang Luwei 
> > + *   Xiao Guangrong 
> > + *   Wu Hao 
> > + *   Joseph Grecco 
> > + *   Enno Luebbers 
> > + *   Tim Whisonant 
> > + *   Ananda Ravuri 
> > + *   Christopher Rauer 
> > + *   Henry Mitchel 
> > + *
> > + * This work is licensed under the terms of the GNU GPL version 2.
> > + * SPDX-License-Identifier: GPL-2.0
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "fpga-dfl.h"
> > +#include "dfl-fme.h"
> > +
> > +#define PR_WAIT_TIMEOUT   800
> > +#define PR_HOST_STATUS_IDLE0
> > +
> > +struct fme_mgr_priv {
> > +   void __iomem *ioaddr;
> > +   u64 pr_error;
> > +};
> > +
> > +static ssize_t interface_id_show(struct device *dev,
> > +struct device_attribute *attr, char *buf)
> > +{
> > +   struct fpga_manager *mgr = dev_get_drvdata(dev);
> > +   struct fme_mgr_priv *priv = mgr->priv;
> > +   u64 intfc_id_l, intfc_id_h;
> > +
> > +

Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2018-02-01 Thread Alan Tull
On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao  wrote:

Hi Hao,

A few comments below.   Besides that, looks good.

> This patch adds fpga manager driver for FPGA Management Engine (FME). It
> implements fpga_manager_ops for FPGA Partial Reconfiguration function.
>
> 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 
> 
> v3: rename driver to dfl-fpga-fme-mgr
> implemented status callback for fpga manager
> rebased due to fpga api changes
> ---
>  .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
>  drivers/fpga/Kconfig   |   6 +
>  drivers/fpga/Makefile  |   1 +
>  drivers/fpga/fpga-dfl-fme-mgr.c| 318 
> +
>  drivers/fpga/fpga-dfl.h|  39 ++-
>  5 files changed, 371 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
>  create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
> b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> new file mode 100644
> index 000..2d4f917
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
> @@ -0,0 +1,8 @@
> +What:  /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
> +Date:  November 2017
> +KernelVersion:  4.15
> +Contact:   Wu Hao 
> +Description:   Read-only. It returns interface id of partial reconfiguration
> +   hardware. Userspace could use this information to check if
> +   current hardware is compatible with given image before FPGA
> +   programming.

I'm a little confused by this.  I can understand that the PR bitstream
has a dependency on the FPGA's static image, but I don't understand
the dependency of the bistream on the hardware that is used to program
the bitstream to the FPGA.

> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 57da904..0171ecb 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -150,6 +150,12 @@ config FPGA_DFL_FME
>   FPGA platform level management features. There shall be 1 FME
>   per DFL based FPGA device.
>
> +config FPGA_DFL_FME_MGR
> +   tristate "FPGA DFL FME Manager Driver"
> +   depends on FPGA_DFL_FME
> +   help
> + Say Y to enable FPGA Manager driver for FPGA Management Engine.
> +
>  config INTEL_FPGA_DFL_PCI
> tristate "Intel FPGA DFL PCIe Device Driver"
> depends on PCI && FPGA_DFL
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index cc75bb3..6378580 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION)  += of-fpga-region.o
>  # FPGA Device Feature List Support
>  obj-$(CONFIG_FPGA_DFL) += fpga-dfl.o
>  obj-$(CONFIG_FPGA_DFL_FME) += fpga-dfl-fme.o
> +obj-$(CONFIG_FPGA_DFL_FME_MGR) += fpga-dfl-fme-mgr.o
>
>  fpga-dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
>
> diff --git a/drivers/fpga/fpga-dfl-fme-mgr.c b/drivers/fpga/fpga-dfl-fme-mgr.c
> new file mode 100644
> index 000..70356ce
> --- /dev/null
> +++ b/drivers/fpga/fpga-dfl-fme-mgr.c
> @@ -0,0 +1,318 @@
> +/*
> + * FPGA Manager Driver for FPGA Management Engine (FME)
> + *
> + * Copyright (C) 2017 Intel Corporation, Inc.
> + *
> + * Authors:
> + *   Kang Luwei 
> + *   Xiao Guangrong 
> + *   Wu Hao 
> + *   Joseph Grecco 
> + *   Enno Luebbers 
> + *   Tim Whisonant 
> + *   Ananda Ravuri 
> + *   Christopher Rauer 
> + *   Henry Mitchel 
> + *
> + * This work is licensed under the terms of the GNU GPL version 2.
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "fpga-dfl.h"
> +#include "dfl-fme.h"
> +
> +#define PR_WAIT_TIMEOUT   800
> +#define PR_HOST_STATUS_IDLE0
> +
> +struct fme_mgr_priv {
> +   void __iomem *ioaddr;
> +   u64 pr_error;
> +};
> +
> +static ssize_t interface_id_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> +   struct fpga_manager *mgr = dev_get_drvdata(dev);
> +   struct fme_mgr_priv *priv = mgr->priv;
> +   u64 intfc_id_l, intfc_id_h;
> +
> +   intfc_id_l = readq(priv->ioaddr + FME_PR_INTFC_ID_L);
> +   intfc_id_h = readq(priv->ioaddr + FME_PR_INTFC_ID_H);
> +
> +   return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n",
> +   (unsigned long long)intfc_id_h,
> +   (unsigned long long)intfc_id_l);
> +}
> +static DEVICE_ATTR_RO(interface_id);
> +
> +static const struct attribute *fme_mgr_attrs[] = {
> +   &dev_attr_interface_id.attr,
> +   NULL,
> +};
> +
> +static u64 pr_error_to_mgr_status(u64 err)
> +{
> +   u64 st

[PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME

2017-11-26 Thread Wu Hao
This patch adds fpga manager driver for FPGA Management Engine (FME). It
implements fpga_manager_ops for FPGA Partial Reconfiguration function.

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 

v3: rename driver to dfl-fpga-fme-mgr
implemented status callback for fpga manager
rebased due to fpga api changes
---
 .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr|   8 +
 drivers/fpga/Kconfig   |   6 +
 drivers/fpga/Makefile  |   1 +
 drivers/fpga/fpga-dfl-fme-mgr.c| 318 +
 drivers/fpga/fpga-dfl.h|  39 ++-
 5 files changed, 371 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
 create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c

diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr 
b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
new file mode 100644
index 000..2d4f917
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr
@@ -0,0 +1,8 @@
+What:  /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interface_id
+Date:  November 2017
+KernelVersion:  4.15
+Contact:   Wu Hao 
+Description:   Read-only. It returns interface id of partial reconfiguration
+   hardware. Userspace could use this information to check if
+   current hardware is compatible with given image before FPGA
+   programming.
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 57da904..0171ecb 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -150,6 +150,12 @@ config FPGA_DFL_FME
  FPGA platform level management features. There shall be 1 FME
  per DFL based FPGA device.
 
+config FPGA_DFL_FME_MGR
+   tristate "FPGA DFL FME Manager Driver"
+   depends on FPGA_DFL_FME
+   help
+ Say Y to enable FPGA Manager driver for FPGA Management Engine.
+
 config INTEL_FPGA_DFL_PCI
tristate "Intel FPGA DFL PCIe Device Driver"
depends on PCI && FPGA_DFL
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index cc75bb3..6378580 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION)  += of-fpga-region.o
 # FPGA Device Feature List Support
 obj-$(CONFIG_FPGA_DFL) += fpga-dfl.o
 obj-$(CONFIG_FPGA_DFL_FME) += fpga-dfl-fme.o
+obj-$(CONFIG_FPGA_DFL_FME_MGR) += fpga-dfl-fme-mgr.o
 
 fpga-dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o
 
diff --git a/drivers/fpga/fpga-dfl-fme-mgr.c b/drivers/fpga/fpga-dfl-fme-mgr.c
new file mode 100644
index 000..70356ce
--- /dev/null
+++ b/drivers/fpga/fpga-dfl-fme-mgr.c
@@ -0,0 +1,318 @@
+/*
+ * FPGA Manager Driver for FPGA Management Engine (FME)
+ *
+ * Copyright (C) 2017 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Kang Luwei 
+ *   Xiao Guangrong 
+ *   Wu Hao 
+ *   Joseph Grecco 
+ *   Enno Luebbers 
+ *   Tim Whisonant 
+ *   Ananda Ravuri 
+ *   Christopher Rauer 
+ *   Henry Mitchel 
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+
+#include "fpga-dfl.h"
+#include "dfl-fme.h"
+
+#define PR_WAIT_TIMEOUT   800
+#define PR_HOST_STATUS_IDLE0
+
+struct fme_mgr_priv {
+   void __iomem *ioaddr;
+   u64 pr_error;
+};
+
+static ssize_t interface_id_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+   struct fpga_manager *mgr = dev_get_drvdata(dev);
+   struct fme_mgr_priv *priv = mgr->priv;
+   u64 intfc_id_l, intfc_id_h;
+
+   intfc_id_l = readq(priv->ioaddr + FME_PR_INTFC_ID_L);
+   intfc_id_h = readq(priv->ioaddr + FME_PR_INTFC_ID_H);
+
+   return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n",
+   (unsigned long long)intfc_id_h,
+   (unsigned long long)intfc_id_l);
+}
+static DEVICE_ATTR_RO(interface_id);
+
+static const struct attribute *fme_mgr_attrs[] = {
+   &dev_attr_interface_id.attr,
+   NULL,
+};
+
+static u64 pr_error_to_mgr_status(u64 err)
+{
+   u64 status = 0;
+
+   if (err & FME_PR_ERR_OPERATION_ERR)
+   status |= FPGA_MGR_STATUS_OPERATION_ERR;
+   if (err & FME_PR_ERR_CRC_ERR)
+   status |= FPGA_MGR_STATUS_CRC_ERR;
+   if (err & FME_PR_ERR_INCOMPATIBLE_BS)
+   status |= FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR;
+   if (err & FME_PR_ERR_PROTOCOL_ERR)
+   status |= FPGA_MGR_STATUS_IP_PROTOCOL_ERR;
+   if (err & FME_PR_ERR_FIFO_OVERFLOW)
+   status |= FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR;
+
+   return status;
+}
+
+static u64 fme_mgr_pr_error_handle(void __iomem *fme_pr)
+