New function of_fpga_region_parse_ov added, moving code from fpga_region_notify_pre_apply. This function gets the FPGA image info from the overlay and is able to simplify some of the logic involved.
This is a baby step in refactoring the FPGA region code to separate out common code from Device Tree overlay support. Signed-off-by: Alan Tull <at...@kernel.org> --- v2: split out from another patch v3: update comments minor changes in flow v4: no change to this patch in this version of patchset v5: no change to this patch in this version of patchset --- drivers/fpga/fpga-region.c | 122 +++++++++++++++++++++++++++------------------ 1 file changed, 73 insertions(+), 49 deletions(-) diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index eaacf50..2a8621d 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -321,33 +321,22 @@ static int child_regions_with_firmware(struct device_node *overlay) } /** - * fpga_region_notify_pre_apply - pre-apply overlay notification - * - * @region: FPGA region that the overlay was applied to - * @nd: overlay notification data - * - * Called after when an overlay targeted to a FPGA Region is about to be - * applied. Function will check the properties that will be added to the FPGA - * region. If the checks pass, it will program the FPGA. - * - * The checks are: - * The overlay must add either firmware-name or external-fpga-config property - * to the FPGA Region. - * - * firmware-name : program the FPGA - * external-fpga-config : FPGA is already programmed - * encrypted-fpga-config : FPGA bitstream is encrypted + * of_fpga_region_parse_ov - parse and check overlay applied to region * - * The overlay can add other FPGA regions, but child FPGA regions cannot have a - * firmware-name property since those regions don't exist yet. + * @region: FPGA region + * @overlay: overlay applied to the FPGA region * - * If the overlay that breaks the rules, notifier returns an error and the - * overlay is rejected before it goes into the main tree. + * Given an overlay applied to a FPGA region, parse the FPGA image specific + * info in the overlay and do some checking. * - * Returns 0 for success or negative error code for failure. + * 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 int fpga_region_notify_pre_apply(struct fpga_region *region, - struct of_overlay_notify_data *nd) +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; @@ -356,7 +345,7 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, if (region->info) { dev_err(dev, "Region already has overlay applied.\n"); - return -EINVAL; + return ERR_PTR(-EINVAL); } /* @@ -364,67 +353,102 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, * 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(nd->overlay); + ret = child_regions_with_firmware(overlay); if (ret) - return ret; + return ERR_PTR(ret); info = fpga_image_info_alloc(dev); if (!info) - return -ENOMEM; + return ERR_PTR(-ENOMEM); - info->overlay = nd->overlay; + info->overlay = overlay; /* Read FPGA region properties from the overlay */ - if (of_property_read_bool(nd->overlay, "partial-fpga-config")) + if (of_property_read_bool(overlay, "partial-fpga-config")) info->flags |= FPGA_MGR_PARTIAL_RECONFIG; - if (of_property_read_bool(nd->overlay, "external-fpga-config")) + if (of_property_read_bool(overlay, "external-fpga-config")) info->flags |= FPGA_MGR_EXTERNAL_CONFIG; - if (of_property_read_bool(nd->overlay, "encrypted-fpga-config")) + if (of_property_read_bool(overlay, "encrypted-fpga-config")) info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM; - if (!of_property_read_string(nd->overlay, "firmware-name", + 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 -ENOMEM; + return ERR_PTR(-ENOMEM); } - of_property_read_u32(nd->overlay, "region-unfreeze-timeout-us", + of_property_read_u32(overlay, "region-unfreeze-timeout-us", &info->enable_timeout_us); - of_property_read_u32(nd->overlay, "region-freeze-timeout-us", + of_property_read_u32(overlay, "region-freeze-timeout-us", &info->disable_timeout_us); - of_property_read_u32(nd->overlay, "config-complete-timeout-us", + of_property_read_u32(overlay, "config-complete-timeout-us", &info->config_complete_timeout_us); - /* If FPGA was externally programmed, don't specify firmware */ - if ((info->flags & FPGA_MGR_EXTERNAL_CONFIG) && info->firmware_name) { - dev_err(dev, "error: specified firmware and external-fpga-config"); - fpga_image_info_free(info); - return -EINVAL; + /* If overlay is not programming the FPGA, don't need FPGA image info */ + if (!info->firmware_name) { + ret = 0; + goto ret_no_info; } - /* FPGA is already configured externally. We're done. */ + /* + * If overlay informs us FPGA was externally programmed, specifying + * firmware here would be ambiguous. + */ if (info->flags & FPGA_MGR_EXTERNAL_CONFIG) { - fpga_image_info_free(info); - return 0; + dev_err(dev, "error: specified firmware and external-fpga-config"); + ret = -EINVAL; + goto ret_no_info; } - /* If we got this far, we should be programming the FPGA */ - if (!info->firmware_name) { - dev_err(dev, "should specify firmware-name or external-fpga-config\n"); - fpga_image_info_free(info); + return info; +ret_no_info: + fpga_image_info_free(info); + return ERR_PTR(ret); +} + +/** + * 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 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; } - region->info = info; + 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; } -- 2.7.4