| 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
| ||
|
Message-ID: <6e93c43e6e513559f0306085211245578c2c9d3f.camel@redhat.com>
Date: Mon, 26 Aug 2024 08:30:23 +0200
From: Philipp Stanner <pstanner@...hat.com>
To: Serge Semin <fancer.lancer@...il.com>
Cc: Jonathan Corbet <corbet@....net>, Jens Axboe <axboe@...nel.dk>, Wu Hao
<hao.wu@...el.com>, Tom Rix <trix@...hat.com>, Moritz Fischer
<mdf@...nel.org>, Xu Yilun <yilun.xu@...el.com>, Andy Shevchenko
<andy@...nel.org>, Linus Walleij <linus.walleij@...aro.org>, Bartosz
Golaszewski <brgl@...ev.pl>, "David S. Miller" <davem@...emloft.net>, Eric
Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
Abeni <pabeni@...hat.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>, Maxime Coquelin
<mcoquelin.stm32@...il.com>, Bjorn Helgaas <bhelgaas@...gle.com>, Alvaro
Karsz <alvaro.karsz@...id-run.com>, "Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Eugenio Pérez <eperezma@...hat.com>, Richard Cochran
<richardcochran@...il.com>, Mark Brown <broonie@...nel.org>, David Lechner
<dlechner@...libre.com>, Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>, Damien Le Moal <dlemoal@...nel.org>,
Hannes Reinecke <hare@...e.de>, Chaitanya Kulkarni <kch@...dia.com>,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-block@...r.kernel.org, linux-fpga@...r.kernel.org,
linux-gpio@...r.kernel.org, netdev@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-pci@...r.kernel.org,
virtualization@...ts.linux.dev
Subject: Re: [PATCH v3 6/9] ethernet: stmicro: Simplify PCI devres usage
On Fri, 2024-08-23 at 12:29 +0300, Serge Semin wrote:
> Hi Philipp
>
> On Thu, Aug 22, 2024 at 03:47:38PM +0200, Philipp Stanner wrote:
> > stmicro uses PCI devres in the wrong way. Resources requested
> > through pcim_* functions don't need to be cleaned up manually in
> > the
> > remove() callback or in the error unwind path of a probe()
> > function.
> >
> > Moreover, there is an unnecessary loop which only requests and
> > ioremaps
> > BAR 0, but iterates over all BARs nevertheless.
> >
> > Furthermore, pcim_iomap_regions() and pcim_iomap_table() have been
> > deprecated by the PCI subsystem in commit e354bb84a4c1 ("PCI:
> > Deprecate
> > pcim_iomap_table(), pcim_iomap_regions_request_all()").
> >
> > Replace these functions with pcim_iomap_region().
> >
> > Remove the unnecessary manual pcim_* cleanup calls.
> >
> > Remove the unnecessary loop over all BARs.
> >
> > Signed-off-by: Philipp Stanner <pstanner@...hat.com>
>
> Thanks for the series. But please note the network subsystem
> dev-process requires to submit the cleanup/feature changes on top of
> the net-next tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/
That seems a policy I haven't seen so far; usually the assumption is
that you branch out from Linus's master.
Anyways, I of course am going to help with setting up something
mergeable
>
> Just recently a Yanteng' (+cc) series
> https://lore.kernel.org/netdev/cover.1723014611.git.siyanteng@loongson.cn/
> was merged in which significantly refactored the Loongson MAC driver.
> Seeing your patch isn't based on these changes, there is a high
> probability that the patch won't get cleanly applied onto the
> net-next tree. So please either rebase your patch onto the net-next
> tree, or at least merge in the Yanteng' series in your tree and
> rebase the patch onto it and let's hope there have been no other
> conflicting patches merged in into the net-next tree.
I'll take a look into that, thx
P.
>
> -Serge(y)
>
>
> > ---
> > .../ethernet/stmicro/stmmac/dwmac-loongson.c | 25 +++++----------
> > ----
> > .../net/ethernet/stmicro/stmmac/stmmac_pci.c | 18 +++++--------
> > 2 files changed, 12 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > index 9e40c28d453a..5d42a9fad672 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > @@ -50,7 +50,7 @@ static int loongson_dwmac_probe(struct pci_dev
> > *pdev, const struct pci_device_id
> > struct plat_stmmacenet_data *plat;
> > struct stmmac_resources res;
> > struct device_node *np;
> > - int ret, i, phy_mode;
> > + int ret, phy_mode;
> >
> > np = dev_of_node(&pdev->dev);
> >
> > @@ -88,14 +88,11 @@ static int loongson_dwmac_probe(struct pci_dev
> > *pdev, const struct pci_device_id
> > goto err_put_node;
> > }
> >
> > - /* Get the base address of device */
> > - for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > - if (pci_resource_len(pdev, i) == 0)
> > - continue;
> > - ret = pcim_iomap_regions(pdev, BIT(0),
> > pci_name(pdev));
> > - if (ret)
> > - goto err_disable_device;
> > - break;
> > + memset(&res, 0, sizeof(res));
> > + res.addr = pcim_iomap_region(pdev, 0, pci_name(pdev));
> > + if (IS_ERR(res.addr)) {
> > + ret = PTR_ERR(res.addr);
> > + goto err_disable_device;
> > }
> >
> > plat->bus_id = of_alias_get_id(np, "ethernet");
> > @@ -116,8 +113,6 @@ static int loongson_dwmac_probe(struct pci_dev
> > *pdev, const struct pci_device_id
> >
> > loongson_default_data(plat);
> > pci_enable_msi(pdev);
> > - memset(&res, 0, sizeof(res));
> > - res.addr = pcim_iomap_table(pdev)[0];
> >
> > res.irq = of_irq_get_byname(np, "macirq");
> > if (res.irq < 0) {
> > @@ -158,18 +153,10 @@ static void loongson_dwmac_remove(struct
> > pci_dev *pdev)
> > {
> > struct net_device *ndev = dev_get_drvdata(&pdev->dev);
> > struct stmmac_priv *priv = netdev_priv(ndev);
> > - int i;
> >
> > of_node_put(priv->plat->mdio_node);
> > stmmac_dvr_remove(&pdev->dev);
> >
> > - for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > - if (pci_resource_len(pdev, i) == 0)
> > - continue;
> > - pcim_iounmap_regions(pdev, BIT(i));
> > - break;
> > - }
> > -
> > pci_disable_msi(pdev);
> > pci_disable_device(pdev);
> > }
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > index 352b01678c22..f89a8a54c4e8 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> > @@ -188,11 +188,11 @@ static int stmmac_pci_probe(struct pci_dev
> > *pdev,
> > return ret;
> > }
> >
> > - /* Get the base address of device */
> > + /* Request the base address BAR of device */
> > for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > if (pci_resource_len(pdev, i) == 0)
> > continue;
> > - ret = pcim_iomap_regions(pdev, BIT(i),
> > pci_name(pdev));
> > + ret = pcim_request_region(pdev, i,
> > pci_name(pdev));
> > if (ret)
> > return ret;
> > break;
> > @@ -205,7 +205,10 @@ static int stmmac_pci_probe(struct pci_dev
> > *pdev,
> > return ret;
> >
> > memset(&res, 0, sizeof(res));
> > - res.addr = pcim_iomap_table(pdev)[i];
> > + /* Get the base address of device */
> > + res.addr = pcim_iomap(pdev, i, 0);
> > + if (!res.addr)
> > + return -ENOMEM;
> > res.wol_irq = pdev->irq;
> > res.irq = pdev->irq;
> >
> > @@ -231,16 +234,7 @@ static int stmmac_pci_probe(struct pci_dev
> > *pdev,
> > */
> > static void stmmac_pci_remove(struct pci_dev *pdev)
> > {
> > - int i;
> > -
> > stmmac_dvr_remove(&pdev->dev);
> > -
> > - for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > - if (pci_resource_len(pdev, i) == 0)
> > - continue;
> > - pcim_iounmap_regions(pdev, BIT(i));
> > - break;
> > - }
> > }
> >
> > static int __maybe_unused stmmac_pci_suspend(struct device *dev)
> > --
> > 2.46.0
> >
> >
>
Powered by blists - more mailing lists