linux-kernel - Re: [PATCH net-next] wifi: ath9k: use devm for irq and ioremap resource

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: <CAKxU2N8y6Kj6hfx4LxC+K0Jc9VRYfE-LHBVBAaC-4EcXy_6GqQ@mail.gmail.com>
Date: Wed, 9 Apr 2025 18:42:45 -0700
From: Rosen Penev <rosenp@...il.com>
To: linux-wireless@...r.kernel.org
Cc: Toke Høiland-Jørgensen <toke@...e.dk>, 
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] wifi: ath9k: use devm for irq and ioremap resource

On Wed, Apr 9, 2025 at 5:41 PM Rosen Penev <rosenp@...il.com> wrote:
>
> Avoids having to manually free. Both of these get called and removed in
> probe only and are safe to convert.
>
> devm_platform_ioremap_resource is different as it also calls
> devm_request_memory_region, which requires non overlapping memory
> regions. Luckily, that seems to be the case.
>
> Tested on a TP-Link Archer C7v2.
>
> Signed-off-by: Rosen Penev <rosenp@...il.com>
whoops. net-next should not be in the title.
> ---
>  drivers/net/wireless/ath/ath9k/ahb.c | 22 ++++++----------------
>  drivers/net/wireless/ath/ath9k/pci.c |  9 +++------
>  2 files changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c
> index d4805e02b927..636a487bf9b4 100644
> --- a/drivers/net/wireless/ath/ath9k/ahb.c
> +++ b/drivers/net/wireless/ath/ath9k/ahb.c
> @@ -74,7 +74,6 @@ static int ath_ahb_probe(struct platform_device *pdev)
>         void __iomem *mem;
>         struct ath_softc *sc;
>         struct ieee80211_hw *hw;
> -       struct resource *res;
>         const struct platform_device_id *id = platform_get_device_id(pdev);
>         int irq;
>         int ret = 0;
> @@ -86,16 +85,10 @@ static int ath_ahb_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       if (res == NULL) {
> -               dev_err(&pdev->dev, "no memory resource found\n");
> -               return -ENXIO;
> -       }
> -
> -       mem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> -       if (mem == NULL) {
> +       mem = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(mem)) {
>                 dev_err(&pdev->dev, "ioremap failed\n");
> -               return -ENOMEM;
> +               return PTR_ERR(mem);
>         }
>
>         irq = platform_get_irq(pdev, 0);
> @@ -118,16 +111,16 @@ static int ath_ahb_probe(struct platform_device *pdev)
>         sc->mem = mem;
>         sc->irq = irq;
>
> -       ret = request_irq(irq, ath_isr, IRQF_SHARED, "ath9k", sc);
> +       ret = devm_request_irq(&pdev->dev, irq, ath_isr, IRQF_SHARED, "ath9k", sc);
>         if (ret) {
>                 dev_err(&pdev->dev, "request_irq failed\n");
> -               goto err_free_hw;
> +               return ret;
>         }
>
>         ret = ath9k_init_device(id->driver_data, sc, &ath_ahb_bus_ops);
>         if (ret) {
>                 dev_err(&pdev->dev, "failed to initialize device\n");
> -               goto err_irq;
> +               goto err_free_hw;
>         }
>
>         ah = sc->sc_ah;
> @@ -137,8 +130,6 @@ static int ath_ahb_probe(struct platform_device *pdev)
>
>         return 0;
>
> - err_irq:
> -       free_irq(irq, sc);
>   err_free_hw:
>         ieee80211_free_hw(hw);
>         return ret;
> @@ -152,7 +143,6 @@ static void ath_ahb_remove(struct platform_device *pdev)
>                 struct ath_softc *sc = hw->priv;
>
>                 ath9k_deinit_device(sc);
> -               free_irq(sc->irq, sc);
>                 ieee80211_free_hw(sc->hw);
>         }
>  }
> diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
> index 27d4034c814e..48c7cae11e37 100644
> --- a/drivers/net/wireless/ath/ath9k/pci.c
> +++ b/drivers/net/wireless/ath/ath9k/pci.c
> @@ -965,9 +965,9 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>         }
>
>         if (!msi_enabled)
> -               ret = request_irq(pdev->irq, ath_isr, IRQF_SHARED, "ath9k", sc);
> +               ret = devm_request_irq(&pdev->dev, pdev->irq, ath_isr, IRQF_SHARED, "ath9k", sc);
>         else
> -               ret = request_irq(pdev->irq, ath_isr, 0, "ath9k", sc);
> +               ret = devm_request_irq(&pdev->dev, pdev->irq, ath_isr, 0, "ath9k", sc);
>
>         if (ret) {
>                 dev_err(&pdev->dev, "request_irq failed\n");
> @@ -979,7 +979,7 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>         ret = ath9k_init_device(id->device, sc, &ath_pci_bus_ops);
>         if (ret) {
>                 dev_err(&pdev->dev, "Failed to initialize device\n");
> -               goto err_init;
> +               goto err_irq;
>         }
>
>         sc->sc_ah->msi_enabled = msi_enabled;
> @@ -991,8 +991,6 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
>         return 0;
>
> -err_init:
> -       free_irq(sc->irq, sc);
>  err_irq:
>         ieee80211_free_hw(hw);
>         return ret;
> @@ -1006,7 +1004,6 @@ static void ath_pci_remove(struct pci_dev *pdev)
>         if (!is_ath9k_unloaded)
>                 sc->sc_ah->ah_flags |= AH_UNPLUGGED;
>         ath9k_deinit_device(sc);
> -       free_irq(sc->irq, sc);
>         ieee80211_free_hw(sc->hw);
>  }
>
> --
> 2.49.0
>