Re: [PATCH v3 08/21] fpga: add Intel FPGA DFL PCIe device

[Wu Hao]

On Thu, Feb 01, 2018 at 03:59:21PM -0600, Alan Tull wrote:
> On Tue, Dec 5, 2017 at 11:30 PM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> > On Tue, Dec 05, 2017 at 11:00:22AM -0600, Alan Tull wrote:
> >> On Mon, Dec 4, 2017 at 9:33 PM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> >> > On Mon, Dec 04, 2017 at 01:46:59PM -0600, Alan Tull wrote:
> >> >> On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> >> >> > On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
> >> >> >> From: Wu Hao
> >> >> >> > Sent: 27 November 2017 06:42
> >> >> >> > From: Zhang Yi <yi.z.zhang@xxxxxxxxx>
> >> >> >> >
> >> >> >> > The Intel FPGA device appears as a PCIe device on the system. This patch
> >> >> >> > implements the basic framework of the driver for Intel PCIe device which
> >> >> >> > is located between CPU and Accelerated Function Units (AFUs), and has
> >> >> >> > the Device Feature List (DFL) implemented in its MMIO space.
> >> >> >>
> >> >> >> This ought to have a better name than 'Intel FPGA'.
> >> >> >> An fpga can be used for all sorts of things, this looks like
> >> >> >> a very specific architecture using a common VHDL environment to
> >> >> >> allow certain types of user VHDL be accessed over PCIe.
> >> >> >
> >> >> > Hi David
> >> >> >
> >> >> > This patch adds a pcie device driver for Intel FPGA devices which implements
> >> >> > the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
> >> >> > Acceleration Cards. They are pcie devices, and all have DFL implemented in
> >> >> > the MMIO space, so we would like to use one kernel driver to handle them.
> >> >> >
> >> >> > With this full patchset, it just provides user the interfaces to configure
> >> >> > and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
> >> >> > users can develop and build their own logics via tools provided by Intel,
> >> >> > program them to accelerators on these Intel FPGA devices, and access them
> >> >> > for their workloads.
> >> >>
> >> >> I don't see anything Intel specific here.  This could all be named dfl-*
> >> >
> >> > The maybe some device specific things, e.g Intel FPGA devices supported by this
> >> > driver always have FME DFL at the beginning on the BAR0 for PF device.
> 
> I'm thinking that another user could add their PCI id's and a static
> FPGA image that has a DFL in the right place for this to work for
> them.
> 
> >> >
> >> > But I think this should be the right direction for better code reuse, it could
> >> > save efforts for other vendors who want to use DFL and follow the same way.
> 
> I appreciate your understanding here.
> 
> >> >
> >> > Thanks for the comments. I will rename this driver in the next version.
> >>
> >> Thanks!
> >>
> >> Regarding file names, it seems like the files added to drivers/fpga
> >> could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
> >> other are currently dfl-*.[ch] currently.
> >
> > Sure, will have all related drivers files renamed to dfl-*.[ch].
> 
> Actually, I'll reverse that a bit.  The enumeration code, including
> the pcie part is all sufficiently general to run on anything that has
> a DFL struct located in the right place.  But individual feature
> drivers (currently only the fme-mgr) will be vendor specific and could
> be named intel-*.

Hi Alan,

I unified all the drivers to use dfl-* in the v4 patchset, including fme-mgr.
As I feel it's hard to say which FME functions (sub features, registers) are
vendor specific and which FME functions are not, from the spec, they all belong
to FME, and people can be reused for sure. So I didn't rename it back to Intel
driver in the v4 patchset. :)

Thanks
Hao


> 
> Alan