Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751451AbdINPuN (ORCPT ); Thu, 14 Sep 2017 11:50:13 -0400 Received: from mga04.intel.com ([192.55.52.120]:49577 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751255AbdINPuM (ORCPT ); Thu, 14 Sep 2017 11:50:12 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,393,1500966000"; d="scan'208";a="1014462913" Date: Thu, 14 Sep 2017 08:50:06 -0700 (PDT) From: matthew.gerlach@linux.intel.com X-X-Sender: mgerlach@mgerlach-VirtualBox To: Alan Tull cc: Moritz Fischer , linux-kernel@vger.kernel.org, linux-fpga@vger.kernel.org Subject: Re: [PATCH v4 15/18] fpga: region: move device tree support to of-fpga-region.c In-Reply-To: <20170913204841.2730-16-atull@kernel.org> Message-ID: References: <20170913204841.2730-1-atull@kernel.org> <20170913204841.2730-16-atull@kernel.org> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 32491 Lines: 1118 Hi Alan, Just a couple of minor nits. Matthew Gerlach On Wed, 13 Sep 2017, Alan Tull wrote: > Create of-fpga-region.c and ove the following functions without s/ove/move/ > modification from fpga-region.c. > > * of_fpga_region_find > * of_fpga_region_get_mgr > * of_fpga_region_get_bridges > * child_regions_with_firmware > * of_fpga_region_parse_ov > * of_fpga_region_notify_pre_apply > * of_fpga_region_notify_post_remove > * of_fpga_region_notify > * of_fpga_region_probe > * of_fpga_region_remove > > Create two new function with some code from fpga_region_init/exit. s/function/functions/ > > * of_fpga_region_init > * of_fpga_region_exit > > Signed-off-by: Alan Tull > --- > v2: split out code changes into other patches, only move code here > v3: updated to move changed code to of-fpga-region.c > v4: rebase on current for-next > remove fpga-bridge dependency on CONFIG_OF > --- > drivers/fpga/Kconfig | 14 +- > drivers/fpga/Makefile | 1 + > drivers/fpga/fpga-region.c | 451 +------------------------------------- > drivers/fpga/of-fpga-region.c | 495 ++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 507 insertions(+), 454 deletions(-) > create mode 100644 drivers/fpga/of-fpga-region.c > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > index ad5448f..529b729 100644 > --- a/drivers/fpga/Kconfig > +++ b/drivers/fpga/Kconfig > @@ -13,10 +13,17 @@ if FPGA > > config FPGA_REGION > tristate "FPGA Region" > - depends on OF && FPGA_BRIDGE > + depends on FPGA_BRIDGE > + help > + FPGA Region common code. A FPGA Region controls a FPGA Manager > + and the FPGA Bridges associated with either a reconfigurable > + region of an FPGA or a whole FPGA. > + > +config OF_FPGA_REGION > + tristate "FPGA Region Device Tree Overlay Support" > + depends on OF && FPGA_REGION > help > - FPGA Regions allow loading FPGA images under control of > - the Device Tree. > + Support for loading FPGA images under control of Device Tree. Support for loading FPGA images by applying a Device Tree Overlay. > > config FPGA_MGR_ICE40_SPI > tristate "Lattice iCE40 SPI" > @@ -74,7 +81,6 @@ config FPGA_MGR_ZYNQ_FPGA > > config FPGA_BRIDGE > tristate "FPGA Bridge Framework" > - depends on OF > help > Say Y here if you want to support bridges connected between host > processors and FPGAs or between FPGAs. > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index e09895f..8f56766 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -25,3 +25,4 @@ obj-$(CONFIG_XILINX_PR_DECOUPLER) += xilinx-pr-decoupler.o > > # High Level Interfaces > obj-$(CONFIG_FPGA_REGION) += fpga-region.o > +obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o > diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c > index 5f2fa5f6..afc6188 100644 > --- a/drivers/fpga/fpga-region.c > +++ b/drivers/fpga/fpga-region.c > @@ -2,6 +2,7 @@ > * FPGA Region - Device Tree support for FPGA programming under Linux > * > * 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, > @@ -23,7 +24,6 @@ > #include > #include > #include > -#include > #include > #include > > @@ -44,30 +44,6 @@ struct fpga_region *fpga_region_class_find( > } > EXPORT_SYMBOL_GPL(fpga_region_class_find); > > -static const struct of_device_id fpga_region_of_match[] = { > - { .compatible = "fpga-region", }, > - {}, > -}; > -MODULE_DEVICE_TABLE(of, fpga_region_of_match); > - > -static int fpga_region_of_node_match(struct device *dev, const void *data) > -{ > - return dev->of_node == data; > -} > - > -/** > - * of_fpga_region_find - find FPGA region > - * @np: device node of FPGA Region > - * > - * Caller will need to put_device(®ion->dev) when done. > - * > - * Returns FPGA Region struct or NULL > - */ > -static struct fpga_region *of_fpga_region_find(struct device_node *np) > -{ > - return fpga_region_class_find(NULL, np, fpga_region_of_node_match); > -} > - > /** > * fpga_region_get - get an exclusive reference to a fpga region > * @region: FPGA Region struct > @@ -116,102 +92,6 @@ static void fpga_region_put(struct fpga_region *region) > } > > /** > - * of_fpga_region_get_mgr - get reference for FPGA manager > - * @np: device node of FPGA region > - * > - * Get FPGA Manager from "fpga-mgr" property or from ancestor region. > - * > - * Caller should call fpga_mgr_put() when done with manager. > - * > - * Return: fpga manager struct or IS_ERR() condition containing error code. > - */ > -static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np) > -{ > - struct device_node *mgr_node; > - struct fpga_manager *mgr; > - > - of_node_get(np); > - while (np) { > - if (of_device_is_compatible(np, "fpga-region")) { > - mgr_node = of_parse_phandle(np, "fpga-mgr", 0); > - if (mgr_node) { > - mgr = of_fpga_mgr_get(mgr_node); > - of_node_put(np); > - return mgr; > - } > - } > - np = of_get_next_parent(np); > - } > - of_node_put(np); > - > - return ERR_PTR(-EINVAL); > -} > - > -/** > - * of_fpga_region_get_bridges - create a list of bridges > - * @region: FPGA region > - * > - * Create a list of bridges including the parent bridge and the bridges > - * specified by "fpga-bridges" property. Note that the > - * fpga_bridges_enable/disable/put functions are all fine with an empty list > - * if that happens. > - * > - * Caller should call fpga_bridges_put(®ion->bridge_list) when > - * done with the bridges. > - * > - * Return 0 for success (even if there are no bridges specified) > - * or -EBUSY if any of the bridges are in use. > - */ > -static int of_fpga_region_get_bridges(struct fpga_region *region) > -{ > - struct device *dev = ®ion->dev; > - struct device_node *region_np = dev->of_node; > - struct fpga_image_info *info = region->info; > - struct device_node *br, *np, *parent_br = NULL; > - int i, ret; > - > - /* If parent is a bridge, add to list */ > - ret = of_fpga_bridge_get_to_list(region_np->parent, info, > - ®ion->bridge_list); > - > - /* -EBUSY means parent is a bridge that is under use. Give up. */ > - if (ret == -EBUSY) > - return ret; > - > - /* Zero return code means parent was a bridge and was added to list. */ > - if (!ret) > - parent_br = region_np->parent; > - > - /* If overlay has a list of bridges, use it. */ > - if (of_parse_phandle(info->overlay, "fpga-bridges", 0)) > - np = info->overlay; > - else > - np = region_np; > - > - for (i = 0; ; i++) { > - br = of_parse_phandle(np, "fpga-bridges", i); > - if (!br) > - break; > - > - /* If parent bridge is in list, skip it. */ > - if (br == parent_br) > - continue; > - > - /* If node is a bridge, get it and add to list */ > - ret = of_fpga_bridge_get_to_list(br, info, > - ®ion->bridge_list); > - > - /* If any of the bridges are in use, give up */ > - if (ret == -EBUSY) { > - fpga_bridges_put(®ion->bridge_list); > - return -EBUSY; > - } > - } > - > - return 0; > -} > - > -/** > * fpga_region_program_fpga - program FPGA > * @region: FPGA region > * Program an FPGA using fpga image info (region->info). > @@ -282,259 +162,6 @@ int fpga_region_program_fpga(struct fpga_region *region) > } > EXPORT_SYMBOL_GPL(fpga_region_program_fpga); > > -/** > - * child_regions_with_firmware > - * @overlay: device node of the overlay > - * > - * If the overlay adds child FPGA regions, they are not allowed to have > - * firmware-name property. > - * > - * Return 0 for OK or -EINVAL if child FPGA region adds firmware-name. > - */ > -static int child_regions_with_firmware(struct device_node *overlay) > -{ > - struct device_node *child_region; > - const char *child_firmware_name; > - int ret = 0; > - > - of_node_get(overlay); > - > - child_region = of_find_matching_node(overlay, fpga_region_of_match); > - while (child_region) { > - if (!of_property_read_string(child_region, "firmware-name", > - &child_firmware_name)) { > - ret = -EINVAL; > - break; > - } > - child_region = of_find_matching_node(child_region, > - fpga_region_of_match); > - } > - > - of_node_put(child_region); > - > - if (ret) > - pr_err("firmware-name not allowed in child FPGA region: %pOF", > - child_region); > - > - return ret; > -} > - > -/** > - * of_fpga_region_parse_ov - parse and check overlay applied to region > - * > - * @region: FPGA region > - * @overlay: overlay applied to the FPGA region > - * > - * Given an overlay applied to a FPGA region, parse the FPGA image specific > - * info in the overlay and do some checking. > - * > - * Returns: > - * NULL if overlay doesn't direct us to program the FPGA. > - * fpga_image_info struct if there is an image to program. > - * error code for invalid overlay. > - */ > -static struct fpga_image_info *of_fpga_region_parse_ov( > - struct fpga_region *region, > - struct device_node *overlay) > -{ > - struct device *dev = ®ion->dev; > - struct fpga_image_info *info; > - const char *firmware_name; > - int ret; > - > - if (region->info) { > - dev_err(dev, "Region already has overlay applied.\n"); > - return ERR_PTR(-EINVAL); > - } > - > - /* > - * Reject overlay if child FPGA Regions added in the overlay have > - * firmware-name property (would mean that an FPGA region that has > - * not been added to the live tree yet is doing FPGA programming). > - */ > - ret = child_regions_with_firmware(overlay); > - if (ret) > - return ERR_PTR(ret); > - > - info = fpga_image_info_alloc(dev); > - if (!info) > - return ERR_PTR(-ENOMEM); > - > - info->overlay = overlay; > - > - /* Read FPGA region properties from the overlay */ > - if (of_property_read_bool(overlay, "partial-fpga-config")) > - info->flags |= FPGA_MGR_PARTIAL_RECONFIG; > - > - if (of_property_read_bool(overlay, "external-fpga-config")) > - info->flags |= FPGA_MGR_EXTERNAL_CONFIG; > - > - if (of_property_read_bool(overlay, "encrypted-fpga-config")) > - info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM; > - > - if (!of_property_read_string(overlay, "firmware-name", > - &firmware_name)) { > - info->firmware_name = devm_kstrdup(dev, firmware_name, > - GFP_KERNEL); > - if (!info->firmware_name) > - return ERR_PTR(-ENOMEM); > - } > - > - of_property_read_u32(overlay, "region-unfreeze-timeout-us", > - &info->enable_timeout_us); > - > - of_property_read_u32(overlay, "region-freeze-timeout-us", > - &info->disable_timeout_us); > - > - of_property_read_u32(overlay, "config-complete-timeout-us", > - &info->config_complete_timeout_us); > - > - /* If overlay is not programming the FPGA, don't need FPGA image info */ > - if (!info->firmware_name) { > - ret = 0; > - goto ret_no_info; > - } > - > - /* > - * If overlay informs us FPGA was externally programmed, specifying > - * firmware here would be ambiguous. > - */ > - if (info->flags & FPGA_MGR_EXTERNAL_CONFIG) { > - dev_err(dev, "error: specified firmware and external-fpga-config"); > - ret = -EINVAL; > - goto ret_no_info; > - } > - > - return info; > -ret_no_info: > - fpga_image_info_free(info); > - return ERR_PTR(ret); > -} > - > -/** > - * of_fpga_region_notify_pre_apply - pre-apply overlay notification > - * > - * @region: FPGA region that the overlay was applied to > - * @nd: overlay notification data > - * > - * Called when an overlay targeted to a FPGA Region is about to be applied. > - * Parses the overlay for properties that influence how the FPGA will be > - * programmed and does some checking. If the checks pass, programs the FPGA. > - * If the checks fail, overlay is rejected and does not get added to the > - * live tree. > - * > - * Returns 0 for success or negative error code for failure. > - */ > -static int of_fpga_region_notify_pre_apply(struct fpga_region *region, > - struct of_overlay_notify_data *nd) > -{ > - struct device *dev = ®ion->dev; > - struct fpga_image_info *info; > - int ret; > - > - if (region->info) { > - dev_err(dev, "Region already has overlay applied.\n"); > - return -EINVAL; > - } > - > - info = of_fpga_region_parse_ov(region, nd->overlay); > - if (IS_ERR(info)) > - return PTR_ERR(info); > - > - if (!info) > - return 0; > - > - region->info = info; > - ret = fpga_region_program_fpga(region); > - if (ret) { > - /* error; reject overlay */ > - fpga_image_info_free(info); > - region->info = NULL; > - } > - > - return ret; > -} > - > -/** > - * of_fpga_region_notify_post_remove - post-remove overlay notification > - * > - * @region: FPGA region that was targeted by the overlay that was removed > - * @nd: overlay notification data > - * > - * Called after an overlay has been removed if the overlay's target was a > - * FPGA region. > - */ > -static void of_fpga_region_notify_post_remove(struct fpga_region *region, > - struct of_overlay_notify_data *nd) > -{ > - fpga_bridges_disable(®ion->bridge_list); > - fpga_bridges_put(®ion->bridge_list); > - fpga_image_info_free(region->info); > - region->info = NULL; > -} > - > -/** > - * of_fpga_region_notify - reconfig notifier for dynamic DT changes > - * @nb: notifier block > - * @action: notifier action > - * @arg: reconfig data > - * > - * This notifier handles programming a FPGA when a "firmware-name" property is > - * added to a fpga-region. > - * > - * Returns NOTIFY_OK or error if FPGA programming fails. > - */ > -static int of_fpga_region_notify(struct notifier_block *nb, > - unsigned long action, void *arg) > -{ > - struct of_overlay_notify_data *nd = arg; > - struct fpga_region *region; > - int ret; > - > - switch (action) { > - case OF_OVERLAY_PRE_APPLY: > - pr_debug("%s OF_OVERLAY_PRE_APPLY\n", __func__); > - break; > - case OF_OVERLAY_POST_APPLY: > - pr_debug("%s OF_OVERLAY_POST_APPLY\n", __func__); > - return NOTIFY_OK; /* not for us */ > - case OF_OVERLAY_PRE_REMOVE: > - pr_debug("%s OF_OVERLAY_PRE_REMOVE\n", __func__); > - return NOTIFY_OK; /* not for us */ > - case OF_OVERLAY_POST_REMOVE: > - pr_debug("%s OF_OVERLAY_POST_REMOVE\n", __func__); > - break; > - default: /* should not happen */ > - return NOTIFY_OK; > - } > - > - region = of_fpga_region_find(nd->target); > - if (!region) > - return NOTIFY_OK; > - > - ret = 0; > - switch (action) { > - case OF_OVERLAY_PRE_APPLY: > - ret = of_fpga_region_notify_pre_apply(region, nd); > - break; > - > - case OF_OVERLAY_POST_REMOVE: > - of_fpga_region_notify_post_remove(region, nd); > - break; > - } > - > - put_device(®ion->dev); > - > - if (ret) > - return notifier_from_errno(ret); > - > - return NOTIFY_OK; > -} > - > -static struct notifier_block fpga_region_of_nb = { > - .notifier_call = of_fpga_region_notify, > -}; > - > int fpga_region_register(struct device *dev, struct fpga_region *region) > { > int id, ret = 0; > @@ -576,63 +203,6 @@ int fpga_region_unregister(struct fpga_region *region) > } > EXPORT_SYMBOL_GPL(fpga_region_unregister); > > -static int of_fpga_region_probe(struct platform_device *pdev) > -{ > - struct device *dev = &pdev->dev; > - struct device_node *np = dev->of_node; > - struct fpga_region *region; > - struct fpga_manager *mgr; > - int ret; > - > - /* Find the FPGA mgr specified by region or parent region. */ > - mgr = of_fpga_region_get_mgr(np); > - if (IS_ERR(mgr)) > - return -EPROBE_DEFER; > - > - region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL); > - if (!region) { > - ret = -ENOMEM; > - goto eprobe_mgr_put; > - } > - > - region->mgr = mgr; > - > - /* Specify how to get bridges for this type of region. */ > - region->get_bridges = of_fpga_region_get_bridges; > - > - ret = fpga_region_register(dev, region); > - if (ret) > - goto eprobe_mgr_put; > - > - of_platform_populate(np, fpga_region_of_match, NULL, ®ion->dev); > - > - dev_info(dev, "FPGA Region probed\n"); > - > - return 0; > - > -eprobe_mgr_put: > - fpga_mgr_put(mgr); > - return ret; > -} > - > -static int of_fpga_region_remove(struct platform_device *pdev) > -{ > - struct fpga_region *region = platform_get_drvdata(pdev); > - > - fpga_region_unregister(region); > - > - return 0; > -} > - > -static struct platform_driver of_fpga_region_driver = { > - .probe = of_fpga_region_probe, > - .remove = of_fpga_region_remove, > - .driver = { > - .name = "fpga-region", > - .of_match_table = of_match_ptr(fpga_region_of_match), > - }, > -}; > - > static void fpga_region_dev_release(struct device *dev) > { > struct fpga_region *region = to_fpga_region(dev); > @@ -646,36 +216,17 @@ static void fpga_region_dev_release(struct device *dev) > */ > static int __init fpga_region_init(void) > { > - int ret; > - > fpga_region_class = class_create(THIS_MODULE, "fpga_region"); > if (IS_ERR(fpga_region_class)) > return PTR_ERR(fpga_region_class); > > fpga_region_class->dev_release = fpga_region_dev_release; > > - ret = of_overlay_notifier_register(&fpga_region_of_nb); > - if (ret) > - goto err_class; > - > - ret = platform_driver_register(&of_fpga_region_driver); > - if (ret) > - goto err_plat; > - > return 0; > - > -err_plat: > - of_overlay_notifier_unregister(&fpga_region_of_nb); > -err_class: > - class_destroy(fpga_region_class); > - ida_destroy(&fpga_region_ida); > - return ret; > } > > static void __exit fpga_region_exit(void) > { > - platform_driver_unregister(&of_fpga_region_driver); > - of_overlay_notifier_unregister(&fpga_region_of_nb); > class_destroy(fpga_region_class); > ida_destroy(&fpga_region_ida); > } > diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c > new file mode 100644 > index 0000000..1797057 > --- /dev/null > +++ b/drivers/fpga/of-fpga-region.c > @@ -0,0 +1,495 @@ > +/* > + * FPGA Region - Device Tree support for FPGA programming under Linux > + * > + * 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, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program. If not, see . > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +static const struct of_device_id fpga_region_of_match[] = { > + { .compatible = "fpga-region", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, fpga_region_of_match); > + > +static int fpga_region_of_node_match(struct device *dev, const void *data) > +{ > + return dev->of_node == data; > +} > + > +/** > + * of_fpga_region_find - find FPGA region > + * @np: device node of FPGA Region > + * > + * Caller will need to put_device(®ion->dev) when done. > + * > + * Returns FPGA Region struct or NULL > + */ > +static struct fpga_region *of_fpga_region_find(struct device_node *np) > +{ > + return fpga_region_class_find(NULL, np, fpga_region_of_node_match); > +} > + > +/** > + * of_fpga_region_get_mgr - get reference for FPGA manager > + * @np: device node of FPGA region > + * > + * Get FPGA Manager from "fpga-mgr" property or from ancestor region. > + * > + * Caller should call fpga_mgr_put() when done with manager. > + * > + * Return: fpga manager struct or IS_ERR() condition containing error code. > + */ > +static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np) > +{ > + struct device_node *mgr_node; > + struct fpga_manager *mgr; > + > + of_node_get(np); > + while (np) { > + if (of_device_is_compatible(np, "fpga-region")) { > + mgr_node = of_parse_phandle(np, "fpga-mgr", 0); > + if (mgr_node) { > + mgr = of_fpga_mgr_get(mgr_node); > + of_node_put(np); > + return mgr; > + } > + } > + np = of_get_next_parent(np); > + } > + of_node_put(np); > + > + return ERR_PTR(-EINVAL); > +} > + > +/** > + * of_fpga_region_get_bridges - create a list of bridges > + * @region: FPGA region > + * > + * Create a list of bridges including the parent bridge and the bridges > + * specified by "fpga-bridges" property. Note that the > + * fpga_bridges_enable/disable/put functions are all fine with an empty list > + * if that happens. > + * > + * Caller should call fpga_bridges_put(®ion->bridge_list) when > + * done with the bridges. > + * > + * Return 0 for success (even if there are no bridges specified) > + * or -EBUSY if any of the bridges are in use. > + */ > +static int of_fpga_region_get_bridges(struct fpga_region *region) > +{ > + struct device *dev = ®ion->dev; > + struct device_node *region_np = dev->of_node; > + struct fpga_image_info *info = region->info; > + struct device_node *br, *np, *parent_br = NULL; > + int i, ret; > + > + /* If parent is a bridge, add to list */ > + ret = of_fpga_bridge_get_to_list(region_np->parent, info, > + ®ion->bridge_list); > + > + /* -EBUSY means parent is a bridge that is under use. Give up. */ > + if (ret == -EBUSY) > + return ret; > + > + /* Zero return code means parent was a bridge and was added to list. */ > + if (!ret) > + parent_br = region_np->parent; > + > + /* If overlay has a list of bridges, use it. */ > + if (of_parse_phandle(info->overlay, "fpga-bridges", 0)) > + np = info->overlay; > + else > + np = region_np; > + > + for (i = 0; ; i++) { > + br = of_parse_phandle(np, "fpga-bridges", i); > + if (!br) > + break; > + > + /* If parent bridge is in list, skip it. */ > + if (br == parent_br) > + continue; > + > + /* If node is a bridge, get it and add to list */ > + ret = of_fpga_bridge_get_to_list(br, info, > + ®ion->bridge_list); > + > + /* If any of the bridges are in use, give up */ > + if (ret == -EBUSY) { > + fpga_bridges_put(®ion->bridge_list); > + return -EBUSY; > + } > + } > + > + return 0; > +} > + > +/** > + * child_regions_with_firmware > + * @overlay: device node of the overlay > + * > + * If the overlay adds child FPGA regions, they are not allowed to have > + * firmware-name property. > + * > + * Return 0 for OK or -EINVAL if child FPGA region adds firmware-name. > + */ > +static int child_regions_with_firmware(struct device_node *overlay) > +{ > + struct device_node *child_region; > + const char *child_firmware_name; > + int ret = 0; > + > + of_node_get(overlay); > + > + child_region = of_find_matching_node(overlay, fpga_region_of_match); > + while (child_region) { > + if (!of_property_read_string(child_region, "firmware-name", > + &child_firmware_name)) { > + ret = -EINVAL; > + break; > + } > + child_region = of_find_matching_node(child_region, > + fpga_region_of_match); > + } > + > + of_node_put(child_region); > + > + if (ret) > + pr_err("firmware-name not allowed in child FPGA region: %pOF", > + child_region); > + > + return ret; > +} > + > +/** > + * of_fpga_region_parse_ov - parse and check overlay applied to region > + * > + * @region: FPGA region > + * @overlay: overlay applied to the FPGA region > + * > + * Given an overlay applied to a FPGA region, parse the FPGA image specific > + * info in the overlay and do some checking. > + * > + * Returns: > + * NULL if overlay doesn't direct us to program the FPGA. > + * fpga_image_info struct if there is an image to program. > + * error code for invalid overlay. > + */ > +static struct fpga_image_info *of_fpga_region_parse_ov( > + struct fpga_region *region, > + struct device_node *overlay) > +{ > + struct device *dev = ®ion->dev; > + struct fpga_image_info *info; > + const char *firmware_name; > + int ret; > + > + if (region->info) { > + dev_err(dev, "Region already has overlay applied.\n"); > + return ERR_PTR(-EINVAL); > + } > + > + /* > + * Reject overlay if child FPGA Regions added in the overlay have > + * firmware-name property (would mean that an FPGA region that has > + * not been added to the live tree yet is doing FPGA programming). > + */ > + ret = child_regions_with_firmware(overlay); > + if (ret) > + return ERR_PTR(ret); > + > + info = fpga_image_info_alloc(dev); > + if (!info) > + return ERR_PTR(-ENOMEM); > + > + info->overlay = overlay; > + > + /* Read FPGA region properties from the overlay */ > + if (of_property_read_bool(overlay, "partial-fpga-config")) > + info->flags |= FPGA_MGR_PARTIAL_RECONFIG; > + > + if (of_property_read_bool(overlay, "external-fpga-config")) > + info->flags |= FPGA_MGR_EXTERNAL_CONFIG; > + > + if (of_property_read_bool(overlay, "encrypted-fpga-config")) > + info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM; > + > + if (!of_property_read_string(overlay, "firmware-name", > + &firmware_name)) { > + info->firmware_name = devm_kstrdup(dev, firmware_name, > + GFP_KERNEL); > + if (!info->firmware_name) > + return ERR_PTR(-ENOMEM); > + } > + > + of_property_read_u32(overlay, "region-unfreeze-timeout-us", > + &info->enable_timeout_us); > + > + of_property_read_u32(overlay, "region-freeze-timeout-us", > + &info->disable_timeout_us); > + > + of_property_read_u32(overlay, "config-complete-timeout-us", > + &info->config_complete_timeout_us); > + > + /* If overlay is not programming the FPGA, don't need FPGA image info */ > + if (!info->firmware_name) { > + ret = 0; > + goto ret_no_info; > + } > + > + /* > + * If overlay informs us FPGA was externally programmed, specifying > + * firmware here would be ambiguous. > + */ > + if (info->flags & FPGA_MGR_EXTERNAL_CONFIG) { > + dev_err(dev, "error: specified firmware and external-fpga-config"); > + ret = -EINVAL; > + goto ret_no_info; > + } > + > + return info; > +ret_no_info: > + fpga_image_info_free(info); > + return ERR_PTR(ret); > +} > + > +/** > + * of_fpga_region_notify_pre_apply - pre-apply overlay notification > + * > + * @region: FPGA region that the overlay was applied to > + * @nd: overlay notification data > + * > + * Called when an overlay targeted to a FPGA Region is about to be applied. > + * Parses the overlay for properties that influence how the FPGA will be > + * programmed and does some checking. If the checks pass, programs the FPGA. > + * If the checks fail, overlay is rejected and does not get added to the > + * live tree. > + * > + * Returns 0 for success or negative error code for failure. > + */ > +static int of_fpga_region_notify_pre_apply(struct fpga_region *region, > + struct of_overlay_notify_data *nd) > +{ > + struct device *dev = ®ion->dev; > + struct fpga_image_info *info; > + int ret; > + > + if (region->info) { > + dev_err(dev, "Region already has overlay applied.\n"); > + return -EINVAL; > + } > + > + info = of_fpga_region_parse_ov(region, nd->overlay); > + if (IS_ERR(info)) > + return PTR_ERR(info); > + > + if (!info) > + return 0; > + > + region->info = info; > + ret = fpga_region_program_fpga(region); > + if (ret) { > + /* error; reject overlay */ > + fpga_image_info_free(info); > + region->info = NULL; > + } > + > + return ret; > +} > + > +/** > + * of_fpga_region_notify_post_remove - post-remove overlay notification > + * > + * @region: FPGA region that was targeted by the overlay that was removed > + * @nd: overlay notification data > + * > + * Called after an overlay has been removed if the overlay's target was a > + * FPGA region. > + */ > +static void of_fpga_region_notify_post_remove(struct fpga_region *region, > + struct of_overlay_notify_data *nd) > +{ > + fpga_bridges_disable(®ion->bridge_list); > + fpga_bridges_put(®ion->bridge_list); > + fpga_image_info_free(region->info); > + region->info = NULL; > +} > + > +/** > + * of_fpga_region_notify - reconfig notifier for dynamic DT changes > + * @nb: notifier block > + * @action: notifier action > + * @arg: reconfig data > + * > + * This notifier handles programming a FPGA when a "firmware-name" property is > + * added to a fpga-region. > + * > + * Returns NOTIFY_OK or error if FPGA programming fails. > + */ > +static int of_fpga_region_notify(struct notifier_block *nb, > + unsigned long action, void *arg) > +{ > + struct of_overlay_notify_data *nd = arg; > + struct fpga_region *region; > + int ret; > + > + switch (action) { > + case OF_OVERLAY_PRE_APPLY: > + pr_debug("%s OF_OVERLAY_PRE_APPLY\n", __func__); > + break; > + case OF_OVERLAY_POST_APPLY: > + pr_debug("%s OF_OVERLAY_POST_APPLY\n", __func__); > + return NOTIFY_OK; /* not for us */ > + case OF_OVERLAY_PRE_REMOVE: > + pr_debug("%s OF_OVERLAY_PRE_REMOVE\n", __func__); > + return NOTIFY_OK; /* not for us */ > + case OF_OVERLAY_POST_REMOVE: > + pr_debug("%s OF_OVERLAY_POST_REMOVE\n", __func__); > + break; > + default: /* should not happen */ > + return NOTIFY_OK; If this should not happen, then shouldn't an error be returned? > + } > + > + region = of_fpga_region_find(nd->target); > + if (!region) > + return NOTIFY_OK; > + > + ret = 0; > + switch (action) { > + case OF_OVERLAY_PRE_APPLY: > + ret = of_fpga_region_notify_pre_apply(region, nd); > + break; > + > + case OF_OVERLAY_POST_REMOVE: > + of_fpga_region_notify_post_remove(region, nd); > + break; > + } > + > + put_device(®ion->dev); > + > + if (ret) > + return notifier_from_errno(ret); > + > + return NOTIFY_OK; > +} > + > +static struct notifier_block fpga_region_of_nb = { > + .notifier_call = of_fpga_region_notify, > +}; > + > +static int of_fpga_region_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct fpga_region *region; > + struct fpga_manager *mgr; > + int ret; > + > + /* Find the FPGA mgr specified by region or parent region. */ > + mgr = of_fpga_region_get_mgr(np); > + if (IS_ERR(mgr)) > + return -EPROBE_DEFER; > + > + region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL); > + if (!region) { > + ret = -ENOMEM; > + goto eprobe_mgr_put; > + } > + > + region->mgr = mgr; > + > + /* Specify how to get bridges for this type of region. */ > + region->get_bridges = of_fpga_region_get_bridges; > + > + ret = fpga_region_register(dev, region); > + if (ret) > + goto eprobe_mgr_put; > + > + of_platform_populate(np, fpga_region_of_match, NULL, ®ion->dev); > + > + dev_info(dev, "FPGA Region probed\n"); > + > + return 0; > + > +eprobe_mgr_put: > + fpga_mgr_put(mgr); > + return ret; > +} > + > +static int of_fpga_region_remove(struct platform_device *pdev) > +{ > + struct fpga_region *region = platform_get_drvdata(pdev); > + > + fpga_region_unregister(region); > + > + return 0; > +} > + > +static struct platform_driver of_fpga_region_driver = { > + .probe = of_fpga_region_probe, > + .remove = of_fpga_region_remove, > + .driver = { > + .name = "of-fpga-region", > + .of_match_table = of_match_ptr(fpga_region_of_match), > + }, > +}; > + > +/** > + * fpga_region_init - init function for fpga_region class > + * Creates the fpga_region class and registers a reconfig notifier. > + */ > +static int __init of_fpga_region_init(void) > +{ > + int ret; > + > + ret = of_overlay_notifier_register(&fpga_region_of_nb); > + if (ret) > + return ret; > + > + ret = platform_driver_register(&of_fpga_region_driver); > + if (ret) > + goto err_plat; > + > + return 0; > + > +err_plat: > + of_overlay_notifier_unregister(&fpga_region_of_nb); > + return ret; > +} > + > +static void __exit of_fpga_region_exit(void) > +{ > + platform_driver_unregister(&of_fpga_region_driver); > + of_overlay_notifier_unregister(&fpga_region_of_nb); > +} > + > +subsys_initcall(of_fpga_region_init); > +module_exit(of_fpga_region_exit); > + > +MODULE_DESCRIPTION("FPGA Region"); > +MODULE_AUTHOR("Alan Tull "); > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4 > > -- > 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 >