* [PATCH 0/3] Use scope based cleanup in drivers/soc/ti/
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Do scope based "struct device_node" cleanup in drivers/soc/ti/*. This
covers all the possible cases.
Kousik Sanagavarapu (3):
soc: ti: pruss: do device_node auto cleanup
soc: ti: knav_qmss_queue: do device_node auto cleanup
soc: ti: pm33xx: do device_node auto cleanup
drivers/soc/ti/knav_qmss_queue.c | 17 ++++++-----
drivers/soc/ti/pm33xx.c | 20 +++++--------
drivers/soc/ti/pruss.c | 48 +++++++++++---------------------
3 files changed, 31 insertions(+), 54 deletions(-)
--
2.45.0.rc1.8.ge326e52010
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] soc: ti: pruss: do device_node auto cleanup
2024-05-10 7:13 ` Kousik Sanagavarapu
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
-1 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Use scope based cleanup instead of manual of_node_put() calls, hence
simplifying the handling of error paths at various places.
Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
drivers/soc/ti/pruss.c | 48 ++++++++++++++----------------------------
1 file changed, 16 insertions(+), 32 deletions(-)
diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
index 24a42e0b645c..8f238d59eca9 100644
--- a/drivers/soc/ti/pruss.c
+++ b/drivers/soc/ti/pruss.c
@@ -381,13 +381,13 @@ static int pruss_clk_mux_setup(struct pruss *pruss, struct clk *clk_mux,
static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
{
const struct pruss_private_data *data;
- struct device_node *clks_np;
struct device *dev = pruss->dev;
int ret = 0;
data = of_device_get_match_data(dev);
- clks_np = of_get_child_by_name(cfg_node, "clocks");
+ struct device_node *clks_np __free(device_node) =
+ of_get_child_by_name(cfg_node, "clocks");
if (!clks_np) {
dev_err(dev, "%pOF is missing its 'clocks' node\n", cfg_node);
return -ENODEV;
@@ -398,7 +398,7 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
"coreclk-mux", clks_np);
if (ret) {
dev_err(dev, "failed to setup coreclk-mux\n");
- goto put_clks_node;
+ return ret;
}
}
@@ -406,13 +406,10 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
clks_np);
if (ret) {
dev_err(dev, "failed to setup iepclk-mux\n");
- goto put_clks_node;
+ return ret;
}
-put_clks_node:
- of_node_put(clks_np);
-
- return ret;
+ return 0;
}
static struct regmap_config regmap_conf = {
@@ -424,26 +421,22 @@ static struct regmap_config regmap_conf = {
static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
{
struct device_node *np = dev_of_node(dev);
- struct device_node *child;
+ struct device_node *child __free(device_node) =
+ of_get_child_by_name(np, "cfg");
struct resource res;
int ret;
- child = of_get_child_by_name(np, "cfg");
if (!child) {
dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
return -ENODEV;
}
- if (of_address_to_resource(child, 0, &res)) {
- ret = -ENOMEM;
- goto node_put;
- }
+ if (of_address_to_resource(child, 0, &res))
+ return -ENOMEM;
pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
- if (!pruss->cfg_base) {
- ret = -ENOMEM;
- goto node_put;
- }
+ if (!pruss->cfg_base)
+ return -ENOMEM;
regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
(u64)res.start);
@@ -455,16 +448,13 @@ static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
if (IS_ERR(pruss->cfg_regmap)) {
dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
PTR_ERR(pruss->cfg_regmap));
- ret = PTR_ERR(pruss->cfg_regmap);
- goto node_put;
+ return PTR_ERR(pruss->cfg_regmap);
}
ret = pruss_clk_init(pruss, child);
if (ret)
dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
-node_put:
- of_node_put(child);
return ret;
}
@@ -472,7 +462,6 @@ static int pruss_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *np = dev_of_node(dev);
- struct device_node *child;
struct pruss *pruss;
struct resource res;
int ret, i, index;
@@ -494,7 +483,8 @@ static int pruss_probe(struct platform_device *pdev)
pruss->dev = dev;
mutex_init(&pruss->lock);
- child = of_get_child_by_name(np, "memories");
+ struct device_node *child __free(device_node) =
+ of_get_child_by_name(np, "memories");
if (!child) {
dev_err(dev, "%pOF is missing its 'memories' node\n", child);
return -ENODEV;
@@ -510,22 +500,17 @@ static int pruss_probe(struct platform_device *pdev)
index = of_property_match_string(child, "reg-names",
mem_names[i]);
- if (index < 0) {
- of_node_put(child);
+ if (index < 0)
return index;
- }
- if (of_address_to_resource(child, index, &res)) {
- of_node_put(child);
+ if (of_address_to_resource(child, index, &res))
return -EINVAL;
- }
pruss->mem_regions[i].va = devm_ioremap(dev, res.start,
resource_size(&res));
if (!pruss->mem_regions[i].va) {
dev_err(dev, "failed to parse and map memory resource %d %s\n",
i, mem_names[i]);
- of_node_put(child);
return -ENOMEM;
}
pruss->mem_regions[i].pa = res.start;
@@ -535,7 +520,6 @@ static int pruss_probe(struct platform_device *pdev)
mem_names[i], &pruss->mem_regions[i].pa,
pruss->mem_regions[i].size, pruss->mem_regions[i].va);
}
- of_node_put(child);
platform_set_drvdata(pdev, pruss);
--
2.45.0.rc1.8.ge326e52010
_______________________________________________
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] 24+ messages in thread* [PATCH 1/3] soc: ti: pruss: do device_node auto cleanup
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Use scope based cleanup instead of manual of_node_put() calls, hence
simplifying the handling of error paths at various places.
Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
drivers/soc/ti/pruss.c | 48 ++++++++++++++----------------------------
1 file changed, 16 insertions(+), 32 deletions(-)
diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
index 24a42e0b645c..8f238d59eca9 100644
--- a/drivers/soc/ti/pruss.c
+++ b/drivers/soc/ti/pruss.c
@@ -381,13 +381,13 @@ static int pruss_clk_mux_setup(struct pruss *pruss, struct clk *clk_mux,
static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
{
const struct pruss_private_data *data;
- struct device_node *clks_np;
struct device *dev = pruss->dev;
int ret = 0;
data = of_device_get_match_data(dev);
- clks_np = of_get_child_by_name(cfg_node, "clocks");
+ struct device_node *clks_np __free(device_node) =
+ of_get_child_by_name(cfg_node, "clocks");
if (!clks_np) {
dev_err(dev, "%pOF is missing its 'clocks' node\n", cfg_node);
return -ENODEV;
@@ -398,7 +398,7 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
"coreclk-mux", clks_np);
if (ret) {
dev_err(dev, "failed to setup coreclk-mux\n");
- goto put_clks_node;
+ return ret;
}
}
@@ -406,13 +406,10 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
clks_np);
if (ret) {
dev_err(dev, "failed to setup iepclk-mux\n");
- goto put_clks_node;
+ return ret;
}
-put_clks_node:
- of_node_put(clks_np);
-
- return ret;
+ return 0;
}
static struct regmap_config regmap_conf = {
@@ -424,26 +421,22 @@ static struct regmap_config regmap_conf = {
static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
{
struct device_node *np = dev_of_node(dev);
- struct device_node *child;
+ struct device_node *child __free(device_node) =
+ of_get_child_by_name(np, "cfg");
struct resource res;
int ret;
- child = of_get_child_by_name(np, "cfg");
if (!child) {
dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
return -ENODEV;
}
- if (of_address_to_resource(child, 0, &res)) {
- ret = -ENOMEM;
- goto node_put;
- }
+ if (of_address_to_resource(child, 0, &res))
+ return -ENOMEM;
pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
- if (!pruss->cfg_base) {
- ret = -ENOMEM;
- goto node_put;
- }
+ if (!pruss->cfg_base)
+ return -ENOMEM;
regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
(u64)res.start);
@@ -455,16 +448,13 @@ static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
if (IS_ERR(pruss->cfg_regmap)) {
dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
PTR_ERR(pruss->cfg_regmap));
- ret = PTR_ERR(pruss->cfg_regmap);
- goto node_put;
+ return PTR_ERR(pruss->cfg_regmap);
}
ret = pruss_clk_init(pruss, child);
if (ret)
dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
-node_put:
- of_node_put(child);
return ret;
}
@@ -472,7 +462,6 @@ static int pruss_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct device_node *np = dev_of_node(dev);
- struct device_node *child;
struct pruss *pruss;
struct resource res;
int ret, i, index;
@@ -494,7 +483,8 @@ static int pruss_probe(struct platform_device *pdev)
pruss->dev = dev;
mutex_init(&pruss->lock);
- child = of_get_child_by_name(np, "memories");
+ struct device_node *child __free(device_node) =
+ of_get_child_by_name(np, "memories");
if (!child) {
dev_err(dev, "%pOF is missing its 'memories' node\n", child);
return -ENODEV;
@@ -510,22 +500,17 @@ static int pruss_probe(struct platform_device *pdev)
index = of_property_match_string(child, "reg-names",
mem_names[i]);
- if (index < 0) {
- of_node_put(child);
+ if (index < 0)
return index;
- }
- if (of_address_to_resource(child, index, &res)) {
- of_node_put(child);
+ if (of_address_to_resource(child, index, &res))
return -EINVAL;
- }
pruss->mem_regions[i].va = devm_ioremap(dev, res.start,
resource_size(&res));
if (!pruss->mem_regions[i].va) {
dev_err(dev, "failed to parse and map memory resource %d %s\n",
i, mem_names[i]);
- of_node_put(child);
return -ENOMEM;
}
pruss->mem_regions[i].pa = res.start;
@@ -535,7 +520,6 @@ static int pruss_probe(struct platform_device *pdev)
mem_names[i], &pruss->mem_regions[i].pa,
pruss->mem_regions[i].size, pruss->mem_regions[i].va);
}
- of_node_put(child);
platform_set_drvdata(pdev, pruss);
--
2.45.0.rc1.8.ge326e52010
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 1/3] soc: ti: pruss: do device_node auto cleanup
2024-05-10 7:13 ` Kousik Sanagavarapu
@ 2024-05-16 14:41 ` Jonathan Cameron
-1 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-05-16 14:41 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: Nishanth Menon, Santosh Shilimkar, Julia Lawall, Shuah Khan,
Javier Carrasco, linux-kernel, linux-arm-kernel
On Fri, 10 May 2024 12:43:22 +0530
Kousik Sanagavarapu <five231003@gmail.com> wrote:
> Use scope based cleanup instead of manual of_node_put() calls, hence
> simplifying the handling of error paths at various places.
>
> Suggested-by: Julia Lawall <julia.lawall@inria.fr>
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---
> drivers/soc/ti/pruss.c | 48 ++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index 24a42e0b645c..8f238d59eca9 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -381,13 +381,13 @@ static int pruss_clk_mux_setup(struct pruss *pruss, struct clk *clk_mux,
> static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> {
> const struct pruss_private_data *data;
> - struct device_node *clks_np;
> struct device *dev = pruss->dev;
> int ret = 0;
>
> data = of_device_get_match_data(dev);
>
> - clks_np = of_get_child_by_name(cfg_node, "clocks");
> + struct device_node *clks_np __free(device_node) =
> + of_get_child_by_name(cfg_node, "clocks");
> if (!clks_np) {
> dev_err(dev, "%pOF is missing its 'clocks' node\n", cfg_node);
> return -ENODEV;
> @@ -398,7 +398,7 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> "coreclk-mux", clks_np);
> if (ret) {
> dev_err(dev, "failed to setup coreclk-mux\n");
> - goto put_clks_node;
> + return ret;
This driver would also benefit from use of dev_err_probe() for all cases
like this that are called only from the probe() callback.
return dev_err_probe(dev, ret, "failed to setup coreclk-mux\n2);
Handles any deferred cases nicely, but in general simplifies code by
making these printer error message and return cases a single line.
> }
> }
>
> @@ -406,13 +406,10 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> clks_np);
> if (ret) {
> dev_err(dev, "failed to setup iepclk-mux\n");
> - goto put_clks_node;
> + return ret;
> }
>
> -put_clks_node:
> - of_node_put(clks_np);
> -
> - return ret;
> + return 0;
> }
>
> static struct regmap_config regmap_conf = {
> @@ -424,26 +421,22 @@ static struct regmap_config regmap_conf = {
> static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
> {
> struct device_node *np = dev_of_node(dev);
> - struct device_node *child;
> + struct device_node *child __free(device_node) =
> + of_get_child_by_name(np, "cfg");
> struct resource res;
> int ret;
>
> - child = of_get_child_by_name(np, "cfg");
> if (!child) {
> dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
> return -ENODEV;
> }
>
> - if (of_address_to_resource(child, 0, &res)) {
> - ret = -ENOMEM;
> - goto node_put;
> - }
> + if (of_address_to_resource(child, 0, &res))
> + return -ENOMEM;
>
> pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
> - if (!pruss->cfg_base) {
> - ret = -ENOMEM;
> - goto node_put;
> - }
> + if (!pruss->cfg_base)
> + return -ENOMEM;
>
> regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
> (u64)res.start);
> @@ -455,16 +448,13 @@ static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
> if (IS_ERR(pruss->cfg_regmap)) {
> dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
> PTR_ERR(pruss->cfg_regmap));
> - ret = PTR_ERR(pruss->cfg_regmap);
> - goto node_put;
> + return PTR_ERR(pruss->cfg_regmap);
> }
>
> ret = pruss_clk_init(pruss, child);
> if (ret)
> dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
>
> -node_put:
> - of_node_put(child);
> return ret;
> }
>
> @@ -472,7 +462,6 @@ static int pruss_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev_of_node(dev);
> - struct device_node *child;
> struct pruss *pruss;
> struct resource res;
> int ret, i, index;
> @@ -494,7 +483,8 @@ static int pruss_probe(struct platform_device *pdev)
> pruss->dev = dev;
> mutex_init(&pruss->lock);
>
> - child = of_get_child_by_name(np, "memories");
> + struct device_node *child __free(device_node) =
> + of_get_child_by_name(np, "memories");
In this case, the scope of the device_node being held is increased quite
a bit. Whilst that shouldn't be a problem, it is a bit ugly.
Perhaps factor this code out into another function similar to the ones above?
Jonathan
> if (!child) {
> dev_err(dev, "%pOF is missing its 'memories' node\n", child);
> return -ENODEV;
> @@ -510,22 +500,17 @@ static int pruss_probe(struct platform_device *pdev)
>
> index = of_property_match_string(child, "reg-names",
> mem_names[i]);
> - if (index < 0) {
> - of_node_put(child);
> + if (index < 0)
> return index;
> - }
>
> - if (of_address_to_resource(child, index, &res)) {
> - of_node_put(child);
> + if (of_address_to_resource(child, index, &res))
> return -EINVAL;
> - }
>
> pruss->mem_regions[i].va = devm_ioremap(dev, res.start,
> resource_size(&res));
> if (!pruss->mem_regions[i].va) {
> dev_err(dev, "failed to parse and map memory resource %d %s\n",
> i, mem_names[i]);
> - of_node_put(child);
> return -ENOMEM;
> }
> pruss->mem_regions[i].pa = res.start;
> @@ -535,7 +520,6 @@ static int pruss_probe(struct platform_device *pdev)
> mem_names[i], &pruss->mem_regions[i].pa,
> pruss->mem_regions[i].size, pruss->mem_regions[i].va);
> }
> - of_node_put(child);
>
> platform_set_drvdata(pdev, pruss);
>
_______________________________________________
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] 24+ messages in thread* Re: [PATCH 1/3] soc: ti: pruss: do device_node auto cleanup
@ 2024-05-16 14:41 ` Jonathan Cameron
0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-05-16 14:41 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: Nishanth Menon, Santosh Shilimkar, Julia Lawall, Shuah Khan,
Javier Carrasco, linux-kernel, linux-arm-kernel
On Fri, 10 May 2024 12:43:22 +0530
Kousik Sanagavarapu <five231003@gmail.com> wrote:
> Use scope based cleanup instead of manual of_node_put() calls, hence
> simplifying the handling of error paths at various places.
>
> Suggested-by: Julia Lawall <julia.lawall@inria.fr>
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---
> drivers/soc/ti/pruss.c | 48 ++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c
> index 24a42e0b645c..8f238d59eca9 100644
> --- a/drivers/soc/ti/pruss.c
> +++ b/drivers/soc/ti/pruss.c
> @@ -381,13 +381,13 @@ static int pruss_clk_mux_setup(struct pruss *pruss, struct clk *clk_mux,
> static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> {
> const struct pruss_private_data *data;
> - struct device_node *clks_np;
> struct device *dev = pruss->dev;
> int ret = 0;
>
> data = of_device_get_match_data(dev);
>
> - clks_np = of_get_child_by_name(cfg_node, "clocks");
> + struct device_node *clks_np __free(device_node) =
> + of_get_child_by_name(cfg_node, "clocks");
> if (!clks_np) {
> dev_err(dev, "%pOF is missing its 'clocks' node\n", cfg_node);
> return -ENODEV;
> @@ -398,7 +398,7 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> "coreclk-mux", clks_np);
> if (ret) {
> dev_err(dev, "failed to setup coreclk-mux\n");
> - goto put_clks_node;
> + return ret;
This driver would also benefit from use of dev_err_probe() for all cases
like this that are called only from the probe() callback.
return dev_err_probe(dev, ret, "failed to setup coreclk-mux\n2);
Handles any deferred cases nicely, but in general simplifies code by
making these printer error message and return cases a single line.
> }
> }
>
> @@ -406,13 +406,10 @@ static int pruss_clk_init(struct pruss *pruss, struct device_node *cfg_node)
> clks_np);
> if (ret) {
> dev_err(dev, "failed to setup iepclk-mux\n");
> - goto put_clks_node;
> + return ret;
> }
>
> -put_clks_node:
> - of_node_put(clks_np);
> -
> - return ret;
> + return 0;
> }
>
> static struct regmap_config regmap_conf = {
> @@ -424,26 +421,22 @@ static struct regmap_config regmap_conf = {
> static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
> {
> struct device_node *np = dev_of_node(dev);
> - struct device_node *child;
> + struct device_node *child __free(device_node) =
> + of_get_child_by_name(np, "cfg");
> struct resource res;
> int ret;
>
> - child = of_get_child_by_name(np, "cfg");
> if (!child) {
> dev_err(dev, "%pOF is missing its 'cfg' node\n", child);
> return -ENODEV;
> }
>
> - if (of_address_to_resource(child, 0, &res)) {
> - ret = -ENOMEM;
> - goto node_put;
> - }
> + if (of_address_to_resource(child, 0, &res))
> + return -ENOMEM;
>
> pruss->cfg_base = devm_ioremap(dev, res.start, resource_size(&res));
> - if (!pruss->cfg_base) {
> - ret = -ENOMEM;
> - goto node_put;
> - }
> + if (!pruss->cfg_base)
> + return -ENOMEM;
>
> regmap_conf.name = kasprintf(GFP_KERNEL, "%pOFn@%llx", child,
> (u64)res.start);
> @@ -455,16 +448,13 @@ static int pruss_cfg_of_init(struct device *dev, struct pruss *pruss)
> if (IS_ERR(pruss->cfg_regmap)) {
> dev_err(dev, "regmap_init_mmio failed for cfg, ret = %ld\n",
> PTR_ERR(pruss->cfg_regmap));
> - ret = PTR_ERR(pruss->cfg_regmap);
> - goto node_put;
> + return PTR_ERR(pruss->cfg_regmap);
> }
>
> ret = pruss_clk_init(pruss, child);
> if (ret)
> dev_err(dev, "pruss_clk_init failed, ret = %d\n", ret);
>
> -node_put:
> - of_node_put(child);
> return ret;
> }
>
> @@ -472,7 +462,6 @@ static int pruss_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev_of_node(dev);
> - struct device_node *child;
> struct pruss *pruss;
> struct resource res;
> int ret, i, index;
> @@ -494,7 +483,8 @@ static int pruss_probe(struct platform_device *pdev)
> pruss->dev = dev;
> mutex_init(&pruss->lock);
>
> - child = of_get_child_by_name(np, "memories");
> + struct device_node *child __free(device_node) =
> + of_get_child_by_name(np, "memories");
In this case, the scope of the device_node being held is increased quite
a bit. Whilst that shouldn't be a problem, it is a bit ugly.
Perhaps factor this code out into another function similar to the ones above?
Jonathan
> if (!child) {
> dev_err(dev, "%pOF is missing its 'memories' node\n", child);
> return -ENODEV;
> @@ -510,22 +500,17 @@ static int pruss_probe(struct platform_device *pdev)
>
> index = of_property_match_string(child, "reg-names",
> mem_names[i]);
> - if (index < 0) {
> - of_node_put(child);
> + if (index < 0)
> return index;
> - }
>
> - if (of_address_to_resource(child, index, &res)) {
> - of_node_put(child);
> + if (of_address_to_resource(child, index, &res))
> return -EINVAL;
> - }
>
> pruss->mem_regions[i].va = devm_ioremap(dev, res.start,
> resource_size(&res));
> if (!pruss->mem_regions[i].va) {
> dev_err(dev, "failed to parse and map memory resource %d %s\n",
> i, mem_names[i]);
> - of_node_put(child);
> return -ENOMEM;
> }
> pruss->mem_regions[i].pa = res.start;
> @@ -535,7 +520,6 @@ static int pruss_probe(struct platform_device *pdev)
> mem_names[i], &pruss->mem_regions[i].pa,
> pruss->mem_regions[i].size, pruss->mem_regions[i].va);
> }
> - of_node_put(child);
>
> platform_set_drvdata(pdev, pruss);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-10 7:13 ` Kousik Sanagavarapu
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
-1 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Use scope based cleanup, instead of manual of_node_put() calls, which
automatically free()s "struct device_node".
Doing the cleanup this way has the advantage of reducing the chance of
memory leaks in case we need to read from new OF nodes in the future
when we probe.
Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
drivers/soc/ti/knav_qmss_queue.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 06fb5505c22c..237946312080 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -1755,7 +1755,6 @@ MODULE_DEVICE_TABLE(of, keystone_qmss_of_match);
static int knav_queue_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
- struct device_node *qmgrs, *queue_pools, *regions, *pdsps;
struct device *dev = &pdev->dev;
u32 temp[2];
int ret;
@@ -1799,19 +1798,20 @@ static int knav_queue_probe(struct platform_device *pdev)
kdev->num_queues = temp[1];
/* Initialize queue managers using device tree configuration */
- qmgrs = of_get_child_by_name(node, "qmgrs");
+ struct device_node *qmgrs __free(device_node) =
+ of_get_child_by_name(node, "qmgrs");
if (!qmgrs) {
dev_err(dev, "queue manager info not specified\n");
ret = -ENODEV;
goto err;
}
ret = knav_queue_init_qmgrs(kdev, qmgrs);
- of_node_put(qmgrs);
if (ret)
goto err;
/* get pdsp configuration values from device tree */
- pdsps = of_get_child_by_name(node, "pdsps");
+ struct device_node *pdsps __free(device_node) =
+ of_get_child_by_name(node, "pdsps");
if (pdsps) {
ret = knav_queue_init_pdsps(kdev, pdsps);
if (ret)
@@ -1821,17 +1821,16 @@ static int knav_queue_probe(struct platform_device *pdev)
if (ret)
goto err;
}
- of_node_put(pdsps);
/* get usable queue range values from device tree */
- queue_pools = of_get_child_by_name(node, "queue-pools");
+ struct device_node *queue_pools __free(device_node) =
+ of_get_child_by_name(node, "queue-pools");
if (!queue_pools) {
dev_err(dev, "queue-pools not specified\n");
ret = -ENODEV;
goto err;
}
ret = knav_setup_queue_pools(kdev, queue_pools);
- of_node_put(queue_pools);
if (ret)
goto err;
@@ -1853,14 +1852,14 @@ static int knav_queue_probe(struct platform_device *pdev)
if (ret)
goto err;
- regions = of_get_child_by_name(node, "descriptor-regions");
+ struct device_node *regions __free(device_node) =
+ of_get_child_by_name(node, "descriptor-regions");
if (!regions) {
dev_err(dev, "descriptor-regions not specified\n");
ret = -ENODEV;
goto err;
}
ret = knav_queue_setup_regions(kdev, regions);
- of_node_put(regions);
if (ret)
goto err;
--
2.45.0.rc1.8.ge326e52010
_______________________________________________
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] 24+ messages in thread* [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Use scope based cleanup, instead of manual of_node_put() calls, which
automatically free()s "struct device_node".
Doing the cleanup this way has the advantage of reducing the chance of
memory leaks in case we need to read from new OF nodes in the future
when we probe.
Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
drivers/soc/ti/knav_qmss_queue.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 06fb5505c22c..237946312080 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -1755,7 +1755,6 @@ MODULE_DEVICE_TABLE(of, keystone_qmss_of_match);
static int knav_queue_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
- struct device_node *qmgrs, *queue_pools, *regions, *pdsps;
struct device *dev = &pdev->dev;
u32 temp[2];
int ret;
@@ -1799,19 +1798,20 @@ static int knav_queue_probe(struct platform_device *pdev)
kdev->num_queues = temp[1];
/* Initialize queue managers using device tree configuration */
- qmgrs = of_get_child_by_name(node, "qmgrs");
+ struct device_node *qmgrs __free(device_node) =
+ of_get_child_by_name(node, "qmgrs");
if (!qmgrs) {
dev_err(dev, "queue manager info not specified\n");
ret = -ENODEV;
goto err;
}
ret = knav_queue_init_qmgrs(kdev, qmgrs);
- of_node_put(qmgrs);
if (ret)
goto err;
/* get pdsp configuration values from device tree */
- pdsps = of_get_child_by_name(node, "pdsps");
+ struct device_node *pdsps __free(device_node) =
+ of_get_child_by_name(node, "pdsps");
if (pdsps) {
ret = knav_queue_init_pdsps(kdev, pdsps);
if (ret)
@@ -1821,17 +1821,16 @@ static int knav_queue_probe(struct platform_device *pdev)
if (ret)
goto err;
}
- of_node_put(pdsps);
/* get usable queue range values from device tree */
- queue_pools = of_get_child_by_name(node, "queue-pools");
+ struct device_node *queue_pools __free(device_node) =
+ of_get_child_by_name(node, "queue-pools");
if (!queue_pools) {
dev_err(dev, "queue-pools not specified\n");
ret = -ENODEV;
goto err;
}
ret = knav_setup_queue_pools(kdev, queue_pools);
- of_node_put(queue_pools);
if (ret)
goto err;
@@ -1853,14 +1852,14 @@ static int knav_queue_probe(struct platform_device *pdev)
if (ret)
goto err;
- regions = of_get_child_by_name(node, "descriptor-regions");
+ struct device_node *regions __free(device_node) =
+ of_get_child_by_name(node, "descriptor-regions");
if (!regions) {
dev_err(dev, "descriptor-regions not specified\n");
ret = -ENODEV;
goto err;
}
ret = knav_queue_setup_regions(kdev, regions);
- of_node_put(regions);
if (ret)
goto err;
--
2.45.0.rc1.8.ge326e52010
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-10 7:13 ` Kousik Sanagavarapu
@ 2024-05-11 10:12 ` kernel test robot
-1 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2024-05-11 10:12 UTC (permalink / raw)
To: Kousik Sanagavarapu, Nishanth Menon, Santosh Shilimkar,
Julia Lawall
Cc: llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco, linux-kernel,
linux-arm-kernel, Kousik Sanagavarapu
Hi Kousik,
kernel test robot noticed the following build errors:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on soc/for-next linus/master v6.9-rc7 next-20240510]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kousik-Sanagavarapu/soc-ti-pruss-do-device_node-auto-cleanup/20240510-151656
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20240510071432.62913-3-five231003%40gmail.com
patch subject: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240511/202405111846.3m9z398l-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240511/202405111846.3m9z398l-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405111846.3m9z398l-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *pdsps __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *pdsps __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *pdsps __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *qmgrs __free(device_node) =
^
9 errors generated.
vim +1853 drivers/soc/ti/knav_qmss_queue.c
350601b4f7ab45 Murali Karicheri 2018-04-17 1754
41f93af900a20d Sandeep Nair 2014-02-28 1755 static int knav_queue_probe(struct platform_device *pdev)
41f93af900a20d Sandeep Nair 2014-02-28 1756 {
41f93af900a20d Sandeep Nair 2014-02-28 1757 struct device_node *node = pdev->dev.of_node;
41f93af900a20d Sandeep Nair 2014-02-28 1758 struct device *dev = &pdev->dev;
41f93af900a20d Sandeep Nair 2014-02-28 1759 u32 temp[2];
41f93af900a20d Sandeep Nair 2014-02-28 1760 int ret;
41f93af900a20d Sandeep Nair 2014-02-28 1761
41f93af900a20d Sandeep Nair 2014-02-28 1762 if (!node) {
41f93af900a20d Sandeep Nair 2014-02-28 1763 dev_err(dev, "device tree info unavailable\n");
41f93af900a20d Sandeep Nair 2014-02-28 1764 return -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1765 }
41f93af900a20d Sandeep Nair 2014-02-28 1766
41f93af900a20d Sandeep Nair 2014-02-28 1767 kdev = devm_kzalloc(dev, sizeof(struct knav_device), GFP_KERNEL);
41f93af900a20d Sandeep Nair 2014-02-28 1768 if (!kdev) {
41f93af900a20d Sandeep Nair 2014-02-28 1769 dev_err(dev, "memory allocation failed\n");
41f93af900a20d Sandeep Nair 2014-02-28 1770 return -ENOMEM;
41f93af900a20d Sandeep Nair 2014-02-28 1771 }
41f93af900a20d Sandeep Nair 2014-02-28 1772
50c01a942b2874 Rob Herring 2023-10-09 1773 if (device_get_match_data(dev))
350601b4f7ab45 Murali Karicheri 2018-04-17 1774 kdev->version = QMSS_66AK2G;
350601b4f7ab45 Murali Karicheri 2018-04-17 1775
41f93af900a20d Sandeep Nair 2014-02-28 1776 platform_set_drvdata(pdev, kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1777 kdev->dev = dev;
41f93af900a20d Sandeep Nair 2014-02-28 1778 INIT_LIST_HEAD(&kdev->queue_ranges);
41f93af900a20d Sandeep Nair 2014-02-28 1779 INIT_LIST_HEAD(&kdev->qmgrs);
41f93af900a20d Sandeep Nair 2014-02-28 1780 INIT_LIST_HEAD(&kdev->pools);
41f93af900a20d Sandeep Nair 2014-02-28 1781 INIT_LIST_HEAD(&kdev->regions);
41f93af900a20d Sandeep Nair 2014-02-28 1782 INIT_LIST_HEAD(&kdev->pdsps);
41f93af900a20d Sandeep Nair 2014-02-28 1783
41f93af900a20d Sandeep Nair 2014-02-28 1784 pm_runtime_enable(&pdev->dev);
12eeb74925da70 Minghao Chi 2022-04-18 1785 ret = pm_runtime_resume_and_get(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1786 if (ret < 0) {
e961c0f19450fd Zhang Qilong 2022-11-08 1787 pm_runtime_disable(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1788 dev_err(dev, "Failed to enable QMSS\n");
41f93af900a20d Sandeep Nair 2014-02-28 1789 return ret;
41f93af900a20d Sandeep Nair 2014-02-28 1790 }
41f93af900a20d Sandeep Nair 2014-02-28 1791
41f93af900a20d Sandeep Nair 2014-02-28 1792 if (of_property_read_u32_array(node, "queue-range", temp, 2)) {
41f93af900a20d Sandeep Nair 2014-02-28 1793 dev_err(dev, "queue-range not specified\n");
41f93af900a20d Sandeep Nair 2014-02-28 1794 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1795 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1796 }
41f93af900a20d Sandeep Nair 2014-02-28 1797 kdev->base_id = temp[0];
41f93af900a20d Sandeep Nair 2014-02-28 1798 kdev->num_queues = temp[1];
41f93af900a20d Sandeep Nair 2014-02-28 1799
41f93af900a20d Sandeep Nair 2014-02-28 1800 /* Initialize queue managers using device tree configuration */
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1801 struct device_node *qmgrs __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1802 of_get_child_by_name(node, "qmgrs");
41f93af900a20d Sandeep Nair 2014-02-28 1803 if (!qmgrs) {
41f93af900a20d Sandeep Nair 2014-02-28 1804 dev_err(dev, "queue manager info not specified\n");
41f93af900a20d Sandeep Nair 2014-02-28 1805 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1806 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1807 }
41f93af900a20d Sandeep Nair 2014-02-28 1808 ret = knav_queue_init_qmgrs(kdev, qmgrs);
41f93af900a20d Sandeep Nair 2014-02-28 1809 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1810 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1811
41f93af900a20d Sandeep Nair 2014-02-28 1812 /* get pdsp configuration values from device tree */
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1813 struct device_node *pdsps __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1814 of_get_child_by_name(node, "pdsps");
41f93af900a20d Sandeep Nair 2014-02-28 1815 if (pdsps) {
41f93af900a20d Sandeep Nair 2014-02-28 1816 ret = knav_queue_init_pdsps(kdev, pdsps);
41f93af900a20d Sandeep Nair 2014-02-28 1817 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1818 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1819
41f93af900a20d Sandeep Nair 2014-02-28 1820 ret = knav_queue_start_pdsps(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1821 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1822 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1823 }
41f93af900a20d Sandeep Nair 2014-02-28 1824
41f93af900a20d Sandeep Nair 2014-02-28 1825 /* get usable queue range values from device tree */
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1826 struct device_node *queue_pools __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1827 of_get_child_by_name(node, "queue-pools");
41f93af900a20d Sandeep Nair 2014-02-28 1828 if (!queue_pools) {
41f93af900a20d Sandeep Nair 2014-02-28 1829 dev_err(dev, "queue-pools not specified\n");
41f93af900a20d Sandeep Nair 2014-02-28 1830 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1831 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1832 }
41f93af900a20d Sandeep Nair 2014-02-28 1833 ret = knav_setup_queue_pools(kdev, queue_pools);
41f93af900a20d Sandeep Nair 2014-02-28 1834 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1835 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1836
41f93af900a20d Sandeep Nair 2014-02-28 1837 ret = knav_get_link_ram(kdev, "linkram0", &kdev->link_rams[0]);
41f93af900a20d Sandeep Nair 2014-02-28 1838 if (ret) {
41f93af900a20d Sandeep Nair 2014-02-28 1839 dev_err(kdev->dev, "could not setup linking ram\n");
41f93af900a20d Sandeep Nair 2014-02-28 1840 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1841 }
41f93af900a20d Sandeep Nair 2014-02-28 1842
41f93af900a20d Sandeep Nair 2014-02-28 1843 ret = knav_get_link_ram(kdev, "linkram1", &kdev->link_rams[1]);
41f93af900a20d Sandeep Nair 2014-02-28 1844 if (ret) {
41f93af900a20d Sandeep Nair 2014-02-28 1845 /*
41f93af900a20d Sandeep Nair 2014-02-28 1846 * nothing really, we have one linking ram already, so we just
41f93af900a20d Sandeep Nair 2014-02-28 1847 * live within our means
41f93af900a20d Sandeep Nair 2014-02-28 1848 */
41f93af900a20d Sandeep Nair 2014-02-28 1849 }
41f93af900a20d Sandeep Nair 2014-02-28 1850
41f93af900a20d Sandeep Nair 2014-02-28 1851 ret = knav_queue_setup_link_ram(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1852 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 @1853 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1854
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1855 struct device_node *regions __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1856 of_get_child_by_name(node, "descriptor-regions");
41f93af900a20d Sandeep Nair 2014-02-28 1857 if (!regions) {
41f93af900a20d Sandeep Nair 2014-02-28 1858 dev_err(dev, "descriptor-regions not specified\n");
4cba398f37f868 Zhihao Cheng 2020-11-21 1859 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1860 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1861 }
41f93af900a20d Sandeep Nair 2014-02-28 1862 ret = knav_queue_setup_regions(kdev, regions);
41f93af900a20d Sandeep Nair 2014-02-28 1863 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1864 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1865
41f93af900a20d Sandeep Nair 2014-02-28 1866 ret = knav_queue_init_queues(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1867 if (ret < 0) {
41f93af900a20d Sandeep Nair 2014-02-28 1868 dev_err(dev, "hwqueue initialization failed\n");
41f93af900a20d Sandeep Nair 2014-02-28 1869 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1870 }
41f93af900a20d Sandeep Nair 2014-02-28 1871
41f93af900a20d Sandeep Nair 2014-02-28 1872 debugfs_create_file("qmss", S_IFREG | S_IRUGO, NULL, NULL,
74e0e43a09cea3 Qinglang Miao 2020-09-20 1873 &knav_queue_debug_fops);
a2dd6877b43ef1 Murali Karicheri 2018-04-17 1874 device_ready = true;
41f93af900a20d Sandeep Nair 2014-02-28 1875 return 0;
41f93af900a20d Sandeep Nair 2014-02-28 1876
41f93af900a20d Sandeep Nair 2014-02-28 1877 err:
41f93af900a20d Sandeep Nair 2014-02-28 1878 knav_queue_stop_pdsps(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1879 knav_queue_free_regions(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1880 knav_free_queue_ranges(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1881 pm_runtime_put_sync(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1882 pm_runtime_disable(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1883 return ret;
41f93af900a20d Sandeep Nair 2014-02-28 1884 }
41f93af900a20d Sandeep Nair 2014-02-28 1885
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-11 10:12 ` kernel test robot
0 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2024-05-11 10:12 UTC (permalink / raw)
To: Kousik Sanagavarapu, Nishanth Menon, Santosh Shilimkar,
Julia Lawall
Cc: llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco, linux-kernel,
linux-arm-kernel, Kousik Sanagavarapu
Hi Kousik,
kernel test robot noticed the following build errors:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on soc/for-next linus/master v6.9-rc7 next-20240510]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kousik-Sanagavarapu/soc-ti-pruss-do-device_node-auto-cleanup/20240510-151656
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20240510071432.62913-3-five231003%40gmail.com
patch subject: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240511/202405111846.3m9z398l-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240511/202405111846.3m9z398l-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405111846.3m9z398l-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *pdsps __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *pdsps __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
goto err;
^
drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *regions __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *queue_pools __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *pdsps __free(device_node) =
^
drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
struct device_node *qmgrs __free(device_node) =
^
9 errors generated.
vim +1853 drivers/soc/ti/knav_qmss_queue.c
350601b4f7ab45 Murali Karicheri 2018-04-17 1754
41f93af900a20d Sandeep Nair 2014-02-28 1755 static int knav_queue_probe(struct platform_device *pdev)
41f93af900a20d Sandeep Nair 2014-02-28 1756 {
41f93af900a20d Sandeep Nair 2014-02-28 1757 struct device_node *node = pdev->dev.of_node;
41f93af900a20d Sandeep Nair 2014-02-28 1758 struct device *dev = &pdev->dev;
41f93af900a20d Sandeep Nair 2014-02-28 1759 u32 temp[2];
41f93af900a20d Sandeep Nair 2014-02-28 1760 int ret;
41f93af900a20d Sandeep Nair 2014-02-28 1761
41f93af900a20d Sandeep Nair 2014-02-28 1762 if (!node) {
41f93af900a20d Sandeep Nair 2014-02-28 1763 dev_err(dev, "device tree info unavailable\n");
41f93af900a20d Sandeep Nair 2014-02-28 1764 return -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1765 }
41f93af900a20d Sandeep Nair 2014-02-28 1766
41f93af900a20d Sandeep Nair 2014-02-28 1767 kdev = devm_kzalloc(dev, sizeof(struct knav_device), GFP_KERNEL);
41f93af900a20d Sandeep Nair 2014-02-28 1768 if (!kdev) {
41f93af900a20d Sandeep Nair 2014-02-28 1769 dev_err(dev, "memory allocation failed\n");
41f93af900a20d Sandeep Nair 2014-02-28 1770 return -ENOMEM;
41f93af900a20d Sandeep Nair 2014-02-28 1771 }
41f93af900a20d Sandeep Nair 2014-02-28 1772
50c01a942b2874 Rob Herring 2023-10-09 1773 if (device_get_match_data(dev))
350601b4f7ab45 Murali Karicheri 2018-04-17 1774 kdev->version = QMSS_66AK2G;
350601b4f7ab45 Murali Karicheri 2018-04-17 1775
41f93af900a20d Sandeep Nair 2014-02-28 1776 platform_set_drvdata(pdev, kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1777 kdev->dev = dev;
41f93af900a20d Sandeep Nair 2014-02-28 1778 INIT_LIST_HEAD(&kdev->queue_ranges);
41f93af900a20d Sandeep Nair 2014-02-28 1779 INIT_LIST_HEAD(&kdev->qmgrs);
41f93af900a20d Sandeep Nair 2014-02-28 1780 INIT_LIST_HEAD(&kdev->pools);
41f93af900a20d Sandeep Nair 2014-02-28 1781 INIT_LIST_HEAD(&kdev->regions);
41f93af900a20d Sandeep Nair 2014-02-28 1782 INIT_LIST_HEAD(&kdev->pdsps);
41f93af900a20d Sandeep Nair 2014-02-28 1783
41f93af900a20d Sandeep Nair 2014-02-28 1784 pm_runtime_enable(&pdev->dev);
12eeb74925da70 Minghao Chi 2022-04-18 1785 ret = pm_runtime_resume_and_get(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1786 if (ret < 0) {
e961c0f19450fd Zhang Qilong 2022-11-08 1787 pm_runtime_disable(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1788 dev_err(dev, "Failed to enable QMSS\n");
41f93af900a20d Sandeep Nair 2014-02-28 1789 return ret;
41f93af900a20d Sandeep Nair 2014-02-28 1790 }
41f93af900a20d Sandeep Nair 2014-02-28 1791
41f93af900a20d Sandeep Nair 2014-02-28 1792 if (of_property_read_u32_array(node, "queue-range", temp, 2)) {
41f93af900a20d Sandeep Nair 2014-02-28 1793 dev_err(dev, "queue-range not specified\n");
41f93af900a20d Sandeep Nair 2014-02-28 1794 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1795 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1796 }
41f93af900a20d Sandeep Nair 2014-02-28 1797 kdev->base_id = temp[0];
41f93af900a20d Sandeep Nair 2014-02-28 1798 kdev->num_queues = temp[1];
41f93af900a20d Sandeep Nair 2014-02-28 1799
41f93af900a20d Sandeep Nair 2014-02-28 1800 /* Initialize queue managers using device tree configuration */
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1801 struct device_node *qmgrs __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1802 of_get_child_by_name(node, "qmgrs");
41f93af900a20d Sandeep Nair 2014-02-28 1803 if (!qmgrs) {
41f93af900a20d Sandeep Nair 2014-02-28 1804 dev_err(dev, "queue manager info not specified\n");
41f93af900a20d Sandeep Nair 2014-02-28 1805 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1806 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1807 }
41f93af900a20d Sandeep Nair 2014-02-28 1808 ret = knav_queue_init_qmgrs(kdev, qmgrs);
41f93af900a20d Sandeep Nair 2014-02-28 1809 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1810 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1811
41f93af900a20d Sandeep Nair 2014-02-28 1812 /* get pdsp configuration values from device tree */
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1813 struct device_node *pdsps __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1814 of_get_child_by_name(node, "pdsps");
41f93af900a20d Sandeep Nair 2014-02-28 1815 if (pdsps) {
41f93af900a20d Sandeep Nair 2014-02-28 1816 ret = knav_queue_init_pdsps(kdev, pdsps);
41f93af900a20d Sandeep Nair 2014-02-28 1817 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1818 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1819
41f93af900a20d Sandeep Nair 2014-02-28 1820 ret = knav_queue_start_pdsps(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1821 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1822 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1823 }
41f93af900a20d Sandeep Nair 2014-02-28 1824
41f93af900a20d Sandeep Nair 2014-02-28 1825 /* get usable queue range values from device tree */
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1826 struct device_node *queue_pools __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1827 of_get_child_by_name(node, "queue-pools");
41f93af900a20d Sandeep Nair 2014-02-28 1828 if (!queue_pools) {
41f93af900a20d Sandeep Nair 2014-02-28 1829 dev_err(dev, "queue-pools not specified\n");
41f93af900a20d Sandeep Nair 2014-02-28 1830 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1831 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1832 }
41f93af900a20d Sandeep Nair 2014-02-28 1833 ret = knav_setup_queue_pools(kdev, queue_pools);
41f93af900a20d Sandeep Nair 2014-02-28 1834 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1835 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1836
41f93af900a20d Sandeep Nair 2014-02-28 1837 ret = knav_get_link_ram(kdev, "linkram0", &kdev->link_rams[0]);
41f93af900a20d Sandeep Nair 2014-02-28 1838 if (ret) {
41f93af900a20d Sandeep Nair 2014-02-28 1839 dev_err(kdev->dev, "could not setup linking ram\n");
41f93af900a20d Sandeep Nair 2014-02-28 1840 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1841 }
41f93af900a20d Sandeep Nair 2014-02-28 1842
41f93af900a20d Sandeep Nair 2014-02-28 1843 ret = knav_get_link_ram(kdev, "linkram1", &kdev->link_rams[1]);
41f93af900a20d Sandeep Nair 2014-02-28 1844 if (ret) {
41f93af900a20d Sandeep Nair 2014-02-28 1845 /*
41f93af900a20d Sandeep Nair 2014-02-28 1846 * nothing really, we have one linking ram already, so we just
41f93af900a20d Sandeep Nair 2014-02-28 1847 * live within our means
41f93af900a20d Sandeep Nair 2014-02-28 1848 */
41f93af900a20d Sandeep Nair 2014-02-28 1849 }
41f93af900a20d Sandeep Nair 2014-02-28 1850
41f93af900a20d Sandeep Nair 2014-02-28 1851 ret = knav_queue_setup_link_ram(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1852 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 @1853 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1854
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1855 struct device_node *regions __free(device_node) =
10e6f93cca367b Kousik Sanagavarapu 2024-05-10 1856 of_get_child_by_name(node, "descriptor-regions");
41f93af900a20d Sandeep Nair 2014-02-28 1857 if (!regions) {
41f93af900a20d Sandeep Nair 2014-02-28 1858 dev_err(dev, "descriptor-regions not specified\n");
4cba398f37f868 Zhihao Cheng 2020-11-21 1859 ret = -ENODEV;
41f93af900a20d Sandeep Nair 2014-02-28 1860 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1861 }
41f93af900a20d Sandeep Nair 2014-02-28 1862 ret = knav_queue_setup_regions(kdev, regions);
41f93af900a20d Sandeep Nair 2014-02-28 1863 if (ret)
41f93af900a20d Sandeep Nair 2014-02-28 1864 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1865
41f93af900a20d Sandeep Nair 2014-02-28 1866 ret = knav_queue_init_queues(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1867 if (ret < 0) {
41f93af900a20d Sandeep Nair 2014-02-28 1868 dev_err(dev, "hwqueue initialization failed\n");
41f93af900a20d Sandeep Nair 2014-02-28 1869 goto err;
41f93af900a20d Sandeep Nair 2014-02-28 1870 }
41f93af900a20d Sandeep Nair 2014-02-28 1871
41f93af900a20d Sandeep Nair 2014-02-28 1872 debugfs_create_file("qmss", S_IFREG | S_IRUGO, NULL, NULL,
74e0e43a09cea3 Qinglang Miao 2020-09-20 1873 &knav_queue_debug_fops);
a2dd6877b43ef1 Murali Karicheri 2018-04-17 1874 device_ready = true;
41f93af900a20d Sandeep Nair 2014-02-28 1875 return 0;
41f93af900a20d Sandeep Nair 2014-02-28 1876
41f93af900a20d Sandeep Nair 2014-02-28 1877 err:
41f93af900a20d Sandeep Nair 2014-02-28 1878 knav_queue_stop_pdsps(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1879 knav_queue_free_regions(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1880 knav_free_queue_ranges(kdev);
41f93af900a20d Sandeep Nair 2014-02-28 1881 pm_runtime_put_sync(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1882 pm_runtime_disable(&pdev->dev);
41f93af900a20d Sandeep Nair 2014-02-28 1883 return ret;
41f93af900a20d Sandeep Nair 2014-02-28 1884 }
41f93af900a20d Sandeep Nair 2014-02-28 1885
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
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] 24+ messages in thread* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-11 10:12 ` kernel test robot
@ 2024-05-12 10:26 ` Kousik Sanagavarapu
-1 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-12 10:26 UTC (permalink / raw)
To: kernel test robot
Cc: Nishanth Menon, Santosh Shilimkar, Julia Lawall, llvm,
oe-kbuild-all, Shuah Khan, Javier Carrasco, linux-kernel,
linux-arm-kernel
On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> Hi Kousik,
>
> kernel test robot noticed the following build errors:
>
[...]
> All errors (new ones prefixed by >>):
>
> >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *pdsps __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *pdsps __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *pdsps __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *qmgrs __free(device_node) =
> ^
> 9 errors generated.
Seems like gcc didn't catch this when I compiled locally.
Normally, this would be fixed if we placed braces around the individual
initialization blocks, that is, say
{
struct device_node *qmgrs __free(device_node) =
of_get_child_by_name(node, "qmgrs");
...
}
That would make the code look a lot more dirty though and is purely
unnecessary. So I'd say I'd drop this patch and do a v2 with the
remaining two patches. Thoughts?
There's also some stuff with classes but that too is not really worth
doing because the code will end up looking very ugly.
Thanks
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-12 10:26 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-12 10:26 UTC (permalink / raw)
To: kernel test robot
Cc: Nishanth Menon, Santosh Shilimkar, Julia Lawall, llvm,
oe-kbuild-all, Shuah Khan, Javier Carrasco, linux-kernel,
linux-arm-kernel
On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> Hi Kousik,
>
> kernel test robot noticed the following build errors:
>
[...]
> All errors (new ones prefixed by >>):
>
> >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *pdsps __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *pdsps __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
> goto err;
> ^
> drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *regions __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *queue_pools __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *pdsps __free(device_node) =
> ^
> drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> struct device_node *qmgrs __free(device_node) =
> ^
> 9 errors generated.
Seems like gcc didn't catch this when I compiled locally.
Normally, this would be fixed if we placed braces around the individual
initialization blocks, that is, say
{
struct device_node *qmgrs __free(device_node) =
of_get_child_by_name(node, "qmgrs");
...
}
That would make the code look a lot more dirty though and is purely
unnecessary. So I'd say I'd drop this patch and do a v2 with the
remaining two patches. Thoughts?
There's also some stuff with classes but that too is not really worth
doing because the code will end up looking very ugly.
Thanks
_______________________________________________
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] 24+ messages in thread* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-12 10:26 ` Kousik Sanagavarapu
@ 2024-05-12 10:36 ` Julia Lawall
-1 siblings, 0 replies; 24+ messages in thread
From: Julia Lawall @ 2024-05-12 10:36 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: kernel test robot, Nishanth Menon, Santosh Shilimkar,
Julia Lawall, llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco,
linux-kernel, linux-arm-kernel
On Sun, 12 May 2024, Kousik Sanagavarapu wrote:
> On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > Hi Kousik,
> >
> > kernel test robot noticed the following build errors:
> >
>
> [...]
>
> > All errors (new ones prefixed by >>):
> >
> > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *qmgrs __free(device_node) =
> > ^
> > 9 errors generated.
>
> Seems like gcc didn't catch this when I compiled locally.
>
> Normally, this would be fixed if we placed braces around the individual
> initialization blocks, that is, say
>
> {
> struct device_node *qmgrs __free(device_node) =
> of_get_child_by_name(node, "qmgrs");
> ...
> }
>
>
> That would make the code look a lot more dirty though and is purely
> unnecessary. So I'd say I'd drop this patch and do a v2 with the
> remaining two patches. Thoughts?
>
> There's also some stuff with classes but that too is not really worth
> doing because the code will end up looking very ugly.
Please include the patch in such a message so that one can see everything
at once. I'm not sure it's necessary to show all the error messages
either. Just a few that help illustrate the problem.
julia
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-12 10:36 ` Julia Lawall
0 siblings, 0 replies; 24+ messages in thread
From: Julia Lawall @ 2024-05-12 10:36 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: kernel test robot, Nishanth Menon, Santosh Shilimkar,
Julia Lawall, llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco,
linux-kernel, linux-arm-kernel
On Sun, 12 May 2024, Kousik Sanagavarapu wrote:
> On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > Hi Kousik,
> >
> > kernel test robot noticed the following build errors:
> >
>
> [...]
>
> > All errors (new ones prefixed by >>):
> >
> > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *qmgrs __free(device_node) =
> > ^
> > 9 errors generated.
>
> Seems like gcc didn't catch this when I compiled locally.
>
> Normally, this would be fixed if we placed braces around the individual
> initialization blocks, that is, say
>
> {
> struct device_node *qmgrs __free(device_node) =
> of_get_child_by_name(node, "qmgrs");
> ...
> }
>
>
> That would make the code look a lot more dirty though and is purely
> unnecessary. So I'd say I'd drop this patch and do a v2 with the
> remaining two patches. Thoughts?
>
> There's also some stuff with classes but that too is not really worth
> doing because the code will end up looking very ugly.
Please include the patch in such a message so that one can see everything
at once. I'm not sure it's necessary to show all the error messages
either. Just a few that help illustrate the problem.
julia
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-12 10:26 ` Kousik Sanagavarapu
@ 2024-05-13 6:44 ` Nathan Chancellor
-1 siblings, 0 replies; 24+ messages in thread
From: Nathan Chancellor @ 2024-05-13 6:44 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: kernel test robot, Nishanth Menon, Santosh Shilimkar,
Julia Lawall, llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco,
linux-kernel, linux-arm-kernel
On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:
> On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > Hi Kousik,
> >
> > kernel test robot noticed the following build errors:
> >
>
> [...]
>
> > All errors (new ones prefixed by >>):
> >
> > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *qmgrs __free(device_node) =
> > ^
> > 9 errors generated.
>
> Seems like gcc didn't catch this when I compiled locally.
FWIW, you may notice this as you do more conversions. The fact that GCC
does not warn at all is a GCC bug as far as I am aware (i.e., clang's
error is correct):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
which has come up in other places:
https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/
Cheers,
Nathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-13 6:44 ` Nathan Chancellor
0 siblings, 0 replies; 24+ messages in thread
From: Nathan Chancellor @ 2024-05-13 6:44 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: kernel test robot, Nishanth Menon, Santosh Shilimkar,
Julia Lawall, llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco,
linux-kernel, linux-arm-kernel
On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:
> On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > Hi Kousik,
> >
> > kernel test robot noticed the following build errors:
> >
>
> [...]
>
> > All errors (new ones prefixed by >>):
> >
> > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1840:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1835:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1831:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1822:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1818:4: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1810:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1806:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1795:3: error: cannot jump from this goto statement to its label
> > goto err;
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *regions __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1826:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *queue_pools __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1813:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *pdsps __free(device_node) =
> > ^
> > drivers/soc/ti/knav_qmss_queue.c:1801:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > struct device_node *qmgrs __free(device_node) =
> > ^
> > 9 errors generated.
>
> Seems like gcc didn't catch this when I compiled locally.
FWIW, you may notice this as you do more conversions. The fact that GCC
does not warn at all is a GCC bug as far as I am aware (i.e., clang's
error is correct):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
which has come up in other places:
https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/
Cheers,
Nathan
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-13 6:44 ` Nathan Chancellor
@ 2024-05-13 7:23 ` Kousik Sanagavarapu
-1 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-13 7:23 UTC (permalink / raw)
To: Nathan Chancellor
Cc: kernel test robot, Nishanth Menon, Santosh Shilimkar,
Julia Lawall, llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco,
linux-kernel, linux-arm-kernel
On Sun, May 12, 2024 at 11:44:51PM -0700, Nathan Chancellor wrote:
> On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:
> > On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > > Hi Kousik,
> > >
> > > kernel test robot noticed the following build errors:
> > >
> >
> > [...]
> >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > > goto err;
> > > ^
> > > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > > struct device_node *regions __free(device_node) =
> > > ^
[...]
> > Seems like gcc didn't catch this when I compiled locally.
>
> FWIW, you may notice this as you do more conversions. The fact that GCC
> does not warn at all is a GCC bug as far as I am aware (i.e., clang's
> error is correct):
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
>
> which has come up in other places:
>
> https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/
Thank you so much for these links :)
All my internet searches ended up at stackoverflow posts which didn't
even describe the problem correctly, which also lead me to write an
email explaining a partly erroneous solution, which is sitting in my
mailbox ;)
Thanks again, these will help a lot.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-13 7:23 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-13 7:23 UTC (permalink / raw)
To: Nathan Chancellor
Cc: kernel test robot, Nishanth Menon, Santosh Shilimkar,
Julia Lawall, llvm, oe-kbuild-all, Shuah Khan, Javier Carrasco,
linux-kernel, linux-arm-kernel
On Sun, May 12, 2024 at 11:44:51PM -0700, Nathan Chancellor wrote:
> On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:
> > On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > > Hi Kousik,
> > >
> > > kernel test robot noticed the following build errors:
> > >
> >
> > [...]
> >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > > goto err;
> > > ^
> > > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > > struct device_node *regions __free(device_node) =
> > > ^
[...]
> > Seems like gcc didn't catch this when I compiled locally.
>
> FWIW, you may notice this as you do more conversions. The fact that GCC
> does not warn at all is a GCC bug as far as I am aware (i.e., clang's
> error is correct):
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
>
> which has come up in other places:
>
> https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/
Thank you so much for these links :)
All my internet searches ended up at stackoverflow posts which didn't
even describe the problem correctly, which also lead me to write an
email explaining a partly erroneous solution, which is sitting in my
mailbox ;)
Thanks again, these will help a lot.
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
2024-05-13 7:23 ` Kousik Sanagavarapu
@ 2024-05-16 14:48 ` Jonathan Cameron
-1 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-05-16 14:48 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: Nathan Chancellor, kernel test robot, Nishanth Menon,
Santosh Shilimkar, Julia Lawall, llvm, oe-kbuild-all, Shuah Khan,
Javier Carrasco, linux-kernel, linux-arm-kernel
On Mon, 13 May 2024 12:53:27 +0530
Kousik Sanagavarapu <five231003@gmail.com> wrote:
> On Sun, May 12, 2024 at 11:44:51PM -0700, Nathan Chancellor wrote:
> > On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:
> > > On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > > > Hi Kousik,
> > > >
> > > > kernel test robot noticed the following build errors:
> > > >
> > >
> > > [...]
> > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > > > goto err;
> > > > ^
> > > > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > > > struct device_node *regions __free(device_node) =
> > > > ^
>
> [...]
>
> > > Seems like gcc didn't catch this when I compiled locally.
> >
> > FWIW, you may notice this as you do more conversions. The fact that GCC
> > does not warn at all is a GCC bug as far as I am aware (i.e., clang's
> > error is correct):
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
> >
> > which has come up in other places:
> >
> > https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/
>
> Thank you so much for these links :)
>
> All my internet searches ended up at stackoverflow posts which didn't
> even describe the problem correctly, which also lead me to write an
> email explaining a partly erroneous solution, which is sitting in my
> mailbox ;)
>
> Thanks again, these will help a lot.
Independent of all this, it's not a good idea form a readability point
of view to mix automated and manual cleanup. So in cases like this
where you want to do scope based cleanup, use separate functions
that have appropriately defined scope (or brackets for the really minor
cases).
Here, you may just be able to push the device_node get into
knav_queue_setup_regions() for example.
Jonathan
>
> _______________________________________________
> 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] 24+ messages in thread
* Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
@ 2024-05-16 14:48 ` Jonathan Cameron
0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2024-05-16 14:48 UTC (permalink / raw)
To: Kousik Sanagavarapu
Cc: Nathan Chancellor, kernel test robot, Nishanth Menon,
Santosh Shilimkar, Julia Lawall, llvm, oe-kbuild-all, Shuah Khan,
Javier Carrasco, linux-kernel, linux-arm-kernel
On Mon, 13 May 2024 12:53:27 +0530
Kousik Sanagavarapu <five231003@gmail.com> wrote:
> On Sun, May 12, 2024 at 11:44:51PM -0700, Nathan Chancellor wrote:
> > On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:
> > > On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:
> > > > Hi Kousik,
> > > >
> > > > kernel test robot noticed the following build errors:
> > > >
> > >
> > > [...]
> > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label
> > > > goto err;
> > > > ^
> > > > drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > > > struct device_node *regions __free(device_node) =
> > > > ^
>
> [...]
>
> > > Seems like gcc didn't catch this when I compiled locally.
> >
> > FWIW, you may notice this as you do more conversions. The fact that GCC
> > does not warn at all is a GCC bug as far as I am aware (i.e., clang's
> > error is correct):
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
> >
> > which has come up in other places:
> >
> > https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/
>
> Thank you so much for these links :)
>
> All my internet searches ended up at stackoverflow posts which didn't
> even describe the problem correctly, which also lead me to write an
> email explaining a partly erroneous solution, which is sitting in my
> mailbox ;)
>
> Thanks again, these will help a lot.
Independent of all this, it's not a good idea form a readability point
of view to mix automated and manual cleanup. So in cases like this
where you want to do scope based cleanup, use separate functions
that have appropriately defined scope (or brackets for the really minor
cases).
Here, you may just be able to push the device_node get into
knav_queue_setup_regions() for example.
Jonathan
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
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] 24+ messages in thread
* [PATCH 3/3] soc: ti: pm33xx: do device_node auto cleanup
2024-05-10 7:13 ` Kousik Sanagavarapu
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
-1 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Use scope based cleanup instead of manual of_node_put() calls, hence
simplifying the handling of error paths.
Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
drivers/soc/ti/pm33xx.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c
index 8e983c3c4e03..40988c45ed00 100644
--- a/drivers/soc/ti/pm33xx.c
+++ b/drivers/soc/ti/pm33xx.c
@@ -383,10 +383,9 @@ static void am33xx_pm_free_sram(void)
*/
static int am33xx_pm_alloc_sram(void)
{
- struct device_node *np;
- int ret = 0;
+ struct device_node *np __free(device_node) =
+ of_find_compatible_node(NULL, NULL, "ti,omap3-mpu");
- np = of_find_compatible_node(NULL, NULL, "ti,omap3-mpu");
if (!np) {
np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu");
if (!np) {
@@ -400,24 +399,21 @@ static int am33xx_pm_alloc_sram(void)
if (!sram_pool) {
dev_err(pm33xx_dev, "PM: %s: Unable to get sram pool for ocmcram\n",
__func__);
- ret = -ENODEV;
- goto mpu_put_node;
+ return -ENODEV;
}
sram_pool_data = of_gen_pool_get(np, "pm-sram", 1);
if (!sram_pool_data) {
dev_err(pm33xx_dev, "PM: %s: Unable to get sram data pool for ocmcram\n",
__func__);
- ret = -ENODEV;
- goto mpu_put_node;
+ return -ENODEV;
}
ocmcram_location = gen_pool_alloc(sram_pool, *pm_sram->do_wfi_sz);
if (!ocmcram_location) {
dev_err(pm33xx_dev, "PM: %s: Unable to allocate memory from ocmcram\n",
__func__);
- ret = -ENOMEM;
- goto mpu_put_node;
+ return -ENOMEM;
}
ocmcram_location_data = gen_pool_alloc(sram_pool_data,
@@ -425,12 +421,10 @@ static int am33xx_pm_alloc_sram(void)
if (!ocmcram_location_data) {
dev_err(pm33xx_dev, "PM: Unable to allocate memory from ocmcram\n");
gen_pool_free(sram_pool, ocmcram_location, *pm_sram->do_wfi_sz);
- ret = -ENOMEM;
+ return -ENOMEM;
}
-mpu_put_node:
- of_node_put(np);
- return ret;
+ return 0;
}
static int am33xx_pm_rtc_setup(void)
--
2.45.0.rc1.8.ge326e52010
_______________________________________________
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] 24+ messages in thread* [PATCH 3/3] soc: ti: pm33xx: do device_node auto cleanup
@ 2024-05-10 7:13 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-10 7:13 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel,
Kousik Sanagavarapu
Use scope based cleanup instead of manual of_node_put() calls, hence
simplifying the handling of error paths.
Suggested-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
drivers/soc/ti/pm33xx.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c
index 8e983c3c4e03..40988c45ed00 100644
--- a/drivers/soc/ti/pm33xx.c
+++ b/drivers/soc/ti/pm33xx.c
@@ -383,10 +383,9 @@ static void am33xx_pm_free_sram(void)
*/
static int am33xx_pm_alloc_sram(void)
{
- struct device_node *np;
- int ret = 0;
+ struct device_node *np __free(device_node) =
+ of_find_compatible_node(NULL, NULL, "ti,omap3-mpu");
- np = of_find_compatible_node(NULL, NULL, "ti,omap3-mpu");
if (!np) {
np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu");
if (!np) {
@@ -400,24 +399,21 @@ static int am33xx_pm_alloc_sram(void)
if (!sram_pool) {
dev_err(pm33xx_dev, "PM: %s: Unable to get sram pool for ocmcram\n",
__func__);
- ret = -ENODEV;
- goto mpu_put_node;
+ return -ENODEV;
}
sram_pool_data = of_gen_pool_get(np, "pm-sram", 1);
if (!sram_pool_data) {
dev_err(pm33xx_dev, "PM: %s: Unable to get sram data pool for ocmcram\n",
__func__);
- ret = -ENODEV;
- goto mpu_put_node;
+ return -ENODEV;
}
ocmcram_location = gen_pool_alloc(sram_pool, *pm_sram->do_wfi_sz);
if (!ocmcram_location) {
dev_err(pm33xx_dev, "PM: %s: Unable to allocate memory from ocmcram\n",
__func__);
- ret = -ENOMEM;
- goto mpu_put_node;
+ return -ENOMEM;
}
ocmcram_location_data = gen_pool_alloc(sram_pool_data,
@@ -425,12 +421,10 @@ static int am33xx_pm_alloc_sram(void)
if (!ocmcram_location_data) {
dev_err(pm33xx_dev, "PM: Unable to allocate memory from ocmcram\n");
gen_pool_free(sram_pool, ocmcram_location, *pm_sram->do_wfi_sz);
- ret = -ENOMEM;
+ return -ENOMEM;
}
-mpu_put_node:
- of_node_put(np);
- return ret;
+ return 0;
}
static int am33xx_pm_rtc_setup(void)
--
2.45.0.rc1.8.ge326e52010
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 0/3] Use scope based cleanup in drivers/soc/ti/
2024-05-10 7:13 ` Kousik Sanagavarapu
@ 2024-05-11 9:06 ` Kousik Sanagavarapu
-1 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-11 9:06 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel
On Fri, May 10, 2024 at 08:17:54AM -0500, Nishanth Menon wrote:
> On 12:38-20240510, Kousik Sanagavarapu wrote:
> > Do scope based "struct device_node" cleanup in drivers/soc/ti/*. This
> > series converts all the possible cases.
>
> Thank you - also will be great if you can indicate how this was caught?
This is kind of a refactoring. I grepped for them manually.
As for the motivation itself, I would say
https://lwn.net/Articles/934679/
Me and the other LKMP mentees are working on doing this to various parts
of the source with the help of Julia, Shuah and Javier.
As mentioned in the commit messages, this refactoring has the advantage
of reducing the number of leaks in case we add new code in the future
introducing new OF stuff.
Thanks
PS: I sent out the cover letter once before without CCing the mailing
lists and Nishanth replied to that. So that mail can't be seen here
independently.
_______________________________________________
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] 24+ messages in thread
* Re: [PATCH 0/3] Use scope based cleanup in drivers/soc/ti/
@ 2024-05-11 9:06 ` Kousik Sanagavarapu
0 siblings, 0 replies; 24+ messages in thread
From: Kousik Sanagavarapu @ 2024-05-11 9:06 UTC (permalink / raw)
To: Nishanth Menon, Santosh Shilimkar, Julia Lawall
Cc: Shuah Khan, Javier Carrasco, linux-kernel, linux-arm-kernel
On Fri, May 10, 2024 at 08:17:54AM -0500, Nishanth Menon wrote:
> On 12:38-20240510, Kousik Sanagavarapu wrote:
> > Do scope based "struct device_node" cleanup in drivers/soc/ti/*. This
> > series converts all the possible cases.
>
> Thank you - also will be great if you can indicate how this was caught?
This is kind of a refactoring. I grepped for them manually.
As for the motivation itself, I would say
https://lwn.net/Articles/934679/
Me and the other LKMP mentees are working on doing this to various parts
of the source with the help of Julia, Shuah and Javier.
As mentioned in the commit messages, this refactoring has the advantage
of reducing the number of leaks in case we add new code in the future
introducing new OF stuff.
Thanks
PS: I sent out the cover letter once before without CCing the mailing
lists and Nishanth replied to that. So that mail can't be seen here
independently.
^ permalink raw reply [flat|nested] 24+ messages in thread