* [PATCH v1 0/3] fpga: Use standard class dev_release function @ 2021-05-21 1:03 Russ Weight 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Russ Weight @ 2021-05-21 1:03 UTC (permalink / raw) To: mdf, linux-fpga, linux-kernel Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight The FPGA framework has a convention of using managed resource functions to allow parent drivers to manage the data structures allocated by the class drivers. They use an empty *_dev_release() function to satisfy the class driver. This is inconsistent with linux driver model. These changes remove the managed resource functions and populate the class dev_release callback functions. They also merge the create and register functions into a single register function for each of the fpga-mgr, fpga-region, and fpga-bridge class drivers. For more context, refer to this email thread: https://marc.info/?l=linux-fpga&m=162127412218557&w=2 I turned on the configs assocated with each of the modified files, but I must have been missing some dependencies, because not all of them compiled. I did a run-time test specifically with the dfl-fme infrastructure. This would have exercised the region, bridge, and fpga-mgr frameworks. - Russ Russ Weight (3): fpga: mgr: Use standard dev_release for class driver fpga: bridge: Use standard dev_release for class driver fpga: region: Use standard dev_release for class driver drivers/fpga/altera-cvp.c | 12 +- drivers/fpga/altera-fpga2sdram.c | 12 +- drivers/fpga/altera-freeze-bridge.c | 10 +- drivers/fpga/altera-hps2fpga.c | 12 +- drivers/fpga/altera-pr-ip-core.c | 6 +- drivers/fpga/altera-ps-spi.c | 8 +- drivers/fpga/dfl-fme-br.c | 10 +- drivers/fpga/dfl-fme-mgr.c | 10 +- drivers/fpga/dfl-fme-region.c | 10 +- drivers/fpga/dfl.c | 10 +- drivers/fpga/fpga-bridge.c | 113 ++++-------------- drivers/fpga/fpga-mgr.c | 177 ++++------------------------ drivers/fpga/fpga-region.c | 97 +++------------ drivers/fpga/ice40-spi.c | 8 +- drivers/fpga/machxo2-spi.c | 8 +- drivers/fpga/of-fpga-region.c | 10 +- drivers/fpga/socfpga-a10.c | 16 +-- drivers/fpga/socfpga.c | 8 +- drivers/fpga/stratix10-soc.c | 15 +-- drivers/fpga/ts73xx-fpga.c | 8 +- drivers/fpga/xilinx-pr-decoupler.c | 17 +-- drivers/fpga/xilinx-spi.c | 10 +- drivers/fpga/zynq-fpga.c | 16 +-- drivers/fpga/zynqmp-fpga.c | 8 +- include/linux/fpga/fpga-bridge.h | 12 +- include/linux/fpga/fpga-mgr.h | 14 +-- include/linux/fpga/fpga-region.h | 12 +- 27 files changed, 150 insertions(+), 499 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver 2021-05-21 1:03 [PATCH v1 0/3] fpga: Use standard class dev_release function Russ Weight @ 2021-05-21 1:03 ` Russ Weight 2021-05-21 15:27 ` Xu Yilun ` (2 more replies) 2021-05-21 1:03 ` [PATCH v1 2/3] fpga: bridge: " Russ Weight 2021-05-21 1:03 ` [PATCH v1 3/3] fpga: region: " Russ Weight 2 siblings, 3 replies; 11+ messages in thread From: Russ Weight @ 2021-05-21 1:03 UTC (permalink / raw) To: mdf, linux-fpga, linux-kernel Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight The FPGA manager class driver data structure is being treated as a managed resource instead of using standard dev_release call-back to release the class data structure. This change removes the managed resource code and combines the create() and register() functions into a single register() function. Signed-off-by: Russ Weight <russell.h.weight@intel.com> --- drivers/fpga/altera-cvp.c | 12 +-- drivers/fpga/altera-pr-ip-core.c | 6 +- drivers/fpga/altera-ps-spi.c | 8 +- drivers/fpga/dfl-fme-mgr.c | 10 +- drivers/fpga/fpga-mgr.c | 177 ++++--------------------------- drivers/fpga/ice40-spi.c | 8 +- drivers/fpga/machxo2-spi.c | 8 +- drivers/fpga/socfpga-a10.c | 16 ++- drivers/fpga/socfpga.c | 8 +- drivers/fpga/stratix10-soc.c | 15 +-- drivers/fpga/ts73xx-fpga.c | 8 +- drivers/fpga/xilinx-spi.c | 10 +- drivers/fpga/zynq-fpga.c | 16 ++- drivers/fpga/zynqmp-fpga.c | 8 +- include/linux/fpga/fpga-mgr.h | 14 +-- 15 files changed, 74 insertions(+), 250 deletions(-) diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c index 4e0edb60bfba..b61020ceb0a1 100644 --- a/drivers/fpga/altera-cvp.c +++ b/drivers/fpga/altera-cvp.c @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev, snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s", ALTERA_CVP_MGR_NAME, pci_name(pdev)); - mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name, - &altera_cvp_ops, conf); - if (!mgr) { - ret = -ENOMEM; + mgr = fpga_mgr_register(&pdev->dev, conf->mgr_name, + &altera_cvp_ops, conf); + if (IS_ERR(mgr)) { + ret = PTR_ERR(mgr); goto err_unmap; } pci_set_drvdata(pdev, mgr); - ret = fpga_mgr_register(mgr); - if (ret) - goto err_unmap; - return 0; err_unmap: diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c index 5b130c4d9882..2eb5505a7031 100644 --- a/drivers/fpga/altera-pr-ip-core.c +++ b/drivers/fpga/altera-pr-ip-core.c @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base) (val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT, (int)(val & ALT_PR_CSR_PR_START)); - mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv); - return devm_fpga_mgr_register(dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } EXPORT_SYMBOL_GPL(alt_pr_register); diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c index 23bfd4d1ad0f..c9693d852a18 100644 --- a/drivers/fpga/altera-ps-spi.c +++ b/drivers/fpga/altera-ps-spi.c @@ -302,12 +302,10 @@ static int altera_ps_probe(struct spi_device *spi) snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s", dev_driver_string(&spi->dev), dev_name(&spi->dev)); - mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name, - &altera_ps_ops, conf); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(&spi->dev, conf->mgr_name, + &altera_ps_ops, conf); - return devm_fpga_mgr_register(&spi->dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } static const struct spi_device_id altera_ps_spi_ids[] = { diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c index d5861d13b306..dd8ff7c4ec10 100644 --- a/drivers/fpga/dfl-fme-mgr.c +++ b/drivers/fpga/dfl-fme-mgr.c @@ -308,14 +308,14 @@ static int fme_mgr_probe(struct platform_device *pdev) fme_mgr_get_compat_id(priv->ioaddr, compat_id); - mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager", - &fme_mgr_ops, priv); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(dev, "DFL FME FPGA Manager", + &fme_mgr_ops, priv); + if (IS_ERR(mgr)) + return PTR_ERR(mgr); mgr->compat_id = compat_id; - return devm_fpga_mgr_register(dev, mgr); + return 0; } static struct platform_driver fme_mgr_driver = { diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index b85bc47c91a9..e95226fbf18c 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -21,10 +21,6 @@ static DEFINE_IDA(fpga_mgr_ida); static struct class *fpga_mgr_class; -struct fpga_mgr_devres { - struct fpga_manager *mgr; -}; - /** * fpga_image_info_alloc - Allocate a FPGA image info struct * @dev: owning device @@ -550,20 +546,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr) EXPORT_SYMBOL_GPL(fpga_mgr_unlock); /** - * fpga_mgr_create - create and initialize a FPGA manager struct + * fpga_mgr_register - create and register a FPGA manager struct * @dev: fpga manager device from pdev * @name: fpga manager name * @mops: pointer to structure of fpga manager ops * @priv: fpga manager private data * - * The caller of this function is responsible for freeing the struct with - * fpga_mgr_free(). Using devm_fpga_mgr_create() instead is recommended. - * - * Return: pointer to struct fpga_manager or NULL + * Returns a struct fpga_manager pointer on success, or ERR_PTR() on error. */ -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, - const struct fpga_manager_ops *mops, - void *priv) +struct fpga_manager * +fpga_mgr_register(struct device *dev, const char *name, + const struct fpga_manager_ops *mops, void *priv) { struct fpga_manager *mgr; int id, ret; @@ -572,17 +565,17 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, !mops->write_init || (!mops->write && !mops->write_sg) || (mops->write && mops->write_sg)) { dev_err(dev, "Attempt to register without fpga_manager_ops\n"); - return NULL; + return ERR_PTR(-EINVAL); } if (!name || !strlen(name)) { dev_err(dev, "Attempt to register with no name!\n"); - return NULL; + return ERR_PTR(-EINVAL); } mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); if (!mgr) - return NULL; + return ERR_PTR(-ENOMEM); id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); if (id < 0) @@ -594,7 +587,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, mgr->mops = mops; mgr->priv = priv; - device_initialize(&mgr->dev); mgr->dev.class = fpga_mgr_class; mgr->dev.groups = mops->groups; mgr->dev.parent = dev; @@ -605,84 +597,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, if (ret) goto error_device; - return mgr; - -error_device: - ida_simple_remove(&fpga_mgr_ida, id); -error_kfree: - kfree(mgr); - - return NULL; -} -EXPORT_SYMBOL_GPL(fpga_mgr_create); - -/** - * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create() - * @mgr: fpga manager struct - */ -void fpga_mgr_free(struct fpga_manager *mgr) -{ - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); - kfree(mgr); -} -EXPORT_SYMBOL_GPL(fpga_mgr_free); - -static void devm_fpga_mgr_release(struct device *dev, void *res) -{ - struct fpga_mgr_devres *dr = res; - - fpga_mgr_free(dr->mgr); -} - -/** - * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct - * @dev: fpga manager device from pdev - * @name: fpga manager name - * @mops: pointer to structure of fpga manager ops - * @priv: fpga manager private data - * - * This function is intended for use in a FPGA manager driver's probe function. - * After the manager driver creates the manager struct with - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register(). The - * manager driver's remove function should call fpga_mgr_unregister(). The - * manager struct allocated with this function will be freed automatically on - * driver detach. This includes the case of a probe function returning error - * before calling fpga_mgr_register(), the struct will still get cleaned up. - * - * Return: pointer to struct fpga_manager or NULL - */ -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, - const struct fpga_manager_ops *mops, - void *priv) -{ - struct fpga_mgr_devres *dr; - - dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL); - if (!dr) - return NULL; - - dr->mgr = fpga_mgr_create(dev, name, mops, priv); - if (!dr->mgr) { - devres_free(dr); - return NULL; - } - - devres_add(dev, dr); - - return dr->mgr; -} -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create); - -/** - * fpga_mgr_register - register a FPGA manager - * @mgr: fpga manager struct - * - * Return: 0 on success, negative error code otherwise. - */ -int fpga_mgr_register(struct fpga_manager *mgr) -{ - int ret; - /* * Initialize framework state by requesting low level driver read state * from device. FPGA may be in reset mode or may have been programmed @@ -690,18 +604,20 @@ int fpga_mgr_register(struct fpga_manager *mgr) */ mgr->state = mgr->mops->state(mgr); - ret = device_add(&mgr->dev); - if (ret) - goto error_device; - - dev_info(&mgr->dev, "%s registered\n", mgr->name); + ret = device_register(&mgr->dev); + if (ret) { + put_device(&mgr->dev); + return ERR_PTR(ret); + } - return 0; + return mgr; error_device: - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); + ida_simple_remove(&fpga_mgr_ida, id); +error_kfree: + kfree(mgr); - return ret; + return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(fpga_mgr_register); @@ -726,61 +642,12 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) } EXPORT_SYMBOL_GPL(fpga_mgr_unregister); -static int fpga_mgr_devres_match(struct device *dev, void *res, - void *match_data) -{ - struct fpga_mgr_devres *dr = res; - - return match_data == dr->mgr; -} - -static void devm_fpga_mgr_unregister(struct device *dev, void *res) -{ - struct fpga_mgr_devres *dr = res; - - fpga_mgr_unregister(dr->mgr); -} - -/** - * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register() - * @dev: managing device for this FPGA manager - * @mgr: fpga manager struct - * - * This is the devres variant of fpga_mgr_register() for which the unregister - * function will be called automatically when the managing device is detached. - */ -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr) -{ - struct fpga_mgr_devres *dr; - int ret; - - /* - * Make sure that the struct fpga_manager * that is passed in is - * managed itself. - */ - if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release, - fpga_mgr_devres_match, mgr))) - return -EINVAL; - - dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL); - if (!dr) - return -ENOMEM; - - ret = fpga_mgr_register(mgr); - if (ret) { - devres_free(dr); - return ret; - } - - dr->mgr = mgr; - devres_add(dev, dr); - - return 0; -} -EXPORT_SYMBOL_GPL(devm_fpga_mgr_register); - static void fpga_mgr_dev_release(struct device *dev) { + struct fpga_manager *mgr = to_fpga_manager(dev); + + ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); + kfree(mgr); } static int __init fpga_mgr_class_init(void) diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c index 69dec5af23c3..665db9e68875 100644 --- a/drivers/fpga/ice40-spi.c +++ b/drivers/fpga/ice40-spi.c @@ -178,12 +178,10 @@ static int ice40_fpga_probe(struct spi_device *spi) return ret; } - mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager", - &ice40_fpga_ops, priv); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager", + &ice40_fpga_ops, priv); - return devm_fpga_mgr_register(dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } static const struct of_device_id ice40_fpga_of_match[] = { diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c index 114a64d2b7a4..859750b6b578 100644 --- a/drivers/fpga/machxo2-spi.c +++ b/drivers/fpga/machxo2-spi.c @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi) return -EINVAL; } - mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager", - &machxo2_ops, spi); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager", + &machxo2_ops, spi); - return devm_fpga_mgr_register(dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } static const struct of_device_id of_match[] = { diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c index 573d88bdf730..ac8e89b8a5cc 100644 --- a/drivers/fpga/socfpga-a10.c +++ b/drivers/fpga/socfpga-a10.c @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev) return -EBUSY; } - mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager", - &socfpga_a10_fpga_mgr_ops, priv); - if (!mgr) - return -ENOMEM; - - platform_set_drvdata(pdev, mgr); - - ret = fpga_mgr_register(mgr); - if (ret) { + mgr = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager", + &socfpga_a10_fpga_mgr_ops, priv); + if (IS_ERR(mgr)) { clk_disable_unprepare(priv->clk); - return ret; + return PTR_ERR(mgr); } + platform_set_drvdata(pdev, mgr); + return 0; } diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c index 1f467173fc1f..1dbe3b44418d 100644 --- a/drivers/fpga/socfpga.c +++ b/drivers/fpga/socfpga.c @@ -571,12 +571,10 @@ static int socfpga_fpga_probe(struct platform_device *pdev) if (ret) return ret; - mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager", - &socfpga_fpga_ops, priv); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager", + &socfpga_fpga_ops, priv); - return devm_fpga_mgr_register(dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } #ifdef CONFIG_OF diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c index 657a70c5fc99..e0bd91b3f7f8 100644 --- a/drivers/fpga/stratix10-soc.c +++ b/drivers/fpga/stratix10-soc.c @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev) init_completion(&priv->status_return_completion); - mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager", - &s10_ops, priv); - if (!mgr) { - dev_err(dev, "unable to create FPGA manager\n"); - ret = -ENOMEM; - goto probe_err; - } - - ret = fpga_mgr_register(mgr); - if (ret) { + mgr = fpga_mgr_register(dev, "Stratix10 SOC FPGA Manager", + &s10_ops, priv); + if (IS_ERR(mgr)) { dev_err(dev, "unable to register FPGA manager\n"); - fpga_mgr_free(mgr); + ret = PTR_ERR(mgr); goto probe_err; } diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c index 101f016c6ed8..581c561a8980 100644 --- a/drivers/fpga/ts73xx-fpga.c +++ b/drivers/fpga/ts73xx-fpga.c @@ -122,12 +122,10 @@ static int ts73xx_fpga_probe(struct platform_device *pdev) if (IS_ERR(priv->io_base)) return PTR_ERR(priv->io_base); - mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager", - &ts73xx_fpga_ops, priv); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(kdev, "TS-73xx FPGA Manager", + &ts73xx_fpga_ops, priv); - return devm_fpga_mgr_register(kdev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } static struct platform_driver ts73xx_fpga_driver = { diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c index fee4d0abf6bf..720b8a78ffc6 100644 --- a/drivers/fpga/xilinx-spi.c +++ b/drivers/fpga/xilinx-spi.c @@ -247,13 +247,11 @@ static int xilinx_spi_probe(struct spi_device *spi) return dev_err_probe(&spi->dev, PTR_ERR(conf->done), "Failed to get DONE gpio\n"); - mgr = devm_fpga_mgr_create(&spi->dev, - "Xilinx Slave Serial FPGA Manager", - &xilinx_spi_ops, conf); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(&spi->dev, + "Xilinx Slave Serial FPGA Manager", + &xilinx_spi_ops, conf); - return devm_fpga_mgr_register(&spi->dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } static const struct of_device_id xlnx_spi_of_match[] = { diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c index 07fa8d9ec675..74bbb9710f4e 100644 --- a/drivers/fpga/zynq-fpga.c +++ b/drivers/fpga/zynq-fpga.c @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device *pdev) clk_disable(priv->clk); - mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager", - &zynq_fpga_ops, priv); - if (!mgr) - return -ENOMEM; - - platform_set_drvdata(pdev, mgr); - - err = fpga_mgr_register(mgr); - if (err) { + mgr = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager", + &zynq_fpga_ops, priv); + if (IS_ERR(mgr)) { dev_err(dev, "unable to register FPGA manager\n"); clk_unprepare(priv->clk); - return err; + return PTR_ERR(mgr); } + platform_set_drvdata(pdev, mgr); + return 0; } diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c index 125743c9797f..1b2eae3f0e03 100644 --- a/drivers/fpga/zynqmp-fpga.c +++ b/drivers/fpga/zynqmp-fpga.c @@ -102,12 +102,10 @@ static int zynqmp_fpga_probe(struct platform_device *pdev) priv->dev = dev; - mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager", - &zynqmp_fpga_ops, priv); - if (!mgr) - return -ENOMEM; + mgr = fpga_mgr_register(dev, "Xilinx ZynqMP FPGA Manager", + &zynqmp_fpga_ops, priv); - return devm_fpga_mgr_register(dev, mgr); + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; } static const struct of_device_id zynqmp_fpga_of_match[] = { diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h index 2bc3030a69e5..8ab5f4a67192 100644 --- a/include/linux/fpga/fpga-mgr.h +++ b/include/linux/fpga/fpga-mgr.h @@ -191,17 +191,9 @@ struct fpga_manager *fpga_mgr_get(struct device *dev); void fpga_mgr_put(struct fpga_manager *mgr); -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, - const struct fpga_manager_ops *mops, - void *priv); -void fpga_mgr_free(struct fpga_manager *mgr); -int fpga_mgr_register(struct fpga_manager *mgr); +struct fpga_manager *fpga_mgr_register(struct device *dev, const char *name, + const struct fpga_manager_ops *mops, + void *priv); void fpga_mgr_unregister(struct fpga_manager *mgr); -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr); - -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, - const struct fpga_manager_ops *mops, - void *priv); - #endif /*_LINUX_FPGA_MGR_H */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight @ 2021-05-21 15:27 ` Xu Yilun 2021-05-21 16:30 ` Moritz Fischer 2021-05-22 20:35 ` kernel test robot 2021-05-24 0:55 ` kernel test robot 2 siblings, 1 reply; 11+ messages in thread From: Xu Yilun @ 2021-05-21 15:27 UTC (permalink / raw) To: Russ Weight Cc: mdf, linux-fpga, linux-kernel, trix, lgoncalv, hao.wu, matthew.gerlach, richard.gong On Thu, May 20, 2021 at 06:03:57PM -0700, Russ Weight wrote: > The FPGA manager class driver data structure is being treated as a > managed resource instead of using standard dev_release call-back > to release the class data structure. This change removes the > managed resource code and combines the create() and register() > functions into a single register() function. Using the dev_release to release the class dev data is good to me, but I think we don't have to remove the devm_fpga_mgr_register function, which saves users from calling fpga_mgr_unregister every time. > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > --- > drivers/fpga/altera-cvp.c | 12 +-- > drivers/fpga/altera-pr-ip-core.c | 6 +- > drivers/fpga/altera-ps-spi.c | 8 +- > drivers/fpga/dfl-fme-mgr.c | 10 +- > drivers/fpga/fpga-mgr.c | 177 ++++--------------------------- > drivers/fpga/ice40-spi.c | 8 +- > drivers/fpga/machxo2-spi.c | 8 +- > drivers/fpga/socfpga-a10.c | 16 ++- > drivers/fpga/socfpga.c | 8 +- > drivers/fpga/stratix10-soc.c | 15 +-- > drivers/fpga/ts73xx-fpga.c | 8 +- > drivers/fpga/xilinx-spi.c | 10 +- > drivers/fpga/zynq-fpga.c | 16 ++- > drivers/fpga/zynqmp-fpga.c | 8 +- > include/linux/fpga/fpga-mgr.h | 14 +-- > 15 files changed, 74 insertions(+), 250 deletions(-) > > diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c > index 4e0edb60bfba..b61020ceb0a1 100644 > --- a/drivers/fpga/altera-cvp.c > +++ b/drivers/fpga/altera-cvp.c > @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev, > snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s", > ALTERA_CVP_MGR_NAME, pci_name(pdev)); > > - mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name, > - &altera_cvp_ops, conf); > - if (!mgr) { > - ret = -ENOMEM; > + mgr = fpga_mgr_register(&pdev->dev, conf->mgr_name, > + &altera_cvp_ops, conf); > + if (IS_ERR(mgr)) { > + ret = PTR_ERR(mgr); > goto err_unmap; > } > > pci_set_drvdata(pdev, mgr); > > - ret = fpga_mgr_register(mgr); > - if (ret) > - goto err_unmap; > - > return 0; > > err_unmap: > diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c > index 5b130c4d9882..2eb5505a7031 100644 > --- a/drivers/fpga/altera-pr-ip-core.c > +++ b/drivers/fpga/altera-pr-ip-core.c > @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base) > (val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT, > (int)(val & ALT_PR_CSR_PR_START)); > > - mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv); > > - return devm_fpga_mgr_register(dev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; Many maintainers don't prefer unnecessary ternery operators which makes code hard to read. We may change to normal conditional statement. Same below. > } > EXPORT_SYMBOL_GPL(alt_pr_register); > > diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c > index 23bfd4d1ad0f..c9693d852a18 100644 > --- a/drivers/fpga/altera-ps-spi.c > +++ b/drivers/fpga/altera-ps-spi.c > @@ -302,12 +302,10 @@ static int altera_ps_probe(struct spi_device *spi) > snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s", > dev_driver_string(&spi->dev), dev_name(&spi->dev)); > > - mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name, > - &altera_ps_ops, conf); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(&spi->dev, conf->mgr_name, > + &altera_ps_ops, conf); > > - return devm_fpga_mgr_register(&spi->dev, mgr); You need to implement fpga_mgr_unregister on remove() for some drivers after you delete the devm_ functions. Or maybe better, implement the devm_fpga_mgr_register. Same below. > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > static const struct spi_device_id altera_ps_spi_ids[] = { > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c > index d5861d13b306..dd8ff7c4ec10 100644 > --- a/drivers/fpga/dfl-fme-mgr.c > +++ b/drivers/fpga/dfl-fme-mgr.c > @@ -308,14 +308,14 @@ static int fme_mgr_probe(struct platform_device *pdev) > > fme_mgr_get_compat_id(priv->ioaddr, compat_id); > > - mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager", > - &fme_mgr_ops, priv); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(dev, "DFL FME FPGA Manager", > + &fme_mgr_ops, priv); > + if (IS_ERR(mgr)) > + return PTR_ERR(mgr); > > mgr->compat_id = compat_id; > > - return devm_fpga_mgr_register(dev, mgr); > + return 0; > } > > static struct platform_driver fme_mgr_driver = { > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c > index b85bc47c91a9..e95226fbf18c 100644 > --- a/drivers/fpga/fpga-mgr.c > +++ b/drivers/fpga/fpga-mgr.c > @@ -21,10 +21,6 @@ > static DEFINE_IDA(fpga_mgr_ida); > static struct class *fpga_mgr_class; > > -struct fpga_mgr_devres { > - struct fpga_manager *mgr; > -}; > - > /** > * fpga_image_info_alloc - Allocate a FPGA image info struct > * @dev: owning device > @@ -550,20 +546,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr) > EXPORT_SYMBOL_GPL(fpga_mgr_unlock); > > /** > - * fpga_mgr_create - create and initialize a FPGA manager struct > + * fpga_mgr_register - create and register a FPGA manager struct > * @dev: fpga manager device from pdev > * @name: fpga manager name > * @mops: pointer to structure of fpga manager ops > * @priv: fpga manager private data > * > - * The caller of this function is responsible for freeing the struct with > - * fpga_mgr_free(). Using devm_fpga_mgr_create() instead is recommended. > - * > - * Return: pointer to struct fpga_manager or NULL > + * Returns a struct fpga_manager pointer on success, or ERR_PTR() on error. > */ > -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > - const struct fpga_manager_ops *mops, > - void *priv) > +struct fpga_manager * > +fpga_mgr_register(struct device *dev, const char *name, > + const struct fpga_manager_ops *mops, void *priv) > { > struct fpga_manager *mgr; > int id, ret; > @@ -572,17 +565,17 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > !mops->write_init || (!mops->write && !mops->write_sg) || > (mops->write && mops->write_sg)) { > dev_err(dev, "Attempt to register without fpga_manager_ops\n"); > - return NULL; > + return ERR_PTR(-EINVAL); > } > > if (!name || !strlen(name)) { > dev_err(dev, "Attempt to register with no name!\n"); > - return NULL; > + return ERR_PTR(-EINVAL); > } > > mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); > if (!mgr) > - return NULL; > + return ERR_PTR(-ENOMEM); > > id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); > if (id < 0) > @@ -594,7 +587,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > mgr->mops = mops; > mgr->priv = priv; > > - device_initialize(&mgr->dev); > mgr->dev.class = fpga_mgr_class; > mgr->dev.groups = mops->groups; > mgr->dev.parent = dev; > @@ -605,84 +597,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > if (ret) > goto error_device; > > - return mgr; > - > -error_device: > - ida_simple_remove(&fpga_mgr_ida, id); > -error_kfree: > - kfree(mgr); > - > - return NULL; > -} > -EXPORT_SYMBOL_GPL(fpga_mgr_create); > - > -/** > - * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create() > - * @mgr: fpga manager struct > - */ > -void fpga_mgr_free(struct fpga_manager *mgr) > -{ > - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > - kfree(mgr); > -} > -EXPORT_SYMBOL_GPL(fpga_mgr_free); > - > -static void devm_fpga_mgr_release(struct device *dev, void *res) > -{ > - struct fpga_mgr_devres *dr = res; > - > - fpga_mgr_free(dr->mgr); > -} > - > -/** > - * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct > - * @dev: fpga manager device from pdev > - * @name: fpga manager name > - * @mops: pointer to structure of fpga manager ops > - * @priv: fpga manager private data > - * > - * This function is intended for use in a FPGA manager driver's probe function. > - * After the manager driver creates the manager struct with > - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register(). The > - * manager driver's remove function should call fpga_mgr_unregister(). The > - * manager struct allocated with this function will be freed automatically on > - * driver detach. This includes the case of a probe function returning error > - * before calling fpga_mgr_register(), the struct will still get cleaned up. > - * > - * Return: pointer to struct fpga_manager or NULL > - */ > -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, > - const struct fpga_manager_ops *mops, > - void *priv) > -{ > - struct fpga_mgr_devres *dr; > - > - dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL); > - if (!dr) > - return NULL; > - > - dr->mgr = fpga_mgr_create(dev, name, mops, priv); > - if (!dr->mgr) { > - devres_free(dr); > - return NULL; > - } > - > - devres_add(dev, dr); > - > - return dr->mgr; > -} > -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create); > - > -/** > - * fpga_mgr_register - register a FPGA manager > - * @mgr: fpga manager struct > - * > - * Return: 0 on success, negative error code otherwise. > - */ > -int fpga_mgr_register(struct fpga_manager *mgr) > -{ > - int ret; > - > /* > * Initialize framework state by requesting low level driver read state > * from device. FPGA may be in reset mode or may have been programmed > @@ -690,18 +604,20 @@ int fpga_mgr_register(struct fpga_manager *mgr) > */ > mgr->state = mgr->mops->state(mgr); > > - ret = device_add(&mgr->dev); > - if (ret) > - goto error_device; > - > - dev_info(&mgr->dev, "%s registered\n", mgr->name); > + ret = device_register(&mgr->dev); > + if (ret) { > + put_device(&mgr->dev); > + return ERR_PTR(ret); > + } > > - return 0; > + return mgr; > > error_device: > - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > + ida_simple_remove(&fpga_mgr_ida, id); > +error_kfree: > + kfree(mgr); > > - return ret; > + return ERR_PTR(ret); > } > EXPORT_SYMBOL_GPL(fpga_mgr_register); > > @@ -726,61 +642,12 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) > } > EXPORT_SYMBOL_GPL(fpga_mgr_unregister); > > -static int fpga_mgr_devres_match(struct device *dev, void *res, > - void *match_data) > -{ > - struct fpga_mgr_devres *dr = res; > - > - return match_data == dr->mgr; > -} > - > -static void devm_fpga_mgr_unregister(struct device *dev, void *res) > -{ > - struct fpga_mgr_devres *dr = res; > - > - fpga_mgr_unregister(dr->mgr); > -} > - > -/** > - * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register() > - * @dev: managing device for this FPGA manager > - * @mgr: fpga manager struct > - * > - * This is the devres variant of fpga_mgr_register() for which the unregister > - * function will be called automatically when the managing device is detached. > - */ > -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr) > -{ > - struct fpga_mgr_devres *dr; > - int ret; > - > - /* > - * Make sure that the struct fpga_manager * that is passed in is > - * managed itself. > - */ > - if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release, > - fpga_mgr_devres_match, mgr))) > - return -EINVAL; > - > - dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL); > - if (!dr) > - return -ENOMEM; > - > - ret = fpga_mgr_register(mgr); > - if (ret) { > - devres_free(dr); > - return ret; > - } > - > - dr->mgr = mgr; > - devres_add(dev, dr); > - > - return 0; > -} > -EXPORT_SYMBOL_GPL(devm_fpga_mgr_register); > - > static void fpga_mgr_dev_release(struct device *dev) > { > + struct fpga_manager *mgr = to_fpga_manager(dev); > + > + ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > + kfree(mgr); > } > > static int __init fpga_mgr_class_init(void) > diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c > index 69dec5af23c3..665db9e68875 100644 > --- a/drivers/fpga/ice40-spi.c > +++ b/drivers/fpga/ice40-spi.c > @@ -178,12 +178,10 @@ static int ice40_fpga_probe(struct spi_device *spi) > return ret; > } > > - mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager", > - &ice40_fpga_ops, priv); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager", > + &ice40_fpga_ops, priv); > > - return devm_fpga_mgr_register(dev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > static const struct of_device_id ice40_fpga_of_match[] = { > diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c > index 114a64d2b7a4..859750b6b578 100644 > --- a/drivers/fpga/machxo2-spi.c > +++ b/drivers/fpga/machxo2-spi.c > @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi) > return -EINVAL; > } > > - mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager", > - &machxo2_ops, spi); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager", > + &machxo2_ops, spi); > > - return devm_fpga_mgr_register(dev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > static const struct of_device_id of_match[] = { > diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c > index 573d88bdf730..ac8e89b8a5cc 100644 > --- a/drivers/fpga/socfpga-a10.c > +++ b/drivers/fpga/socfpga-a10.c > @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev) > return -EBUSY; > } > > - mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager", > - &socfpga_a10_fpga_mgr_ops, priv); > - if (!mgr) > - return -ENOMEM; > - > - platform_set_drvdata(pdev, mgr); > - > - ret = fpga_mgr_register(mgr); > - if (ret) { > + mgr = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager", > + &socfpga_a10_fpga_mgr_ops, priv); > + if (IS_ERR(mgr)) { > clk_disable_unprepare(priv->clk); > - return ret; > + return PTR_ERR(mgr); > } > > + platform_set_drvdata(pdev, mgr); > + > return 0; > } > > diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c > index 1f467173fc1f..1dbe3b44418d 100644 > --- a/drivers/fpga/socfpga.c > +++ b/drivers/fpga/socfpga.c > @@ -571,12 +571,10 @@ static int socfpga_fpga_probe(struct platform_device *pdev) > if (ret) > return ret; > > - mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager", > - &socfpga_fpga_ops, priv); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager", > + &socfpga_fpga_ops, priv); > > - return devm_fpga_mgr_register(dev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > #ifdef CONFIG_OF > diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c > index 657a70c5fc99..e0bd91b3f7f8 100644 > --- a/drivers/fpga/stratix10-soc.c > +++ b/drivers/fpga/stratix10-soc.c > @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev) > > init_completion(&priv->status_return_completion); > > - mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager", > - &s10_ops, priv); > - if (!mgr) { > - dev_err(dev, "unable to create FPGA manager\n"); > - ret = -ENOMEM; > - goto probe_err; > - } > - > - ret = fpga_mgr_register(mgr); > - if (ret) { > + mgr = fpga_mgr_register(dev, "Stratix10 SOC FPGA Manager", > + &s10_ops, priv); > + if (IS_ERR(mgr)) { > dev_err(dev, "unable to register FPGA manager\n"); > - fpga_mgr_free(mgr); > + ret = PTR_ERR(mgr); > goto probe_err; > } > > diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c > index 101f016c6ed8..581c561a8980 100644 > --- a/drivers/fpga/ts73xx-fpga.c > +++ b/drivers/fpga/ts73xx-fpga.c > @@ -122,12 +122,10 @@ static int ts73xx_fpga_probe(struct platform_device *pdev) > if (IS_ERR(priv->io_base)) > return PTR_ERR(priv->io_base); > > - mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager", > - &ts73xx_fpga_ops, priv); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(kdev, "TS-73xx FPGA Manager", > + &ts73xx_fpga_ops, priv); > > - return devm_fpga_mgr_register(kdev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > static struct platform_driver ts73xx_fpga_driver = { > diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c > index fee4d0abf6bf..720b8a78ffc6 100644 > --- a/drivers/fpga/xilinx-spi.c > +++ b/drivers/fpga/xilinx-spi.c > @@ -247,13 +247,11 @@ static int xilinx_spi_probe(struct spi_device *spi) > return dev_err_probe(&spi->dev, PTR_ERR(conf->done), > "Failed to get DONE gpio\n"); > > - mgr = devm_fpga_mgr_create(&spi->dev, > - "Xilinx Slave Serial FPGA Manager", > - &xilinx_spi_ops, conf); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(&spi->dev, > + "Xilinx Slave Serial FPGA Manager", > + &xilinx_spi_ops, conf); > > - return devm_fpga_mgr_register(&spi->dev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > static const struct of_device_id xlnx_spi_of_match[] = { > diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c > index 07fa8d9ec675..74bbb9710f4e 100644 > --- a/drivers/fpga/zynq-fpga.c > +++ b/drivers/fpga/zynq-fpga.c > @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device *pdev) > > clk_disable(priv->clk); > > - mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager", > - &zynq_fpga_ops, priv); > - if (!mgr) > - return -ENOMEM; > - > - platform_set_drvdata(pdev, mgr); > - > - err = fpga_mgr_register(mgr); > - if (err) { > + mgr = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager", > + &zynq_fpga_ops, priv); > + if (IS_ERR(mgr)) { > dev_err(dev, "unable to register FPGA manager\n"); > clk_unprepare(priv->clk); > - return err; > + return PTR_ERR(mgr); > } > > + platform_set_drvdata(pdev, mgr); > + > return 0; > } > > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c > index 125743c9797f..1b2eae3f0e03 100644 > --- a/drivers/fpga/zynqmp-fpga.c > +++ b/drivers/fpga/zynqmp-fpga.c > @@ -102,12 +102,10 @@ static int zynqmp_fpga_probe(struct platform_device *pdev) > > priv->dev = dev; > > - mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager", > - &zynqmp_fpga_ops, priv); > - if (!mgr) > - return -ENOMEM; > + mgr = fpga_mgr_register(dev, "Xilinx ZynqMP FPGA Manager", > + &zynqmp_fpga_ops, priv); > > - return devm_fpga_mgr_register(dev, mgr); > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > } > > static const struct of_device_id zynqmp_fpga_of_match[] = { > diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h > index 2bc3030a69e5..8ab5f4a67192 100644 > --- a/include/linux/fpga/fpga-mgr.h > +++ b/include/linux/fpga/fpga-mgr.h > @@ -191,17 +191,9 @@ struct fpga_manager *fpga_mgr_get(struct device *dev); > > void fpga_mgr_put(struct fpga_manager *mgr); > > -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > - const struct fpga_manager_ops *mops, > - void *priv); > -void fpga_mgr_free(struct fpga_manager *mgr); > -int fpga_mgr_register(struct fpga_manager *mgr); > +struct fpga_manager *fpga_mgr_register(struct device *dev, const char *name, > + const struct fpga_manager_ops *mops, > + void *priv); > void fpga_mgr_unregister(struct fpga_manager *mgr); > > -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr); > - > -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, > - const struct fpga_manager_ops *mops, > - void *priv); > - > #endif /*_LINUX_FPGA_MGR_H */ Thanks, Yilun > -- > 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver 2021-05-21 15:27 ` Xu Yilun @ 2021-05-21 16:30 ` Moritz Fischer 2021-05-21 16:33 ` Russ Weight 0 siblings, 1 reply; 11+ messages in thread From: Moritz Fischer @ 2021-05-21 16:30 UTC (permalink / raw) To: Xu Yilun Cc: Russ Weight, mdf, linux-fpga, linux-kernel, trix, lgoncalv, hao.wu, matthew.gerlach, richard.gong On Fri, May 21, 2021 at 11:27:19PM +0800, Xu Yilun wrote: > On Thu, May 20, 2021 at 06:03:57PM -0700, Russ Weight wrote: > > The FPGA manager class driver data structure is being treated as a > > managed resource instead of using standard dev_release call-back > > to release the class data structure. This change removes the > > managed resource code and combines the create() and register() > > functions into a single register() function. > > Using the dev_release to release the class dev data is good to me, but I > think we don't have to remove the devm_fpga_mgr_register function, which > saves users from calling fpga_mgr_unregister every time. > > > > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > > --- > > drivers/fpga/altera-cvp.c | 12 +-- > > drivers/fpga/altera-pr-ip-core.c | 6 +- > > drivers/fpga/altera-ps-spi.c | 8 +- > > drivers/fpga/dfl-fme-mgr.c | 10 +- > > drivers/fpga/fpga-mgr.c | 177 ++++--------------------------- > > drivers/fpga/ice40-spi.c | 8 +- > > drivers/fpga/machxo2-spi.c | 8 +- > > drivers/fpga/socfpga-a10.c | 16 ++- > > drivers/fpga/socfpga.c | 8 +- > > drivers/fpga/stratix10-soc.c | 15 +-- > > drivers/fpga/ts73xx-fpga.c | 8 +- > > drivers/fpga/xilinx-spi.c | 10 +- > > drivers/fpga/zynq-fpga.c | 16 ++- > > drivers/fpga/zynqmp-fpga.c | 8 +- > > include/linux/fpga/fpga-mgr.h | 14 +-- > > 15 files changed, 74 insertions(+), 250 deletions(-) > > > > diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c > > index 4e0edb60bfba..b61020ceb0a1 100644 > > --- a/drivers/fpga/altera-cvp.c > > +++ b/drivers/fpga/altera-cvp.c > > @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev, > > snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s", > > ALTERA_CVP_MGR_NAME, pci_name(pdev)); > > > > - mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name, > > - &altera_cvp_ops, conf); > > - if (!mgr) { > > - ret = -ENOMEM; > > + mgr = fpga_mgr_register(&pdev->dev, conf->mgr_name, > > + &altera_cvp_ops, conf); > > + if (IS_ERR(mgr)) { > > + ret = PTR_ERR(mgr); > > goto err_unmap; > > } > > > > pci_set_drvdata(pdev, mgr); > > > > - ret = fpga_mgr_register(mgr); > > - if (ret) > > - goto err_unmap; > > - > > return 0; > > > > err_unmap: > > diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c > > index 5b130c4d9882..2eb5505a7031 100644 > > --- a/drivers/fpga/altera-pr-ip-core.c > > +++ b/drivers/fpga/altera-pr-ip-core.c > > @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base) > > (val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT, > > (int)(val & ALT_PR_CSR_PR_START)); > > > > - mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv); > > > > - return devm_fpga_mgr_register(dev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > Many maintainers don't prefer unnecessary ternery operators which makes code > hard to read. We may change to normal conditional statement. > > Same below. > > > } > > EXPORT_SYMBOL_GPL(alt_pr_register); > > > > diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c > > index 23bfd4d1ad0f..c9693d852a18 100644 > > --- a/drivers/fpga/altera-ps-spi.c > > +++ b/drivers/fpga/altera-ps-spi.c > > @@ -302,12 +302,10 @@ static int altera_ps_probe(struct spi_device *spi) > > snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s", > > dev_driver_string(&spi->dev), dev_name(&spi->dev)); > > > > - mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name, > > - &altera_ps_ops, conf); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(&spi->dev, conf->mgr_name, > > + &altera_ps_ops, conf); > > > > - return devm_fpga_mgr_register(&spi->dev, mgr); > > You need to implement fpga_mgr_unregister on remove() for some drivers > after you delete the devm_ functions. > > Or maybe better, implement the devm_fpga_mgr_register. Yes, I don't think we want to completely get rid of the devres code, we just need to fix it. > > Same below. > > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > static const struct spi_device_id altera_ps_spi_ids[] = { > > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c > > index d5861d13b306..dd8ff7c4ec10 100644 > > --- a/drivers/fpga/dfl-fme-mgr.c > > +++ b/drivers/fpga/dfl-fme-mgr.c > > @@ -308,14 +308,14 @@ static int fme_mgr_probe(struct platform_device *pdev) > > > > fme_mgr_get_compat_id(priv->ioaddr, compat_id); > > > > - mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager", > > - &fme_mgr_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(dev, "DFL FME FPGA Manager", > > + &fme_mgr_ops, priv); > > + if (IS_ERR(mgr)) > > + return PTR_ERR(mgr); > > > > mgr->compat_id = compat_id; > > > > - return devm_fpga_mgr_register(dev, mgr); > > + return 0; > > } > > > > static struct platform_driver fme_mgr_driver = { > > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c > > index b85bc47c91a9..e95226fbf18c 100644 > > --- a/drivers/fpga/fpga-mgr.c > > +++ b/drivers/fpga/fpga-mgr.c > > @@ -21,10 +21,6 @@ > > static DEFINE_IDA(fpga_mgr_ida); > > static struct class *fpga_mgr_class; > > > > -struct fpga_mgr_devres { > > - struct fpga_manager *mgr; > > -}; > > - > > /** > > * fpga_image_info_alloc - Allocate a FPGA image info struct > > * @dev: owning device > > @@ -550,20 +546,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr) > > EXPORT_SYMBOL_GPL(fpga_mgr_unlock); > > > > /** > > - * fpga_mgr_create - create and initialize a FPGA manager struct > > + * fpga_mgr_register - create and register a FPGA manager struct > > * @dev: fpga manager device from pdev > > * @name: fpga manager name > > * @mops: pointer to structure of fpga manager ops > > * @priv: fpga manager private data > > * > > - * The caller of this function is responsible for freeing the struct with > > - * fpga_mgr_free(). Using devm_fpga_mgr_create() instead is recommended. > > - * > > - * Return: pointer to struct fpga_manager or NULL > > + * Returns a struct fpga_manager pointer on success, or ERR_PTR() on error. > > */ > > -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > > - const struct fpga_manager_ops *mops, > > - void *priv) > > +struct fpga_manager * > > +fpga_mgr_register(struct device *dev, const char *name, > > + const struct fpga_manager_ops *mops, void *priv) > > { > > struct fpga_manager *mgr; > > int id, ret; > > @@ -572,17 +565,17 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > > !mops->write_init || (!mops->write && !mops->write_sg) || > > (mops->write && mops->write_sg)) { > > dev_err(dev, "Attempt to register without fpga_manager_ops\n"); > > - return NULL; > > + return ERR_PTR(-EINVAL); > > } > > > > if (!name || !strlen(name)) { > > dev_err(dev, "Attempt to register with no name!\n"); > > - return NULL; > > + return ERR_PTR(-EINVAL); > > } > > > > mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); > > if (!mgr) > > - return NULL; > > + return ERR_PTR(-ENOMEM); > > > > id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); > > if (id < 0) > > @@ -594,7 +587,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > > mgr->mops = mops; > > mgr->priv = priv; > > > > - device_initialize(&mgr->dev); > > mgr->dev.class = fpga_mgr_class; > > mgr->dev.groups = mops->groups; > > mgr->dev.parent = dev; > > @@ -605,84 +597,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > > if (ret) > > goto error_device; > > > > - return mgr; > > - > > -error_device: > > - ida_simple_remove(&fpga_mgr_ida, id); > > -error_kfree: > > - kfree(mgr); > > - > > - return NULL; > > -} > > -EXPORT_SYMBOL_GPL(fpga_mgr_create); > > - > > -/** > > - * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create() > > - * @mgr: fpga manager struct > > - */ > > -void fpga_mgr_free(struct fpga_manager *mgr) > > -{ > > - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > > - kfree(mgr); > > -} > > -EXPORT_SYMBOL_GPL(fpga_mgr_free); > > - > > -static void devm_fpga_mgr_release(struct device *dev, void *res) > > -{ > > - struct fpga_mgr_devres *dr = res; > > - > > - fpga_mgr_free(dr->mgr); > > -} > > - > > -/** > > - * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct > > - * @dev: fpga manager device from pdev > > - * @name: fpga manager name > > - * @mops: pointer to structure of fpga manager ops > > - * @priv: fpga manager private data > > - * > > - * This function is intended for use in a FPGA manager driver's probe function. > > - * After the manager driver creates the manager struct with > > - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register(). The > > - * manager driver's remove function should call fpga_mgr_unregister(). The > > - * manager struct allocated with this function will be freed automatically on > > - * driver detach. This includes the case of a probe function returning error > > - * before calling fpga_mgr_register(), the struct will still get cleaned up. > > - * > > - * Return: pointer to struct fpga_manager or NULL > > - */ > > -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, > > - const struct fpga_manager_ops *mops, > > - void *priv) > > -{ > > - struct fpga_mgr_devres *dr; > > - > > - dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL); > > - if (!dr) > > - return NULL; > > - > > - dr->mgr = fpga_mgr_create(dev, name, mops, priv); > > - if (!dr->mgr) { > > - devres_free(dr); > > - return NULL; > > - } > > - > > - devres_add(dev, dr); > > - > > - return dr->mgr; > > -} > > -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create); > > - > > -/** > > - * fpga_mgr_register - register a FPGA manager > > - * @mgr: fpga manager struct > > - * > > - * Return: 0 on success, negative error code otherwise. > > - */ > > -int fpga_mgr_register(struct fpga_manager *mgr) > > -{ > > - int ret; > > - > > /* > > * Initialize framework state by requesting low level driver read state > > * from device. FPGA may be in reset mode or may have been programmed > > @@ -690,18 +604,20 @@ int fpga_mgr_register(struct fpga_manager *mgr) > > */ > > mgr->state = mgr->mops->state(mgr); > > > > - ret = device_add(&mgr->dev); > > - if (ret) > > - goto error_device; > > - > > - dev_info(&mgr->dev, "%s registered\n", mgr->name); > > + ret = device_register(&mgr->dev); > > + if (ret) { > > + put_device(&mgr->dev); > > + return ERR_PTR(ret); > > + } > > > > - return 0; > > + return mgr; > > > > error_device: > > - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > > + ida_simple_remove(&fpga_mgr_ida, id); > > +error_kfree: > > + kfree(mgr); > > > > - return ret; > > + return ERR_PTR(ret); > > } > > EXPORT_SYMBOL_GPL(fpga_mgr_register); > > > > @@ -726,61 +642,12 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) > > } > > EXPORT_SYMBOL_GPL(fpga_mgr_unregister); > > > > -static int fpga_mgr_devres_match(struct device *dev, void *res, > > - void *match_data) > > -{ > > - struct fpga_mgr_devres *dr = res; > > - > > - return match_data == dr->mgr; > > -} > > - > > -static void devm_fpga_mgr_unregister(struct device *dev, void *res) > > -{ > > - struct fpga_mgr_devres *dr = res; > > - > > - fpga_mgr_unregister(dr->mgr); > > -} > > - > > -/** > > - * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register() > > - * @dev: managing device for this FPGA manager > > - * @mgr: fpga manager struct > > - * > > - * This is the devres variant of fpga_mgr_register() for which the unregister > > - * function will be called automatically when the managing device is detached. > > - */ > > -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr) > > -{ > > - struct fpga_mgr_devres *dr; > > - int ret; > > - > > - /* > > - * Make sure that the struct fpga_manager * that is passed in is > > - * managed itself. > > - */ > > - if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release, > > - fpga_mgr_devres_match, mgr))) > > - return -EINVAL; > > - > > - dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL); > > - if (!dr) > > - return -ENOMEM; > > - > > - ret = fpga_mgr_register(mgr); > > - if (ret) { > > - devres_free(dr); > > - return ret; > > - } > > - > > - dr->mgr = mgr; > > - devres_add(dev, dr); > > - > > - return 0; > > -} > > -EXPORT_SYMBOL_GPL(devm_fpga_mgr_register); > > - > > static void fpga_mgr_dev_release(struct device *dev) > > { > > + struct fpga_manager *mgr = to_fpga_manager(dev); > > + > > + ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); > > + kfree(mgr); > > } > > > > static int __init fpga_mgr_class_init(void) > > diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c > > index 69dec5af23c3..665db9e68875 100644 > > --- a/drivers/fpga/ice40-spi.c > > +++ b/drivers/fpga/ice40-spi.c > > @@ -178,12 +178,10 @@ static int ice40_fpga_probe(struct spi_device *spi) > > return ret; > > } > > > > - mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager", > > - &ice40_fpga_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager", > > + &ice40_fpga_ops, priv); > > > > - return devm_fpga_mgr_register(dev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > static const struct of_device_id ice40_fpga_of_match[] = { > > diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c > > index 114a64d2b7a4..859750b6b578 100644 > > --- a/drivers/fpga/machxo2-spi.c > > +++ b/drivers/fpga/machxo2-spi.c > > @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi) > > return -EINVAL; > > } > > > > - mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager", > > - &machxo2_ops, spi); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager", > > + &machxo2_ops, spi); > > > > - return devm_fpga_mgr_register(dev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > static const struct of_device_id of_match[] = { > > diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c > > index 573d88bdf730..ac8e89b8a5cc 100644 > > --- a/drivers/fpga/socfpga-a10.c > > +++ b/drivers/fpga/socfpga-a10.c > > @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev) > > return -EBUSY; > > } > > > > - mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager", > > - &socfpga_a10_fpga_mgr_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > - > > - platform_set_drvdata(pdev, mgr); > > - > > - ret = fpga_mgr_register(mgr); > > - if (ret) { > > + mgr = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager", > > + &socfpga_a10_fpga_mgr_ops, priv); > > + if (IS_ERR(mgr)) { > > clk_disable_unprepare(priv->clk); > > - return ret; > > + return PTR_ERR(mgr); > > } > > > > + platform_set_drvdata(pdev, mgr); > > + > > return 0; > > } > > > > diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c > > index 1f467173fc1f..1dbe3b44418d 100644 > > --- a/drivers/fpga/socfpga.c > > +++ b/drivers/fpga/socfpga.c > > @@ -571,12 +571,10 @@ static int socfpga_fpga_probe(struct platform_device *pdev) > > if (ret) > > return ret; > > > > - mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager", > > - &socfpga_fpga_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager", > > + &socfpga_fpga_ops, priv); > > > > - return devm_fpga_mgr_register(dev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > #ifdef CONFIG_OF > > diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c > > index 657a70c5fc99..e0bd91b3f7f8 100644 > > --- a/drivers/fpga/stratix10-soc.c > > +++ b/drivers/fpga/stratix10-soc.c > > @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev) > > > > init_completion(&priv->status_return_completion); > > > > - mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager", > > - &s10_ops, priv); > > - if (!mgr) { > > - dev_err(dev, "unable to create FPGA manager\n"); > > - ret = -ENOMEM; > > - goto probe_err; > > - } > > - > > - ret = fpga_mgr_register(mgr); > > - if (ret) { > > + mgr = fpga_mgr_register(dev, "Stratix10 SOC FPGA Manager", > > + &s10_ops, priv); > > + if (IS_ERR(mgr)) { > > dev_err(dev, "unable to register FPGA manager\n"); > > - fpga_mgr_free(mgr); > > + ret = PTR_ERR(mgr); > > goto probe_err; > > } > > > > diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c > > index 101f016c6ed8..581c561a8980 100644 > > --- a/drivers/fpga/ts73xx-fpga.c > > +++ b/drivers/fpga/ts73xx-fpga.c > > @@ -122,12 +122,10 @@ static int ts73xx_fpga_probe(struct platform_device *pdev) > > if (IS_ERR(priv->io_base)) > > return PTR_ERR(priv->io_base); > > > > - mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager", > > - &ts73xx_fpga_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(kdev, "TS-73xx FPGA Manager", > > + &ts73xx_fpga_ops, priv); > > > > - return devm_fpga_mgr_register(kdev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > static struct platform_driver ts73xx_fpga_driver = { > > diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c > > index fee4d0abf6bf..720b8a78ffc6 100644 > > --- a/drivers/fpga/xilinx-spi.c > > +++ b/drivers/fpga/xilinx-spi.c > > @@ -247,13 +247,11 @@ static int xilinx_spi_probe(struct spi_device *spi) > > return dev_err_probe(&spi->dev, PTR_ERR(conf->done), > > "Failed to get DONE gpio\n"); > > > > - mgr = devm_fpga_mgr_create(&spi->dev, > > - "Xilinx Slave Serial FPGA Manager", > > - &xilinx_spi_ops, conf); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(&spi->dev, > > + "Xilinx Slave Serial FPGA Manager", > > + &xilinx_spi_ops, conf); > > > > - return devm_fpga_mgr_register(&spi->dev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > static const struct of_device_id xlnx_spi_of_match[] = { > > diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c > > index 07fa8d9ec675..74bbb9710f4e 100644 > > --- a/drivers/fpga/zynq-fpga.c > > +++ b/drivers/fpga/zynq-fpga.c > > @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device *pdev) > > > > clk_disable(priv->clk); > > > > - mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager", > > - &zynq_fpga_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > - > > - platform_set_drvdata(pdev, mgr); > > - > > - err = fpga_mgr_register(mgr); > > - if (err) { > > + mgr = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager", > > + &zynq_fpga_ops, priv); > > + if (IS_ERR(mgr)) { > > dev_err(dev, "unable to register FPGA manager\n"); > > clk_unprepare(priv->clk); > > - return err; > > + return PTR_ERR(mgr); > > } > > > > + platform_set_drvdata(pdev, mgr); > > + > > return 0; > > } > > > > diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c > > index 125743c9797f..1b2eae3f0e03 100644 > > --- a/drivers/fpga/zynqmp-fpga.c > > +++ b/drivers/fpga/zynqmp-fpga.c > > @@ -102,12 +102,10 @@ static int zynqmp_fpga_probe(struct platform_device *pdev) > > > > priv->dev = dev; > > > > - mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager", > > - &zynqmp_fpga_ops, priv); > > - if (!mgr) > > - return -ENOMEM; > > + mgr = fpga_mgr_register(dev, "Xilinx ZynqMP FPGA Manager", > > + &zynqmp_fpga_ops, priv); > > > > - return devm_fpga_mgr_register(dev, mgr); > > + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; > > } > > > > static const struct of_device_id zynqmp_fpga_of_match[] = { > > diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h > > index 2bc3030a69e5..8ab5f4a67192 100644 > > --- a/include/linux/fpga/fpga-mgr.h > > +++ b/include/linux/fpga/fpga-mgr.h > > @@ -191,17 +191,9 @@ struct fpga_manager *fpga_mgr_get(struct device *dev); > > > > void fpga_mgr_put(struct fpga_manager *mgr); > > > > -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > > - const struct fpga_manager_ops *mops, > > - void *priv); > > -void fpga_mgr_free(struct fpga_manager *mgr); > > -int fpga_mgr_register(struct fpga_manager *mgr); > > +struct fpga_manager *fpga_mgr_register(struct device *dev, const char *name, > > + const struct fpga_manager_ops *mops, > > + void *priv); > > void fpga_mgr_unregister(struct fpga_manager *mgr); > > > > -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr); > > - > > -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, > > - const struct fpga_manager_ops *mops, > > - void *priv); > > - > > #endif /*_LINUX_FPGA_MGR_H */ > > Thanks, > Yilun > > > -- > > 2.25.1 - Moritz ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver 2021-05-21 16:30 ` Moritz Fischer @ 2021-05-21 16:33 ` Russ Weight 0 siblings, 0 replies; 11+ messages in thread From: Russ Weight @ 2021-05-21 16:33 UTC (permalink / raw) To: Moritz Fischer, Xu Yilun Cc: linux-fpga, linux-kernel, trix, lgoncalv, hao.wu, matthew.gerlach, richard.gong On 5/21/21 9:30 AM, Moritz Fischer wrote: > On Fri, May 21, 2021 at 11:27:19PM +0800, Xu Yilun wrote: >> On Thu, May 20, 2021 at 06:03:57PM -0700, Russ Weight wrote: >>> The FPGA manager class driver data structure is being treated as a >>> managed resource instead of using standard dev_release call-back >>> to release the class data structure. This change removes the >>> managed resource code and combines the create() and register() >>> functions into a single register() function. >> Using the dev_release to release the class dev data is good to me, but I >> think we don't have to remove the devm_fpga_mgr_register function, which >> saves users from calling fpga_mgr_unregister every time. >> >>> Signed-off-by: Russ Weight <russell.h.weight@intel.com> >>> --- >>> drivers/fpga/altera-cvp.c | 12 +-- >>> drivers/fpga/altera-pr-ip-core.c | 6 +- >>> drivers/fpga/altera-ps-spi.c | 8 +- >>> drivers/fpga/dfl-fme-mgr.c | 10 +- >>> drivers/fpga/fpga-mgr.c | 177 ++++--------------------------- >>> drivers/fpga/ice40-spi.c | 8 +- >>> drivers/fpga/machxo2-spi.c | 8 +- >>> drivers/fpga/socfpga-a10.c | 16 ++- >>> drivers/fpga/socfpga.c | 8 +- >>> drivers/fpga/stratix10-soc.c | 15 +-- >>> drivers/fpga/ts73xx-fpga.c | 8 +- >>> drivers/fpga/xilinx-spi.c | 10 +- >>> drivers/fpga/zynq-fpga.c | 16 ++- >>> drivers/fpga/zynqmp-fpga.c | 8 +- >>> include/linux/fpga/fpga-mgr.h | 14 +-- >>> 15 files changed, 74 insertions(+), 250 deletions(-) >>> >>> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c >>> index 4e0edb60bfba..b61020ceb0a1 100644 >>> --- a/drivers/fpga/altera-cvp.c >>> +++ b/drivers/fpga/altera-cvp.c >>> @@ -652,19 +652,15 @@ static int altera_cvp_probe(struct pci_dev *pdev, >>> snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s @%s", >>> ALTERA_CVP_MGR_NAME, pci_name(pdev)); >>> >>> - mgr = devm_fpga_mgr_create(&pdev->dev, conf->mgr_name, >>> - &altera_cvp_ops, conf); >>> - if (!mgr) { >>> - ret = -ENOMEM; >>> + mgr = fpga_mgr_register(&pdev->dev, conf->mgr_name, >>> + &altera_cvp_ops, conf); >>> + if (IS_ERR(mgr)) { >>> + ret = PTR_ERR(mgr); >>> goto err_unmap; >>> } >>> >>> pci_set_drvdata(pdev, mgr); >>> >>> - ret = fpga_mgr_register(mgr); >>> - if (ret) >>> - goto err_unmap; >>> - >>> return 0; >>> >>> err_unmap: >>> diff --git a/drivers/fpga/altera-pr-ip-core.c b/drivers/fpga/altera-pr-ip-core.c >>> index 5b130c4d9882..2eb5505a7031 100644 >>> --- a/drivers/fpga/altera-pr-ip-core.c >>> +++ b/drivers/fpga/altera-pr-ip-core.c >>> @@ -191,11 +191,9 @@ int alt_pr_register(struct device *dev, void __iomem *reg_base) >>> (val & ALT_PR_CSR_STATUS_MSK) >> ALT_PR_CSR_STATUS_SFT, >>> (int)(val & ALT_PR_CSR_PR_START)); >>> >>> - mgr = devm_fpga_mgr_create(dev, dev_name(dev), &alt_pr_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(dev, dev_name(dev), &alt_pr_ops, priv); >>> >>> - return devm_fpga_mgr_register(dev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >> Many maintainers don't prefer unnecessary ternery operators which makes code >> hard to read. We may change to normal conditional statement. >> >> Same below. >> >>> } >>> EXPORT_SYMBOL_GPL(alt_pr_register); >>> >>> diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c >>> index 23bfd4d1ad0f..c9693d852a18 100644 >>> --- a/drivers/fpga/altera-ps-spi.c >>> +++ b/drivers/fpga/altera-ps-spi.c >>> @@ -302,12 +302,10 @@ static int altera_ps_probe(struct spi_device *spi) >>> snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s", >>> dev_driver_string(&spi->dev), dev_name(&spi->dev)); >>> >>> - mgr = devm_fpga_mgr_create(&spi->dev, conf->mgr_name, >>> - &altera_ps_ops, conf); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(&spi->dev, conf->mgr_name, >>> + &altera_ps_ops, conf); >>> >>> - return devm_fpga_mgr_register(&spi->dev, mgr); >> You need to implement fpga_mgr_unregister on remove() for some drivers >> after you delete the devm_ functions. >> >> Or maybe better, implement the devm_fpga_mgr_register. > Yes, I don't think we want to completely get rid of the devres code, we > just need to fix it. Agreed. I'll create a second patch-set and restore the some of the devres code. Thanks, - Russ >> Same below. >> >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> static const struct spi_device_id altera_ps_spi_ids[] = { >>> diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c >>> index d5861d13b306..dd8ff7c4ec10 100644 >>> --- a/drivers/fpga/dfl-fme-mgr.c >>> +++ b/drivers/fpga/dfl-fme-mgr.c >>> @@ -308,14 +308,14 @@ static int fme_mgr_probe(struct platform_device *pdev) >>> >>> fme_mgr_get_compat_id(priv->ioaddr, compat_id); >>> >>> - mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager", >>> - &fme_mgr_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(dev, "DFL FME FPGA Manager", >>> + &fme_mgr_ops, priv); >>> + if (IS_ERR(mgr)) >>> + return PTR_ERR(mgr); >>> >>> mgr->compat_id = compat_id; >>> >>> - return devm_fpga_mgr_register(dev, mgr); >>> + return 0; >>> } >>> >>> static struct platform_driver fme_mgr_driver = { >>> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c >>> index b85bc47c91a9..e95226fbf18c 100644 >>> --- a/drivers/fpga/fpga-mgr.c >>> +++ b/drivers/fpga/fpga-mgr.c >>> @@ -21,10 +21,6 @@ >>> static DEFINE_IDA(fpga_mgr_ida); >>> static struct class *fpga_mgr_class; >>> >>> -struct fpga_mgr_devres { >>> - struct fpga_manager *mgr; >>> -}; >>> - >>> /** >>> * fpga_image_info_alloc - Allocate a FPGA image info struct >>> * @dev: owning device >>> @@ -550,20 +546,17 @@ void fpga_mgr_unlock(struct fpga_manager *mgr) >>> EXPORT_SYMBOL_GPL(fpga_mgr_unlock); >>> >>> /** >>> - * fpga_mgr_create - create and initialize a FPGA manager struct >>> + * fpga_mgr_register - create and register a FPGA manager struct >>> * @dev: fpga manager device from pdev >>> * @name: fpga manager name >>> * @mops: pointer to structure of fpga manager ops >>> * @priv: fpga manager private data >>> * >>> - * The caller of this function is responsible for freeing the struct with >>> - * fpga_mgr_free(). Using devm_fpga_mgr_create() instead is recommended. >>> - * >>> - * Return: pointer to struct fpga_manager or NULL >>> + * Returns a struct fpga_manager pointer on success, or ERR_PTR() on error. >>> */ >>> -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >>> - const struct fpga_manager_ops *mops, >>> - void *priv) >>> +struct fpga_manager * >>> +fpga_mgr_register(struct device *dev, const char *name, >>> + const struct fpga_manager_ops *mops, void *priv) >>> { >>> struct fpga_manager *mgr; >>> int id, ret; >>> @@ -572,17 +565,17 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >>> !mops->write_init || (!mops->write && !mops->write_sg) || >>> (mops->write && mops->write_sg)) { >>> dev_err(dev, "Attempt to register without fpga_manager_ops\n"); >>> - return NULL; >>> + return ERR_PTR(-EINVAL); >>> } >>> >>> if (!name || !strlen(name)) { >>> dev_err(dev, "Attempt to register with no name!\n"); >>> - return NULL; >>> + return ERR_PTR(-EINVAL); >>> } >>> >>> mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); >>> if (!mgr) >>> - return NULL; >>> + return ERR_PTR(-ENOMEM); >>> >>> id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); >>> if (id < 0) >>> @@ -594,7 +587,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >>> mgr->mops = mops; >>> mgr->priv = priv; >>> >>> - device_initialize(&mgr->dev); >>> mgr->dev.class = fpga_mgr_class; >>> mgr->dev.groups = mops->groups; >>> mgr->dev.parent = dev; >>> @@ -605,84 +597,6 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >>> if (ret) >>> goto error_device; >>> >>> - return mgr; >>> - >>> -error_device: >>> - ida_simple_remove(&fpga_mgr_ida, id); >>> -error_kfree: >>> - kfree(mgr); >>> - >>> - return NULL; >>> -} >>> -EXPORT_SYMBOL_GPL(fpga_mgr_create); >>> - >>> -/** >>> - * fpga_mgr_free - free a FPGA manager created with fpga_mgr_create() >>> - * @mgr: fpga manager struct >>> - */ >>> -void fpga_mgr_free(struct fpga_manager *mgr) >>> -{ >>> - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); >>> - kfree(mgr); >>> -} >>> -EXPORT_SYMBOL_GPL(fpga_mgr_free); >>> - >>> -static void devm_fpga_mgr_release(struct device *dev, void *res) >>> -{ >>> - struct fpga_mgr_devres *dr = res; >>> - >>> - fpga_mgr_free(dr->mgr); >>> -} >>> - >>> -/** >>> - * devm_fpga_mgr_create - create and initialize a managed FPGA manager struct >>> - * @dev: fpga manager device from pdev >>> - * @name: fpga manager name >>> - * @mops: pointer to structure of fpga manager ops >>> - * @priv: fpga manager private data >>> - * >>> - * This function is intended for use in a FPGA manager driver's probe function. >>> - * After the manager driver creates the manager struct with >>> - * devm_fpga_mgr_create(), it should register it with fpga_mgr_register(). The >>> - * manager driver's remove function should call fpga_mgr_unregister(). The >>> - * manager struct allocated with this function will be freed automatically on >>> - * driver detach. This includes the case of a probe function returning error >>> - * before calling fpga_mgr_register(), the struct will still get cleaned up. >>> - * >>> - * Return: pointer to struct fpga_manager or NULL >>> - */ >>> -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, >>> - const struct fpga_manager_ops *mops, >>> - void *priv) >>> -{ >>> - struct fpga_mgr_devres *dr; >>> - >>> - dr = devres_alloc(devm_fpga_mgr_release, sizeof(*dr), GFP_KERNEL); >>> - if (!dr) >>> - return NULL; >>> - >>> - dr->mgr = fpga_mgr_create(dev, name, mops, priv); >>> - if (!dr->mgr) { >>> - devres_free(dr); >>> - return NULL; >>> - } >>> - >>> - devres_add(dev, dr); >>> - >>> - return dr->mgr; >>> -} >>> -EXPORT_SYMBOL_GPL(devm_fpga_mgr_create); >>> - >>> -/** >>> - * fpga_mgr_register - register a FPGA manager >>> - * @mgr: fpga manager struct >>> - * >>> - * Return: 0 on success, negative error code otherwise. >>> - */ >>> -int fpga_mgr_register(struct fpga_manager *mgr) >>> -{ >>> - int ret; >>> - >>> /* >>> * Initialize framework state by requesting low level driver read state >>> * from device. FPGA may be in reset mode or may have been programmed >>> @@ -690,18 +604,20 @@ int fpga_mgr_register(struct fpga_manager *mgr) >>> */ >>> mgr->state = mgr->mops->state(mgr); >>> >>> - ret = device_add(&mgr->dev); >>> - if (ret) >>> - goto error_device; >>> - >>> - dev_info(&mgr->dev, "%s registered\n", mgr->name); >>> + ret = device_register(&mgr->dev); >>> + if (ret) { >>> + put_device(&mgr->dev); >>> + return ERR_PTR(ret); >>> + } >>> >>> - return 0; >>> + return mgr; >>> >>> error_device: >>> - ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); >>> + ida_simple_remove(&fpga_mgr_ida, id); >>> +error_kfree: >>> + kfree(mgr); >>> >>> - return ret; >>> + return ERR_PTR(ret); >>> } >>> EXPORT_SYMBOL_GPL(fpga_mgr_register); >>> >>> @@ -726,61 +642,12 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) >>> } >>> EXPORT_SYMBOL_GPL(fpga_mgr_unregister); >>> >>> -static int fpga_mgr_devres_match(struct device *dev, void *res, >>> - void *match_data) >>> -{ >>> - struct fpga_mgr_devres *dr = res; >>> - >>> - return match_data == dr->mgr; >>> -} >>> - >>> -static void devm_fpga_mgr_unregister(struct device *dev, void *res) >>> -{ >>> - struct fpga_mgr_devres *dr = res; >>> - >>> - fpga_mgr_unregister(dr->mgr); >>> -} >>> - >>> -/** >>> - * devm_fpga_mgr_register - resource managed variant of fpga_mgr_register() >>> - * @dev: managing device for this FPGA manager >>> - * @mgr: fpga manager struct >>> - * >>> - * This is the devres variant of fpga_mgr_register() for which the unregister >>> - * function will be called automatically when the managing device is detached. >>> - */ >>> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr) >>> -{ >>> - struct fpga_mgr_devres *dr; >>> - int ret; >>> - >>> - /* >>> - * Make sure that the struct fpga_manager * that is passed in is >>> - * managed itself. >>> - */ >>> - if (WARN_ON(!devres_find(dev, devm_fpga_mgr_release, >>> - fpga_mgr_devres_match, mgr))) >>> - return -EINVAL; >>> - >>> - dr = devres_alloc(devm_fpga_mgr_unregister, sizeof(*dr), GFP_KERNEL); >>> - if (!dr) >>> - return -ENOMEM; >>> - >>> - ret = fpga_mgr_register(mgr); >>> - if (ret) { >>> - devres_free(dr); >>> - return ret; >>> - } >>> - >>> - dr->mgr = mgr; >>> - devres_add(dev, dr); >>> - >>> - return 0; >>> -} >>> -EXPORT_SYMBOL_GPL(devm_fpga_mgr_register); >>> - >>> static void fpga_mgr_dev_release(struct device *dev) >>> { >>> + struct fpga_manager *mgr = to_fpga_manager(dev); >>> + >>> + ida_simple_remove(&fpga_mgr_ida, mgr->dev.id); >>> + kfree(mgr); >>> } >>> >>> static int __init fpga_mgr_class_init(void) >>> diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c >>> index 69dec5af23c3..665db9e68875 100644 >>> --- a/drivers/fpga/ice40-spi.c >>> +++ b/drivers/fpga/ice40-spi.c >>> @@ -178,12 +178,10 @@ static int ice40_fpga_probe(struct spi_device *spi) >>> return ret; >>> } >>> >>> - mgr = devm_fpga_mgr_create(dev, "Lattice iCE40 FPGA Manager", >>> - &ice40_fpga_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager", >>> + &ice40_fpga_ops, priv); >>> >>> - return devm_fpga_mgr_register(dev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> static const struct of_device_id ice40_fpga_of_match[] = { >>> diff --git a/drivers/fpga/machxo2-spi.c b/drivers/fpga/machxo2-spi.c >>> index 114a64d2b7a4..859750b6b578 100644 >>> --- a/drivers/fpga/machxo2-spi.c >>> +++ b/drivers/fpga/machxo2-spi.c >>> @@ -366,12 +366,10 @@ static int machxo2_spi_probe(struct spi_device *spi) >>> return -EINVAL; >>> } >>> >>> - mgr = devm_fpga_mgr_create(dev, "Lattice MachXO2 SPI FPGA Manager", >>> - &machxo2_ops, spi); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager", >>> + &machxo2_ops, spi); >>> >>> - return devm_fpga_mgr_register(dev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> static const struct of_device_id of_match[] = { >>> diff --git a/drivers/fpga/socfpga-a10.c b/drivers/fpga/socfpga-a10.c >>> index 573d88bdf730..ac8e89b8a5cc 100644 >>> --- a/drivers/fpga/socfpga-a10.c >>> +++ b/drivers/fpga/socfpga-a10.c >>> @@ -508,19 +508,15 @@ static int socfpga_a10_fpga_probe(struct platform_device *pdev) >>> return -EBUSY; >>> } >>> >>> - mgr = devm_fpga_mgr_create(dev, "SoCFPGA Arria10 FPGA Manager", >>> - &socfpga_a10_fpga_mgr_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> - >>> - platform_set_drvdata(pdev, mgr); >>> - >>> - ret = fpga_mgr_register(mgr); >>> - if (ret) { >>> + mgr = fpga_mgr_register(dev, "SoCFPGA Arria10 FPGA Manager", >>> + &socfpga_a10_fpga_mgr_ops, priv); >>> + if (IS_ERR(mgr)) { >>> clk_disable_unprepare(priv->clk); >>> - return ret; >>> + return PTR_ERR(mgr); >>> } >>> >>> + platform_set_drvdata(pdev, mgr); >>> + >>> return 0; >>> } >>> >>> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c >>> index 1f467173fc1f..1dbe3b44418d 100644 >>> --- a/drivers/fpga/socfpga.c >>> +++ b/drivers/fpga/socfpga.c >>> @@ -571,12 +571,10 @@ static int socfpga_fpga_probe(struct platform_device *pdev) >>> if (ret) >>> return ret; >>> >>> - mgr = devm_fpga_mgr_create(dev, "Altera SOCFPGA FPGA Manager", >>> - &socfpga_fpga_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(dev, "Altera SOCFPGA FPGA Manager", >>> + &socfpga_fpga_ops, priv); >>> >>> - return devm_fpga_mgr_register(dev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> #ifdef CONFIG_OF >>> diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c >>> index 657a70c5fc99..e0bd91b3f7f8 100644 >>> --- a/drivers/fpga/stratix10-soc.c >>> +++ b/drivers/fpga/stratix10-soc.c >>> @@ -425,18 +425,11 @@ static int s10_probe(struct platform_device *pdev) >>> >>> init_completion(&priv->status_return_completion); >>> >>> - mgr = fpga_mgr_create(dev, "Stratix10 SOC FPGA Manager", >>> - &s10_ops, priv); >>> - if (!mgr) { >>> - dev_err(dev, "unable to create FPGA manager\n"); >>> - ret = -ENOMEM; >>> - goto probe_err; >>> - } >>> - >>> - ret = fpga_mgr_register(mgr); >>> - if (ret) { >>> + mgr = fpga_mgr_register(dev, "Stratix10 SOC FPGA Manager", >>> + &s10_ops, priv); >>> + if (IS_ERR(mgr)) { >>> dev_err(dev, "unable to register FPGA manager\n"); >>> - fpga_mgr_free(mgr); >>> + ret = PTR_ERR(mgr); >>> goto probe_err; >>> } >>> >>> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c >>> index 101f016c6ed8..581c561a8980 100644 >>> --- a/drivers/fpga/ts73xx-fpga.c >>> +++ b/drivers/fpga/ts73xx-fpga.c >>> @@ -122,12 +122,10 @@ static int ts73xx_fpga_probe(struct platform_device *pdev) >>> if (IS_ERR(priv->io_base)) >>> return PTR_ERR(priv->io_base); >>> >>> - mgr = devm_fpga_mgr_create(kdev, "TS-73xx FPGA Manager", >>> - &ts73xx_fpga_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(kdev, "TS-73xx FPGA Manager", >>> + &ts73xx_fpga_ops, priv); >>> >>> - return devm_fpga_mgr_register(kdev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> static struct platform_driver ts73xx_fpga_driver = { >>> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c >>> index fee4d0abf6bf..720b8a78ffc6 100644 >>> --- a/drivers/fpga/xilinx-spi.c >>> +++ b/drivers/fpga/xilinx-spi.c >>> @@ -247,13 +247,11 @@ static int xilinx_spi_probe(struct spi_device *spi) >>> return dev_err_probe(&spi->dev, PTR_ERR(conf->done), >>> "Failed to get DONE gpio\n"); >>> >>> - mgr = devm_fpga_mgr_create(&spi->dev, >>> - "Xilinx Slave Serial FPGA Manager", >>> - &xilinx_spi_ops, conf); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(&spi->dev, >>> + "Xilinx Slave Serial FPGA Manager", >>> + &xilinx_spi_ops, conf); >>> >>> - return devm_fpga_mgr_register(&spi->dev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> static const struct of_device_id xlnx_spi_of_match[] = { >>> diff --git a/drivers/fpga/zynq-fpga.c b/drivers/fpga/zynq-fpga.c >>> index 07fa8d9ec675..74bbb9710f4e 100644 >>> --- a/drivers/fpga/zynq-fpga.c >>> +++ b/drivers/fpga/zynq-fpga.c >>> @@ -609,20 +609,16 @@ static int zynq_fpga_probe(struct platform_device *pdev) >>> >>> clk_disable(priv->clk); >>> >>> - mgr = devm_fpga_mgr_create(dev, "Xilinx Zynq FPGA Manager", >>> - &zynq_fpga_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> - >>> - platform_set_drvdata(pdev, mgr); >>> - >>> - err = fpga_mgr_register(mgr); >>> - if (err) { >>> + mgr = fpga_mgr_register(dev, "Xilinx Zynq FPGA Manager", >>> + &zynq_fpga_ops, priv); >>> + if (IS_ERR(mgr)) { >>> dev_err(dev, "unable to register FPGA manager\n"); >>> clk_unprepare(priv->clk); >>> - return err; >>> + return PTR_ERR(mgr); >>> } >>> >>> + platform_set_drvdata(pdev, mgr); >>> + >>> return 0; >>> } >>> >>> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c >>> index 125743c9797f..1b2eae3f0e03 100644 >>> --- a/drivers/fpga/zynqmp-fpga.c >>> +++ b/drivers/fpga/zynqmp-fpga.c >>> @@ -102,12 +102,10 @@ static int zynqmp_fpga_probe(struct platform_device *pdev) >>> >>> priv->dev = dev; >>> >>> - mgr = devm_fpga_mgr_create(dev, "Xilinx ZynqMP FPGA Manager", >>> - &zynqmp_fpga_ops, priv); >>> - if (!mgr) >>> - return -ENOMEM; >>> + mgr = fpga_mgr_register(dev, "Xilinx ZynqMP FPGA Manager", >>> + &zynqmp_fpga_ops, priv); >>> >>> - return devm_fpga_mgr_register(dev, mgr); >>> + return (IS_ERR(mgr)) ? PTR_ERR(mgr) : 0; >>> } >>> >>> static const struct of_device_id zynqmp_fpga_of_match[] = { >>> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h >>> index 2bc3030a69e5..8ab5f4a67192 100644 >>> --- a/include/linux/fpga/fpga-mgr.h >>> +++ b/include/linux/fpga/fpga-mgr.h >>> @@ -191,17 +191,9 @@ struct fpga_manager *fpga_mgr_get(struct device *dev); >>> >>> void fpga_mgr_put(struct fpga_manager *mgr); >>> >>> -struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >>> - const struct fpga_manager_ops *mops, >>> - void *priv); >>> -void fpga_mgr_free(struct fpga_manager *mgr); >>> -int fpga_mgr_register(struct fpga_manager *mgr); >>> +struct fpga_manager *fpga_mgr_register(struct device *dev, const char *name, >>> + const struct fpga_manager_ops *mops, >>> + void *priv); >>> void fpga_mgr_unregister(struct fpga_manager *mgr); >>> >>> -int devm_fpga_mgr_register(struct device *dev, struct fpga_manager *mgr); >>> - >>> -struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, >>> - const struct fpga_manager_ops *mops, >>> - void *priv); >>> - >>> #endif /*_LINUX_FPGA_MGR_H */ >> Thanks, >> Yilun >> >>> -- >>> 2.25.1 > - Moritz ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight 2021-05-21 15:27 ` Xu Yilun @ 2021-05-22 20:35 ` kernel test robot 2021-05-24 0:55 ` kernel test robot 2 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2021-05-22 20:35 UTC (permalink / raw) To: Russ Weight, mdf, linux-fpga, linux-kernel Cc: kbuild-all, clang-built-linux, trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight [-- Attachment #1: Type: text/plain, Size: 8019 bytes --] Hi Russ, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.13-rc2 next-20210521] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd860052c99b1e088352bdd4fb7aef46f8d2ef47 config: x86_64-randconfig-a014-20210522 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/fe4ee184f8a9b9acbf00d536ec38d8d793f8dcee git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 git checkout fe4ee184f8a9b9acbf00d536ec38d8d793f8dcee # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/fpga/fpga-mgr.c:581:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (id < 0) ^~~~~~ drivers/fpga/fpga-mgr.c:620:17: note: uninitialized use occurs here return ERR_PTR(ret); ^~~ drivers/fpga/fpga-mgr.c:581:2: note: remove the 'if' if its condition is always false if (id < 0) ^~~~~~~~~~~ drivers/fpga/fpga-mgr.c:562:13: note: initialize the variable 'ret' to silence this warning int id, ret; ^ = 0 1 warning generated. vim +581 drivers/fpga/fpga-mgr.c ebf877a51ad7b6 Alan Tull 2017-11-15 547 6a8c3be7ec8eb3 Alan Tull 2015-10-07 548 /** fe4ee184f8a9b9 Russ Weight 2021-05-20 549 * fpga_mgr_register - create and register a FPGA manager struct 6a8c3be7ec8eb3 Alan Tull 2015-10-07 550 * @dev: fpga manager device from pdev 6a8c3be7ec8eb3 Alan Tull 2015-10-07 551 * @name: fpga manager name 6a8c3be7ec8eb3 Alan Tull 2015-10-07 552 * @mops: pointer to structure of fpga manager ops 6a8c3be7ec8eb3 Alan Tull 2015-10-07 553 * @priv: fpga manager private data 6a8c3be7ec8eb3 Alan Tull 2015-10-07 554 * fe4ee184f8a9b9 Russ Weight 2021-05-20 555 * Returns a struct fpga_manager pointer on success, or ERR_PTR() on error. 6a8c3be7ec8eb3 Alan Tull 2015-10-07 556 */ fe4ee184f8a9b9 Russ Weight 2021-05-20 557 struct fpga_manager * fe4ee184f8a9b9 Russ Weight 2021-05-20 558 fpga_mgr_register(struct device *dev, const char *name, fe4ee184f8a9b9 Russ Weight 2021-05-20 559 const struct fpga_manager_ops *mops, void *priv) 6a8c3be7ec8eb3 Alan Tull 2015-10-07 560 { 6a8c3be7ec8eb3 Alan Tull 2015-10-07 561 struct fpga_manager *mgr; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 562 int id, ret; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 563 baa6d396635129 Jason Gunthorpe 2017-02-01 564 if (!mops || !mops->write_complete || !mops->state || baa6d396635129 Jason Gunthorpe 2017-02-01 565 !mops->write_init || (!mops->write && !mops->write_sg) || baa6d396635129 Jason Gunthorpe 2017-02-01 566 (mops->write && mops->write_sg)) { 6a8c3be7ec8eb3 Alan Tull 2015-10-07 567 dev_err(dev, "Attempt to register without fpga_manager_ops\n"); fe4ee184f8a9b9 Russ Weight 2021-05-20 568 return ERR_PTR(-EINVAL); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 569 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 570 6a8c3be7ec8eb3 Alan Tull 2015-10-07 571 if (!name || !strlen(name)) { 6a8c3be7ec8eb3 Alan Tull 2015-10-07 572 dev_err(dev, "Attempt to register with no name!\n"); fe4ee184f8a9b9 Russ Weight 2021-05-20 573 return ERR_PTR(-EINVAL); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 574 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 575 6a8c3be7ec8eb3 Alan Tull 2015-10-07 576 mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 577 if (!mgr) fe4ee184f8a9b9 Russ Weight 2021-05-20 578 return ERR_PTR(-ENOMEM); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 579 6a8c3be7ec8eb3 Alan Tull 2015-10-07 580 id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); 88aaab9218f87c Tom Rix 2020-06-08 @581 if (id < 0) 6a8c3be7ec8eb3 Alan Tull 2015-10-07 582 goto error_kfree; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 583 6a8c3be7ec8eb3 Alan Tull 2015-10-07 584 mutex_init(&mgr->ref_mutex); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 585 6a8c3be7ec8eb3 Alan Tull 2015-10-07 586 mgr->name = name; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 587 mgr->mops = mops; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 588 mgr->priv = priv; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 589 6a8c3be7ec8eb3 Alan Tull 2015-10-07 590 mgr->dev.class = fpga_mgr_class; 845089bbf589be Alan Tull 2017-11-15 591 mgr->dev.groups = mops->groups; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 592 mgr->dev.parent = dev; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 593 mgr->dev.of_node = dev->of_node; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 594 mgr->dev.id = id; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 595 6a8c3be7ec8eb3 Alan Tull 2015-10-07 596 ret = dev_set_name(&mgr->dev, "fpga%d", id); 07687c031d14a1 Alan Tull 2015-10-29 597 if (ret) 07687c031d14a1 Alan Tull 2015-10-29 598 goto error_device; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 599 7085e2a94f7df5 Alan Tull 2018-05-16 600 /* 7085e2a94f7df5 Alan Tull 2018-05-16 601 * Initialize framework state by requesting low level driver read state 7085e2a94f7df5 Alan Tull 2018-05-16 602 * from device. FPGA may be in reset mode or may have been programmed 7085e2a94f7df5 Alan Tull 2018-05-16 603 * by bootloader or EEPROM. 7085e2a94f7df5 Alan Tull 2018-05-16 604 */ 7085e2a94f7df5 Alan Tull 2018-05-16 605 mgr->state = mgr->mops->state(mgr); 7085e2a94f7df5 Alan Tull 2018-05-16 606 fe4ee184f8a9b9 Russ Weight 2021-05-20 607 ret = device_register(&mgr->dev); fe4ee184f8a9b9 Russ Weight 2021-05-20 608 if (ret) { fe4ee184f8a9b9 Russ Weight 2021-05-20 609 put_device(&mgr->dev); fe4ee184f8a9b9 Russ Weight 2021-05-20 610 return ERR_PTR(ret); fe4ee184f8a9b9 Russ Weight 2021-05-20 611 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 612 fe4ee184f8a9b9 Russ Weight 2021-05-20 613 return mgr; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 614 6a8c3be7ec8eb3 Alan Tull 2015-10-07 615 error_device: fe4ee184f8a9b9 Russ Weight 2021-05-20 616 ida_simple_remove(&fpga_mgr_ida, id); fe4ee184f8a9b9 Russ Weight 2021-05-20 617 error_kfree: fe4ee184f8a9b9 Russ Weight 2021-05-20 618 kfree(mgr); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 619 fe4ee184f8a9b9 Russ Weight 2021-05-20 620 return ERR_PTR(ret); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 621 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 622 EXPORT_SYMBOL_GPL(fpga_mgr_register); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 623 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 40776 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight 2021-05-21 15:27 ` Xu Yilun 2021-05-22 20:35 ` kernel test robot @ 2021-05-24 0:55 ` kernel test robot 2 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2021-05-24 0:55 UTC (permalink / raw) To: Russ Weight, mdf, linux-fpga, linux-kernel Cc: kbuild-all, clang-built-linux, trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight [-- Attachment #1: Type: text/plain, Size: 8019 bytes --] Hi Russ, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.13-rc2 next-20210521] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd860052c99b1e088352bdd4fb7aef46f8d2ef47 config: x86_64-randconfig-b001-20210522 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/fe4ee184f8a9b9acbf00d536ec38d8d793f8dcee git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 git checkout fe4ee184f8a9b9acbf00d536ec38d8d793f8dcee # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/fpga/fpga-mgr.c:581:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (id < 0) ^~~~~~ drivers/fpga/fpga-mgr.c:620:17: note: uninitialized use occurs here return ERR_PTR(ret); ^~~ drivers/fpga/fpga-mgr.c:581:2: note: remove the 'if' if its condition is always false if (id < 0) ^~~~~~~~~~~ drivers/fpga/fpga-mgr.c:562:13: note: initialize the variable 'ret' to silence this warning int id, ret; ^ = 0 1 warning generated. vim +581 drivers/fpga/fpga-mgr.c ebf877a51ad7b6 Alan Tull 2017-11-15 547 6a8c3be7ec8eb3 Alan Tull 2015-10-07 548 /** fe4ee184f8a9b9 Russ Weight 2021-05-20 549 * fpga_mgr_register - create and register a FPGA manager struct 6a8c3be7ec8eb3 Alan Tull 2015-10-07 550 * @dev: fpga manager device from pdev 6a8c3be7ec8eb3 Alan Tull 2015-10-07 551 * @name: fpga manager name 6a8c3be7ec8eb3 Alan Tull 2015-10-07 552 * @mops: pointer to structure of fpga manager ops 6a8c3be7ec8eb3 Alan Tull 2015-10-07 553 * @priv: fpga manager private data 6a8c3be7ec8eb3 Alan Tull 2015-10-07 554 * fe4ee184f8a9b9 Russ Weight 2021-05-20 555 * Returns a struct fpga_manager pointer on success, or ERR_PTR() on error. 6a8c3be7ec8eb3 Alan Tull 2015-10-07 556 */ fe4ee184f8a9b9 Russ Weight 2021-05-20 557 struct fpga_manager * fe4ee184f8a9b9 Russ Weight 2021-05-20 558 fpga_mgr_register(struct device *dev, const char *name, fe4ee184f8a9b9 Russ Weight 2021-05-20 559 const struct fpga_manager_ops *mops, void *priv) 6a8c3be7ec8eb3 Alan Tull 2015-10-07 560 { 6a8c3be7ec8eb3 Alan Tull 2015-10-07 561 struct fpga_manager *mgr; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 562 int id, ret; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 563 baa6d396635129 Jason Gunthorpe 2017-02-01 564 if (!mops || !mops->write_complete || !mops->state || baa6d396635129 Jason Gunthorpe 2017-02-01 565 !mops->write_init || (!mops->write && !mops->write_sg) || baa6d396635129 Jason Gunthorpe 2017-02-01 566 (mops->write && mops->write_sg)) { 6a8c3be7ec8eb3 Alan Tull 2015-10-07 567 dev_err(dev, "Attempt to register without fpga_manager_ops\n"); fe4ee184f8a9b9 Russ Weight 2021-05-20 568 return ERR_PTR(-EINVAL); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 569 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 570 6a8c3be7ec8eb3 Alan Tull 2015-10-07 571 if (!name || !strlen(name)) { 6a8c3be7ec8eb3 Alan Tull 2015-10-07 572 dev_err(dev, "Attempt to register with no name!\n"); fe4ee184f8a9b9 Russ Weight 2021-05-20 573 return ERR_PTR(-EINVAL); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 574 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 575 6a8c3be7ec8eb3 Alan Tull 2015-10-07 576 mgr = kzalloc(sizeof(*mgr), GFP_KERNEL); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 577 if (!mgr) fe4ee184f8a9b9 Russ Weight 2021-05-20 578 return ERR_PTR(-ENOMEM); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 579 6a8c3be7ec8eb3 Alan Tull 2015-10-07 580 id = ida_simple_get(&fpga_mgr_ida, 0, 0, GFP_KERNEL); 88aaab9218f87c Tom Rix 2020-06-08 @581 if (id < 0) 6a8c3be7ec8eb3 Alan Tull 2015-10-07 582 goto error_kfree; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 583 6a8c3be7ec8eb3 Alan Tull 2015-10-07 584 mutex_init(&mgr->ref_mutex); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 585 6a8c3be7ec8eb3 Alan Tull 2015-10-07 586 mgr->name = name; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 587 mgr->mops = mops; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 588 mgr->priv = priv; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 589 6a8c3be7ec8eb3 Alan Tull 2015-10-07 590 mgr->dev.class = fpga_mgr_class; 845089bbf589be Alan Tull 2017-11-15 591 mgr->dev.groups = mops->groups; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 592 mgr->dev.parent = dev; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 593 mgr->dev.of_node = dev->of_node; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 594 mgr->dev.id = id; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 595 6a8c3be7ec8eb3 Alan Tull 2015-10-07 596 ret = dev_set_name(&mgr->dev, "fpga%d", id); 07687c031d14a1 Alan Tull 2015-10-29 597 if (ret) 07687c031d14a1 Alan Tull 2015-10-29 598 goto error_device; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 599 7085e2a94f7df5 Alan Tull 2018-05-16 600 /* 7085e2a94f7df5 Alan Tull 2018-05-16 601 * Initialize framework state by requesting low level driver read state 7085e2a94f7df5 Alan Tull 2018-05-16 602 * from device. FPGA may be in reset mode or may have been programmed 7085e2a94f7df5 Alan Tull 2018-05-16 603 * by bootloader or EEPROM. 7085e2a94f7df5 Alan Tull 2018-05-16 604 */ 7085e2a94f7df5 Alan Tull 2018-05-16 605 mgr->state = mgr->mops->state(mgr); 7085e2a94f7df5 Alan Tull 2018-05-16 606 fe4ee184f8a9b9 Russ Weight 2021-05-20 607 ret = device_register(&mgr->dev); fe4ee184f8a9b9 Russ Weight 2021-05-20 608 if (ret) { fe4ee184f8a9b9 Russ Weight 2021-05-20 609 put_device(&mgr->dev); fe4ee184f8a9b9 Russ Weight 2021-05-20 610 return ERR_PTR(ret); fe4ee184f8a9b9 Russ Weight 2021-05-20 611 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 612 fe4ee184f8a9b9 Russ Weight 2021-05-20 613 return mgr; 6a8c3be7ec8eb3 Alan Tull 2015-10-07 614 6a8c3be7ec8eb3 Alan Tull 2015-10-07 615 error_device: fe4ee184f8a9b9 Russ Weight 2021-05-20 616 ida_simple_remove(&fpga_mgr_ida, id); fe4ee184f8a9b9 Russ Weight 2021-05-20 617 error_kfree: fe4ee184f8a9b9 Russ Weight 2021-05-20 618 kfree(mgr); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 619 fe4ee184f8a9b9 Russ Weight 2021-05-20 620 return ERR_PTR(ret); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 621 } 6a8c3be7ec8eb3 Alan Tull 2015-10-07 622 EXPORT_SYMBOL_GPL(fpga_mgr_register); 6a8c3be7ec8eb3 Alan Tull 2015-10-07 623 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 34393 bytes --] [-- Attachment #3: Type: text/plain, Size: 149 bytes --] _______________________________________________ kbuild mailing list -- kbuild@lists.01.org To unsubscribe send an email to kbuild-leave@lists.01.org ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 2/3] fpga: bridge: Use standard dev_release for class driver 2021-05-21 1:03 [PATCH v1 0/3] fpga: Use standard class dev_release function Russ Weight 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight @ 2021-05-21 1:03 ` Russ Weight 2021-05-21 1:03 ` [PATCH v1 3/3] fpga: region: " Russ Weight 2 siblings, 0 replies; 11+ messages in thread From: Russ Weight @ 2021-05-21 1:03 UTC (permalink / raw) To: mdf, linux-fpga, linux-kernel Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight The FPGA bridge class driver data structure is being treated as a managed resource instead of using standard dev_release call-back to release the class data structure. This change removes the managed resource code and combines the create() and register() functions into a single register() function. Signed-off-by: Russ Weight <russell.h.weight@intel.com> --- drivers/fpga/altera-fpga2sdram.c | 12 +-- drivers/fpga/altera-freeze-bridge.c | 10 +-- drivers/fpga/altera-hps2fpga.c | 12 +-- drivers/fpga/dfl-fme-br.c | 10 +-- drivers/fpga/fpga-bridge.c | 113 ++++++---------------------- drivers/fpga/xilinx-pr-decoupler.c | 17 ++--- include/linux/fpga/fpga-bridge.h | 12 +-- 7 files changed, 48 insertions(+), 138 deletions(-) diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c index a78e49c63c64..66063507116b 100644 --- a/drivers/fpga/altera-fpga2sdram.c +++ b/drivers/fpga/altera-fpga2sdram.c @@ -121,17 +121,13 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev) /* Get f2s bridge configuration saved in handoff register */ regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, &priv->mask); - br = devm_fpga_bridge_create(dev, F2S_BRIDGE_NAME, - &altera_fpga2sdram_br_ops, priv); - if (!br) - return -ENOMEM; + br = fpga_bridge_register(dev, F2S_BRIDGE_NAME, + &altera_fpga2sdram_br_ops, priv); + if (IS_ERR(br)) + return PTR_ERR(mgr); platform_set_drvdata(pdev, br); - ret = fpga_bridge_register(br); - if (ret) - return ret; - dev_info(dev, "driver initialized with handoff %08x\n", priv->mask); if (!of_property_read_u32(dev->of_node, "bridge-enable", &enable)) { diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c index dd58c4aea92e..bfbfa43cd05b 100644 --- a/drivers/fpga/altera-freeze-bridge.c +++ b/drivers/fpga/altera-freeze-bridge.c @@ -244,14 +244,14 @@ static int altera_freeze_br_probe(struct platform_device *pdev) priv->base_addr = base_addr; - br = devm_fpga_bridge_create(dev, FREEZE_BRIDGE_NAME, - &altera_freeze_br_br_ops, priv); - if (!br) - return -ENOMEM; + br = fpga_bridge_register(dev, FREEZE_BRIDGE_NAME, + &altera_freeze_br_br_ops, priv); + if (IS_ERR(br)) + return PTR_ERR(br); platform_set_drvdata(pdev, br); - return fpga_bridge_register(br); + return 0; } static int altera_freeze_br_remove(struct platform_device *pdev) diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c index 77b95f251821..aa758426c22b 100644 --- a/drivers/fpga/altera-hps2fpga.c +++ b/drivers/fpga/altera-hps2fpga.c @@ -180,19 +180,15 @@ static int alt_fpga_bridge_probe(struct platform_device *pdev) } } - br = devm_fpga_bridge_create(dev, priv->name, - &altera_hps2fpga_br_ops, priv); - if (!br) { - ret = -ENOMEM; + br = fpga_bridge_register(dev, priv->name, + &altera_hps2fpga_br_ops, priv); + if (IS_ERR(br)) { + ret = PTR_ERR(br); goto err; } platform_set_drvdata(pdev, br); - ret = fpga_bridge_register(br); - if (ret) - goto err; - return 0; err: diff --git a/drivers/fpga/dfl-fme-br.c b/drivers/fpga/dfl-fme-br.c index 3ff9f3a687ce..808d1f4d76df 100644 --- a/drivers/fpga/dfl-fme-br.c +++ b/drivers/fpga/dfl-fme-br.c @@ -68,14 +68,14 @@ static int fme_br_probe(struct platform_device *pdev) priv->pdata = dev_get_platdata(dev); - br = devm_fpga_bridge_create(dev, "DFL FPGA FME Bridge", - &fme_bridge_ops, priv); - if (!br) - return -ENOMEM; + br = fpga_bridge_register(dev, "DFL FPGA FME Bridge", + &fme_bridge_ops, priv); + if (IS_ERR(br)) + return PTR_ERR(br); platform_set_drvdata(pdev, br); - return fpga_bridge_register(br); + return 0; } static int fme_br_remove(struct platform_device *pdev) diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c index e9266b2a357f..a219408af812 100644 --- a/drivers/fpga/fpga-bridge.c +++ b/drivers/fpga/fpga-bridge.c @@ -312,32 +312,29 @@ static struct attribute *fpga_bridge_attrs[] = { ATTRIBUTE_GROUPS(fpga_bridge); /** - * fpga_bridge_create - create and initialize a struct fpga_bridge + * fpga_bridge_register - create and initialize a struct fpga_bridge * @dev: FPGA bridge device from pdev * @name: FPGA bridge name * @br_ops: pointer to structure of fpga bridge ops * @priv: FPGA bridge private data * - * The caller of this function is responsible for freeing the bridge with - * fpga_bridge_free(). Using devm_fpga_bridge_create() instead is recommended. - * - * Return: struct fpga_bridge or NULL + * Returns a struct fpga_bridge pointer on success, or ERR_PTR() on error. */ -struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name, - const struct fpga_bridge_ops *br_ops, - void *priv) +struct fpga_bridge *fpga_bridge_register(struct device *dev, const char *name, + const struct fpga_bridge_ops *br_ops, + void *priv) { struct fpga_bridge *bridge; int id, ret; if (!name || !strlen(name)) { dev_err(dev, "Attempt to register with no name!\n"); - return NULL; + return ERR_PTR(-EINVAL); } bridge = kzalloc(sizeof(*bridge), GFP_KERNEL); if (!bridge) - return NULL; + return ERR_PTR(-ENOMEM); id = ida_simple_get(&fpga_bridge_ida, 0, 0, GFP_KERNEL); if (id < 0) @@ -350,7 +347,6 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name, bridge->br_ops = br_ops; bridge->priv = priv; - device_initialize(&bridge->dev); bridge->dev.groups = br_ops->groups; bridge->dev.class = fpga_bridge_class; bridge->dev.parent = dev; @@ -361,6 +357,16 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name, if (ret) goto error_device; + ret = device_register(&bridge->dev); + if (ret) { + put_device(&bridge->dev); + return ERR_PTR(ret); + } + + of_platform_populate(dev->of_node, NULL, NULL, dev); + + dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name); + return bridge; error_device: @@ -370,87 +376,6 @@ struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name, return NULL; } -EXPORT_SYMBOL_GPL(fpga_bridge_create); - -/** - * fpga_bridge_free - free a fpga bridge created by fpga_bridge_create() - * @bridge: FPGA bridge struct - */ -void fpga_bridge_free(struct fpga_bridge *bridge) -{ - ida_simple_remove(&fpga_bridge_ida, bridge->dev.id); - kfree(bridge); -} -EXPORT_SYMBOL_GPL(fpga_bridge_free); - -static void devm_fpga_bridge_release(struct device *dev, void *res) -{ - struct fpga_bridge *bridge = *(struct fpga_bridge **)res; - - fpga_bridge_free(bridge); -} - -/** - * devm_fpga_bridge_create - create and init a managed struct fpga_bridge - * @dev: FPGA bridge device from pdev - * @name: FPGA bridge name - * @br_ops: pointer to structure of fpga bridge ops - * @priv: FPGA bridge private data - * - * This function is intended for use in a FPGA bridge driver's probe function. - * After the bridge driver creates the struct with devm_fpga_bridge_create(), it - * should register the bridge with fpga_bridge_register(). The bridge driver's - * remove function should call fpga_bridge_unregister(). The bridge struct - * allocated with this function will be freed automatically on driver detach. - * This includes the case of a probe function returning error before calling - * fpga_bridge_register(), the struct will still get cleaned up. - * - * Return: struct fpga_bridge or NULL - */ -struct fpga_bridge -*devm_fpga_bridge_create(struct device *dev, const char *name, - const struct fpga_bridge_ops *br_ops, void *priv) -{ - struct fpga_bridge **ptr, *bridge; - - ptr = devres_alloc(devm_fpga_bridge_release, sizeof(*ptr), GFP_KERNEL); - if (!ptr) - return NULL; - - bridge = fpga_bridge_create(dev, name, br_ops, priv); - if (!bridge) { - devres_free(ptr); - } else { - *ptr = bridge; - devres_add(dev, ptr); - } - - return bridge; -} -EXPORT_SYMBOL_GPL(devm_fpga_bridge_create); - -/** - * fpga_bridge_register - register a FPGA bridge - * - * @bridge: FPGA bridge struct - * - * Return: 0 for success, error code otherwise. - */ -int fpga_bridge_register(struct fpga_bridge *bridge) -{ - struct device *dev = &bridge->dev; - int ret; - - ret = device_add(dev); - if (ret) - return ret; - - of_platform_populate(dev->of_node, NULL, NULL, dev); - - dev_info(dev->parent, "fpga bridge [%s] registered\n", bridge->name); - - return 0; -} EXPORT_SYMBOL_GPL(fpga_bridge_register); /** @@ -475,6 +400,10 @@ EXPORT_SYMBOL_GPL(fpga_bridge_unregister); static void fpga_bridge_dev_release(struct device *dev) { + struct fpga_bridge *bridge = to_fpga_bridge(dev); + + ida_simple_remove(&fpga_bridge_ida, bridge->dev.id); + kfree(bridge); } static int __init fpga_bridge_dev_init(void) diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c index ea2bde6e5bc4..c004e52b9464 100644 --- a/drivers/fpga/xilinx-pr-decoupler.c +++ b/drivers/fpga/xilinx-pr-decoupler.c @@ -138,22 +138,17 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev) clk_disable(priv->clk); - br = devm_fpga_bridge_create(&pdev->dev, priv->ipconfig->name, - &xlnx_pr_decoupler_br_ops, priv); - if (!br) { - err = -ENOMEM; - goto err_clk; - } - - platform_set_drvdata(pdev, br); - - err = fpga_bridge_register(br); - if (err) { + br = fpga_bridge_register(&pdev->dev, priv->ipconfig->name, + &xlnx_pr_decoupler_br_ops, priv); + if (IS_ERR(br)) { + err = PTR_ERR(br); dev_err(&pdev->dev, "unable to register %s", priv->ipconfig->name); goto err_clk; } + platform_set_drvdata(pdev, br); + return 0; err_clk: diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h index 817600a32c93..86f306c99fa5 100644 --- a/include/linux/fpga/fpga-bridge.h +++ b/include/linux/fpga/fpga-bridge.h @@ -62,15 +62,9 @@ int of_fpga_bridge_get_to_list(struct device_node *np, struct fpga_image_info *info, struct list_head *bridge_list); -struct fpga_bridge *fpga_bridge_create(struct device *dev, const char *name, - const struct fpga_bridge_ops *br_ops, - void *priv); -void fpga_bridge_free(struct fpga_bridge *br); -int fpga_bridge_register(struct fpga_bridge *br); +struct fpga_bridge *fpga_bridge_register(struct device *dev, const char *name, + const struct fpga_bridge_ops *br_ops, + void *priv); void fpga_bridge_unregister(struct fpga_bridge *br); -struct fpga_bridge -*devm_fpga_bridge_create(struct device *dev, const char *name, - const struct fpga_bridge_ops *br_ops, void *priv); - #endif /* _LINUX_FPGA_BRIDGE_H */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 3/3] fpga: region: Use standard dev_release for class driver 2021-05-21 1:03 [PATCH v1 0/3] fpga: Use standard class dev_release function Russ Weight 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight 2021-05-21 1:03 ` [PATCH v1 2/3] fpga: bridge: " Russ Weight @ 2021-05-21 1:03 ` Russ Weight 2021-05-22 23:30 ` kernel test robot 2021-05-22 23:31 ` kernel test robot 2 siblings, 2 replies; 11+ messages in thread From: Russ Weight @ 2021-05-21 1:03 UTC (permalink / raw) To: mdf, linux-fpga, linux-kernel Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight The FPGA region class driver data structure is being treated as a managed resource instead of using standard dev_release call-back to release the class data structure. This change removes the managed resource code and combines the create() and register() functions into a single register() function. Signed-off-by: Russ Weight <russell.h.weight@intel.com> --- drivers/fpga/dfl-fme-region.c | 10 +--- drivers/fpga/dfl.c | 10 +--- drivers/fpga/fpga-region.c | 97 ++++++-------------------------- drivers/fpga/of-fpga-region.c | 10 +--- include/linux/fpga/fpga-region.h | 12 +--- 5 files changed, 28 insertions(+), 111 deletions(-) diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c index 1eeb42af1012..631c8ce160e3 100644 --- a/drivers/fpga/dfl-fme-region.c +++ b/drivers/fpga/dfl-fme-region.c @@ -39,9 +39,9 @@ static int fme_region_probe(struct platform_device *pdev) if (IS_ERR(mgr)) return -EPROBE_DEFER; - region = devm_fpga_region_create(dev, mgr, fme_region_get_bridges); - if (!region) { - ret = -ENOMEM; + region = fpga_region_register(dev, mgr, fme_region_get_bridges); + if (IS_ERR(region)) { + ret = PTR_ERR(region); goto eprobe_mgr_put; } @@ -49,10 +49,6 @@ static int fme_region_probe(struct platform_device *pdev) region->compat_id = mgr->compat_id; platform_set_drvdata(pdev, region); - ret = fpga_region_register(region); - if (ret) - goto eprobe_mgr_put; - dev_dbg(dev, "DFL FME FPGA Region probed\n"); return 0; diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index 511b20ff35a3..2fb3a5bfe4df 100644 --- a/drivers/fpga/dfl.c +++ b/drivers/fpga/dfl.c @@ -1400,9 +1400,9 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info) if (!cdev) return ERR_PTR(-ENOMEM); - cdev->region = devm_fpga_region_create(info->dev, NULL, NULL); - if (!cdev->region) { - ret = -ENOMEM; + cdev->region = fpga_region_register(info->dev, NULL, NULL); + if (IS_ERR(cdev->region)) { + ret = PTR_ERR(cdev->region); goto free_cdev_exit; } @@ -1410,10 +1410,6 @@ dfl_fpga_feature_devs_enumerate(struct dfl_fpga_enum_info *info) mutex_init(&cdev->lock); INIT_LIST_HEAD(&cdev->port_dev_list); - ret = fpga_region_register(cdev->region); - if (ret) - goto free_cdev_exit; - /* create and init build info for enumeration */ binfo = devm_kzalloc(info->dev, sizeof(*binfo), GFP_KERNEL); if (!binfo) { diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index c3134b89c3fe..622b9d598758 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -185,23 +185,18 @@ ATTRIBUTE_GROUPS(fpga_region); * @mgr: manager that programs this region * @get_bridges: optional function to get bridges to a list * - * The caller of this function is responsible for freeing the resulting region - * struct with fpga_region_free(). Using devm_fpga_region_create() instead is - * recommended. - * - * Return: struct fpga_region or NULL + * Returns a struct fpga_region pointer on success, or ERR_PTR() on error. */ -struct fpga_region -*fpga_region_create(struct device *dev, - struct fpga_manager *mgr, - int (*get_bridges)(struct fpga_region *)) +struct fpga_region * +fpga_region_register(struct device *dev, struct fpga_manager *mgr, + int (*get_bridges)(struct fpga_region *)) { struct fpga_region *region; int id, ret = 0; region = kzalloc(sizeof(*region), GFP_KERNEL); if (!region) - return NULL; + return ERR_PTR(-ENOMEM); id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL); if (id < 0) @@ -212,7 +207,6 @@ struct fpga_region 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 = dev->of_node; @@ -222,6 +216,12 @@ struct fpga_region if (ret) goto err_remove; + ret = device_register(®ion->dev); + if (ret) { + put_device(®ion->dev); + return ERR_PTR(ret); + } + return region; err_remove: @@ -229,76 +229,7 @@ struct fpga_region err_free: kfree(region); - return NULL; -} -EXPORT_SYMBOL_GPL(fpga_region_create); - -/** - * fpga_region_free - free a FPGA region created by fpga_region_create() - * @region: FPGA region - */ -void fpga_region_free(struct fpga_region *region) -{ - ida_simple_remove(&fpga_region_ida, region->dev.id); - kfree(region); -} -EXPORT_SYMBOL_GPL(fpga_region_free); - -static void devm_fpga_region_release(struct device *dev, void *res) -{ - struct fpga_region *region = *(struct fpga_region **)res; - - fpga_region_free(region); -} - -/** - * devm_fpga_region_create - create and initialize a managed FPGA region struct - * @dev: device parent - * @mgr: manager that programs this region - * @get_bridges: optional function to get bridges to a list - * - * This function is intended for use in a FPGA region driver's probe function. - * After the region driver creates the region struct with - * devm_fpga_region_create(), it should register it with fpga_region_register(). - * The region driver's remove function should call fpga_region_unregister(). - * The region struct allocated with this function will be freed automatically on - * driver detach. This includes the case of a probe function returning error - * before calling fpga_region_register(), the struct will still get cleaned up. - * - * Return: struct fpga_region or NULL - */ -struct fpga_region -*devm_fpga_region_create(struct device *dev, - struct fpga_manager *mgr, - int (*get_bridges)(struct fpga_region *)) -{ - struct fpga_region **ptr, *region; - - ptr = devres_alloc(devm_fpga_region_release, sizeof(*ptr), GFP_KERNEL); - if (!ptr) - return NULL; - - region = fpga_region_create(dev, mgr, get_bridges); - if (!region) { - devres_free(ptr); - } else { - *ptr = region; - devres_add(dev, ptr); - } - - return region; -} -EXPORT_SYMBOL_GPL(devm_fpga_region_create); - -/** - * fpga_region_register - register a FPGA region - * @region: FPGA region - * - * Return: 0 or -errno - */ -int fpga_region_register(struct fpga_region *region) -{ - return device_add(®ion->dev); + return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(fpga_region_register); @@ -316,6 +247,10 @@ EXPORT_SYMBOL_GPL(fpga_region_unregister); 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); } /** diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c index e405309baadc..466e083654ae 100644 --- a/drivers/fpga/of-fpga-region.c +++ b/drivers/fpga/of-fpga-region.c @@ -405,16 +405,12 @@ static int of_fpga_region_probe(struct platform_device *pdev) if (IS_ERR(mgr)) return -EPROBE_DEFER; - region = devm_fpga_region_create(dev, mgr, of_fpga_region_get_bridges); - if (!region) { - ret = -ENOMEM; + region = fpga_region_register(dev, mgr, of_fpga_region_get_bridges); + if (IS_ERR(region)) { + ret = PTR_ERR(region); goto eprobe_mgr_put; } - ret = fpga_region_register(region); - if (ret) - goto eprobe_mgr_put; - of_platform_populate(np, fpga_region_of_match, NULL, ®ion->dev); platform_set_drvdata(pdev, region); diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h index 27cb706275db..ca793f43af14 100644 --- a/include/linux/fpga/fpga-region.h +++ b/include/linux/fpga/fpga-region.h @@ -37,15 +37,9 @@ struct fpga_region *fpga_region_class_find( int fpga_region_program_fpga(struct fpga_region *region); -struct fpga_region -*fpga_region_create(struct device *dev, struct fpga_manager *mgr, - int (*get_bridges)(struct fpga_region *)); -void fpga_region_free(struct fpga_region *region); -int fpga_region_register(struct fpga_region *region); +struct fpga_region * +fpga_region_register(struct device *dev, struct fpga_manager *mgr, + int (*get_bridges)(struct fpga_region *)); void fpga_region_unregister(struct fpga_region *region); -struct fpga_region -*devm_fpga_region_create(struct device *dev, struct fpga_manager *mgr, - int (*get_bridges)(struct fpga_region *)); - #endif /* _FPGA_REGION_H */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 3/3] fpga: region: Use standard dev_release for class driver 2021-05-21 1:03 ` [PATCH v1 3/3] fpga: region: " Russ Weight @ 2021-05-22 23:30 ` kernel test robot 2021-05-22 23:31 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2021-05-22 23:30 UTC (permalink / raw) To: Russ Weight, mdf, linux-fpga, linux-kernel Cc: kbuild-all, clang-built-linux, trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight [-- Attachment #1: Type: text/plain, Size: 5666 bytes --] Hi Russ, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.13-rc2 next-20210521] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd860052c99b1e088352bdd4fb7aef46f8d2ef47 config: x86_64-randconfig-a014-20210522 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/2bd6e6762866ff1b80bea0d0d8377bcf9d2253ce git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 git checkout 2bd6e6762866ff1b80bea0d0d8377bcf9d2253ce # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/fpga/fpga-region.c:193: warning: expecting prototype for fpga_region_create(). Prototype was for fpga_region_register() instead vim +193 drivers/fpga/fpga-region.c 41a8b2c56470b7 Wu Hao 2018-06-30 181 9f368977b4589e Alan Tull 2018-05-16 182 /** 9f368977b4589e Alan Tull 2018-05-16 183 * fpga_region_create - alloc and init a struct fpga_region 9f368977b4589e Alan Tull 2018-05-16 184 * @dev: device parent 9f368977b4589e Alan Tull 2018-05-16 185 * @mgr: manager that programs this region 9f368977b4589e Alan Tull 2018-05-16 186 * @get_bridges: optional function to get bridges to a list 9f368977b4589e Alan Tull 2018-05-16 187 * 2bd6e6762866ff Russ Weight 2021-05-20 188 * Returns a struct fpga_region pointer on success, or ERR_PTR() on error. 9f368977b4589e Alan Tull 2018-05-16 189 */ 2bd6e6762866ff Russ Weight 2021-05-20 190 struct fpga_region * 2bd6e6762866ff Russ Weight 2021-05-20 191 fpga_region_register(struct device *dev, struct fpga_manager *mgr, 9f368977b4589e Alan Tull 2018-05-16 192 int (*get_bridges)(struct fpga_region *)) 0fa20cdfcc1f68 Alan Tull 2016-11-01 @193 { 9f368977b4589e Alan Tull 2018-05-16 194 struct fpga_region *region; 0fa20cdfcc1f68 Alan Tull 2016-11-01 195 int id, ret = 0; 0fa20cdfcc1f68 Alan Tull 2016-11-01 196 9f368977b4589e Alan Tull 2018-05-16 197 region = kzalloc(sizeof(*region), GFP_KERNEL); 9f368977b4589e Alan Tull 2018-05-16 198 if (!region) 2bd6e6762866ff Russ Weight 2021-05-20 199 return ERR_PTR(-ENOMEM); 9f368977b4589e Alan Tull 2018-05-16 200 0fa20cdfcc1f68 Alan Tull 2016-11-01 201 id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL); 52a3a7ccce07e7 Alan Tull 2017-11-15 202 if (id < 0) 9f368977b4589e Alan Tull 2018-05-16 203 goto err_free; 0fa20cdfcc1f68 Alan Tull 2016-11-01 204 9f368977b4589e Alan Tull 2018-05-16 205 region->mgr = mgr; 9f368977b4589e Alan Tull 2018-05-16 206 region->get_bridges = get_bridges; 0fa20cdfcc1f68 Alan Tull 2016-11-01 207 mutex_init(®ion->mutex); 0fa20cdfcc1f68 Alan Tull 2016-11-01 208 INIT_LIST_HEAD(®ion->bridge_list); 9f368977b4589e Alan Tull 2018-05-16 209 0fa20cdfcc1f68 Alan Tull 2016-11-01 210 region->dev.class = fpga_region_class; 0fa20cdfcc1f68 Alan Tull 2016-11-01 211 region->dev.parent = dev; 52a3a7ccce07e7 Alan Tull 2017-11-15 212 region->dev.of_node = dev->of_node; 0fa20cdfcc1f68 Alan Tull 2016-11-01 213 region->dev.id = id; 0fa20cdfcc1f68 Alan Tull 2016-11-01 214 0fa20cdfcc1f68 Alan Tull 2016-11-01 215 ret = dev_set_name(®ion->dev, "region%d", id); 0fa20cdfcc1f68 Alan Tull 2016-11-01 216 if (ret) 0fa20cdfcc1f68 Alan Tull 2016-11-01 217 goto err_remove; 0fa20cdfcc1f68 Alan Tull 2016-11-01 218 2bd6e6762866ff Russ Weight 2021-05-20 219 ret = device_register(®ion->dev); 2bd6e6762866ff Russ Weight 2021-05-20 220 if (ret) { 2bd6e6762866ff Russ Weight 2021-05-20 221 put_device(®ion->dev); 2bd6e6762866ff Russ Weight 2021-05-20 222 return ERR_PTR(ret); 2bd6e6762866ff Russ Weight 2021-05-20 223 } 2bd6e6762866ff Russ Weight 2021-05-20 224 9f368977b4589e Alan Tull 2018-05-16 225 return region; 52a3a7ccce07e7 Alan Tull 2017-11-15 226 52a3a7ccce07e7 Alan Tull 2017-11-15 227 err_remove: 52a3a7ccce07e7 Alan Tull 2017-11-15 228 ida_simple_remove(&fpga_region_ida, id); 9f368977b4589e Alan Tull 2018-05-16 229 err_free: 9f368977b4589e Alan Tull 2018-05-16 230 kfree(region); 9f368977b4589e Alan Tull 2018-05-16 231 2bd6e6762866ff Russ Weight 2021-05-20 232 return ERR_PTR(ret); 52a3a7ccce07e7 Alan Tull 2017-11-15 233 } 52a3a7ccce07e7 Alan Tull 2017-11-15 234 EXPORT_SYMBOL_GPL(fpga_region_register); 52a3a7ccce07e7 Alan Tull 2017-11-15 235 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 40776 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 3/3] fpga: region: Use standard dev_release for class driver 2021-05-21 1:03 ` [PATCH v1 3/3] fpga: region: " Russ Weight 2021-05-22 23:30 ` kernel test robot @ 2021-05-22 23:31 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2021-05-22 23:31 UTC (permalink / raw) To: Russ Weight, mdf, linux-fpga, linux-kernel Cc: kbuild-all, trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach, richard.gong, Russ Weight [-- Attachment #1: Type: text/plain, Size: 5483 bytes --] Hi Russ, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linux/master] [also build test WARNING on linus/master v5.13-rc2 next-20210521] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd860052c99b1e088352bdd4fb7aef46f8d2ef47 config: arm64-randconfig-p001-20210522 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/2bd6e6762866ff1b80bea0d0d8377bcf9d2253ce git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Russ-Weight/fpga-Use-standard-class-dev_release-function/20210522-205631 git checkout 2bd6e6762866ff1b80bea0d0d8377bcf9d2253ce # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/fpga/fpga-region.c:193: warning: expecting prototype for fpga_region_create(). Prototype was for fpga_region_register() instead vim +193 drivers/fpga/fpga-region.c 41a8b2c56470b7 Wu Hao 2018-06-30 181 9f368977b4589e Alan Tull 2018-05-16 182 /** 9f368977b4589e Alan Tull 2018-05-16 183 * fpga_region_create - alloc and init a struct fpga_region 9f368977b4589e Alan Tull 2018-05-16 184 * @dev: device parent 9f368977b4589e Alan Tull 2018-05-16 185 * @mgr: manager that programs this region 9f368977b4589e Alan Tull 2018-05-16 186 * @get_bridges: optional function to get bridges to a list 9f368977b4589e Alan Tull 2018-05-16 187 * 2bd6e6762866ff Russ Weight 2021-05-20 188 * Returns a struct fpga_region pointer on success, or ERR_PTR() on error. 9f368977b4589e Alan Tull 2018-05-16 189 */ 2bd6e6762866ff Russ Weight 2021-05-20 190 struct fpga_region * 2bd6e6762866ff Russ Weight 2021-05-20 191 fpga_region_register(struct device *dev, struct fpga_manager *mgr, 9f368977b4589e Alan Tull 2018-05-16 192 int (*get_bridges)(struct fpga_region *)) 0fa20cdfcc1f68 Alan Tull 2016-11-01 @193 { 9f368977b4589e Alan Tull 2018-05-16 194 struct fpga_region *region; 0fa20cdfcc1f68 Alan Tull 2016-11-01 195 int id, ret = 0; 0fa20cdfcc1f68 Alan Tull 2016-11-01 196 9f368977b4589e Alan Tull 2018-05-16 197 region = kzalloc(sizeof(*region), GFP_KERNEL); 9f368977b4589e Alan Tull 2018-05-16 198 if (!region) 2bd6e6762866ff Russ Weight 2021-05-20 199 return ERR_PTR(-ENOMEM); 9f368977b4589e Alan Tull 2018-05-16 200 0fa20cdfcc1f68 Alan Tull 2016-11-01 201 id = ida_simple_get(&fpga_region_ida, 0, 0, GFP_KERNEL); 52a3a7ccce07e7 Alan Tull 2017-11-15 202 if (id < 0) 9f368977b4589e Alan Tull 2018-05-16 203 goto err_free; 0fa20cdfcc1f68 Alan Tull 2016-11-01 204 9f368977b4589e Alan Tull 2018-05-16 205 region->mgr = mgr; 9f368977b4589e Alan Tull 2018-05-16 206 region->get_bridges = get_bridges; 0fa20cdfcc1f68 Alan Tull 2016-11-01 207 mutex_init(®ion->mutex); 0fa20cdfcc1f68 Alan Tull 2016-11-01 208 INIT_LIST_HEAD(®ion->bridge_list); 9f368977b4589e Alan Tull 2018-05-16 209 0fa20cdfcc1f68 Alan Tull 2016-11-01 210 region->dev.class = fpga_region_class; 0fa20cdfcc1f68 Alan Tull 2016-11-01 211 region->dev.parent = dev; 52a3a7ccce07e7 Alan Tull 2017-11-15 212 region->dev.of_node = dev->of_node; 0fa20cdfcc1f68 Alan Tull 2016-11-01 213 region->dev.id = id; 0fa20cdfcc1f68 Alan Tull 2016-11-01 214 0fa20cdfcc1f68 Alan Tull 2016-11-01 215 ret = dev_set_name(®ion->dev, "region%d", id); 0fa20cdfcc1f68 Alan Tull 2016-11-01 216 if (ret) 0fa20cdfcc1f68 Alan Tull 2016-11-01 217 goto err_remove; 0fa20cdfcc1f68 Alan Tull 2016-11-01 218 2bd6e6762866ff Russ Weight 2021-05-20 219 ret = device_register(®ion->dev); 2bd6e6762866ff Russ Weight 2021-05-20 220 if (ret) { 2bd6e6762866ff Russ Weight 2021-05-20 221 put_device(®ion->dev); 2bd6e6762866ff Russ Weight 2021-05-20 222 return ERR_PTR(ret); 2bd6e6762866ff Russ Weight 2021-05-20 223 } 2bd6e6762866ff Russ Weight 2021-05-20 224 9f368977b4589e Alan Tull 2018-05-16 225 return region; 52a3a7ccce07e7 Alan Tull 2017-11-15 226 52a3a7ccce07e7 Alan Tull 2017-11-15 227 err_remove: 52a3a7ccce07e7 Alan Tull 2017-11-15 228 ida_simple_remove(&fpga_region_ida, id); 9f368977b4589e Alan Tull 2018-05-16 229 err_free: 9f368977b4589e Alan Tull 2018-05-16 230 kfree(region); 9f368977b4589e Alan Tull 2018-05-16 231 2bd6e6762866ff Russ Weight 2021-05-20 232 return ERR_PTR(ret); 52a3a7ccce07e7 Alan Tull 2017-11-15 233 } 52a3a7ccce07e7 Alan Tull 2017-11-15 234 EXPORT_SYMBOL_GPL(fpga_region_register); 52a3a7ccce07e7 Alan Tull 2017-11-15 235 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 48838 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-05-24 0:56 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-21 1:03 [PATCH v1 0/3] fpga: Use standard class dev_release function Russ Weight 2021-05-21 1:03 ` [PATCH v1 1/3] fpga: mgr: Use standard dev_release for class driver Russ Weight 2021-05-21 15:27 ` Xu Yilun 2021-05-21 16:30 ` Moritz Fischer 2021-05-21 16:33 ` Russ Weight 2021-05-22 20:35 ` kernel test robot 2021-05-24 0:55 ` kernel test robot 2021-05-21 1:03 ` [PATCH v1 2/3] fpga: bridge: " Russ Weight 2021-05-21 1:03 ` [PATCH v1 3/3] fpga: region: " Russ Weight 2021-05-22 23:30 ` kernel test robot 2021-05-22 23:31 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).