Re: [PATCH] regulator/scmi: Add missing of_node_get()

Cristian Marussi
2022-06-22T08:05:32Z

On Wed, Jun 22, 2022 at 11:48:16AM +0800, Liang He wrote:
> In scmi_regulator_probe(), of_find_node_by_name() will decrease
> the refcount of its first argument and we need a of_node_get()
> to keep reference balance.
>
> Signed-off-by: Liang He <windhl@126.com>

Hi Liang,

good catch.

I saw many other drivers that had this same issue fixed, given it is
not so obvious, had also added some explaining comment like:

/* Balance of_node_put() done by of_find_node_by_name() */

so maybe it is a good idea to add something like that, but it depends at
the end on how Mark prefers really. (given it is also in the commit log
indeed...)

Instead, more certainly, I would add also a Fixes: tag to you patch.

Other than this:

Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>

Thanks,
Cristian

> ---
> drivers/regulator/scmi-regulator.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/regulator/scmi-regulator.c b/drivers/regulator/scmi-regulator.c
> index 41ae7ac27ff6..b9918f4fd241 100644
> --- a/drivers/regulator/scmi-regulator.c
> +++ b/drivers/regulator/scmi-regulator.c
> @@ -343,6 +343,7 @@ static int scmi_regulator_probe(struct scmi_device *sdev)
> * plausible SCMI Voltage Domain number, all belonging to this SCMI
> * platform instance node (handle->dev->of_node).
> */
> + of_node_get(handle->dev->of_node);
> np = of_find_node_by_name(handle->dev->of_node, "regulators");
> for_each_child_of_node(np, child) {
> ret = process_scmi_regulator_of_node(sdev, ph, child, rinfo);
> --
> 2.25.1