From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753605AbdEIPZ5 (ORCPT ); Tue, 9 May 2017 11:25:57 -0400 Received: from mail.kernel.org ([198.145.29.136]:59180 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751653AbdEIPZ4 (ORCPT ); Tue, 9 May 2017 11:25:56 -0400 MIME-Version: 1.0 In-Reply-To: <1492697401-11211-11-git-send-email-atull@kernel.org> References: <1492697401-11211-1-git-send-email-atull@kernel.org> <1492697401-11211-11-git-send-email-atull@kernel.org> From: Alan Tull Date: Tue, 9 May 2017 10:25:12 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 10/16] fpga: region: use image info as parameter for programming region To: Moritz Fischer Cc: Alan Tull , linux-kernel , linux-fpga@vger.kernel.org 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 Thu, Apr 20, 2017 at 9:09 AM, Alan Tull wrote: > Use FPGA image info as a parameter when region code is > programming the FPGA. > > This is a baby step in refactoring the FPGA region code to > separate out common FPGA region code from FPGA region > Device Tree overlay support. > > Signed-off-by: Alan Tull > --- > v2: split out from another patch > --- > drivers/fpga/fpga-region.c | 26 ++++++++++++++++---------- > include/linux/fpga/fpga-mgr.h | 4 ++++ > 2 files changed, 20 insertions(+), 10 deletions(-) > > diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c > index d6f2313..1892126 100644 > --- a/drivers/fpga/fpga-region.c > +++ b/drivers/fpga/fpga-region.c > @@ -223,16 +223,21 @@ static int fpga_region_get_bridges(struct fpga_region *region, > /** > * fpga_region_program_fpga - program FPGA > * @region: FPGA region > - * @overlay: device node of the overlay > - * Program an FPGA using information in the region's fpga image info. > + * @info: FPGA image info > + * Program an FPGA using fpga image info. > * Return 0 for success or negative error code. > */ > static int fpga_region_program_fpga(struct fpga_region *region, > - struct device_node *overlay) > + struct fpga_image_info *info) > { > struct device *dev = ®ion->dev; > int ret; > > + if (region->info) { > + dev_err(dev, "region already programmed\n"); > + return PTR_ERR(region); > + } > + Currently if region has info, that is supposed to mean that the region is programmed. I can already see how that could be limiting. I'm think that in v3, I'm going to change this to add a region->state or region->status that can indicate whether the region has been programmed. Having done that, the second parameter of this functon can go away and region->info is used. Alan > region = fpga_region_get(region); > if (IS_ERR(region)) { > dev_err(dev, "failed to get FPGA region\n"); > @@ -245,7 +250,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, > goto err_put_region; > } > > - ret = fpga_region_get_bridges(region, overlay); > + ret = fpga_region_get_bridges(region, info->overlay); > if (ret) { > dev_err(dev, "failed to get FPGA bridges\n"); > goto err_unlock_mgr; > @@ -257,7 +262,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, > goto err_put_br; > } > > - ret = fpga_mgr_load(region->mgr, region->info); > + ret = fpga_mgr_load(region->mgr, info); > if (ret) { > dev_err(dev, "failed to load FPGA image\n"); > goto err_put_br; > @@ -269,6 +274,8 @@ static int fpga_region_program_fpga(struct fpga_region *region, > goto err_put_br; > } > > + region->info = info; > + > fpga_mgr_unlock(region->mgr); > fpga_region_put(region); > > @@ -373,6 +380,8 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, > if (!info) > return -ENOMEM; > > + info->overlay = nd->overlay; > + > /* Read FPGA region properties from the overlay */ > if (of_property_read_bool(nd->overlay, "partial-fpga-config")) > info->flags |= FPGA_MGR_PARTIAL_RECONFIG; > @@ -420,12 +429,9 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, > return -EINVAL; > } > > - region->info = info; > - ret = fpga_region_program_fpga(region, nd->overlay); > - if (ret) { > + ret = fpga_region_program_fpga(region, info); > + if (ret) > fpga_image_info_free(®ion->dev, info); > - region->info = NULL; > - } > > return ret; > } > diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h > index 4e14bdb..66d0e32 100644 > --- a/include/linux/fpga/fpga-mgr.h > +++ b/include/linux/fpga/fpga-mgr.h > @@ -84,6 +84,7 @@ enum fpga_mgr_states { > * @sgt: scatter/gather table containing FPGA image > * @buf: contiguous buffer containing FPGA image > * @count: size of buf > + * @overlay: Device Tree overlay > */ > struct fpga_image_info { > u32 flags; > @@ -94,6 +95,9 @@ struct fpga_image_info { > struct sg_table *sgt; > const char *buf; > size_t count; > +#ifdef CONFIG_OF > + struct device_node *overlay; > +#endif > }; > > /** > -- > 2.7.4 >