On Mon, Feb 5, 2018 at 8:17 PM, Wu Hao <hao.wu@xxxxxxxxx> 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 <hao.wu@xxxxxxxxx> 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 <hao.wu@xxxxxxxxx> 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 <tim.whisonant@xxxxxxxxx> >> >> > > > > Signed-off-by: Enno Luebbers <enno.luebbers@xxxxxxxxx> >> >> > > > > Signed-off-by: Shiva Rao <shiva.rao@xxxxxxxxx> >> >> > > > > Signed-off-by: Christopher Rauer <christopher.rauer@xxxxxxxxx> >> >> > > > > Signed-off-by: Kang Luwei <luwei.kang@xxxxxxxxx> >> >> > > > > Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx> >> >> > > > > Signed-off-by: Wu Hao <hao.wu@xxxxxxxxx> >> >> > > > > ---- >> >> > > > > 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 0000000..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 <hao.wu@xxxxxxxxx> >> >> > > > > +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/bitstream_id > > Hi Alan Hi Hao, > > This bitstream_id refects the full static region version. As you know, PR is > only a sub feature of the FME functional unit, it's possible that we have > different static region (different bitstream_id) but it has the exact same > PR sub feature under the FME, only changes on other sub features or function > units. OK, thanks for that explanation. That makes sense but could have easily been different. Please document this somewhere. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
