* [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers @ 2019-09-15 7:00 Satendra Singh Thakur 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur 2019-09-18 10:27 ` [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Vinod Koul 0 siblings, 2 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:00 UTC (permalink / raw) To: dan.j.williams, vkoul, jun.nie, shawnguo, agross, sean.wang, matthias.bgg, maxime.ripard, wens, lars, afaerber, manivannan.sadhasivam Cc: linux-kernel, linux-mediatek, satendrasingh.thakur, dmaengine, Satendra Singh Thakur, linux-arm-kernel 1. For most of the platform drivers's probe include following steps -memory allocation for driver's private structure -getting io resources -io remapping resources -getting irq number -registering irq -setting driver's private data -getting clock -preparing and enabling clock 2. We have defined a set of macros to combine some or all of the above mentioned steps. This will remove redundant/duplicate code in drivers' probe functions of platform drivers. devm_platform_probe_helper(pdev, priv, clk_name); devm_platform_probe_helper_clk(pdev, priv, clk_name); devm_platform_probe_helper_irq(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); devm_platform_probe_helper_all(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); devm_platform_probe_helper_all_data(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); 3. Code is made devres compatible (wherever required) The functions: clk_get, request_irq, kzalloc, platform_get_resource are replaced with their devm_* counterparts. 4. Few bugs are also fixed. Satendra Singh Thakur (9): probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers probe/dma/jz4740: removed redundant code from jz4740 dma controller's probe function probe/dma/zx: removed redundant code from zx dma controller's probe function probe/dma/qcom-bam: removed redundant code from qcom bam dma controller's probe function probe/dma/mtk-hs: removed redundant code from mediatek hs dma controller's probe function probe/dma/sun6i: removed redundant code from sun6i dma controller's probe function probe/dma/sun4i: removed redundant code from sun4i dma controller's probe function probe/dma/axi: removed redundant code from axi dma controller's probe function probe/dma/owl: removed redundant code from owl dma controller's probe function drivers/dma/dma-axi-dmac.c | 28 ++--- drivers/dma/dma-jz4740.c | 33 +++--- drivers/dma/mediatek/mtk-hsdma.c | 38 +++---- drivers/dma/owl-dma.c | 29 ++--- drivers/dma/qcom/bam_dma.c | 71 +++++------- drivers/dma/sun4i-dma.c | 30 ++---- drivers/dma/sun6i-dma.c | 30 ++---- drivers/dma/zx_dma.c | 35 ++---- include/linux/probe-helper.h | 179 +++++++++++++++++++++++++++++++ 9 files changed, 280 insertions(+), 193 deletions(-) create mode 100644 include/linux/probe-helper.h -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers 2019-09-15 7:00 [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Satendra Singh Thakur @ 2019-09-15 7:26 ` Satendra Singh Thakur 2019-09-15 7:29 ` [PATCH 3/9] probe/dma/zx: removed redundant code from zx dma controller's probe function Satendra Singh Thakur ` (4 more replies) 2019-09-18 10:27 ` [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Vinod Koul 1 sibling, 5 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:26 UTC (permalink / raw) To: dan.j.williams, vkoul, jun.nie, shawnguo, agross, sean.wang, matthias.bgg, maxime.ripard, wens, lars, afaerber, manivannan.sadhasivam Cc: linux-kernel, linux-mediatek, satendrasingh.thakur, dmaengine, Satendra Singh Thakur, linux-arm-kernel 1. For most of the drivers probe include following steps a) memory allocation for driver's private structure b) getting io resources c) io remapping resources d) getting clock e) getting irq number f) registering irq g) preparing and enabling clock i) setting platform's drv data 2. We have defined a set of macros to combine above mentioned steps. This will remove redundant/duplicate code in drivers' probe functions. 3. This macro combines all the steps except f), g) and i). devm_platform_probe_helper(pdev, priv, clk_name); 4. This macro combines all the steps except f) and i). devm_platform_probe_helper_clk(pdev, priv, clk_name); 5. This macro combines all the steps except g) and i). devm_platform_probe_helper_irq(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); 6. This is because, some drivers perform step f) and g) after hw init or subsys registration at very different points in the probe function. The step i) is called at the end of probe function by several drivers; while other drivers call it at different points in probe function. 7. This macro combines above mentioned steps a) to g). devm_platform_probe_helper_all(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); 8. This macro combines all of the above mentioned steps a) to i). devm_platform_probe_helper_all_data(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); 9. Above macros will be useful for wide variety of probe functions of different drivers. Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Signed-off-by: Satendra Singh Thakur <sst2005@gmail.com> --- include/linux/probe-helper.h | 179 +++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 include/linux/probe-helper.h diff --git a/include/linux/probe-helper.h b/include/linux/probe-helper.h new file mode 100644 index 000000000000..7baa468509e3 --- /dev/null +++ b/include/linux/probe-helper.h @@ -0,0 +1,179 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * + * probe_helper.h - helper functions for platform drivers' probe + * function + * Author: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Sep 2019 + * <sst2005@gmail.com> + */ +#ifndef _PROBE_HELPER_H_ +#define _PROBE_HELPER_H_ + +#include <linux/platform_device.h> +#include <linux/clk.h> + +/* devm_platform_probe_helper - Macro for helping probe method + * of platform drivers + * This macro combines the functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq + * @pdev platform device + * @priv driver's private object for memory allocation + * @clk_name clock name as in DT + */ +#define devm_platform_probe_helper(pdev, priv, clk_name) \ +({ \ + __label__ __out; \ + int __ret = 0; \ + priv = devm_kzalloc(&(pdev)->dev, sizeof(*priv), GFP_KERNEL); \ + if (!(priv)) { \ + dev_err(&(pdev)->dev, "devm_kzalloc failed\n"); \ + __ret = -ENOMEM; \ + goto __out; \ + } \ + (priv)->base = devm_platform_ioremap_resource(pdev, 0); \ + if (IS_ERR((priv)->base)) { \ + dev_err(&(pdev)->dev, \ + "devm_platform_ioremap_resource failed\n"); \ + __ret = PTR_ERR((priv)->base); \ + goto __out; \ + } \ + (priv)->clk = devm_clk_get(&(pdev)->dev, clk_name); \ + if (IS_ERR((priv)->clk)) { \ + dev_err(&(pdev)->dev, "devm_clk_get failed\n"); \ + __ret = PTR_ERR((priv)->clk); \ + goto __out; \ + } \ + (priv)->irq = platform_get_irq(pdev, 0); \ + if ((priv)->irq < 0) { \ + dev_err(&(pdev)->dev, "platform_get_irq failed\n"); \ + __ret = (priv)->irq; \ + goto __out; \ + } \ +__out: \ + __ret; \ +}) + +/* devm_platform_probe_helper_irq - Macro for helping probe method + * of platform drivers + * This macro combines the functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq, devm_request_irq + * @pdev platform device + * @priv driver's private object for memory allocation + * @clk_name clock name as in DT + * @irq_hndlr interrupt handler function (isr) + * @irq_flags flags for interrupt registration + * @irq_name name of the interrupt handler + * @irq_devid device identifier for irq + */ +#define devm_platform_probe_helper_irq(pdev, priv, clk_name, \ + irq_hndlr, irq_flags, irq_name, irq_devid) \ +({ \ + __label__ __out; \ + int __ret = 0; \ + __ret = devm_platform_probe_helper(pdev, priv, clk_name); \ + if (__ret < 0) \ + goto __out; \ + __ret = devm_request_irq(&(pdev)->dev, (priv)->irq, irq_hndlr, \ + irq_flags, irq_name, irq_devid); \ + if (__ret < 0) { \ + dev_err(&(pdev)->dev, \ + "devm_request_irq failed for irq num %d\n", \ + (priv)->irq); \ + goto __out; \ + } \ +__out: \ + __ret; \ +}) + +/* devm_platform_probe_helper_clk Macro - for helping probe method + * of platform drivers + * This macro combines the functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq, clk_prepare_enable + * @pdev platform device + * @priv driver's private object for memory allocation + * @clk_name clock name as in DT + */ +#define devm_platform_probe_helper_clk(pdev, priv, clk_name) \ +({ \ + __label__ __out; \ + int __ret = 0; \ + __ret = devm_platform_probe_helper(pdev, priv, clk_name); \ + if (__ret < 0) \ + goto __out; \ + __ret = clk_prepare_enable((priv)->clk); \ + if (__ret < 0) { \ + dev_err(&(pdev)->dev, "clk_prepare_enable failed\n"); \ + goto __out; \ + } \ +__out: \ + __ret; \ +}) + +/* devm_platform_probe_helper_all - Macro for helping probe method + * of platform drivers + * This macro combines the functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq, devm_request_irq, + * clk_prepare_enable + * @pdev platform device + * @priv driver's private object for memory allocation + * @clk_name clock name as in DT + * @irq_hndlr interrupt handler function (isr) + * @irq_flags flags for interrupt registration + * @irq_name name of the interrupt handler + * @irq_devid device identifier for irq + */ +#define devm_platform_probe_helper_all(pdev, priv, clk_name, \ + irq_hndlr, irq_flags, irq_name, irq_devid) \ +({ \ + __label__ __out; \ + int __ret = 0; \ + __ret = devm_platform_probe_helper_clk(pdev, priv, clk_name); \ + if (__ret < 0) \ + goto __out; \ + __ret = devm_request_irq(&(pdev)->dev, (priv)->irq, \ + irq_hndlr, irq_flags, irq_name, irq_devid); \ + if (__ret < 0) { \ + dev_err(&(pdev)->dev, \ + "devm_request_irq failed for irq num %d\n", \ + (priv)->irq); \ + if ((priv)->clk) \ + clk_disable_unprepare((priv)->clk); \ + goto __out; \ + } \ +__out: \ + __ret; \ +}) + +/* devm_platform_probe_helper_all_data - Macro for helping probe method + * of platform drivers + * This macro combines the functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq, devm_request_irq, + * clk_prepare_enable, platform_set_drvdata + * @pdev platform device + * @priv driver's private object for memory allocation + * @clk_name clock name as in DT + * @irq_hndlr interrupt handler function (isr) + * @irq_flags flags for interrupt registration + * @irq_name name of the interrupt handler + * @irq_devid device identifier for irq + */ +#define devm_platform_probe_helper_all_data(pdev, priv, clk_name, \ + irq_hndlr, irq_flags, irq_name, irq_devid) \ +({ \ + __label__ __out; \ + int __ret = 0; \ + __ret = devm_platform_probe_helper_all(pdev, priv, clk_name, \ + irq_hndlr, irq_flags, irq_name, irq_devid); \ + if (__ret < 0) \ + goto __out; \ + platform_set_drvdata(pdev, priv); \ +__out: \ + __ret; \ +}) + +#endif /*_PROBE_HELPER_H_*/ -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/9] probe/dma/zx: removed redundant code from zx dma controller's probe function 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur @ 2019-09-15 7:29 ` Satendra Singh Thakur 2019-09-15 7:30 ` [PATCH 5/9] probe/dma/mtk-hs: removed redundant code from mediatek hs " Satendra Singh Thakur ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:29 UTC (permalink / raw) Cc: linux-kernel, Vinod Koul, satendrasingh.thakur, dmaengine, Dan Williams, Satendra Singh Thakur, Shawn Guo, Jun Nie, linux-arm-kernel 1. In order to remove duplicate code, following functions: platform_get_resource devm_kzalloc devm_ioremap_resource devm_clk_get platform_get_irq devm_request_irq are replaced with a macro devm_platform_probe_helper_irq. 2. Removed dmam_pool_destroy from remove method as dmam_pool_create is already used in probe function. 3. This patch depends on the file include/linux/probe-helper.h which is pushed in previous patch [01/09]. Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Signed-off-by: Satendra Singh Thakur <sst2005@gmail.com> --- drivers/dma/zx_dma.c | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/drivers/dma/zx_dma.c b/drivers/dma/zx_dma.c index 9f4436f7c914..d8c2fbe9766c 100644 --- a/drivers/dma/zx_dma.c +++ b/drivers/dma/zx_dma.c @@ -18,6 +18,7 @@ #include <linux/of.h> #include <linux/clk.h> #include <linux/of_dma.h> +#include <linux/probe-helper.h> #include "virt-dma.h" @@ -754,20 +755,17 @@ static struct dma_chan *zx_of_dma_simple_xlate(struct of_phandle_args *dma_spec, static int zx_dma_probe(struct platform_device *op) { struct zx_dma_dev *d; - struct resource *iores; int i, ret = 0; - iores = platform_get_resource(op, IORESOURCE_MEM, 0); - if (!iores) - return -EINVAL; - - d = devm_kzalloc(&op->dev, sizeof(*d), GFP_KERNEL); - if (!d) - return -ENOMEM; - - d->base = devm_ioremap_resource(&op->dev, iores); - if (IS_ERR(d->base)) - return PTR_ERR(d->base); + /* + * This macro internally combines following functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq, devm_request_irq, + */ + ret = devm_platform_probe_helper_irq(op, d, NULL, + zx_dma_int_handler, 0, DRIVER_NAME, d); + if (ret < 0) + return ret; of_property_read_u32((&op->dev)->of_node, "dma-channels", &d->dma_channels); @@ -776,18 +774,6 @@ static int zx_dma_probe(struct platform_device *op) if (!d->dma_requests || !d->dma_channels) return -EINVAL; - d->clk = devm_clk_get(&op->dev, NULL); - if (IS_ERR(d->clk)) { - dev_err(&op->dev, "no dma clk\n"); - return PTR_ERR(d->clk); - } - - d->irq = platform_get_irq(op, 0); - ret = devm_request_irq(&op->dev, d->irq, zx_dma_int_handler, - 0, DRIVER_NAME, d); - if (ret) - return ret; - /* A DMA memory pool for LLIs, align on 32-byte boundary */ d->pool = dmam_pool_create(DRIVER_NAME, &op->dev, LLI_BLOCK_SIZE, 32, 0); @@ -894,7 +880,6 @@ static int zx_dma_remove(struct platform_device *op) list_del(&c->vc.chan.device_node); } clk_disable_unprepare(d->clk); - dmam_pool_destroy(d->pool); return 0; } -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/9] probe/dma/mtk-hs: removed redundant code from mediatek hs dma controller's probe function 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur 2019-09-15 7:29 ` [PATCH 3/9] probe/dma/zx: removed redundant code from zx dma controller's probe function Satendra Singh Thakur @ 2019-09-15 7:30 ` Satendra Singh Thakur 2019-09-15 7:31 ` [PATCH 6/9] probe/dma/sun6i: removed redundant code from sun6i " Satendra Singh Thakur ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:30 UTC (permalink / raw) Cc: Sean Wang, linux-kernel, dmaengine, Vinod Koul, linux-mediatek, satendrasingh.thakur, Matthias Brugger, Dan Williams, Satendra Singh Thakur, linux-arm-kernel 1. In order to remove duplicate code, following functions: platform_get_resource devm_kzalloc devm_ioremap_resource devm_clk_get platform_get_irq are replaced with a macro devm_platform_probe_helper. 2. Fixed a memory leak when devm_request_irq fails, Called of_dma_controller_free in such case. 3. This patch depends on the file include/linux/probe-helper.h which is pushed in previous patch [01/09]. Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Signed-off-by: Satendra Singh Thakur <sst2005@gmail.com> --- drivers/dma/mediatek/mtk-hsdma.c | 38 ++++++++++---------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c index 1a2028e1c29e..6fc01093aeea 100644 --- a/drivers/dma/mediatek/mtk-hsdma.c +++ b/drivers/dma/mediatek/mtk-hsdma.c @@ -23,6 +23,7 @@ #include <linux/pm_runtime.h> #include <linux/refcount.h> #include <linux/slab.h> +#include <linux/probe-helper.h> #include "../virt-dma.h" @@ -896,41 +897,24 @@ static int mtk_hsdma_probe(struct platform_device *pdev) struct mtk_hsdma_device *hsdma; struct mtk_hsdma_vchan *vc; struct dma_device *dd; - struct resource *res; int i, err; - hsdma = devm_kzalloc(&pdev->dev, sizeof(*hsdma), GFP_KERNEL); - if (!hsdma) - return -ENOMEM; - + /* + * This macro internally combines following functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq + */ + err = devm_platform_probe_helper(pdev, hsdma, "hsdma"); + if (err < 0) + return err; dd = &hsdma->ddev; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - hsdma->base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(hsdma->base)) - return PTR_ERR(hsdma->base); - hsdma->soc = of_device_get_match_data(&pdev->dev); if (!hsdma->soc) { dev_err(&pdev->dev, "No device match found\n"); return -ENODEV; } - hsdma->clk = devm_clk_get(&pdev->dev, "hsdma"); - if (IS_ERR(hsdma->clk)) { - dev_err(&pdev->dev, "No clock for %s\n", - dev_name(&pdev->dev)); - return PTR_ERR(hsdma->clk); - } - - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (!res) { - dev_err(&pdev->dev, "No irq resource for %s\n", - dev_name(&pdev->dev)); - return -EINVAL; - } - hsdma->irq = res->start; - refcount_set(&hsdma->pc_refcnt, 0); spin_lock_init(&hsdma->lock); @@ -997,7 +981,7 @@ static int mtk_hsdma_probe(struct platform_device *pdev) if (err) { dev_err(&pdev->dev, "request_irq failed with err %d\n", err); - goto err_unregister; + goto err_free; } platform_set_drvdata(pdev, hsdma); @@ -1006,6 +990,8 @@ static int mtk_hsdma_probe(struct platform_device *pdev) return 0; +err_free: + of_dma_controller_free(pdev->dev.of_node); err_unregister: dma_async_device_unregister(dd); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 6/9] probe/dma/sun6i: removed redundant code from sun6i dma controller's probe function 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur 2019-09-15 7:29 ` [PATCH 3/9] probe/dma/zx: removed redundant code from zx dma controller's probe function Satendra Singh Thakur 2019-09-15 7:30 ` [PATCH 5/9] probe/dma/mtk-hs: removed redundant code from mediatek hs " Satendra Singh Thakur @ 2019-09-15 7:31 ` Satendra Singh Thakur 2019-09-15 7:32 ` [PATCH 7/9] probe/dma/sun4i: removed redundant code from sun4i " Satendra Singh Thakur 2019-09-15 7:32 ` [PATCH 9/9] probe/dma/owl: removed redundant code from owl " Satendra Singh Thakur 4 siblings, 0 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:31 UTC (permalink / raw) Cc: Maxime Ripard, Chen-Yu Tsai, linux-kernel, Vinod Koul, satendrasingh.thakur, dmaengine, Dan Williams, Satendra Singh Thakur, linux-arm-kernel 1. In order to remove duplicate code, following functions: platform_get_resource devm_kzalloc devm_ioremap_resource devm_clk_get platform_get_irq are replaced with a macro devm_platform_probe_helper. 2. This patch depends on the file include/linux/probe-helper.h which is pushed in previous patch [01/09]. Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Signed-off-by: Satendra Singh Thakur <sst2005@gmail.com> --- drivers/dma/sun6i-dma.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c index ed5b68dcfe50..41ee054bbeeb 100644 --- a/drivers/dma/sun6i-dma.c +++ b/drivers/dma/sun6i-dma.c @@ -19,6 +19,7 @@ #include <linux/reset.h> #include <linux/slab.h> #include <linux/types.h> +#include <linux/probe-helper.h> #include "virt-dma.h" @@ -1234,34 +1235,21 @@ static int sun6i_dma_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct sun6i_dma_dev *sdc; - struct resource *res; int ret, i; - sdc = devm_kzalloc(&pdev->dev, sizeof(*sdc), GFP_KERNEL); - if (!sdc) - return -ENOMEM; + /* + * This macro internally combines following functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq + */ + ret = devm_platform_probe_helper(pdev, sdc, NULL); + if (ret < 0) + return ret; sdc->cfg = of_device_get_match_data(&pdev->dev); if (!sdc->cfg) return -ENODEV; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - sdc->base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(sdc->base)) - return PTR_ERR(sdc->base); - - sdc->irq = platform_get_irq(pdev, 0); - if (sdc->irq < 0) { - dev_err(&pdev->dev, "Cannot claim IRQ\n"); - return sdc->irq; - } - - sdc->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(sdc->clk)) { - dev_err(&pdev->dev, "No clock specified\n"); - return PTR_ERR(sdc->clk); - } - if (sdc->cfg->has_mbus_clk) { sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus"); if (IS_ERR(sdc->clk_mbus)) { -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 7/9] probe/dma/sun4i: removed redundant code from sun4i dma controller's probe function 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur ` (2 preceding siblings ...) 2019-09-15 7:31 ` [PATCH 6/9] probe/dma/sun6i: removed redundant code from sun6i " Satendra Singh Thakur @ 2019-09-15 7:32 ` Satendra Singh Thakur 2019-09-15 7:32 ` [PATCH 9/9] probe/dma/owl: removed redundant code from owl " Satendra Singh Thakur 4 siblings, 0 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:32 UTC (permalink / raw) Cc: Maxime Ripard, Chen-Yu Tsai, linux-kernel, Vinod Koul, satendrasingh.thakur, dmaengine, Dan Williams, Satendra Singh Thakur, linux-arm-kernel 1. In order to remove duplicate code, following functions: platform_get_resource devm_kzalloc devm_ioremap_resource devm_clk_get platform_get_irq are replaced with a macro devm_platform_probe_helper. 2. This patch depends on the file include/linux/probe-helper.h which is pushed in previous patch [01/09]. Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Signed-off-by: Satendra Singh Thakur <sst2005@gmail.com> --- drivers/dma/sun4i-dma.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c index 1f80568b2613..5db139ff43ac 100644 --- a/drivers/dma/sun4i-dma.c +++ b/drivers/dma/sun4i-dma.c @@ -15,6 +15,7 @@ #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/probe-helper.h> #include "virt-dma.h" @@ -1119,29 +1120,16 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id) static int sun4i_dma_probe(struct platform_device *pdev) { struct sun4i_dma_dev *priv; - struct resource *res; int i, j, ret; - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); - if (!priv) - return -ENOMEM; - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - priv->base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(priv->base)) - return PTR_ERR(priv->base); - - priv->irq = platform_get_irq(pdev, 0); - if (priv->irq < 0) { - dev_err(&pdev->dev, "Cannot claim IRQ\n"); - return priv->irq; - } - - priv->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(priv->clk)) { - dev_err(&pdev->dev, "No clock specified\n"); - return PTR_ERR(priv->clk); - } + /* + * This macro internally combines following functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq + */ + ret = devm_platform_probe_helper(pdev, priv, NULL); + if (ret < 0) + return ret; platform_set_drvdata(pdev, priv); spin_lock_init(&priv->lock); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 9/9] probe/dma/owl: removed redundant code from owl dma controller's probe function 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur ` (3 preceding siblings ...) 2019-09-15 7:32 ` [PATCH 7/9] probe/dma/sun4i: removed redundant code from sun4i " Satendra Singh Thakur @ 2019-09-15 7:32 ` Satendra Singh Thakur 4 siblings, 0 replies; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-15 7:32 UTC (permalink / raw) Cc: linux-kernel, Vinod Koul, satendrasingh.thakur, Manivannan Sadhasivam, dmaengine, Dan Williams, Satendra Singh Thakur, Andreas Färber, linux-arm-kernel 1. In order to remove duplicate code, following functions: platform_get_resource devm_kzalloc devm_ioremap_resource devm_clk_get platform_get_irq are replaced with a macro devm_platform_probe_helper. 2. This patch depends on the file include/linux/probe-helper.h which is pushed in previous patch [01/09]. Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com> Signed-off-by: Satendra Singh Thakur <sst2005@gmail.com> --- drivers/dma/owl-dma.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c index 90bbcef99ef8..03e692fc25a1 100644 --- a/drivers/dma/owl-dma.c +++ b/drivers/dma/owl-dma.c @@ -23,6 +23,7 @@ #include <linux/of_device.h> #include <linux/of_dma.h> #include <linux/slab.h> +#include <linux/probe-helper.h> #include "virt-dma.h" #define OWL_DMA_FRAME_MAX_LENGTH 0xfffff @@ -1045,20 +1046,15 @@ static int owl_dma_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct owl_dma *od; - struct resource *res; int ret, i, nr_channels, nr_requests; - - od = devm_kzalloc(&pdev->dev, sizeof(*od), GFP_KERNEL); - if (!od) - return -ENOMEM; - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) - return -EINVAL; - - od->base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(od->base)) - return PTR_ERR(od->base); + /* + * This macro internally combines following functions: + * devm_kzalloc, platform_get_resource, devm_ioremap_resource, + * devm_clk_get, platform_get_irq + */ + ret = devm_platform_probe_helper(pdev, od, NULL); + if (ret < 0) + return ret; ret = of_property_read_u32(np, "dma-channels", &nr_channels); if (ret) { @@ -1105,18 +1101,11 @@ static int owl_dma_probe(struct platform_device *pdev) INIT_LIST_HEAD(&od->dma.channels); - od->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(od->clk)) { - dev_err(&pdev->dev, "unable to get clock\n"); - return PTR_ERR(od->clk); - } - /* * Eventhough the DMA controller is capable of generating 4 * IRQ's for DMA priority feature, we only use 1 IRQ for * simplification. */ - od->irq = platform_get_irq(pdev, 0); ret = devm_request_irq(&pdev->dev, od->irq, owl_dma_interrupt, 0, dev_name(&pdev->dev), od); if (ret) { -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers 2019-09-15 7:00 [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Satendra Singh Thakur 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur @ 2019-09-18 10:27 ` Vinod Koul 2019-09-21 14:57 ` Satendra Singh Thakur 1 sibling, 1 reply; 10+ messages in thread From: Vinod Koul @ 2019-09-18 10:27 UTC (permalink / raw) To: Satendra Singh Thakur Cc: lars, maxime.ripard, sean.wang, linux-kernel, afaerber, dmaengine, wens, agross, linux-mediatek, satendrasingh.thakur, manivannan.sadhasivam, matthias.bgg, dan.j.williams, shawnguo, jun.nie, linux-arm-kernel On 15-09-19, 12:30, Satendra Singh Thakur wrote: > 1. For most of the platform drivers's probe include following steps > > -memory allocation for driver's private structure > -getting io resources > -io remapping resources > -getting irq number > -registering irq > -setting driver's private data > -getting clock > -preparing and enabling clock And we have perfect set of APIs for these tasks! > 2. We have defined a set of macros to combine some or all of > the above mentioned steps. This will remove redundant/duplicate > code in drivers' probe functions of platform drivers. Why, how does it help and you do realize it also introduces bugs > devm_platform_probe_helper(pdev, priv, clk_name); > devm_platform_probe_helper_clk(pdev, priv, clk_name); > devm_platform_probe_helper_irq(pdev, priv, clk_name, > irq_hndlr, irq_flags, irq_name, irq_devid); > devm_platform_probe_helper_all(pdev, priv, clk_name, > irq_hndlr, irq_flags, irq_name, irq_devid); > devm_platform_probe_helper_all_data(pdev, priv, clk_name, > irq_hndlr, irq_flags, irq_name, irq_devid); > > 3. Code is made devres compatible (wherever required) > The functions: clk_get, request_irq, kzalloc, platform_get_resource > are replaced with their devm_* counterparts. We already have devres APIs for people to use! > > 4. Few bugs are also fixed. Which ones..? > > Satendra Singh Thakur (9): > probe/dma : added helper macros to remove redundant/duplicate code > from probe functions of the dma controller drivers > probe/dma/jz4740: removed redundant code from jz4740 dma controller's > probe function > probe/dma/zx: removed redundant code from zx dma controller's probe > function > probe/dma/qcom-bam: removed redundant code from qcom bam dma > controller's probe function > probe/dma/mtk-hs: removed redundant code from mediatek hs dma > controller's probe function > probe/dma/sun6i: removed redundant code from sun6i dma controller's > probe function > probe/dma/sun4i: removed redundant code from sun4i dma controller's > probe function > probe/dma/axi: removed redundant code from axi dma controller's probe > function > probe/dma/owl: removed redundant code from owl dma controller's probe > function > > drivers/dma/dma-axi-dmac.c | 28 ++--- > drivers/dma/dma-jz4740.c | 33 +++--- > drivers/dma/mediatek/mtk-hsdma.c | 38 +++---- > drivers/dma/owl-dma.c | 29 ++--- > drivers/dma/qcom/bam_dma.c | 71 +++++------- > drivers/dma/sun4i-dma.c | 30 ++---- > drivers/dma/sun6i-dma.c | 30 ++---- > drivers/dma/zx_dma.c | 35 ++---- > include/linux/probe-helper.h | 179 +++++++++++++++++++++++++++++++ > 9 files changed, 280 insertions(+), 193 deletions(-) > create mode 100644 include/linux/probe-helper.h > > -- > 2.17.1 -- ~Vinod _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers 2019-09-18 10:27 ` [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Vinod Koul @ 2019-09-21 14:57 ` Satendra Singh Thakur 2019-09-24 19:27 ` Vinod Koul 0 siblings, 1 reply; 10+ messages in thread From: Satendra Singh Thakur @ 2019-09-21 14:57 UTC (permalink / raw) To: dan.j.williams, vkoul, jun.nie, shawnguo, agross, sean.wang, matthias.bgg, maxime.ripard, wens, lars, afaerber, manivannan.sadhasivam Cc: linux-kernel, linux-mediatek, satendrasingh.thakur, dmaengine, Satendra Singh Thakur, linux-arm-kernel On Wed, Sep 18, 2019 at 3:57 PM, Vinod Koul wrote: > On 15-09-19, 12:30, Satendra Singh Thakur wrote: > > 1. For most of the platform drivers's probe include following steps > > > > -memory allocation for driver's private structure > > -getting io resources > > -io remapping resources > > -getting irq number > > -registering irq > > -setting driver's private data > > -getting clock > > -preparing and enabling clock > > And we have perfect set of APIs for these tasks! Hi Vinod, Thanks for the comments. You are right, we already have set of APIs for these tasks. The proposed macros combine the very same APIs to remove duplicate/redundant code. A new driver author can use these macros to easily write probe function without having to worry about signatures of internal APIs. In the past, people have combined some of them e.g. a) clk_prepare_enable combines clk_prepare and clk_enable b) devm_platform_ioremap_resource combines platform_get_resource (for type IORESOURCE_MEM) and devm_ioremap_resource c) module_platform_driver macro encompasses module_init/exit and driver_register/unregister functions. The basic idea is to simplyfy coding. > > 2. We have defined a set of macros to combine some or all of > > the above mentioned steps. This will remove redundant/duplicate > > code in drivers' probe functions of platform drivers. > > Why, how does it help and you do realize it also introduces bugs This will make probe function shorter by removing repeated code. This will also reduce bugs caused due to improper handling of failure cases because of these reasons: a) If the developer calls each API individualy one might miss proper handling of the failure for some API; Whereas the macro properly handles failure of each API. b) Macros are devres compatible which leaves less room for memory leaks. Yes, the macros which enable clock and request irqs might cause bugs if they are not used carefully. For instance, enabling the clock or requesting the irq earlier than actually required. So, the macros with _clk and _irq, _all suffix should be used carefully. Please let me know if I miss any specific type of bug here. > > > devm_platform_probe_helper(pdev, priv, clk_name); > > devm_platform_probe_helper_clk(pdev, priv, clk_name); > > devm_platform_probe_helper_irq(pdev, priv, clk_name, > > irq_hndlr, irq_flags, irq_name, irq_devid); > > devm_platform_probe_helper_all(pdev, priv, clk_name, > > irq_hndlr, irq_flags, irq_name, irq_devid); > > devm_platform_probe_helper_all_data(pdev, priv, clk_name, > > irq_hndlr, irq_flags, irq_name, irq_devid); > > > > 3. Code is made devres compatible (wherever required) > > The functions: clk_get, request_irq, kzalloc, platform_get_resource > > are replaced with their devm_* counterparts. > > We already have devres APIs for people to use! Yes, we have devres APIs and many drivers do use them. But still there are many which don't use them. The proposed macros provides just another way to use devres APIs. > > > > 4. Few bugs are also fixed. > > Which ones..? The bug is that the failure of request_irq is not handled properly in mtk-hsdma.c. This is fixed in patch [5/9]. https://lkml.org/lkml/2019/9/15/35 Please let me know if I am missing something here. Thanks -Satendra _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers 2019-09-21 14:57 ` Satendra Singh Thakur @ 2019-09-24 19:27 ` Vinod Koul 0 siblings, 0 replies; 10+ messages in thread From: Vinod Koul @ 2019-09-24 19:27 UTC (permalink / raw) To: Satendra Singh Thakur Cc: lars, maxime.ripard, sean.wang, linux-kernel, afaerber, dmaengine, wens, agross, linux-mediatek, satendrasingh.thakur, manivannan.sadhasivam, matthias.bgg, dan.j.williams, shawnguo, jun.nie, linux-arm-kernel On 21-09-19, 20:27, Satendra Singh Thakur wrote: > On Wed, Sep 18, 2019 at 3:57 PM, Vinod Koul wrote: > > On 15-09-19, 12:30, Satendra Singh Thakur wrote: > > > 1. For most of the platform drivers's probe include following steps > > > > > > -memory allocation for driver's private structure > > > -getting io resources > > > -io remapping resources > > > -getting irq number > > > -registering irq > > > -setting driver's private data > > > -getting clock > > > -preparing and enabling clock > > > > And we have perfect set of APIs for these tasks! > Hi Vinod, > Thanks for the comments. > You are right, we already have set of APIs for these tasks. > The proposed macros combine the very same APIs to remove > duplicate/redundant code. > A new driver author can use these macros to easily write probe Nope they can write each APIs, know the exact flow and do it! > function without having to worry about signatures of internal APIs. > In the past, people have combined some of them e.g. > a) clk_prepare_enable combines clk_prepare and clk_enable As it is clock, it is called in sequence, whereas different drivers may have different sequencing. Btw I am not for adding maanged irq functions, they are really not the correct way to manage! > b) devm_platform_ioremap_resource combines > platform_get_resource (for type IORESOURCE_MEM) > and devm_ioremap_resource > c) module_platform_driver macro encompasses module_init/exit > and driver_register/unregister functions. All examples you are quoting do a set of functions (clk, resources etc and not do N things! > The basic idea is to simplyfy coding. > > > 2. We have defined a set of macros to combine some or all of > > > the above mentioned steps. This will remove redundant/duplicate > > > code in drivers' probe functions of platform drivers. > > > > Why, how does it help and you do realize it also introduces bugs > This will make probe function shorter by removing repeated code. > This will also reduce bugs caused due to improper handling > of failure cases because of these reasons: > a) If the developer calls each API individualy one might miss > proper handling of the failure for some API; Whereas the macro > properly handles failure of each API. > b) Macros are devres compatible which leaves less room for > memory leaks. No we review the code and if we miss we fix later! > Yes, the macros which enable clock and request irqs > might cause bugs if they are not used carefully. > For instance, enabling the clock or requesting the irq > earlier than actually required. So, the macros with _clk > and _irq, _all suffix should be used carefully. Precisely! > Please let me know if I miss any specific type of bug > here. > > > > > devm_platform_probe_helper(pdev, priv, clk_name); > > > devm_platform_probe_helper_clk(pdev, priv, clk_name); > > > devm_platform_probe_helper_irq(pdev, priv, clk_name, > > > irq_hndlr, irq_flags, irq_name, irq_devid); > > > devm_platform_probe_helper_all(pdev, priv, clk_name, > > > irq_hndlr, irq_flags, irq_name, irq_devid); > > > devm_platform_probe_helper_all_data(pdev, priv, clk_name, > > > irq_hndlr, irq_flags, irq_name, irq_devid); > > > > > > 3. Code is made devres compatible (wherever required) > > > The functions: clk_get, request_irq, kzalloc, platform_get_resource > > > are replaced with their devm_* counterparts. > > > > We already have devres APIs for people to use! > Yes, we have devres APIs and many drivers do use them. > But still there are many which don't use them. > The proposed macros provides just another way to use devres APIs. > > > > > > 4. Few bugs are also fixed. > > > > Which ones..? > The bug is that the failure of request_irq > is not handled properly in mtk-hsdma.c. This is fixed in patch [5/9]. > https://lkml.org/lkml/2019/9/15/35 Please send the fix individually and I would be glad to review. -- ~Vinod _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-09-24 19:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-15 7:00 [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Satendra Singh Thakur 2019-09-15 7:26 ` [PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers Satendra Singh Thakur 2019-09-15 7:29 ` [PATCH 3/9] probe/dma/zx: removed redundant code from zx dma controller's probe function Satendra Singh Thakur 2019-09-15 7:30 ` [PATCH 5/9] probe/dma/mtk-hs: removed redundant code from mediatek hs " Satendra Singh Thakur 2019-09-15 7:31 ` [PATCH 6/9] probe/dma/sun6i: removed redundant code from sun6i " Satendra Singh Thakur 2019-09-15 7:32 ` [PATCH 7/9] probe/dma/sun4i: removed redundant code from sun4i " Satendra Singh Thakur 2019-09-15 7:32 ` [PATCH 9/9] probe/dma/owl: removed redundant code from owl " Satendra Singh Thakur 2019-09-18 10:27 ` [PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers Vinod Koul 2019-09-21 14:57 ` Satendra Singh Thakur 2019-09-24 19:27 ` Vinod Koul
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).