From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755050AbdEEDIp (ORCPT ); Thu, 4 May 2017 23:08:45 -0400 Received: from mga03.intel.com ([134.134.136.65]:32113 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751791AbdEEDIo (ORCPT ); Thu, 4 May 2017 23:08:44 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,290,1491289200"; d="scan'208";a="1126660206" Date: Fri, 5 May 2017 11:03:27 +0800 From: Wu Hao To: "Li, Yi" Cc: atull@kernel.org, moritz.fischer@ettus.com, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, luwei.kang@intel.com, yi.z.zhang@intel.com, Xiao Guangrong , Tim Whisonant , Enno Luebbers , Shiva Rao , Christopher Rauer Subject: Re: [PATCH 04/16] fpga: intel: pcie: parse feature list and create platform device for features. Message-ID: <20170505030327.GA3099@hao-dev> References: <1490875696-15145-1-git-send-email-hao.wu@intel.com> <1490875696-15145-5-git-send-email-hao.wu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 04, 2017 at 10:13:41AM -0500, Li, Yi wrote: > hi Hao > > > On 3/30/2017 7:08 AM, Wu Hao wrote: > >From: Xiao Guangrong > > > >Device Featuer List structure creates a link list of feature headers > >within the MMIO space to provide an extensiable way of adding features. > > > >The Intel FPGA PCIe driver walks through the feature headers to enumerate > >feature devices, FPGA Management Engine (FME) and FPGA Port for Accelerated > >Function Unit (AFU), and their private sub features. For feature devices, > >it creates the platform devices and linked the private sub features into > >their platform data. > > > >Signed-off-by: Tim Whisonant > >Signed-off-by: Enno Luebbers > >Signed-off-by: Shiva Rao > >Signed-off-by: Christopher Rauer > >Signed-off-by: Kang Luwei > >Signed-off-by: Zhang Yi > >Signed-off-by: Xiao Guangrong > >Signed-off-by: Wu Hao > >--- > > drivers/fpga/intel/Makefile | 2 +- > > drivers/fpga/intel/feature-dev.c | 139 +++++++ > > drivers/fpga/intel/feature-dev.h | 342 ++++++++++++++++ > > drivers/fpga/intel/pcie.c | 841 ++++++++++++++++++++++++++++++++++++++- > > 4 files changed, 1321 insertions(+), 3 deletions(-) > > create mode 100644 drivers/fpga/intel/feature-dev.c > > create mode 100644 drivers/fpga/intel/feature-dev.h > > > >..... > >+ > >+static int > >+build_info_create_dev(struct build_feature_devs_info *binfo, > >+ enum fpga_id_type type, int feature_nr, const char *name) > >+{ > >+ struct platform_device *fdev; > >+ struct resource *res; > >+ struct feature_platform_data *pdata; > >+ int ret; > >+ > >+ /* we will create a new device, commit current device first */ > >+ ret = build_info_commit_dev(binfo); > > Looks like the code create the platform device (prepared by previous > feature) when prepare the current feature binfo, which I found is somewhat > confusing. Is there a reason to do so? Hi Yi, Driver only creates new platform device when it switches to new feature device parsing (new feature device found in the 'Device Feature List'), this code just makes sure previous platform device for last feature device is committed (platform_device_add). If there is no previous feature device or previous feature device has been already committed, then this function build_info_commit_dev will return 0 directly. Thanks Hao > > >+ if (ret) > >+ return ret; > >+ > >+ /* > >+ * we use -ENODEV as the initialization indicator which indicates > >+ * whether the id need to be reclaimed > >+ */ > >+ fdev = binfo->feature_dev = platform_device_alloc(name, -ENODEV); > >+ if (!fdev) > >+ return -ENOMEM; > >+ > >+ fdev->id = alloc_fpga_id(type, &fdev->dev); > >+ if (fdev->id < 0) > >+ return fdev->id; > >+ > >+ fdev->dev.parent = &binfo->parent_dev->dev; > >+ > >+ /* > >+ * we need not care the memory which is associated with the > >+ * platform device. After call platform_device_unregister(), > >+ * it will be automatically freed by device's > >+ * release() callback, platform_device_release(). > >+ */ > >+ pdata = feature_platform_data_alloc_and_init(fdev, feature_nr); > >+ if (!pdata) > >+ return -ENOMEM; > >+ > >+ /* > >+ * the count should be initialized to 0 to make sure > >+ *__fpga_port_enable() following __fpga_port_disable() > >+ * works properly for port device. > >+ * and it should always be 0 for fme device. > >+ */ > >+ WARN_ON(pdata->disable_count); > >+ > >+ fdev->dev.platform_data = pdata; > >+ fdev->num_resources = feature_nr; > >+ fdev->resource = kcalloc(feature_nr, sizeof(*res), GFP_KERNEL); > >+ if (!fdev->resource) > >+ return -ENOMEM; > >+ > >+ return 0; > >+} > >+ > >....