Re: [PATCH] net: stmmac: call stmmac_remove_config_dt() in error paths in stmmac_probe_config_dt()

[Krzysztof Kozlowski <krzk@kernel.org>]

On 17/12/2024 04:32, Joe Hattori wrote:
> Current implementation of stmmac_probe_config_dt() does not release the
> OF node reference obtained by of_parse_phandle() when stmmac_dt_phy()
> fails, thus call stmmac_remove_config_dt(). The error_hw_init and
> error_pclk_get labels can be removed as just calling
> stmmac_remove_config_dt() suffices. Also, remove the first argument of
> stmmac_remove_config_dt() as it is not used.
> 
> This bug was found by an experimental static analysis tool that I am
> developing.
> 
> Fixes: 4838a5405028 ("net: stmmac: Fix wrapper drivers not detecting PHY")
> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
> ---
>  .../ethernet/stmicro/stmmac/stmmac_platform.c | 37 +++++++------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 3ac32444e492..4f1a9f7aae6e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -407,13 +407,11 @@ static int stmmac_of_get_mac_mode(struct device_node *np)
>  
>  /**
>   * stmmac_remove_config_dt - undo the effects of stmmac_probe_config_dt()
> - * @pdev: platform_device structure
>   * @plat: driver data platform structure
>   *
>   * Release resources claimed by stmmac_probe_config_dt().
>   */
> -static void stmmac_remove_config_dt(struct platform_device *pdev,
> -				    struct plat_stmmacenet_data *plat)
> +static void stmmac_remove_config_dt(struct plat_stmmacenet_data *plat)
>  {
>  	clk_disable_unprepare(plat->stmmac_clk);
>  	clk_disable_unprepare(plat->pclk);
> @@ -436,7 +434,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>  	struct plat_stmmacenet_data *plat;
>  	struct stmmac_dma_cfg *dma_cfg;
>  	int phy_mode;
> -	void *ret;
>  	int rc;
>  
>  	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
> @@ -490,8 +487,10 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>  		dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>  
>  	rc = stmmac_mdio_setup(plat, np, &pdev->dev);
> -	if (rc)
> +	if (rc) {
> +		stmmac_remove_config_dt(plat);
>  		return ERR_PTR(rc);
You now double unprepare the clocks. This is either buggy or just
confusing (assuming clocks are NULL).

Best regards,
Krzysztof