From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751497AbdGZW3z (ORCPT ); Wed, 26 Jul 2017 18:29:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:59504 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751011AbdGZW3x (ORCPT ); Wed, 26 Jul 2017 18:29:53 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3250022CAE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=atull@kernel.org MIME-Version: 1.0 In-Reply-To: References: <1490875696-15145-1-git-send-email-hao.wu@intel.com> <1490875696-15145-3-git-send-email-hao.wu@intel.com> <20170331060909.GA7621@kroah.com> <20170331074842.GA17067@hao-dev> <20170401121804.GE4804@hao-dev> <20170726095029.GA26841@hao-dev> From: Alan Tull Date: Wed, 26 Jul 2017 17:29:11 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 02/16] fpga: add FPGA device framework To: Wu Hao Cc: matthew.gerlach@linux.intel.com, Greg KH , Moritz Fischer , linux-fpga@vger.kernel.org, linux-kernel , "Kang, Luwei" , "Zhang, Yi Z" , Tim Whisonant , Enno Luebbers , Shiva Rao , Christopher Rauer , Xiao Guangrong Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 26, 2017 at 9:20 AM, Alan Tull wrote: > On Wed, Jul 26, 2017 at 4:50 AM, Wu Hao wrote: >> On Tue, Jul 25, 2017 at 04:32:10PM -0500, Alan Tull wrote: >>> On Sat, Apr 1, 2017 at 7:18 AM, Wu Hao wrote: >>> >>> Hi Hao, >>> >>> > On Fri, Mar 31, 2017 at 12:01:13PM -0700, matthew.gerlach@linux.intel.com wrote: >>> >> On Fri, 31 Mar 2017, Wu Hao wrote: >>> >> >On Fri, Mar 31, 2017 at 08:09:09AM +0200, Greg KH wrote: >>> >> >>On Thu, Mar 30, 2017 at 08:08:02PM +0800, Wu Hao wrote: >>> >> >>>During FPGA device (e.g PCI-based) discovery, platform devices are >>> >> >>>registered for different FPGA function units. But the device node path >>> >> >>>isn't quite friendly to applications. >>> >> >>> >>> >> >>>Consider this case, applications want to access child device's sysfs file >>> >> >>>for some information. >>> >> >>> >>> >> >>>1) Access using bus-based path (e.g PCI) >>> >> >>> >>> >> >>> /sys/bus/pci/devices/xxxxx/fpga_func_a.0/sysfs_file >>> >> >>> >>> >> >>> From the path, it's clear which PCI device is the parent, but not perfect >>> >> >>> solution for applications. PCI device BDF is not fixed, application may >>> >> >>> need to search all PCI device to find the actual FPGA Device. >>> >> >>> >>> >> >>>2) Or access using platform device path >>> >> >>> >>> >> >>> /sys/bus/platform/devices/fpga_func_a.0/sysfs_file >>> >> >>> >>> >> >>> Applications find the actual function by name easily, but no information >>> >> >>> about which fpga device it belongs to. It's quite confusing if multiple >>> >> >>> FPGA devices are in one system. >>> >> >>> >>> >> >>>'FPGA Device' class is introduced to resolve this problem. Each node under >>> >> >>>this class represents a fpga device, which may have one or more child >>> >> >>>devices. Applications only need to search under this FPGA Device class >>> >> >>>folder to find the child device node it needs. >>> >> >>> >>> >> >>>For example, for the platform has 2 fpga devices, each fpga device has >>> >> >>>3 child devices, the hierarchy looks like this. >>> >> >>> >>> >> >>>Two nodes are under /sys/class/fpga/: >>> >> >>>/sys/class/fpga/fpga.0 >>> >> >>>/sys/class/fpga/fpga.1 >>> >> >>> >>> >> >>>Each node has 1 function A device and 2 function B devices: >>> >> >>>/sys/class/fpga/fpga.0/func_a.0 >>> >> >>>/sys/class/fpga/fpga.0/func_b.0 >>> >> >>>/sys/class/fpga/fpga.0/func_b.1 >>> >> >>> >>> >> >>>/sys/class/fpga/fpga.1/func_a.1 >>> >> >>>/sys/class/fpga/fpga.1/func_b.2 >>> >> >>>/sys/class/fpga/fpga.1/func_b.3 >>> >> >>> >>> >> >>>This following APIs are provided by FPGA device framework: >>> >> >>>* fpga_dev_create >>> >> >>> Create fpga device under the given parent device. >>> >> >>>* fpga_dev_destroy >>> >> >>> Destroy fpga device >>> >> >>> >>> >> >>>The following sysfs files are created: >>> >> >>>* /sys/class/fpga//name >>> >> >>> Name of the fpga device. >>> >> >> >>> >> >>How does this interact with the existing "fpga class" that is in the >>> >> >>kernel already? >>> >> > >>> >> >The fpga-dev introduced by this patch, is only a container device, and >>> >> >>> >> I completely understand the need for a container device. The fpga-region is >>> >> also primarily a container, and in some cases the fpga-region may represent >>> >> the entire fpga. Over time this code may become redundant. >>> > >>> > Thanks a lot for your review and comments. >>> > >>> > I feel that the fpga-region implies that it supports reconfiguration, >>> >>> On Arria10, we create base fpga region which does not support full >>> reconfiguration. It corresponds to the whole FPGA area, which was >>> loaded with a static FPGA image in the bootloader. The partial >>> reconfiguration regions are children of the base FPGA region. Any >>> devices in the FPGA are child devices of either the base region or a >>> region which is a child of it. >>> >>> > but >>> > in our cases, the Intel FPGA device, doesn't have base fpga-region for >>> > full reconfiguration, but many accelerators with partial reconfiguration >>> > support. A fpga-region brings together everything needed for the >>> > reconfiguration, and a fpga-dev is trying to brings everything on a FPGA >>> > device together, including fpga-region/bridge/manager, access different >>> > accelerators and other function units. >>> > >>> > I think it's not mandatory to use fpga-dev, as fpga-dev is just trying to >>> > provide one more option here for some complex hardware. >>> >>> Now that you've put out v2 which uses fpga-regions, do you still need >>> fpga-dev class? >> >> Hi Alan >> >> Thanks for the comments. >> >> In v2, I have updated the driver organization section in intel-fpga.txt[1]. > > > I've read your v2 of this document. It's changed as you've said, but > not that much. I should clarify here that, yes I see that in v2 you're now using regions and bridges and I appreciate that. I'm just trying to see what a good relationship between the existing fpga classes and the new fpga-dev class would be. > I'm just continuing the previous conversation. I'll > add further comments on the v2 version. > > Alan > >> The fpga-regions/bridges/manager are created as children of FME module, as >> the partial reconfiguration function is only a sub feature of FME module. >> >> If switch to fpga-region as container device, it may not be easy for user >> space applications to know which one represents a FPGA device and which one >> represents a reconfigurable region as all have the similar name 'regionx' >> in the same sysfs folder. Please consider this case, if we have 5 fpga >> devices on one system and each fpga device has multiple PR regions (e.g 20+). >> Then user space applications need to search all regions to locate the ones >> represent the FPGA device, even we add some attributes to it. >> >> [1]http://marc.info/?l=linux-fpga&m=149844234509825&w=2 >> >> Thanks >> Hao >> >>> >>> Alan >>> >>> > >>> > Thanks >>> > Hao >>> > >>> >> >drivers could register different functions under it. Per my understanding, >>> >> >the existing "fpga class", including fpga-region, fpga-bridge and >>> >> >fpga-manager, is used to provide reconfiguration function for FPGA. So >>> >> >driver can create child node using this existing "fpga class" to provide >>> >> >FPGA reconfiguration function, and more nodes under this container for >>> >> >different functions for given FPGA device. >>> >> > >>> >> >For Intel FPGA device, partial reconfiguration is only one function of >>> >> >Intel FPGA Management Engine (FME). FME driver creates fpga_manager under >>> >> >below path for partial reconfiguration, and other interfaces for more >>> >> >functions, e.g power management, virtualization support and etc. >>> >> > >>> >> >/sys/class/fpga///fpga_manager >>> >> > >>> >> >Thanks >>> >> >Hao >>> >> > >>> >> >> >>> >> >>thanks, >>> >> >> >>> >> >>greg k-h >>> >> >-- >>> >> >To unsubscribe from this list: send the line "unsubscribe linux-fpga" in >>> >> >the body of a message to majordomo@vger.kernel.org >>> >> >More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >