* [PATCH] soc: fsl: cpm1: tsa: switch to for_each_available_child_of_node_scoped()
@ 2024-10-02 20:21 Javier Carrasco
2024-11-14 7:14 ` Herve Codina
0 siblings, 1 reply; 2+ messages in thread
From: Javier Carrasco @ 2024-10-02 20:21 UTC (permalink / raw)
To: Herve Codina, Qiang Zhao, Christophe Leroy
Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, Javier Carrasco
The non-scoped variant of this macro turns error-prone as soon as error
paths are included, because explicit calls to of_node_put() are required
to avoid leaking memory.
Using its scoped counterpart simplifies the code by removing the need of
explicit calls to of_node_put(), as they are automatically triggered as
soon as the child node goes out of scope. Moreover, it is more robust as
it accounts for new error paths without having to worry about
decrementing the object's refcount.
Note that the device_node is declared within the macro, and its explicit
declaration can be dropped as well if it is not used anywhere else.
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
drivers/soc/fsl/qe/tsa.c | 28 ++++------------------------
1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
index f0889b3fcaf2..a1140aadfd6c 100644
--- a/drivers/soc/fsl/qe/tsa.c
+++ b/drivers/soc/fsl/qe/tsa.c
@@ -680,7 +680,6 @@ static inline int tsa_of_parse_tdm_tx_route(struct tsa *tsa,
static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
{
- struct device_node *tdm_np;
struct tsa_tdm *tdm;
struct clk *clk;
u32 tdm_id, val;
@@ -691,11 +690,10 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
for (i = 0; i < ARRAY_SIZE(tsa->tdm); i++)
tsa->tdm[i].is_enable = false;
- for_each_available_child_of_node(np, tdm_np) {
+ for_each_available_child_of_node_scoped(np, tdm_np) {
ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
if (ret) {
dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
- of_node_put(tdm_np);
return ret;
}
switch (tdm_id) {
@@ -719,16 +717,14 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
invalid_tdm:
dev_err(tsa->dev, "%pOF: Invalid tdm_id (%u)\n", tdm_np,
tdm_id);
- of_node_put(tdm_np);
return -EINVAL;
}
}
- for_each_available_child_of_node(np, tdm_np) {
+ for_each_available_child_of_node_scoped(np, tdm_np) {
ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
if (ret) {
dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
- of_node_put(tdm_np);
return ret;
}
@@ -742,14 +738,12 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
dev_err(tsa->dev,
"%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
tdm_np);
- of_node_put(tdm_np);
return ret;
}
if (val > 3) {
dev_err(tsa->dev,
"%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
tdm_np, val);
- of_node_put(tdm_np);
return -EINVAL;
}
tdm->simode_tdm |= TSA_SIMODE_TDM_RFSD(val);
@@ -761,14 +755,12 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
dev_err(tsa->dev,
"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n",
tdm_np);
- of_node_put(tdm_np);
return ret;
}
if (val > 3) {
dev_err(tsa->dev,
"%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
tdm_np, val);
- of_node_put(tdm_np);
return -EINVAL;
}
tdm->simode_tdm |= TSA_SIMODE_TDM_TFSD(val);
@@ -792,13 +784,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
clk = of_clk_get_by_name(tdm_np, tsa_is_qe(tsa) ? "rsync" : "l1rsync");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
- of_node_put(tdm_np);
goto err;
}
ret = clk_prepare_enable(clk);
if (ret) {
clk_put(clk);
- of_node_put(tdm_np);
goto err;
}
tdm->l1rsync_clk = clk;
@@ -806,13 +796,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
clk = of_clk_get_by_name(tdm_np, tsa_is_qe(tsa) ? "rclk" : "l1rclk");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
- of_node_put(tdm_np);
goto err;
}
ret = clk_prepare_enable(clk);
if (ret) {
clk_put(clk);
- of_node_put(tdm_np);
goto err;
}
tdm->l1rclk_clk = clk;
@@ -821,13 +809,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
clk = of_clk_get_by_name(tdm_np, tsa_is_qe(tsa) ? "tsync" : "l1tsync");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
- of_node_put(tdm_np);
goto err;
}
ret = clk_prepare_enable(clk);
if (ret) {
clk_put(clk);
- of_node_put(tdm_np);
goto err;
}
tdm->l1tsync_clk = clk;
@@ -835,13 +821,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
clk = of_clk_get_by_name(tdm_np, tsa_is_qe(tsa) ? "tclk" : "l1tclk");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
- of_node_put(tdm_np);
goto err;
}
ret = clk_prepare_enable(clk);
if (ret) {
clk_put(clk);
- of_node_put(tdm_np);
goto err;
}
tdm->l1tclk_clk = clk;
@@ -859,16 +843,12 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
}
ret = tsa_of_parse_tdm_rx_route(tsa, tdm_np, tsa->tdms, tdm_id);
- if (ret) {
- of_node_put(tdm_np);
+ if (ret)
goto err;
- }
ret = tsa_of_parse_tdm_tx_route(tsa, tdm_np, tsa->tdms, tdm_id);
- if (ret) {
- of_node_put(tdm_np);
+ if (ret)
goto err;
- }
tdm->is_enable = true;
}
---
base-commit: fe21733536749bb1b31c9c84e0b8d2ab8d82ce13
change-id: 20241002-tsa-scoped-7565f74fe3c0
Best regards,
--
Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] soc: fsl: cpm1: tsa: switch to for_each_available_child_of_node_scoped()
2024-10-02 20:21 [PATCH] soc: fsl: cpm1: tsa: switch to for_each_available_child_of_node_scoped() Javier Carrasco
@ 2024-11-14 7:14 ` Herve Codina
0 siblings, 0 replies; 2+ messages in thread
From: Herve Codina @ 2024-11-14 7:14 UTC (permalink / raw)
To: Javier Carrasco
Cc: Qiang Zhao, Christophe Leroy, linuxppc-dev, linux-arm-kernel,
linux-kernel
Hi Javier,
On Wed, 02 Oct 2024 22:21:51 +0200
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> The non-scoped variant of this macro turns error-prone as soon as error
> paths are included, because explicit calls to of_node_put() are required
> to avoid leaking memory.
>
> Using its scoped counterpart simplifies the code by removing the need of
> explicit calls to of_node_put(), as they are automatically triggered as
> soon as the child node goes out of scope. Moreover, it is more robust as
> it accounts for new error paths without having to worry about
> decrementing the object's refcount.
>
> Note that the device_node is declared within the macro, and its explicit
> declaration can be dropped as well if it is not used anywhere else.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> drivers/soc/fsl/qe/tsa.c | 28 ++++------------------------
> 1 file changed, 4 insertions(+), 24 deletions(-)
Acked-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-11-14 7:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 20:21 [PATCH] soc: fsl: cpm1: tsa: switch to for_each_available_child_of_node_scoped() Javier Carrasco
2024-11-14 7:14 ` Herve Codina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).