| Message ID | 20241031-clk-renesas-cpg-mssr-cleanup-v1-2-628274ecbfcb@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | clk: renesas: cpg-mssr: fix 'soc' node handling and automate cleanup | expand |
Hi Javier Carrasco, Thanks for the patch > -----Original Message----- > From: Javier Carrasco <javier.carrasco.cruz@gmail.com> > Sent: 30 October 2024 23:26 > Subject: [PATCH 2/2] clk: renesas: cpg-mssr: automate 'soc' node release in cpg_mssr_reserved_init() > > Switch to a more robust approach by means of the cleanup attribute, which automates the calls to > of_node_put() when 'soc' goes out of scope. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > --- > drivers/clk/renesas/renesas-cpg-mssr.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c > index 5dc89b1009fe..bf85501709f0 100644 > --- a/drivers/clk/renesas/renesas-cpg-mssr.c > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c > @@ -979,7 +979,7 @@ static void __init cpg_mssr_reserved_exit(struct cpg_mssr_priv *priv) static int > __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > const struct cpg_mssr_info *info) { > - struct device_node *soc = of_find_node_by_path("/soc"); > + struct device_node *soc __free(device_node) = > +of_find_node_by_path("/soc"); I guess. by looking at [1] and [2], the cleanup function should be of_node_put(), which translates to __free_of_node_put(), that extracts of_node_put() and execute it for cleanup?? [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/irqchip/irq-renesas-rzg2l.c?h=next-20241030#n535 [2] https://elixir.bootlin.com/linux/v6.12-rc5/source/include/linux/cleanup.h#L198 Cheers, Biju > struct device_node *node; > uint32_t args[MAX_PHANDLE_ARGS]; > unsigned int *ids = NULL; > @@ -1022,7 +1022,6 @@ static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > > ids = krealloc_array(ids, (num + 1), sizeof(*ids), GFP_KERNEL); > if (!ids) { > - of_node_put(soc); > of_node_put(it.node); > return -ENOMEM; > } > @@ -1037,7 +1036,6 @@ static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > num++; > } > } > - of_node_put(soc); > > priv->num_reserved_ids = num; > priv->reserved_ids = ids; > > -- > 2.43.0 >
Hi, > -----Original Message----- > From: Biju Das <biju.das.jz@bp.renesas.com> > Sent: 31 October 2024 06:59 > Subject: RE: [PATCH 2/2] clk: renesas: cpg-mssr: automate 'soc' node release in > cpg_mssr_reserved_init() > > Hi Javier Carrasco, > > Thanks for the patch > > > -----Original Message----- > > From: Javier Carrasco <javier.carrasco.cruz@gmail.com> > > Sent: 30 October 2024 23:26 > > Subject: [PATCH 2/2] clk: renesas: cpg-mssr: automate 'soc' node > > release in cpg_mssr_reserved_init() > > > > Switch to a more robust approach by means of the cleanup attribute, > > which automates the calls to > > of_node_put() when 'soc' goes out of scope. > > > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > > --- > > drivers/clk/renesas/renesas-cpg-mssr.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c > > b/drivers/clk/renesas/renesas-cpg-mssr.c > > index 5dc89b1009fe..bf85501709f0 100644 > > --- a/drivers/clk/renesas/renesas-cpg-mssr.c > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c > > @@ -979,7 +979,7 @@ static void __init cpg_mssr_reserved_exit(struct > > cpg_mssr_priv *priv) static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > > const struct cpg_mssr_info *info) { > > - struct device_node *soc = of_find_node_by_path("/soc"); > > + struct device_node *soc __free(device_node) = > > +of_find_node_by_path("/soc"); > > I guess. by looking at [1] and [2], the cleanup function should be of_node_put(), which translates to > __free_of_node_put(), that extracts of_node_put() and execute it for cleanup?? > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/irqchip/irq- > renesas-rzg2l.c?h=next-20241030#n535 > > [2] https://elixir.bootlin.com/linux/v6.12-rc5/source/include/linux/cleanup.h#L198 Please ignore me. I am wrong. https://elixir.bootlin.com/linux/v6.12-rc5/source/include/linux/of.h#L138 Cheers, Biju > > Cheers, > Biju > > > struct device_node *node; > > uint32_t args[MAX_PHANDLE_ARGS]; > > unsigned int *ids = NULL; > > @@ -1022,7 +1022,6 @@ static int __init cpg_mssr_reserved_init(struct > > cpg_mssr_priv *priv, > > > > ids = krealloc_array(ids, (num + 1), sizeof(*ids), GFP_KERNEL); > > if (!ids) { > > - of_node_put(soc); > > of_node_put(it.node); > > return -ENOMEM; > > } > > @@ -1037,7 +1036,6 @@ static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > > num++; > > } > > } > > - of_node_put(soc); > > > > priv->num_reserved_ids = num; > > priv->reserved_ids = ids; > > > > -- > > 2.43.0 > >
On 31/10/2024 00:25, Javier Carrasco wrote: > Switch to a more robust approach by means of the cleanup attribute, > which automates the calls to of_node_put() when 'soc' goes out of scope. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > --- > drivers/clk/renesas/renesas-cpg-mssr.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c > index 5dc89b1009fe..bf85501709f0 100644 > --- a/drivers/clk/renesas/renesas-cpg-mssr.c > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c > @@ -979,7 +979,7 @@ static void __init cpg_mssr_reserved_exit(struct cpg_mssr_priv *priv) > static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > const struct cpg_mssr_info *info) > { > - struct device_node *soc = of_find_node_by_path("/soc"); > + struct device_node *soc __free(device_node) = of_find_node_by_path("/soc"); > struct device_node *node; > uint32_t args[MAX_PHANDLE_ARGS]; > unsigned int *ids = NULL; > @@ -1022,7 +1022,6 @@ static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, > > ids = krealloc_array(ids, (num + 1), sizeof(*ids), GFP_KERNEL); > if (!ids) { > - of_node_put(soc); You just added this. Don't add code which is immediately removed. It's a noop or wrong code. Best regards, Krzysztof
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c index 5dc89b1009fe..bf85501709f0 100644 --- a/drivers/clk/renesas/renesas-cpg-mssr.c +++ b/drivers/clk/renesas/renesas-cpg-mssr.c @@ -979,7 +979,7 @@ static void __init cpg_mssr_reserved_exit(struct cpg_mssr_priv *priv) static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, const struct cpg_mssr_info *info) { - struct device_node *soc = of_find_node_by_path("/soc"); + struct device_node *soc __free(device_node) = of_find_node_by_path("/soc"); struct device_node *node; uint32_t args[MAX_PHANDLE_ARGS]; unsigned int *ids = NULL; @@ -1022,7 +1022,6 @@ static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, ids = krealloc_array(ids, (num + 1), sizeof(*ids), GFP_KERNEL); if (!ids) { - of_node_put(soc); of_node_put(it.node); return -ENOMEM; } @@ -1037,7 +1036,6 @@ static int __init cpg_mssr_reserved_init(struct cpg_mssr_priv *priv, num++; } } - of_node_put(soc); priv->num_reserved_ids = num; priv->reserved_ids = ids;
Switch to a more robust approach by means of the cleanup attribute, which automates the calls to of_node_put() when 'soc' goes out of scope. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/clk/renesas/renesas-cpg-mssr.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)