| Message ID | 20200927194922.245750969@linutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | net: in_interrupt() cleanup and fixes | expand |
+ Uffe On 9/27/2020 9:49 PM, Thomas Gleixner wrote: > @@ -85,7 +85,7 @@ static void brcmf_sdiod_ib_irqhandler(st > > brcmf_dbg(INTR, "IB intr triggered\n"); > > - brcmf_sdio_isr(sdiodev->bus); > + brcmf_sdio_isr(sdiodev->bus, false); > } Hi Uffe, I assume the above code is okay, but want to confirm. Is the SDIO interrupt guaranteed to be on a worker thread? Thanks, Arend
On Mon, 28 Sep 2020 at 09:35, Arend Van Spriel <arend.vanspriel@broadcom.com> wrote: > > + Uffe > > On 9/27/2020 9:49 PM, Thomas Gleixner wrote: > > @@ -85,7 +85,7 @@ static void brcmf_sdiod_ib_irqhandler(st > > > > brcmf_dbg(INTR, "IB intr triggered\n"); > > > > - brcmf_sdio_isr(sdiodev->bus); > > + brcmf_sdio_isr(sdiodev->bus, false); > > } > > Hi Uffe, > > I assume the above code is okay, but want to confirm. Is the SDIO > interrupt guaranteed to be on a worker thread? Correct. As a matter of fact, the sdio irqs can be delivered through a couple of different paths. The legacy (scheduled for removal), is from a dedicated kthread. The more "modern" way is either from the context of a threaded IRQ handler or via a workqueue. However, there are also so-called out of band SDIO irqs, typically routed via a separate GPIO line. This isn't managed by the MMC/SDIO subsystem, but the SDIO functional driver itself. Kind regards Uffe
On 9/28/2020 11:19 AM, Ulf Hansson wrote: > On Mon, 28 Sep 2020 at 09:35, Arend Van Spriel > <arend.vanspriel@broadcom.com> wrote: >> >> + Uffe >> >> On 9/27/2020 9:49 PM, Thomas Gleixner wrote: >>> @@ -85,7 +85,7 @@ static void brcmf_sdiod_ib_irqhandler(st >>> >>> brcmf_dbg(INTR, "IB intr triggered\n"); >>> >>> - brcmf_sdio_isr(sdiodev->bus); >>> + brcmf_sdio_isr(sdiodev->bus, false); >>> } >> >> Hi Uffe, >> >> I assume the above code is okay, but want to confirm. Is the SDIO >> interrupt guaranteed to be on a worker thread? > > Correct. > > As a matter of fact, the sdio irqs can be delivered through a couple > of different paths. The legacy (scheduled for removal), is from a > dedicated kthread. The more "modern" way is either from the context of > a threaded IRQ handler or via a workqueue. > > However, there are also so-called out of band SDIO irqs, typically > routed via a separate GPIO line. This isn't managed by the MMC/SDIO > subsystem, but the SDIO functional driver itself. In brcmfmac we indeed support the out-of-band interrupt. That scnenario is also taken care of in this patch. Thanks for the confirmation. Regards, Arend
On 9/27/2020 9:49 PM, Thomas Gleixner wrote: > From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > brcmf_sdio_isr() is using in_interrupt() to distinguish if it is called > from a interrupt service routine or from a worker thread. > > Passing such information from the calling context is preferred and > requested by Linus, so add an argument `in_isr' to brcmf_sdio_isr() and let > the callers pass the information about the calling context. After getting confirmation here is my ... Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com> > Cc: Franky Lin <franky.lin@broadcom.com> > Cc: Hante Meuleman <hante.meuleman@broadcom.com> > Cc: Chi-Hsien Lin <chi-hsien.lin@cypress.com> > Cc: Wright Feng <wright.feng@cypress.com> > Cc: Kalle Valo <kvalo@codeaurora.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: linux-wireless@vger.kernel.org > Cc: brcm80211-dev-list.pdl@broadcom.com > Cc: brcm80211-dev-list@cypress.com > Cc: netdev@vger.kernel.org > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 4 ++-- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++-- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-)
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -73,7 +73,7 @@ static irqreturn_t brcmf_sdiod_oob_irqha sdiodev->irq_en = false; } - brcmf_sdio_isr(sdiodev->bus); + brcmf_sdio_isr(sdiodev->bus, true); return IRQ_HANDLED; } @@ -85,7 +85,7 @@ static void brcmf_sdiod_ib_irqhandler(st brcmf_dbg(INTR, "IB intr triggered\n"); - brcmf_sdio_isr(sdiodev->bus); + brcmf_sdio_isr(sdiodev->bus, false); } /* dummy handler for SDIO function 2 interrupt */ --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -3625,7 +3625,7 @@ void brcmf_sdio_trigger_dpc(struct brcmf } } -void brcmf_sdio_isr(struct brcmf_sdio *bus) +void brcmf_sdio_isr(struct brcmf_sdio *bus, bool in_isr) { brcmf_dbg(TRACE, "Enter\n"); @@ -3636,7 +3636,7 @@ void brcmf_sdio_isr(struct brcmf_sdio *b /* Count the interrupt call */ bus->sdcnt.intrcount++; - if (in_interrupt()) + if (in_isr) atomic_set(&bus->ipend, 1); else if (brcmf_sdio_intr_rstatus(bus)) { --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h @@ -372,7 +372,7 @@ int brcmf_sdiod_remove(struct brcmf_sdio struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev); void brcmf_sdio_remove(struct brcmf_sdio *bus); -void brcmf_sdio_isr(struct brcmf_sdio *bus); +void brcmf_sdio_isr(struct brcmf_sdio *bus, bool in_isr); void brcmf_sdio_wd_timer(struct brcmf_sdio *bus, bool active); void brcmf_sdio_wowl_config(struct device *dev, bool enabled);