Re: [PATCH 2/2] stmmac: intel: Simplify intel_eth_pci_remove()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Sat, Jul 30, 2022 at 08:20:02PM +0200, Christophe JAILLET wrote:
> There is no point to call pcim_iounmap_regions() in the remove function,
> this frees a managed resource that would be release by the framework
> anyway.

The patch is fully correct in my opinion. The iounmap() is called exactly in
the same order as if it's done implicitly by managed resources handlers, hence
no need to explicitly call it.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is speculative.
> Sometimes the order of releasing managed resources is tricky.
> 
> Just a few drivers have this pattern, while many call pcim_iomap_regions().
> If I'm right and this patch is reviewed and merged, I'll look at the
> other files if they also can be simplified a bit.
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> index 9f38642f86ce..f68d23051557 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> @@ -1136,8 +1136,6 @@ static void intel_eth_pci_remove(struct pci_dev *pdev)
>  
>  	clk_disable_unprepare(plat->stmmac_clk);
>  	clk_unregister_fixed_rate(priv->plat->stmmac_clk);
> -
> -	pcim_iounmap_regions(pdev, BIT(0));
>  }
>  
>  static int __maybe_unused intel_eth_pci_suspend(struct device *dev)
> -- 
> 2.34.1
> 

-- 
With Best Regards,
Andy Shevchenko