From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753460AbdEHIor (ORCPT ); Mon, 8 May 2017 04:44:47 -0400 Received: from mga06.intel.com ([134.134.136.31]:39132 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751669AbdEHIoq (ORCPT ); Mon, 8 May 2017 04:44:46 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,308,1491289200"; d="scan'208";a="98642501" From: "Wu, Hao" To: Alan Tull CC: Moritz Fischer , linux-kernel , "linux-fpga@vger.kernel.org" , "matthew.gerlach@linux.intel.com" Subject: RE: [PATCH v2 02/16] fpga: bridge: support getting bridge from device Thread-Topic: [PATCH v2 02/16] fpga: bridge: support getting bridge from device Thread-Index: AQHSxEkCCnan/ANkbE2D/UOuk14GkqHkLQWAgAXWm1A= Date: Mon, 8 May 2017 08:44:23 +0000 Message-ID: References: <1492697401-11211-1-git-send-email-atull@kernel.org> <1492697401-11211-3-git-send-email-atull@kernel.org> <20170503115831.GA30448@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 mail.home.local id v488kKQd030417 > On Wed, May 3, 2017 at 3:07 PM, Alan Tull wrote: > > On Wed, May 3, 2017 at 6:58 AM, Wu Hao wrote: > >> On Thu, Apr 20, 2017 at 09:09:47AM -0500, Alan Tull wrote: > >>> Add two functions for getting the FPGA bridge from the device > >>> rather than device tree node. This is to enable writing code > >>> that will support using FPGA bridges without device tree. > >>> Rename one old function to make it clear that it is device > >>> tree-ish. This leaves us with 3 functions for getting a bridge: > >>> > >>> * fpga_bridge_get > >>> Get the bridge given the device. > >>> > >>> * fpga_bridges_get_to_list > >>> Given the device, get the bridge and add it to a list. > >>> > >>> * of_fpga_bridges_get_to_list > >>> Renamed from priviously existing fpga_bridges_get_to_list. > >>> Given the device node, get the bridge and add it to a list. > >>> > >> > >> Hi Alan > >> > >> Thanks a lot for providing this patch set for non device tree support. :) > >> Actually I am reworking the Intel FPGA device drivers based on this patch > >> set, and I find some problems with the existing APIs including fpga bridge > >> and manager. My idea is to create all fpga bridges/regions/manager under > >> the same platform device (FME), it allows FME driver to establish the > >> relationship for the bridges/regions/managers it creates in an easy way. > >> But I found current fpga class API doesn't support this very well. > >> e.g fpga_bridge_get/get_to_list only accept parent device as the input > >> parameter, but it doesn't work if we have multiple bridges (and > >> regions/manager) under the same platform device. fpga_mgr has similar > >> issue, but fpga_region APIs work better, as they accept fpga_region as > >> parameter not the shared parent device. > > > > That's good feedback. I can post a couple patches that apply on top > > of that patchset to add the APIs you need. > > > > Probably what I'll do is add > > > > struct fpga_manager *fpga_mgr_get(struct fpga_manager *mgr); > > > > And rename fpga_bridge_get() to fpga_bridge_dev_get() and add the > following: > > > > struct fpga_bridge *fpga_bridge_get(struct fpga_bridge *br, > > struct fpga_image_info *info); > > > > int of_fpga_bridge_get_to_list(struct fpga_bridge *br, > > struct fpga_image_info *info, > > struct list_head *bridge_list); > > > > Working on it now. > > > >> > >> Do you think if having multiple fpga-* under one parent device is in the > >> right direction? > > > > That should be fine as long as it's coded with an eye on making things > > reusable and seeing beyond the current project. Just thinking of the > > future and of what can be of general usefulness for others. And there > > will be others interested in reusing this. > > > > Alan > > Actually, I don't think you will need the additional APIs we were > just discussing after all. What you have is a multifunction device > (single piece of hardware, multi functions such as in drivers/mfd). > It will have child devices for the mgr, bridges, and regions. When > registering the mgr and bridges you will need to allocate child > devices and use them to create the mgr and bridges. > > Alan Hi Alan I tried to create child devices as the parent device for the mgr and bridges in fme platform driver module. If only creates the device without driver, it doesn't work as try_module_get(dev->parent->driver->owner) always failed in mgr_get and bridge_get functions. If it creates platform devices as child devices, and introduce new platform device drivers for bridge and mgr, then it will be difficult to establish the relationship for region/mgr/bridges (e.g when should region->mgr be configured and cleared, as mgr is created/destroyed when mgr parent device platform driver module is loaded/unload), and it maybe not really necessary to introduce more different driver modules here. But if it allows multiple fpga-* created under one device in one device driver, it will be much easier to avoid above problems. So I asked if it is possible to create multiple fpga-* under one parent device, I feel this will not impact to current fpga drivers a lot, but provide more flexibility for drivers to use fpga-region/bridge/manager to create the topology in a device specific way, especially for non device tree case. Thanks Hao