From 9c1c4b2753fea36a072e78a5efc82fca0d13b455 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:11 -0600 Subject: fpga: bridge: support getting bridge from device Add two functions for getting the FPGA bridge from the device rather than device tree node. This is to enable writing code that will support using FPGA bridges without device tree. Rename one old function to make it clear that it is device tree-ish. This leaves us with 3 functions for getting a bridge: * fpga_bridge_get Get the bridge given the device. * fpga_bridges_get_to_list Given the device, get the bridge and add it to a list. * of_fpga_bridges_get_to_list Renamed from priviously existing fpga_bridges_get_to_list. Given the device node, get the bridge and add it to a list. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index d9ab7c75b14f..9175556215b1 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -183,11 +183,14 @@ static int fpga_region_get_bridges(struct fpga_region *region, int i, ret; /* If parent is a bridge, add to list */ - ret = fpga_bridge_get_to_list(region_np->parent, region->info, - ®ion->bridge_list); + ret = of_fpga_bridge_get_to_list(region_np->parent, region->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; @@ -207,8 +210,8 @@ static int fpga_region_get_bridges(struct fpga_region *region, continue; /* If node is a bridge, get it and add to list */ - ret = fpga_bridge_get_to_list(br, region->info, - ®ion->bridge_list); + ret = of_fpga_bridge_get_to_list(br, region->info, + ®ion->bridge_list); /* If any of the bridges are in use, give up */ if (ret == -EBUSY) { -- cgit v1.2.2 From 5cf0c7f6502f26332b46fa87914553a4d6ae75ac Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:12 -0600 Subject: fpga: mgr: API change to replace fpga load functions with single function 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. API documentation has been updated and a new document for FPGA region has been added. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 9175556215b1..120c496eb7bd 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,15 @@ 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 device *dev = ®ion->dev; 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(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 +378,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 +396,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(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(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(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(info); + region->info = NULL; + } + + return ret; } /** @@ -426,7 +439,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(region->info); region->info = NULL; } -- cgit v1.2.2 From ebf877a51ad7b65e4ab024f021b60a4f7928864a Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:13 -0600 Subject: fpga: mgr: separate getting/locking FPGA manager Previously when the user gets a FPGA manager, it was locked and nobody else could use it for programming. This commit makes it straightforward to save a reference to an FPGA manager and only lock it when programming the FPGA. Add functions that get an FPGA manager's mutex for exclusive use: * fpga_mgr_lock * fpga_mgr_unlock The following functions no longer lock an FPGA manager's mutex: * of_fpga_mgr_get * fpga_mgr_get * fpga_mgr_put Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 120c496eb7bd..1e1640a29306 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -125,7 +125,7 @@ static void fpga_region_put(struct fpga_region *region) } /** - * fpga_region_get_manager - get exclusive reference for FPGA manager + * fpga_region_get_manager - get reference for FPGA manager * @region: FPGA region * * Get FPGA Manager from "fpga-mgr" property or from ancestor region. @@ -233,6 +233,7 @@ static int fpga_region_get_bridges(struct fpga_region *region, static int fpga_region_program_fpga(struct fpga_region *region, struct device_node *overlay) { + struct device *dev = ®ion->dev; struct fpga_manager *mgr; int ret; @@ -249,10 +250,16 @@ static int fpga_region_program_fpga(struct fpga_region *region, goto err_put_region; } + ret = fpga_mgr_lock(mgr); + if (ret) { + dev_err(dev, "FPGA manager is busy\n"); + goto err_put_mgr; + } + ret = fpga_region_get_bridges(region, overlay); if (ret) { pr_err("failed to get fpga region bridges\n"); - goto err_put_mgr; + goto err_unlock_mgr; } ret = fpga_bridges_disable(®ion->bridge_list); @@ -273,6 +280,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, goto err_put_br; } + fpga_mgr_unlock(mgr); fpga_mgr_put(mgr); fpga_region_put(region); @@ -280,6 +288,8 @@ static int fpga_region_program_fpga(struct fpga_region *region, err_put_br: fpga_bridges_put(®ion->bridge_list); +err_unlock_mgr: + fpga_mgr_unlock(mgr); err_put_mgr: fpga_mgr_put(mgr); err_put_region: -- cgit v1.2.2 From c3d971ad32fc19af638ac6f6ffe041aa1ff722b5 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:14 -0600 Subject: fpga: region: use dev_err instead of pr_err Use dev_err messages instead of pr_err. Also s/®ion->dev/dev/ in two places where we already have dev = ®ion->dev. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 1e1640a29306..6b4f9abed043 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -102,7 +102,7 @@ static struct fpga_region *fpga_region_get(struct fpga_region *region) return ERR_PTR(-ENODEV); } - dev_dbg(®ion->dev, "get\n"); + dev_dbg(dev, "get\n"); return region; } @@ -116,7 +116,7 @@ static void fpga_region_put(struct fpga_region *region) { struct device *dev = ®ion->dev; - dev_dbg(®ion->dev, "put\n"); + dev_dbg(dev, "put\n"); module_put(dev->parent->driver->owner); of_node_put(dev->of_node); @@ -239,13 +239,13 @@ static int fpga_region_program_fpga(struct fpga_region *region, region = fpga_region_get(region); if (IS_ERR(region)) { - pr_err("failed to get fpga region\n"); + dev_err(dev, "failed to get FPGA region\n"); return PTR_ERR(region); } mgr = fpga_region_get_manager(region); if (IS_ERR(mgr)) { - pr_err("failed to get fpga region manager\n"); + dev_err(dev, "failed to get FPGA manager\n"); ret = PTR_ERR(mgr); goto err_put_region; } @@ -258,25 +258,25 @@ static int fpga_region_program_fpga(struct fpga_region *region, ret = fpga_region_get_bridges(region, overlay); if (ret) { - pr_err("failed to get fpga region bridges\n"); + dev_err(dev, "failed to get FPGA bridges\n"); goto err_unlock_mgr; } ret = fpga_bridges_disable(®ion->bridge_list); if (ret) { - pr_err("failed to disable region bridges\n"); + dev_err(dev, "failed to disable bridges\n"); goto err_put_br; } ret = fpga_mgr_load(mgr, region->info); if (ret) { - pr_err("failed to load fpga image\n"); + dev_err(dev, "failed to load FPGA image\n"); goto err_put_br; } ret = fpga_bridges_enable(®ion->bridge_list); if (ret) { - pr_err("failed to enable region bridges\n"); + dev_err(dev, "failed to enable region bridges\n"); goto err_put_br; } @@ -407,7 +407,7 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, /* If FPGA was externally programmed, don't specify firmware */ if ((info->flags & FPGA_MGR_EXTERNAL_CONFIG) && info->firmware_name) { - pr_err("error: specified firmware and external-fpga-config"); + dev_err(dev, "error: specified firmware and external-fpga-config"); fpga_image_info_free(info); return -EINVAL; } @@ -420,7 +420,7 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, /* If we got this far, we should be programming the FPGA */ if (!info->firmware_name) { - pr_err("should specify firmware-name or external-fpga-config\n"); + dev_err(dev, "should specify firmware-name or external-fpga-config\n"); fpga_image_info_free(info); return -EINVAL; } -- cgit v1.2.2 From 08bcb4b1a7615762e1e2c87a5c8cc5d9709b60f6 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:15 -0600 Subject: fpga: region: remove unneeded of_node_get and put Remove of_node_get/put in fpga_region_get/put. Not needed and will get in the way when I separate out the common FPGA region code from Device Tree support code. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 6b4f9abed043..352661fc6b14 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -94,9 +94,7 @@ static struct fpga_region *fpga_region_get(struct fpga_region *region) } get_device(dev); - of_node_get(dev->of_node); if (!try_module_get(dev->parent->driver->owner)) { - of_node_put(dev->of_node); put_device(dev); mutex_unlock(®ion->mutex); return ERR_PTR(-ENODEV); @@ -119,7 +117,6 @@ static void fpga_region_put(struct fpga_region *region) dev_dbg(dev, "put\n"); module_put(dev->parent->driver->owner); - of_node_put(dev->of_node); put_device(dev); mutex_unlock(®ion->mutex); } -- cgit v1.2.2 From 1df2dd7f587107ebf3c8e3733410627cf5c3b3ec Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:16 -0600 Subject: fpga: region: get mgr early on Get the FPGA manager during region creation. 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 Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 352661fc6b14..d78f444c1350 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -31,12 +31,14 @@ * @dev: FPGA Region device * @mutex: enforces exclusive reference to region * @bridge_list: list of FPGA bridges specified in region + * @mgr: FPGA manager * @info: fpga image specific information */ struct fpga_region { struct device dev; struct mutex mutex; /* for exclusive reference to region */ struct list_head bridge_list; + struct fpga_manager *mgr; struct fpga_image_info *info; }; @@ -123,7 +125,7 @@ static void fpga_region_put(struct fpga_region *region) /** * fpga_region_get_manager - get reference for FPGA manager - * @region: FPGA region + * @np: device node of FPGA region * * Get FPGA Manager from "fpga-mgr" property or from ancestor region. * @@ -131,10 +133,8 @@ static void fpga_region_put(struct fpga_region *region) * * Return: fpga manager struct or IS_ERR() condition containing error code. */ -static struct fpga_manager *fpga_region_get_manager(struct fpga_region *region) +static struct fpga_manager *fpga_region_get_manager(struct device_node *np) { - struct device *dev = ®ion->dev; - struct device_node *np = dev->of_node; struct device_node *mgr_node; struct fpga_manager *mgr; @@ -231,7 +231,6 @@ static int fpga_region_program_fpga(struct fpga_region *region, struct device_node *overlay) { struct device *dev = ®ion->dev; - struct fpga_manager *mgr; int ret; region = fpga_region_get(region); @@ -240,17 +239,10 @@ static int fpga_region_program_fpga(struct fpga_region *region, return PTR_ERR(region); } - mgr = fpga_region_get_manager(region); - if (IS_ERR(mgr)) { - dev_err(dev, "failed to get FPGA manager\n"); - ret = PTR_ERR(mgr); - goto err_put_region; - } - - ret = fpga_mgr_lock(mgr); + ret = fpga_mgr_lock(region->mgr); if (ret) { dev_err(dev, "FPGA manager is busy\n"); - goto err_put_mgr; + goto err_put_region; } ret = fpga_region_get_bridges(region, overlay); @@ -265,7 +257,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, goto err_put_br; } - ret = fpga_mgr_load(mgr, region->info); + ret = fpga_mgr_load(region->mgr, region->info); if (ret) { dev_err(dev, "failed to load FPGA image\n"); goto err_put_br; @@ -277,8 +269,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, goto err_put_br; } - fpga_mgr_unlock(mgr); - fpga_mgr_put(mgr); + fpga_mgr_unlock(region->mgr); fpga_region_put(region); return 0; @@ -286,9 +277,7 @@ static int fpga_region_program_fpga(struct fpga_region *region, err_put_br: fpga_bridges_put(®ion->bridge_list); err_unlock_mgr: - fpga_mgr_unlock(mgr); -err_put_mgr: - fpga_mgr_put(mgr); + fpga_mgr_unlock(region->mgr); err_put_region: fpga_region_put(region); @@ -517,11 +506,20 @@ static int 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 id, ret = 0; + mgr = fpga_region_get_manager(np); + if (IS_ERR(mgr)) + return -EPROBE_DEFER; + region = kzalloc(sizeof(*region), GFP_KERNEL); - if (!region) - return -ENOMEM; + if (!region) { + ret = -ENOMEM; + goto err_put_mgr; + } + + region->mgr = mgr; id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL); if (id < 0) { @@ -557,6 +555,8 @@ err_remove: ida_simple_remove(&fpga_region_ida, id); err_kfree: kfree(region); +err_put_mgr: + fpga_mgr_put(mgr); return ret; } @@ -566,6 +566,7 @@ static int fpga_region_remove(struct platform_device *pdev) struct fpga_region *region = platform_get_drvdata(pdev); device_unregister(®ion->dev); + fpga_mgr_put(region->mgr); return 0; } -- cgit v1.2.2 From 1743df83ae5a949037eb86a12f225abd4374d176 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:17 -0600 Subject: fpga: region: check for child regions before allocing image info During a device tree overlay pre-apply notification, the check for child FPGA regions can happen slightly earlier. This saves us from allocating the FPGA image info that just gets thrown away. 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 Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index d78f444c1350..afac5433978b 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -355,15 +355,19 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, const char *firmware_name; int ret; - info = fpga_image_info_alloc(dev); - if (!info) - return -ENOMEM; - - /* Reject overlay if child FPGA Regions have firmware-name property */ + /* + * 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(nd->overlay); if (ret) return ret; + info = fpga_image_info_alloc(dev); + if (!info) + return -ENOMEM; + /* Read FPGA region properties from the overlay */ if (of_property_read_bool(nd->overlay, "partial-fpga-config")) info->flags |= FPGA_MGR_PARTIAL_RECONFIG; -- cgit v1.2.2 From ed81f5fc3c35c22d0fc62813cfa4e11b6aea0a64 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:18 -0600 Subject: fpga: region: fix slow warning with more than one overlay When DT overlays are applied, each FPGA region keeps track of the fpga image info as region->info. This pointer is assigned only if an overlay causes the FPGA to be programmed. As it stands, this pointer can be overwritten, causing a slow warning later when overlays are removed. This patch fixes this by changing the allowed behaviour. If a region has received an overlay that programmed the FPGA, reject other overlays that try to program the FPGA. To reprogram the FPGA, first remove the overlay. This makes sense as removing the overlay also removes the devices cleanly. Note that overlays that make DT changes without reprogramming the FPGA are exempt from this restriction. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index afac5433978b..35af952a889a 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -355,6 +355,11 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, const char *firmware_name; int ret; + if (region->info) { + dev_err(dev, "Region already has overlay applied.\n"); + return -EINVAL; + } + /* * Reject overlay if child FPGA Regions added in the overlay have * firmware-name property (would mean that an FPGA region that has -- cgit v1.2.2 From 61c32102391ff38dfd4aba835dd0f99db6b46908 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:19 -0600 Subject: fpga: region: use image info as parameter for programming region Use FPGA image info (region->info) when region code is programming the FPGA to pass in multiple parameters. 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 Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 35af952a889a..eaacf5049381 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -223,14 +223,13 @@ 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. + * Program an FPGA using fpga image info (region->info). * Return 0 for success or negative error code. */ -static int fpga_region_program_fpga(struct fpga_region *region, - struct device_node *overlay) +static int fpga_region_program_fpga(struct fpga_region *region) { struct device *dev = ®ion->dev; + struct fpga_image_info *info = region->info; int ret; region = fpga_region_get(region); @@ -245,7 +244,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 +256,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; @@ -373,6 +372,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; @@ -421,7 +422,8 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, } region->info = info; - ret = fpga_region_program_fpga(region, nd->overlay); + + ret = fpga_region_program_fpga(region); if (ret) { fpga_image_info_free(info); region->info = NULL; -- cgit v1.2.2 From c8898eda81e0b949ca214e1a45ce1b56677eb849 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:20 -0600 Subject: fpga: region: separate out code that parses the overlay 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 Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 122 +++++++++++++++++++++++++++------------------ 1 file changed, 73 insertions(+), 49 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index eaacf5049381..2a8621db5f5b 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; } -- cgit v1.2.2 From 59460a9305458ac3e7f2415b602dbaa6cfcb8a3b Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:21 -0600 Subject: fpga: region: add fpga-region.h header * Create fpga-region.h. * Export fpga_region_program_fpga. * Move struct fpga_region and other things to the header. This is a step in separating FPGA region common code from Device Tree support. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 2a8621db5f5b..402d0b68b97a 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -26,24 +27,6 @@ #include #include -/** - * struct fpga_region - FPGA Region structure - * @dev: FPGA Region device - * @mutex: enforces exclusive reference to region - * @bridge_list: list of FPGA bridges specified in region - * @mgr: FPGA manager - * @info: fpga image specific information - */ -struct fpga_region { - struct device dev; - struct mutex mutex; /* for exclusive reference to region */ - struct list_head bridge_list; - struct fpga_manager *mgr; - struct fpga_image_info *info; -}; - -#define to_fpga_region(d) container_of(d, struct fpga_region, dev) - static DEFINE_IDA(fpga_region_ida); static struct class *fpga_region_class; @@ -226,7 +209,7 @@ static int fpga_region_get_bridges(struct fpga_region *region, * Program an FPGA using fpga image info (region->info). * Return 0 for success or negative error code. */ -static int fpga_region_program_fpga(struct fpga_region *region) +int fpga_region_program_fpga(struct fpga_region *region) { struct device *dev = ®ion->dev; struct fpga_image_info *info = region->info; @@ -282,6 +265,7 @@ err_put_region: return ret; } +EXPORT_SYMBOL_GPL(fpga_region_program_fpga); /** * child_regions_with_firmware @@ -667,5 +651,5 @@ subsys_initcall(fpga_region_init); module_exit(fpga_region_exit); MODULE_DESCRIPTION("FPGA Region"); -MODULE_AUTHOR("Alan Tull "); +MODULE_AUTHOR("Alan Tull "); MODULE_LICENSE("GPL v2"); -- cgit v1.2.2 From 3b49537f8084af15ccaac542eaf317e01c6869e6 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:22 -0600 Subject: fpga: region: rename some functions prior to moving Rename some functions that will be moved to of-fpga-region.c. Also change some parameters and export a function to help with refactoring. This is a step towards the larger goal of separating device tree support from FPGA region common code. * fpga_region_get_manager -> of_fpga_region_get_mgr * add 'of_' prefix to the following: * fpga_region_find * fpga_region_get_bridges * fpga_region_notify_pre_apply * fpga_region_notify_post_remove), * fpga_region_probe/remove Parameter changes: * of_fpga_region_find change parameter to be the device node of the region. * of_fpga_region_get_bridges change second parameter to FPGA image info. Export of_fpga_region_find as well. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 60 ++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 29 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 402d0b68b97a..92ab21651aeb 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -42,12 +42,14 @@ static int fpga_region_of_node_match(struct device *dev, const void *data) } /** - * fpga_region_find - find FPGA region + * 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 *fpga_region_find(struct device_node *np) +static struct fpga_region *of_fpga_region_find(struct device_node *np) { struct device *dev; @@ -107,7 +109,7 @@ static void fpga_region_put(struct fpga_region *region) } /** - * fpga_region_get_manager - get reference for FPGA manager + * 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. @@ -116,7 +118,7 @@ static void fpga_region_put(struct fpga_region *region) * * Return: fpga manager struct or IS_ERR() condition containing error code. */ -static struct fpga_manager *fpga_region_get_manager(struct device_node *np) +static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np) { struct device_node *mgr_node; struct fpga_manager *mgr; @@ -139,9 +141,9 @@ static struct fpga_manager *fpga_region_get_manager(struct device_node *np) } /** - * fpga_region_get_bridges - create a list of bridges + * of_fpga_region_get_bridges - create a list of bridges * @region: FPGA region - * @overlay: device node of the overlay + * @info: FPGA image info * * Create a list of bridges including the parent bridge and the bridges * specified by "fpga-bridges" property. Note that the @@ -154,8 +156,8 @@ static struct fpga_manager *fpga_region_get_manager(struct device_node *np) * Return 0 for success (even if there are no bridges specified) * or -EBUSY if any of the bridges are in use. */ -static int fpga_region_get_bridges(struct fpga_region *region, - struct device_node *overlay) +static int of_fpga_region_get_bridges(struct fpga_region *region, + struct fpga_image_info *info) { struct device *dev = ®ion->dev; struct device_node *region_np = dev->of_node; @@ -163,7 +165,7 @@ static int fpga_region_get_bridges(struct fpga_region *region, int i, ret; /* If parent is a bridge, add to list */ - ret = of_fpga_bridge_get_to_list(region_np->parent, region->info, + 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. */ @@ -175,8 +177,8 @@ static int fpga_region_get_bridges(struct fpga_region *region, parent_br = region_np->parent; /* If overlay has a list of bridges, use it. */ - if (of_parse_phandle(overlay, "fpga-bridges", 0)) - np = overlay; + if (of_parse_phandle(info->overlay, "fpga-bridges", 0)) + np = info->overlay; else np = region_np; @@ -227,7 +229,7 @@ int fpga_region_program_fpga(struct fpga_region *region) goto err_put_region; } - ret = fpga_region_get_bridges(region, info->overlay); + ret = of_fpga_region_get_bridges(region, info); if (ret) { dev_err(dev, "failed to get FPGA bridges\n"); goto err_unlock_mgr; @@ -397,7 +399,7 @@ ret_no_info: } /** - * fpga_region_notify_pre_apply - pre-apply overlay notification + * of_fpga_region_notify_pre_apply - pre-apply overlay notification * * @region: FPGA region that the overlay was applied to * @nd: overlay notification data @@ -410,8 +412,8 @@ ret_no_info: * * 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) +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; @@ -441,7 +443,7 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, } /** - * fpga_region_notify_post_remove - post-remove overlay notification + * 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 @@ -449,8 +451,8 @@ static int fpga_region_notify_pre_apply(struct fpga_region *region, * Called after an overlay has been removed if the overlay's target was a * FPGA region. */ -static void fpga_region_notify_post_remove(struct fpga_region *region, - struct of_overlay_notify_data *nd) +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); @@ -493,18 +495,18 @@ static int of_fpga_region_notify(struct notifier_block *nb, return NOTIFY_OK; } - region = fpga_region_find(nd->target); + region = of_fpga_region_find(nd->target); if (!region) return NOTIFY_OK; ret = 0; switch (action) { case OF_OVERLAY_PRE_APPLY: - ret = fpga_region_notify_pre_apply(region, nd); + ret = of_fpga_region_notify_pre_apply(region, nd); break; case OF_OVERLAY_POST_REMOVE: - fpga_region_notify_post_remove(region, nd); + of_fpga_region_notify_post_remove(region, nd); break; } @@ -520,7 +522,7 @@ static struct notifier_block fpga_region_of_nb = { .notifier_call = of_fpga_region_notify, }; -static int fpga_region_probe(struct platform_device *pdev) +static int of_fpga_region_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; @@ -528,7 +530,7 @@ static int fpga_region_probe(struct platform_device *pdev) struct fpga_manager *mgr; int id, ret = 0; - mgr = fpga_region_get_manager(np); + mgr = of_fpga_region_get_mgr(np); if (IS_ERR(mgr)) return -EPROBE_DEFER; @@ -580,7 +582,7 @@ err_put_mgr: return ret; } -static int fpga_region_remove(struct platform_device *pdev) +static int of_fpga_region_remove(struct platform_device *pdev) { struct fpga_region *region = platform_get_drvdata(pdev); @@ -590,9 +592,9 @@ static int fpga_region_remove(struct platform_device *pdev) return 0; } -static struct platform_driver fpga_region_driver = { - .probe = fpga_region_probe, - .remove = fpga_region_remove, +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), @@ -625,7 +627,7 @@ static int __init fpga_region_init(void) if (ret) goto err_class; - ret = platform_driver_register(&fpga_region_driver); + ret = platform_driver_register(&of_fpga_region_driver); if (ret) goto err_plat; @@ -641,7 +643,7 @@ err_class: static void __exit fpga_region_exit(void) { - platform_driver_unregister(&fpga_region_driver); + 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); -- cgit v1.2.2 From 52a3a7ccce07e73323fc1bae9eb0b0b63375391c Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:23 -0600 Subject: fpga: region: add register/unregister functions Another step in separating common code from device tree specific code for FPGA regions. * add FPGA region register/unregister functions. * add the register/unregister functions to the header * use devm_kzalloc to alloc the region. * add a method for getting bridges to the region struct * add priv to the region struct * use region->info in of_fpga_region_get_bridges Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 105 ++++++++++++++++++++++++++++----------------- 1 file changed, 65 insertions(+), 40 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 92ab21651aeb..76db81de2cc0 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -143,7 +143,6 @@ static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np) /** * of_fpga_region_get_bridges - create a list of bridges * @region: FPGA region - * @info: FPGA image info * * Create a list of bridges including the parent bridge and the bridges * specified by "fpga-bridges" property. Note that the @@ -156,11 +155,11 @@ static struct fpga_manager *of_fpga_region_get_mgr(struct device_node *np) * 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 fpga_image_info *info) +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; @@ -192,7 +191,7 @@ static int of_fpga_region_get_bridges(struct fpga_region *region, continue; /* If node is a bridge, get it and add to list */ - ret = of_fpga_bridge_get_to_list(br, region->info, + ret = of_fpga_bridge_get_to_list(br, info, ®ion->bridge_list); /* If any of the bridges are in use, give up */ @@ -229,10 +228,16 @@ int fpga_region_program_fpga(struct fpga_region *region) goto err_put_region; } - ret = of_fpga_region_get_bridges(region, info); - if (ret) { - dev_err(dev, "failed to get FPGA bridges\n"); - goto err_unlock_mgr; + /* + * In some cases, we already have a list of bridges in the + * fpga region struct. Or we don't have any bridges. + */ + if (region->get_bridges) { + ret = region->get_bridges(region); + if (ret) { + dev_err(dev, "failed to get fpga region bridges\n"); + goto err_unlock_mgr; + } } ret = fpga_bridges_disable(®ion->bridge_list); @@ -259,7 +264,8 @@ int fpga_region_program_fpga(struct fpga_region *region) return 0; err_put_br: - fpga_bridges_put(®ion->bridge_list); + if (region->get_bridges) + fpga_bridges_put(®ion->bridge_list); err_unlock_mgr: fpga_mgr_unlock(region->mgr); err_put_region: @@ -522,39 +528,20 @@ static struct notifier_block fpga_region_of_nb = { .notifier_call = of_fpga_region_notify, }; -static int of_fpga_region_probe(struct platform_device *pdev) +int fpga_region_register(struct device *dev, struct fpga_region *region) { - struct device *dev = &pdev->dev; - struct device_node *np = dev->of_node; - struct fpga_region *region; - struct fpga_manager *mgr; int id, ret = 0; - mgr = of_fpga_region_get_mgr(np); - if (IS_ERR(mgr)) - return -EPROBE_DEFER; - - region = kzalloc(sizeof(*region), GFP_KERNEL); - if (!region) { - ret = -ENOMEM; - goto err_put_mgr; - } - - region->mgr = mgr; - id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL); - if (id < 0) { - ret = id; - goto err_kfree; - } + if (id < 0) + return id; mutex_init(®ion->mutex); INIT_LIST_HEAD(®ion->bridge_list); - device_initialize(®ion->dev); region->dev.class = fpga_region_class; region->dev.parent = dev; - region->dev.of_node = np; + region->dev.of_node = dev->of_node; region->dev.id = id; dev_set_drvdata(dev, region); @@ -566,19 +553,58 @@ static int of_fpga_region_probe(struct platform_device *pdev) if (ret) goto err_remove; + return 0; + +err_remove: + ida_simple_remove(&fpga_region_ida, id); + return ret; +} +EXPORT_SYMBOL_GPL(fpga_region_register); + +int fpga_region_unregister(struct fpga_region *region) +{ + device_unregister(®ion->dev); + + return 0; +} +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; -err_remove: - ida_simple_remove(&fpga_region_ida, id); -err_kfree: - kfree(region); -err_put_mgr: +eprobe_mgr_put: fpga_mgr_put(mgr); - return ret; } @@ -586,7 +612,7 @@ static int of_fpga_region_remove(struct platform_device *pdev) { struct fpga_region *region = platform_get_drvdata(pdev); - device_unregister(®ion->dev); + fpga_region_unregister(region); fpga_mgr_put(region->mgr); return 0; @@ -606,7 +632,6 @@ static void fpga_region_dev_release(struct device *dev) struct fpga_region *region = to_fpga_region(dev); ida_simple_remove(&fpga_region_ida, region->dev.id); - kfree(region); } /** -- cgit v1.2.2 From 503d4b7a446b3838785fa7f21e339941a5d1c2d5 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:24 -0600 Subject: fpga: region: add fpga_region_class_find Add a function for searching the fpga-region class. This will be useful when device tree code is no longer in the same file that declares the fpga-region class. Another step in separating common FPGA region code from device tree support. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 76db81de2cc0..5c0869576cd1 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -30,6 +30,20 @@ static DEFINE_IDA(fpga_region_ida); static struct class *fpga_region_class; +struct fpga_region *fpga_region_class_find( + struct device *start, const void *data, + int (*match)(struct device *, const void *)) +{ + struct device *dev; + + dev = class_find_device(fpga_region_class, start, data, match); + if (!dev) + return NULL; + + return to_fpga_region(dev); +} +EXPORT_SYMBOL_GPL(fpga_region_class_find); + static const struct of_device_id fpga_region_of_match[] = { { .compatible = "fpga-region", }, {}, @@ -51,14 +65,7 @@ static int fpga_region_of_node_match(struct device *dev, const void *data) */ static struct fpga_region *of_fpga_region_find(struct device_node *np) { - struct device *dev; - - dev = class_find_device(fpga_region_class, NULL, np, - fpga_region_of_node_match); - if (!dev) - return NULL; - - return to_fpga_region(dev); + return fpga_region_class_find(NULL, np, fpga_region_of_node_match); } /** -- cgit v1.2.2 From ef3acdd820752e0abb5f1ec899025967d0dccf3d Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:25 -0600 Subject: fpga: region: move device tree support to of-fpga-region.c Create of-fpga-region.c and move the following functions without 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 functions with some code from fpga_region_init/exit. * of_fpga_region_init * of_fpga_region_exit Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 452 +-------------------------------------------- 1 file changed, 1 insertion(+), 451 deletions(-) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index 5c0869576cd1..afc61885a601 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 @@ -115,102 +91,6 @@ static void fpga_region_put(struct fpga_region *region) mutex_unlock(®ion->mutex); } -/** - * 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 @@ -282,259 +162,6 @@ err_put_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,64 +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); - fpga_mgr_put(region->mgr); - - 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); @@ -647,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); } -- cgit v1.2.2 From 845089bbf589be75143d0c9fb326d5595c1b5787 Mon Sep 17 00:00:00 2001 From: Alan Tull Date: Wed, 15 Nov 2017 14:20:28 -0600 Subject: fpga: add attribute groups Make it easy to add attributes to low level FPGA drivers the right way. Add attribute groups pointers to structures that are used when registering a manager, bridge, or group. When the low level driver registers, set the device attribute group. The attributes are created in device_add. Signed-off-by: Alan Tull Acked-by: Moritz Fischer Signed-off-by: Greg Kroah-Hartman --- drivers/fpga/fpga-region.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/fpga/fpga-region.c') diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index afc61885a601..edab2a2e03ef 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -173,6 +173,7 @@ int fpga_region_register(struct device *dev, struct fpga_region *region) mutex_init(®ion->mutex); INIT_LIST_HEAD(®ion->bridge_list); device_initialize(®ion->dev); + region->dev.groups = region->groups; region->dev.class = fpga_region_class; region->dev.parent = dev; region->dev.of_node = dev->of_node; -- cgit v1.2.2