| Message ID | 20240731210243.7467-1-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 92da4ce847bc5d942ddfdb102dba92f4e2797a59 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | net: ath9k: use devm for request_irq | expand |
Rosen Penev <rosenp@gmail.com> writes: > Avoids having to manually call free_irq. Simplifies code slightly. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> The title prefix should be "wifi:". I can fix that, no need to resend because of this.
Rosen Penev <rosenp@gmail.com> writes: > Avoids having to manually call free_irq. Simplifies code slightly. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Rosen Penev <rosenp@gmail.com> wrote: > Avoids having to manually call free_irq(). Simplifies code slightly. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > Acked-by: Toke Høiland-Jørgensen <toke@toke.dk> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. 92da4ce847bc wifi: ath9k: use devm for request_irq()
On 31.07.24 23:02, Rosen Penev wrote: > Avoids having to manually call free_irq. Simplifies code slightly. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ > 2 files changed, 5 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c > index 1a6697b6e3b4..29f67ded8fe2 100644 > --- a/drivers/net/wireless/ath/ath9k/ahb.c > +++ b/drivers/net/wireless/ath/ath9k/ahb.c > @@ -118,7 +118,7 @@ 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); Sorry for the late response, but I think this patch is wrong any may need to be reverted. If there is an error during probe, and the IRQ fires for some reason, there could be an use-after-free bug when the IRQ handler accesses the data in sc. The explicit freq_irq calls were preventing that from happening. - Felix
On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: > > On 31.07.24 23:02, Rosen Penev wrote: > > Avoids having to manually call free_irq. Simplifies code slightly. > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > --- > > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- > > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ > > 2 files changed, 5 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c > > index 1a6697b6e3b4..29f67ded8fe2 100644 > > --- a/drivers/net/wireless/ath/ath9k/ahb.c > > +++ b/drivers/net/wireless/ath/ath9k/ahb.c > > @@ -118,7 +118,7 @@ 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); > Sorry for the late response, but I think this patch is wrong any may > need to be reverted. If there is an error during probe, and the IRQ > fires for some reason, there could be an use-after-free bug when the IRQ > handler accesses the data in sc. > The explicit freq_irq calls were preventing that from happening. How about keeping the devm variant and replacing free_irq with devm_free_irq in probe? > > - Felix >
On 07.08.24 20:52, Rosen Penev wrote: > On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: >> >> On 31.07.24 23:02, Rosen Penev wrote: >> > Avoids having to manually call free_irq. Simplifies code slightly. >> > >> > Signed-off-by: Rosen Penev <rosenp@gmail.com> >> > --- >> > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- >> > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ >> > 2 files changed, 5 insertions(+), 11 deletions(-) >> > >> > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c >> > index 1a6697b6e3b4..29f67ded8fe2 100644 >> > --- a/drivers/net/wireless/ath/ath9k/ahb.c >> > +++ b/drivers/net/wireless/ath/ath9k/ahb.c >> > @@ -118,7 +118,7 @@ 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); >> Sorry for the late response, but I think this patch is wrong any may >> need to be reverted. If there is an error during probe, and the IRQ >> fires for some reason, there could be an use-after-free bug when the IRQ >> handler accesses the data in sc. >> The explicit freq_irq calls were preventing that from happening. > How about keeping the devm variant and replacing free_irq with > devm_free_irq in probe? If you do that, then using the devm variant is completely pointless. I think a full revert is the best option. - Felix
On Wed, Aug 7, 2024 at 1:05 PM Felix Fietkau <nbd@nbd.name> wrote: > > On 07.08.24 20:52, Rosen Penev wrote: > > On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: > >> > >> On 31.07.24 23:02, Rosen Penev wrote: > >> > Avoids having to manually call free_irq. Simplifies code slightly. > >> > > >> > Signed-off-by: Rosen Penev <rosenp@gmail.com> > >> > --- > >> > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- > >> > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ > >> > 2 files changed, 5 insertions(+), 11 deletions(-) > >> > > >> > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c > >> > index 1a6697b6e3b4..29f67ded8fe2 100644 > >> > --- a/drivers/net/wireless/ath/ath9k/ahb.c > >> > +++ b/drivers/net/wireless/ath/ath9k/ahb.c > >> > @@ -118,7 +118,7 @@ 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); > >> Sorry for the late response, but I think this patch is wrong any may > >> need to be reverted. If there is an error during probe, and the IRQ > >> fires for some reason, there could be an use-after-free bug when the IRQ > >> handler accesses the data in sc. > >> The explicit freq_irq calls were preventing that from happening. > > How about keeping the devm variant and replacing free_irq with > > devm_free_irq in probe? > > If you do that, then using the devm variant is completely pointless. > I think a full revert is the best option. OTOH it still allows removing free_irq from _remove, but I see your point. > > - Felix
On 07.08.24 22:07, Rosen Penev wrote: > On Wed, Aug 7, 2024 at 1:05 PM Felix Fietkau <nbd@nbd.name> wrote: >> >> On 07.08.24 20:52, Rosen Penev wrote: >> > On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: >> >> >> >> On 31.07.24 23:02, Rosen Penev wrote: >> >> > Avoids having to manually call free_irq. Simplifies code slightly. >> >> > >> >> > Signed-off-by: Rosen Penev <rosenp@gmail.com> >> >> > --- >> >> > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- >> >> > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ >> >> > 2 files changed, 5 insertions(+), 11 deletions(-) >> >> > >> >> > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c >> >> > index 1a6697b6e3b4..29f67ded8fe2 100644 >> >> > --- a/drivers/net/wireless/ath/ath9k/ahb.c >> >> > +++ b/drivers/net/wireless/ath/ath9k/ahb.c >> >> > @@ -118,7 +118,7 @@ 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); >> >> Sorry for the late response, but I think this patch is wrong any may >> >> need to be reverted. If there is an error during probe, and the IRQ >> >> fires for some reason, there could be an use-after-free bug when the IRQ >> >> handler accesses the data in sc. >> >> The explicit freq_irq calls were preventing that from happening. >> > How about keeping the devm variant and replacing free_irq with >> > devm_free_irq in probe? >> >> If you do that, then using the devm variant is completely pointless. >> I think a full revert is the best option. > OTOH it still allows removing free_irq from _remove, but I see your point. No, because you'd have the same use-after-free bug there as well. - Felix
Felix Fietkau <nbd@nbd.name> writes: > On 07.08.24 22:07, Rosen Penev wrote: >> On Wed, Aug 7, 2024 at 1:05 PM Felix Fietkau <nbd@nbd.name> wrote: >>> >>> On 07.08.24 20:52, Rosen Penev wrote: >>> > On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: >>> >> >>> >> On 31.07.24 23:02, Rosen Penev wrote: >>> >> > Avoids having to manually call free_irq. Simplifies code slightly. >>> >> > >>> >> > Signed-off-by: Rosen Penev <rosenp@gmail.com> >>> >> > --- >>> >> > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- >>> >> > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ >>> >> > 2 files changed, 5 insertions(+), 11 deletions(-) >>> >> > >>> >> > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c >>> >> > index 1a6697b6e3b4..29f67ded8fe2 100644 >>> >> > --- a/drivers/net/wireless/ath/ath9k/ahb.c >>> >> > +++ b/drivers/net/wireless/ath/ath9k/ahb.c >>> >> > @@ -118,7 +118,7 @@ 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); >>> >> Sorry for the late response, but I think this patch is wrong any may >>> >> need to be reverted. If there is an error during probe, and the IRQ >>> >> fires for some reason, there could be an use-after-free bug when the IRQ >>> >> handler accesses the data in sc. >>> >> The explicit freq_irq calls were preventing that from happening. >>> > How about keeping the devm variant and replacing free_irq with >>> > devm_free_irq in probe? >>> >>> If you do that, then using the devm variant is completely pointless. >>> I think a full revert is the best option. >> OTOH it still allows removing free_irq from _remove, but I see your point. > > No, because you'd have the same use-after-free bug there as well. Alright, let's revert. Kalle, can you just do the revert, or should I send a patch for it? -Toke
Toke Høiland-Jørgensen <toke@kernel.org> writes: > Felix Fietkau <nbd@nbd.name> writes: > >> On 07.08.24 22:07, Rosen Penev wrote: >>> On Wed, Aug 7, 2024 at 1:05 PM Felix Fietkau <nbd@nbd.name> wrote: >>>> >>>> On 07.08.24 20:52, Rosen Penev wrote: >>>> > On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: >>>> >> >>>> >> On 31.07.24 23:02, Rosen Penev wrote: >>>> >> > Avoids having to manually call free_irq. Simplifies code slightly. >>>> >> > >>>> >> > Signed-off-by: Rosen Penev <rosenp@gmail.com> >>>> >> > --- >>>> >> > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- >>>> >> > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ >>>> >> > 2 files changed, 5 insertions(+), 11 deletions(-) >>>> >> > >>>> >> > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c >>>> >> > index 1a6697b6e3b4..29f67ded8fe2 100644 >>>> >> > --- a/drivers/net/wireless/ath/ath9k/ahb.c >>>> >> > +++ b/drivers/net/wireless/ath/ath9k/ahb.c >>>> >> > @@ -118,7 +118,7 @@ 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); >>>> >> Sorry for the late response, but I think this patch is wrong any may >>>> >> need to be reverted. If there is an error during probe, and the IRQ >>>> >> fires for some reason, there could be an use-after-free bug when the IRQ >>>> >> handler accesses the data in sc. >>>> >> The explicit freq_irq calls were preventing that from happening. >>>> > How about keeping the devm variant and replacing free_irq with >>>> > devm_free_irq in probe? >>>> >>>> If you do that, then using the devm variant is completely pointless. >>>> I think a full revert is the best option. >>> OTOH it still allows removing free_irq from _remove, but I see your point. >> >> No, because you'd have the same use-after-free bug there as well. > > Alright, let's revert. Kalle, can you just do the revert, or should I > send a patch for it? Thanks, the best is to send a patch. But honestly more and more I'm starting to think that we should just reject all these "drive-by cleanups". We have better things to do than fixing unnecessary their bugs. Thoughts?
Kalle Valo <kvalo@kernel.org> writes: > Toke Høiland-Jørgensen <toke@kernel.org> writes: > >> Felix Fietkau <nbd@nbd.name> writes: >> >>> On 07.08.24 22:07, Rosen Penev wrote: >>>> On Wed, Aug 7, 2024 at 1:05 PM Felix Fietkau <nbd@nbd.name> wrote: >>>>> >>>>> On 07.08.24 20:52, Rosen Penev wrote: >>>>> > On Wed, Aug 7, 2024 at 10:47 AM Felix Fietkau <nbd@nbd.name> wrote: >>>>> >> >>>>> >> On 31.07.24 23:02, Rosen Penev wrote: >>>>> >> > Avoids having to manually call free_irq. Simplifies code slightly. >>>>> >> > >>>>> >> > Signed-off-by: Rosen Penev <rosenp@gmail.com> >>>>> >> > --- >>>>> >> > drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- >>>>> >> > drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ >>>>> >> > 2 files changed, 5 insertions(+), 11 deletions(-) >>>>> >> > >>>>> >> > diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c >>>>> >> > index 1a6697b6e3b4..29f67ded8fe2 100644 >>>>> >> > --- a/drivers/net/wireless/ath/ath9k/ahb.c >>>>> >> > +++ b/drivers/net/wireless/ath/ath9k/ahb.c >>>>> >> > @@ -118,7 +118,7 @@ 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); >>>>> >> Sorry for the late response, but I think this patch is wrong any may >>>>> >> need to be reverted. If there is an error during probe, and the IRQ >>>>> >> fires for some reason, there could be an use-after-free bug when the IRQ >>>>> >> handler accesses the data in sc. >>>>> >> The explicit freq_irq calls were preventing that from happening. >>>>> > How about keeping the devm variant and replacing free_irq with >>>>> > devm_free_irq in probe? >>>>> >>>>> If you do that, then using the devm variant is completely pointless. >>>>> I think a full revert is the best option. >>>> OTOH it still allows removing free_irq from _remove, but I see your point. >>> >>> No, because you'd have the same use-after-free bug there as well. >> >> Alright, let's revert. Kalle, can you just do the revert, or should I >> send a patch for it? > > Thanks, the best is to send a patch. Alright, will do. > But honestly more and more I'm starting to think that we should just > reject all these "drive-by cleanups". We have better things to do than > fixing unnecessary their bugs. Thoughts? Hmm, yeah, maybe. I do kinda like the fact that people send patches to improve small things, though. We all started out as new to the kernel, and I appreciate the fact that people try to improve our "commons" in this way even if it's small things. I do try to be critical of things that can break stuff before ack'ing these fixes, but I'll admit that it seems like I don't have that great of a track record for judging "correct" in this context (cf this one, and that debugfs regression). So I guess you're right that I should at least raise the bar somewhat; will try to recalibrate and say no more :) -Toke
Toke Høiland-Jørgensen <toke@kernel.org> writes: >> But honestly more and more I'm starting to think that we should just >> reject all these "drive-by cleanups". We have better things to do than >> fixing unnecessary their bugs. Thoughts? > > Hmm, yeah, maybe. I do kinda like the fact that people send patches to > improve small things, though. We all started out as new to the kernel, > and I appreciate the fact that people try to improve our "commons" in > this way even if it's small things. Yeah, you have a point. It's just that the extra work from cleanups feels so unnecessary compared to the practical benefits. And most of the time we don't hear from these people ever again, that's why I call them "drive-by cleanup". > I do try to be critical of things that can break stuff before ack'ing > these fixes, but I'll admit that it seems like I don't have that great > of a track record for judging "correct" in this context (cf this one, > and that debugfs regression). So I guess you're right that I should at > least raise the bar somewhat; will try to recalibrate and say no more :) You are doing a great job :) Nobody can catch all bugs in review, I would say there is a small percentage (5%?) of all cleanup patches that cause issues. Though it would be cool to see some real statistics.
Kalle Valo <kvalo@kernel.org> writes: > Toke Høiland-Jørgensen <toke@kernel.org> writes: > >>> But honestly more and more I'm starting to think that we should just >>> reject all these "drive-by cleanups". We have better things to do than >>> fixing unnecessary their bugs. Thoughts? >> >> Hmm, yeah, maybe. I do kinda like the fact that people send patches to >> improve small things, though. We all started out as new to the kernel, >> and I appreciate the fact that people try to improve our "commons" in >> this way even if it's small things. > > Yeah, you have a point. It's just that the extra work from cleanups > feels so unnecessary compared to the practical benefits. And most of the > time we don't hear from these people ever again, that's why I call them > "drive-by cleanup". Sure, I share the frustration, and not everyone turns into regular contributors. But I like to think that even those who don't get something else out of it at least. And on our side I guess there's a balance to be struck between being welcoming and not expending too much effort on it :) >> I do try to be critical of things that can break stuff before ack'ing >> these fixes, but I'll admit that it seems like I don't have that great >> of a track record for judging "correct" in this context (cf this one, >> and that debugfs regression). So I guess you're right that I should at >> least raise the bar somewhat; will try to recalibrate and say no more :) > > You are doing a great job :) Nobody can catch all bugs in review, I > would say there is a small percentage (5%?) of all cleanup patches that > cause issues. Though it would be cool to see some real statistics. Thanks! Yeah, would love to see some statistics, but I suppose it's not trivial to identity "cleanup patches" in the first place in a way that can be automated. Maybe something to poke at sometime one has time to spare (ha!) or needs a break from the regular drudgery :) -Toke
diff --git a/drivers/net/wireless/ath/ath9k/ahb.c b/drivers/net/wireless/ath/ath9k/ahb.c index 1a6697b6e3b4..29f67ded8fe2 100644 --- a/drivers/net/wireless/ath/ath9k/ahb.c +++ b/drivers/net/wireless/ath/ath9k/ahb.c @@ -118,7 +118,7 @@ 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; @@ -127,7 +127,7 @@ static int ath_ahb_probe(struct platform_device *pdev) 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 +137,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 +150,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 1ff53520f0a3..ccf73886199a 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); }
Avoids having to manually call free_irq. Simplifies code slightly. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- drivers/net/wireless/ath/ath9k/ahb.c | 7 ++----- drivers/net/wireless/ath/ath9k/pci.c | 9 +++------ 2 files changed, 5 insertions(+), 11 deletions(-)