RE: [PATCH RFC] fpga: add FPGA Bus device framework

[Wu, Hao]

> On Mon, Aug 14, 2017 at 5:59 AM, Wu, Hao <hao.wu@xxxxxxxxx> wrote:
> >> On Tue, Aug 8, 2017 at 1:25 PM, Wu, Hao <hao.wu@xxxxxxxxx> wrote:
> >> >> On Thu, Aug 3, 2017 at 2:53 AM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> >> >> > On Wed, Aug 02, 2017 at 04:16:32PM -0500, Alan Tull wrote:
> >> >> >> On Wed, Aug 2, 2017 at 7:19 AM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
> >> >> >> > This patch is a RFC patch which replaces the patch[1] which
> >> >> >> > creates 'fpga-dev' class as container device. It introduces
> >> >> >> > a 'fpga' bus type, and provides interfaces to create/destroy
> >> >> >> > fpga bus devices. This fpga bus device only could be used as
> >> >> >> > a container device, and no drivers needed for it.
> >> >> >> >
> >> >> >> > There is no interface change, so this patch could be used
> >> >> >> > together with other patches of the original patch set[2].
> >> >> >> >
> >> >> >>
> >> >> >> I am wondering whether this could be added to fpga-bridge.c so that
> >> >> >> fpga-bridge becomes the fpga bus and fpga bus devices are under it.
> >> >> >> The reasons for doing this are discussed in the other thread.
> >> >> >>
> >> >> >> > This following APIs are provided by FPGA Bus device framework:
> >> >> >> > * fpga_dev_create
> >> >> >> >   Create fpga bus device under the given parent device.
> >> >> >> > * fpga_dev_destroy
> >> >> >> >   Destroy fpga bus device
> >> >> >>
> >> >> >> This is being used in such that each fpga-dev is a container for
> >> >> >> platform devices rather than fpga devices.  That's not what I was
> >> >> >> expecting. :)
> >> >> >
> >> >> > Hi Alan
> >> >> >
> >> >> > So does that mean in Intel FPGA PCIe driver, it needs to create
> >> >> > a fpga-bridge (as base bridge?),
> >> >>
> >> >> Yes
> >> >>
> >> >> > and this fpga-bridge should register
> >> >> > a fpga-bus and have a fpga bus device as its child, after that we can
> >> >> > use this fpga bus device as container device,
> >> >>
> >> >> Could the bus code be added to fpga-bridge?  Then the base bridge is
> >> >> the container device.  A fpga-region would be under that and the AFU
> >> >> and FME fpga devices would be under it.
> >> >>
> >> >> > and create sub feature
> >> >> > devices (e.g AFU and FME platform device)
> >> >>
> >> >> We're talking about adding a new bus to the kernel here, not platform bus.
> >> >>
> >> >> > under it, and user application
> >> >> > could locate it in /sys/bus/fpga/devices/. Is my understanding correct? :)
> >> >>
> >> >>  So sysfs may end up something like this in your case:
> >> >>
> >> >> /sys/bus/fpga/devices/fpga.0/intel-fpga-fme.0
> >> >> /sys/bus/fpga/devices/fpga.0/intel-fpga-port.0
> >> >> /sys/bus/fpga/devices/fpga.0/intel-fpga-port.1
> >> >> /sys/bus/fpga/devices/fpga.0/fpga-mgr0
> >> >> /sys/bus/fpga/devices/fpga.0/fpga-br1
> >> >> /sys/bus/fpga/devices/fpga.0/fpga-br2
> >> >
> >> > Hi Alan
> >> >
> >> > I am a little confused on this.
> >> >
> >> > It seems that we could not have multiple fpga-br/region/mgr under one
> device.
> >> > As in patch set2, intel-fpga-fme.0 creates platform devices as children, and
> >> register
> >> > fpga-bridges/regions/mgr under these children platform devices. This is
> why 3
> >> new
> >> > platform device driver introduced in this patch set 2 to match with those
> new
> >> created
> >> > children platform devices.
> >> >
> >> > So it is something like this
> >> > /sys/bus/fpga/devices/fpga.0/intel-fpga-fme.0/intel-fpga-fme-
> >> region.0/fpga_region/region0
> >> > /sys/bus/fpga/devices/fpga.0/intel-fpga-fme.0/intel-fpga-fme-
> >> region.1/fpga_region/region1
> >> > /sys/bus/fpga/devices/fpga.0/intel-fpga-fme.0/intel-fpga-fme-
> >> br.0/fpga_bridge/br1
> >> > /sys/bus/fpga/devices/fpga.0/intel-fpga-fme.0/intel-fpga-fme-
> >> br.1/fpga_region/br2
> >> > /sys/bus/fpga/devices/fpga.1
> >> > /sys/bus/fpga/devices/fpga.2
> >>
> >> Hi Hao,
> >>
> >> OK I see that now.  Because the regions, mgr, and bridges are all
> >> children of the fme's.
> >>
> >> >
> >> > br0 should be the base bridge. fpga.1 and 2 are the child fpga bus device of
> >> fpga bridge.
> >> >
> >> >>
> >> >> /sys/bus/fpga/devices/fpga.1/fpga-region1
> >> >> /sys/bus/fpga/devices/fpga.2/fpga-region2
> >> >>
> >> >> /sys/bus/fpga/devices/fpga.3/intel-fpga-fme.1
> >> >> /sys/bus/fpga/devices/fpga.3/intel-fpga-port.2
> >> >> /sys/bus/fpga/devices/fpga.3/intel-fpga-port.3
> >> >> /sys/bus/fpga/devices/fpga.0/fpga-mgr1
> >> >> /sys/bus/fpga/devices/fpga.3/fpga-br4
> >> >> /sys/bus/fpga/devices/fpga.3/fpga-br5
> >> >>
> >> >> /sys/bus/fpga/devices/fpga.4/fpga-region4
> >> >> /sys/bus/fpga/devices/fpga.5/fpga-region5
> >> >>
> >> >> fpga-br0 and 3 are base bridges (on top of PCIe) which show up as
> >> >> fpga.0 and 3.  Regions 0 and 3 are base regions.
> >> >>
> >> >> fpga.0 and fpga.3 correspond to the real FPGA devices.
> >> >>
> >> >> >
> >> >> > And if we have second level fpga-bridge for PR regions, then they
> >> >> > should register fpga-bus and fpga bus type device as child too?
> >> >> > If yes, then we need a method for user application to distinguish
> >> >> > which one represents the FPGA device in /sys/bus/fpga/devices/, right?
> >> >>
> >> >> To find a bus that is a fpga, userspace only needs to look for busses
> >> >> that have an FME (or a mgr).
> >> >
> >> > Do you mean that check all fpga-dev.x folder to see if anyone has FME?
> >> >
> >> > Then it is still not friendly to user space, as we may have a lot of bridges
> >> > (and regions) on one system.
> >>
> >> It doesn't sound too hard for userspace code to go through sysfs once
> >> and find the FME's.
> >>
> >> >
> >> > And looks like no big difference that we reuse base fpga-region as
> >> > container. Search all regionx in /sys/class/fpga_region/ to see if anyone
> >> > has a FME.
> >> >
> >> > How do you think? : )
> >>
> >> Yes, looking for
> >> /sys/class/fpga_region/region*/device/intel-fpga-fme.*  That's not so
> >> bad, right? :)
> >>
> >> Then the fpga-dev stuff can go away and we can stop worrying about all
> >> the issues involved in implementing a fpga bus or class.
> >
> > Hi Alan
> >
> > Thanks for your feedback.
> >
> > I think the only concern here is I'm not sure if we will have some fpga devices
> > with a large number fpga regions (e.g 100+) in the future. If there are many
> > regions in the system, then the enduser / application needs to search all the
> > regions one by one, which seems not perfect.
> 
> Well your library could do so, I suppose a clever libudev enumerate could work,
> too, I suppose.

Yes, agree, adding some code in user space library should be able to cover this. : )
Thanks for your suggestion.

Hao

> 
> Cheers,
> 
> Moritz