From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752691AbdHQMNu (ORCPT ); Thu, 17 Aug 2017 08:13:50 -0400 Received: from mga14.intel.com ([192.55.52.115]:23826 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751572AbdHQMNs (ORCPT ); Thu, 17 Aug 2017 08:13:48 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,387,1498546800"; d="scan'208";a="138682373" From: "Wu, Hao" To: Moritz Fischer CC: Alan Tull , Rob Herring , "linux-fpga@vger.kernel.org" , linux-kernel , "Kang, Luwei" , "Zhang, Yi Z" Subject: RE: [PATCH RFC] fpga: add FPGA Bus device framework Thread-Topic: [PATCH RFC] fpga: add FPGA Bus device framework Thread-Index: AQHTC4qo2+f/GmRZrkO7bnSnyVf00qJ5FuzcgAGodlCAAsYygIAGVvWg///FSYCABOqqQA== Date: Thu, 17 Aug 2017 12:12:29 +0000 Message-ID: References: <1501676385-32551-1-git-send-email-hao.wu@intel.com> <20170803075335.GA4120@hao-dev> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id v7HCDw1R003554 > On Mon, Aug 14, 2017 at 5:59 AM, Wu, Hao wrote: > >> On Tue, Aug 8, 2017 at 1:25 PM, Wu, Hao wrote: > >> >> On Thu, Aug 3, 2017 at 2:53 AM, Wu Hao 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 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