netdev - Re: [PATCH 2/2] stmmac: intel: Simplify intel_eth_pci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YuWPc8Flkpm4Yt/z@smile.fi.intel.com>
Date:   Sat, 30 Jul 2022 23:07:15 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     vee.khee.wong@...el.com, weifeng.voon@...el.com,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] stmmac: intel: Simplify intel_eth_pci_remove()

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@...ux.intel.com>

> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.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