Re: [PATCH net-next v5 10/10] net: ethernet: ravb: Add gPTP support for Gen4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: ravb: reference leak of of_gptp device node on probe failure
--

commit b1512d2d01596b4db2e1647ebacba3babfa69468
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

net: ethernet: ravb: Add gPTP support for Gen4

This commit utilizes the refactoring of the RAVB gPTP code to add support
for a system-wide clock on Gen4 devices. It adds ravb_gen4_ptp_probe()
which parses the ptp-timer property and stores the device node reference.

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index ea1c7e536791e..5a07cc0180d8b 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c

[ ... ]

> @@ -2973,12 +3067,10 @@ static int ravb_probe(struct platform_device *pdev)
>  		goto out_reset_assert;
>  	}
>  
> -	if (info->gptp_ref_clk) {
> -		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
> -		if (IS_ERR(priv->gptp_clk)) {
> -			error = PTR_ERR(priv->gptp_clk);
> +	if (info->ptp && info->ptp->probe) {
> +		error = info->ptp->probe(ndev);
> +		if (error)
>  			goto out_reset_assert;
> -		}
>  	}
>  
>  	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");

[Severity: Medium]
Does this code leak the device node reference on probe failure?

When info->ptp->probe() succeeds, ravb_gen4_ptp_probe() acquires a reference
to the ptp-timer node via of_parse_phandle() and stores it in priv->of_gptp.

If a subsequent initialization step in ravb_probe() fails (for example,
devm_clk_get_optional() or pm_runtime_resume_and_get()), the function jumps to
error labels that return without dropping the reference:

drivers/net/ethernet/renesas/ravb_main.c:ravb_probe() {
    ...
out_reset_assert:
	reset_control_assert(rstc);
out_free_netdev:
	free_netdev(ndev);
	return error;
}

> @@ -3157,6 +3245,7 @@ static void ravb_remove(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_dont_use_autosuspend(dev);
>  	clk_unprepare(priv->refclk);
> +	of_node_put(priv->of_gptp);
>  	reset_control_assert(priv->rstc);
>  	free_netdev(ndev);
>  	platform_set_drvdata(pdev, NULL);

This properly drops the reference during removal, but shouldn't we also
have an of_node_put(priv->of_gptp) in the error path of ravb_probe()
to handle cold probe failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921175037.4084310-1-niklas.soderlund+renesas@ragnatech.se?part=10