From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032522AbdDTOPh (ORCPT ); Thu, 20 Apr 2017 10:15:37 -0400 Received: from mail.kernel.org ([198.145.29.136]:59800 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S970220AbdDTOKc (ORCPT ); Thu, 20 Apr 2017 10:10:32 -0400 From: Alan Tull To: Moritz Fischer Cc: Alan Tull , linux-kernel@vger.kernel.org, linux-fpga@vger.kernel.org Subject: [PATCH v2 03/16] fpga: mgr: API change to replace fpga load functions with single function Date: Thu, 20 Apr 2017 09:09:48 -0500 Message-Id: <1492697401-11211-4-git-send-email-atull@kernel.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1492697401-11211-1-git-send-email-atull@kernel.org> References: <1492697401-11211-1-git-send-email-atull@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org fpga-mgr has three methods for programming FPGAs, depending on whether the image is in a scatter gather list, a contiguous buffer, or a firmware file. This makes it difficult to write upper layers as the caller has to assume whether the FPGA image is in a sg table, as a single buffer, or a firmware file. This commit moves these parameters to struct fpga_image_info and adds a single function for programming fpgas. New functions: * fpga_mgr_load - given fpga manager and struct fpga_image_info, program the fpga. * fpga_image_info_alloc - alloc a struct fpga_image_info. * fpga_image_info_free - free a struct fpga_image_info. These three functions are unexported: * fpga_mgr_buf_load_sg * fpga_mgr_buf_load * fpga_mgr_firmware_load Also use devm_kstrdup to copy firmware_name so we aren't making assumptions about where it comes from when allocing/freeing the struct fpga_image_info. Signed-off-by: Alan Tull --- v2: add fpga_image_info_alloc/free update copyright and author email --- drivers/fpga/fpga-mgr.c | 59 +++++++++++++++++++++++++++++++++++-------- drivers/fpga/fpga-region.c | 42 +++++++++++++++++++----------- include/linux/fpga/fpga-mgr.h | 22 ++++++++++------ 3 files changed, 89 insertions(+), 34 deletions(-) diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index 188ffef..3478aab 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -2,6 +2,7 @@ * FPGA Manager Core * * Copyright (C) 2013-2015 Altera Corporation + * Copyright (C) 2017 Intel Corporation * * With code from the mailing list: * Copyright (C) 2013 Xilinx, Inc. @@ -31,6 +32,31 @@ static DEFINE_IDA(fpga_mgr_ida); static struct class *fpga_mgr_class; +struct fpga_image_info *fpga_image_info_alloc(struct device *dev) +{ + struct fpga_image_info *info; + + info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); + if (!info) + return ERR_PTR(-ENOMEM); + + return info; +} +EXPORT_SYMBOL_GPL(fpga_image_info_alloc); + +void fpga_image_info_free(struct device *dev, + struct fpga_image_info *info) +{ + if (!info) + return; + + if (info->firmware_name) + devm_kfree(dev, info->firmware_name); + + devm_kfree(dev, info); +} +EXPORT_SYMBOL_GPL(fpga_image_info_free); + /* * Call the low level driver's write_init function. This will do the * device-specific things to get the FPGA into the state where it is ready to @@ -137,8 +163,9 @@ static int fpga_mgr_write_complete(struct fpga_manager *mgr, * * Return: 0 on success, negative error code otherwise. */ -int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info *info, - struct sg_table *sgt) +static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, + struct fpga_image_info *info, + struct sg_table *sgt) { int ret; @@ -170,7 +197,6 @@ int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info *info, return fpga_mgr_write_complete(mgr, info); } -EXPORT_SYMBOL_GPL(fpga_mgr_buf_load_sg); static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr, struct fpga_image_info *info, @@ -210,8 +236,9 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr, * * Return: 0 on success, negative error code otherwise. */ -int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info, - const char *buf, size_t count) +static int fpga_mgr_buf_load(struct fpga_manager *mgr, + struct fpga_image_info *info, + const char *buf, size_t count) { struct page **pages; struct sg_table sgt; @@ -266,7 +293,6 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info, return rc; } -EXPORT_SYMBOL_GPL(fpga_mgr_buf_load); /** * fpga_mgr_firmware_load - request firmware and load to fpga @@ -282,9 +308,9 @@ EXPORT_SYMBOL_GPL(fpga_mgr_buf_load); * * Return: 0 on success, negative error code otherwise. */ -int fpga_mgr_firmware_load(struct fpga_manager *mgr, - struct fpga_image_info *info, - const char *image_name) +static int fpga_mgr_firmware_load(struct fpga_manager *mgr, + struct fpga_image_info *info, + const char *image_name) { struct device *dev = &mgr->dev; const struct firmware *fw; @@ -307,7 +333,18 @@ int fpga_mgr_firmware_load(struct fpga_manager *mgr, return ret; } -EXPORT_SYMBOL_GPL(fpga_mgr_firmware_load); + +int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info) +{ + if (info->sgt) + return fpga_mgr_buf_load_sg(mgr, info, info->sgt); + if (info->buf && info->count) + return fpga_mgr_buf_load(mgr, info, info->buf, info->count); + if (info->firmware_name) + return fpga_mgr_firmware_load(mgr, info, info->firmware_name); + return -EINVAL; +} +EXPORT_SYMBOL_GPL(fpga_mgr_load); static const char * const state_str[] = { [FPGA_MGR_STATE_UNKNOWN] = "unknown", @@ -578,7 +615,7 @@ static void __exit fpga_mgr_class_exit(void) ida_destroy(&fpga_mgr_ida); } -MODULE_AUTHOR("Alan Tull "); +MODULE_AUTHOR("Alan Tull "); MODULE_DESCRIPTION("FPGA manager framework"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 655940f..93e4003 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -226,14 +226,11 @@ static int fpga_region_get_bridges(struct fpga_region *region, /** * fpga_region_program_fpga - program FPGA * @region: FPGA region - * @firmware_name: name of FPGA image firmware file * @overlay: device node of the overlay - * Program an FPGA using information in the device tree. - * Function assumes that there is a firmware-name property. + * Program an FPGA using information in the region's fpga image info. * Return 0 for success or negative error code. */ static int fpga_region_program_fpga(struct fpga_region *region, - const char *firmware_name, struct device_node *overlay) { struct fpga_manager *mgr; @@ -264,7 +261,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, goto err_put_br; } - ret = fpga_mgr_firmware_load(mgr, region->info, firmware_name); + ret = fpga_mgr_load(mgr, region->info); if (ret) { pr_err("failed to load fpga image\n"); goto err_put_br; @@ -357,16 +354,14 @@ static int child_regions_with_firmware(struct device_node *overlay) static int fpga_region_notify_pre_apply(struct fpga_region *region, struct of_overlay_notify_data *nd) { - const char *firmware_name = NULL; struct fpga_image_info *info; + const char *firmware_name; int ret; - info = devm_kzalloc(®ion->dev, sizeof(*info), GFP_KERNEL); + info = fpga_image_info_alloc(®ion->dev); if (!info) return -ENOMEM; - region->info = info; - /* Reject overlay if child FPGA Regions have firmware-name property */ ret = child_regions_with_firmware(nd->overlay); if (ret) @@ -382,7 +377,13 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, if (of_property_read_bool(nd->overlay, "encrypted-fpga-config")) info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM; - of_property_read_string(nd->overlay, "firmware-name", &firmware_name); + if (!of_property_read_string(nd->overlay, "firmware-name", + &firmware_name)) { + info->firmware_name = devm_kstrdup(dev, firmware_name, + GFP_KERNEL); + if (!info->firmware_name) + return -ENOMEM; + } of_property_read_u32(nd->overlay, "region-unfreeze-timeout-us", &info->enable_timeout_us); @@ -394,22 +395,33 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, &info->config_complete_timeout_us); /* If FPGA was externally programmed, don't specify firmware */ - if ((info->flags & FPGA_MGR_EXTERNAL_CONFIG) && firmware_name) { + if ((info->flags & FPGA_MGR_EXTERNAL_CONFIG) && info->firmware_name) { pr_err("error: specified firmware and external-fpga-config"); + fpga_image_info_free(dev, info); return -EINVAL; } /* FPGA is already configured externally. We're done. */ - if (info->flags & FPGA_MGR_EXTERNAL_CONFIG) + if (info->flags & FPGA_MGR_EXTERNAL_CONFIG) { + fpga_image_info_free(®ion->dev, info); return 0; + } /* If we got this far, we should be programming the FPGA */ - if (!firmware_name) { + if (!info->firmware_name) { pr_err("should specify firmware-name or external-fpga-config\n"); + fpga_image_info_free(dev, info); return -EINVAL; } - return fpga_region_program_fpga(region, firmware_name, nd->overlay); + region->info = info; + ret = fpga_region_program_fpga(region, nd->overlay); + if (ret) { + fpga_image_info_free(®ion->dev, info); + region->info = NULL; + } + + return ret; } /** @@ -426,7 +438,7 @@ static void fpga_region_notify_post_remove(struct fpga_region *region, { fpga_bridges_disable(®ion->bridge_list); fpga_bridges_put(®ion->bridge_list); - devm_kfree(®ion->dev, region->info); + fpga_image_info_free(®ion->dev, region->info); region->info = NULL; } diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h index b4ac24c..1c53aa3 100644 --- a/include/linux/fpga/fpga-mgr.h +++ b/include/linux/fpga/fpga-mgr.h @@ -1,7 +1,8 @@ /* * FPGA Framework * - * Copyright (C) 2013-2015 Altera Corporation + * Copyright (C) 2013-2016 Altera Corporation + * Copyright (C) 2017 Intel Corporation * * This program is free software; you can redistribute it and/or modify it * under the terms and conditions of the GNU General Public License, @@ -79,12 +80,20 @@ enum fpga_mgr_states { * @disable_timeout_us: maximum time to disable traffic through bridge (uSec) * @config_complete_timeout_us: maximum time for FPGA to switch to operating * status in the write_complete op. + * @firmware_name: name of FPGA image firmware file + * @sgt: scatter/gather table containing FPGA image + * @buf: contiguous buffer containing FPGA image + * @count: size of buf */ struct fpga_image_info { u32 flags; u32 enable_timeout_us; u32 disable_timeout_us; u32 config_complete_timeout_us; + char *firmware_name; + struct sg_table *sgt; + const char *buf; + size_t count; }; /** @@ -134,14 +143,11 @@ struct fpga_manager { #define to_fpga_manager(d) container_of(d, struct fpga_manager, dev) -int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info, - const char *buf, size_t count); -int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info *info, - struct sg_table *sgt); +struct fpga_image_info *fpga_image_info_alloc(struct device *dev); -int fpga_mgr_firmware_load(struct fpga_manager *mgr, - struct fpga_image_info *info, - const char *image_name); +void fpga_image_info_free(struct device *dev, struct fpga_image_info *info); + +int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info); struct fpga_manager *of_fpga_mgr_get(struct device_node *node); -- 2.7.4