| Message ID | 1347563526-12513-8-git-send-email-arend@broadcom.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 09/13/2012 01:12 PM, Arend van Spriel wrote: > From: Franky Lin <frankyl@broadcom.com> > > SDIO in-band interrupt is level sensitive according to SDIO standard. > When the register interrupt handler gets called by SDIO stack it is > running in non interrupt context and expected to clear the interrupt > from the dongle. Therefore in-band and out-of-band interrupt need to > be handled differently. Tested-by: Stephen Warren <swarren@wwwdotorg.org> For reference, I took next-20120914, applied this patch series, applied a few patches from Wei Ni to set up the SDIO HW on Tegra correctly, and tested. The last time I tried this without any patches to the brcmfmac, this test caused some system instability issues. This time around, I observed no instability. I tested WPA PSK, ping (or flood ping) plus playing audio through ALSA. Is this series likely to make 3.7? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/14/2012 11:53 PM, Stephen Warren wrote: > On 09/13/2012 01:12 PM, Arend van Spriel wrote: >> From: Franky Lin <frankyl@broadcom.com> >> >> SDIO in-band interrupt is level sensitive according to SDIO standard. >> When the register interrupt handler gets called by SDIO stack it is >> running in non interrupt context and expected to clear the interrupt >> from the dongle. Therefore in-band and out-of-band interrupt need to >> be handled differently. > > Tested-by: Stephen Warren <swarren@wwwdotorg.org> > > For reference, I took next-20120914, applied this patch series, applied > a few patches from Wei Ni to set up the SDIO HW on Tegra correctly, and > tested. The last time I tried this without any patches to the brcmfmac, > this test caused some system instability issues. This time around, I > observed no instability. I tested WPA PSK, ping (or flood ping) plus > playing audio through ALSA. > > Is this series likely to make 3.7? > Thanks, Stephen Good to hear. I suspect we are close to the 3.7 merge window, but we do hope this will make it mainly for the tegra sdio issue that Wei Ni raised. In general the series should resolve issues when used on sdhci/mmc framework with in-band interrupt. Do you still need the patches from Wei Ni and are they SDIO or brcmfmac related? Gr. AvS -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2012 03:19 AM, Arend van Spriel wrote: > On 09/14/2012 11:53 PM, Stephen Warren wrote: >> On 09/13/2012 01:12 PM, Arend van Spriel wrote: >>> From: Franky Lin <frankyl@broadcom.com> >>> >>> SDIO in-band interrupt is level sensitive according to SDIO standard. >>> When the register interrupt handler gets called by SDIO stack it is >>> running in non interrupt context and expected to clear the interrupt >>> from the dongle. Therefore in-band and out-of-band interrupt need to >>> be handled differently. >> >> Tested-by: Stephen Warren <swarren@wwwdotorg.org> >> >> For reference, I took next-20120914, applied this patch series, applied >> a few patches from Wei Ni to set up the SDIO HW on Tegra correctly, and >> tested. The last time I tried this without any patches to the brcmfmac, >> this test caused some system instability issues. This time around, I >> observed no instability. I tested WPA PSK, ping (or flood ping) plus >> playing audio through ALSA. >> >> Is this series likely to make 3.7? >> > > Thanks, Stephen > > Good to hear. I suspect we are close to the 3.7 merge window, but we do > hope this will make it mainly for the tegra sdio issue that Wei Ni > raised. In general the series should resolve issues when used on > sdhci/mmc framework with in-band interrupt. > > Do you still need the patches from Wei Ni and are they SDIO or brcmfmac > related? Some patches are needed, but they're all for Tegra-specific code to configure the pinmux, GPIO, etc. on the Tegra SoC in order to correctly communicate with the SDIO port, and to enable the controller. No patches are required to the brcmfmac driver. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2012 11:12 AM, Stephen Warren wrote: > On 09/15/2012 03:19 AM, Arend van Spriel wrote: >> On 09/14/2012 11:53 PM, Stephen Warren wrote: >>> On 09/13/2012 01:12 PM, Arend van Spriel wrote: >>>> From: Franky Lin <frankyl@broadcom.com> >>>> >>>> SDIO in-band interrupt is level sensitive according to SDIO standard. >>>> When the register interrupt handler gets called by SDIO stack it is >>>> running in non interrupt context and expected to clear the interrupt >>>> from the dongle. Therefore in-band and out-of-band interrupt need to >>>> be handled differently. >>> >>> Tested-by: Stephen Warren <swarren@wwwdotorg.org> >>> >>> For reference, I took next-20120914, applied this patch series, applied >>> a few patches from Wei Ni to set up the SDIO HW on Tegra correctly, and >>> tested. The last time I tried this without any patches to the brcmfmac, >>> this test caused some system instability issues. This time around, I >>> observed no instability. I tested WPA PSK, ping (or flood ping) plus >>> playing audio through ALSA. >>> >>> Is this series likely to make 3.7? >>> >> >> Thanks, Stephen >> >> Good to hear. I suspect we are close to the 3.7 merge window, but we do >> hope this will make it mainly for the tegra sdio issue that Wei Ni >> raised. In general the series should resolve issues when used on >> sdhci/mmc framework with in-band interrupt. >> >> Do you still need the patches from Wei Ni and are they SDIO or brcmfmac >> related? > > Some patches are needed, but they're all for Tegra-specific code to > configure the pinmux, GPIO, etc. on the Tegra SoC in order to correctly > communicate with the SDIO port, and to enable the controller. No patches > are required to the brcmfmac driver. Just to be explicit here: If this patch series (the one at the start of this email thread) makes it into 3.7, I'll apply the Tegra patches to enable brcmfmac on Tegra for 3.8, and everything will work out nicely. If this patch series doesn't make it into 3.7, but gets into 3.8 instead, I'd still like to apply the Tegra patches for 3.8 as well. However, I can't apply the Tegra patches to any branch that doesn't include this patch series, or it'll cause system instability on Tegra. To that end, if this patch series doesn't make 3.7, I'd like to request that it be applied to a separate topic branch in the wireless tree, so that it can be both merged into the wireless tree for any required future work there, /and/ merged into the Tegra tree so that I can use it as a baseline to enable wireless on Tegra. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c index 0c1d08a..3b2c4c2 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c @@ -185,7 +185,7 @@ brcmf_sdcard_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address) return err; } -static int +int brcmf_sdio_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr, void *data, bool write) { diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c index 619f3cd..ff7e829 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c @@ -494,8 +494,8 @@ struct brcmf_sdio { u32 ramsize; /* Size of RAM in SOCRAM (bytes) */ u32 hostintmask; /* Copy of Host Interrupt Mask */ - u32 intstatus; /* Intstatus bits (events) pending */ - bool fcstate; /* State of dongle flow-control */ + atomic_t intstatus; /* Intstatus bits (events) pending */ + atomic_t fcstate; /* State of dongle flow-control */ uint blocksize; /* Block size of SDIO transfers */ uint roundup; /* Max roundup limit */ @@ -2271,20 +2271,53 @@ static inline void brcmf_sdbrcm_adddpctsk(struct brcmf_sdio *bus) spin_unlock_irqrestore(&bus->dpc_tl_lock, flags); } +static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus) +{ + u8 idx; + u32 addr; + unsigned long val; + int n, ret; + + idx = brcmf_sdio_chip_getinfidx(bus->ci, BCMA_CORE_SDIO_DEV); + addr = bus->ci->c_inf[idx].base + + offsetof(struct sdpcmd_regs, intstatus); + + ret = brcmf_sdio_regrw_helper(bus->sdiodev, addr, &val, false); + bus->sdcnt.f1regdata++; + if (ret != 0) + val = 0; + + val &= bus->hostintmask; + atomic_set(&bus->fcstate, !!(val & I_HMB_FC_STATE)); + + /* Clear interrupts */ + if (val) { + ret = brcmf_sdio_regrw_helper(bus->sdiodev, addr, &val, true); + bus->sdcnt.f1regdata++; + } + + if (ret) { + atomic_set(&bus->intstatus, 0); + } else if (val) { + for_each_set_bit(n, &val, 32) + set_bit(n, (unsigned long *)&bus->intstatus.counter); + } + + return ret; +} + static void brcmf_sdbrcm_dpc(struct brcmf_sdio *bus) { - u32 intstatus, newstatus = 0; + u32 newstatus = 0; + unsigned long intstatus; uint rxlimit = bus->rxbound; /* Rx frames to read before resched */ uint txlimit = bus->txbound; /* Tx frames to send before resched */ uint framecnt = 0; /* Temporary counter of tx/rx frames */ bool rxdone = true; /* Flag for no more read data */ - int err = 0; + int err = 0, n; brcmf_dbg(TRACE, "Enter\n"); - /* Start with leftover status bits */ - intstatus = bus->intstatus; - down(&bus->sdsem); /* If waiting for HTAVAIL, check status */ @@ -2339,24 +2372,13 @@ static void brcmf_sdbrcm_dpc(struct brcmf_sdio *bus) /* Pending interrupt indicates new device status */ if (atomic_read(&bus->ipend) > 0) { atomic_set(&bus->ipend, 0); - err = r_sdreg32(bus, &newstatus, - offsetof(struct sdpcmd_regs, intstatus)); - bus->sdcnt.f1regdata++; - if (err != 0) - newstatus = 0; - newstatus &= bus->hostintmask; - bus->fcstate = !!(newstatus & I_HMB_FC_STATE); - if (newstatus) { - err = w_sdreg32(bus, newstatus, - offsetof(struct sdpcmd_regs, - intstatus)); - bus->sdcnt.f1regdata++; - } + sdio_claim_host(bus->sdiodev->func[1]); + err = brcmf_sdio_intr_rstatus(bus); + sdio_release_host(bus->sdiodev->func[1]); } - /* Merge new bits with previous */ - intstatus |= newstatus; - bus->intstatus = 0; + /* Start with leftover status bits */ + intstatus = atomic_xchg(&bus->intstatus, 0); /* Handle flow-control change: read new state in case our ack * crossed another change interrupt. If change still set, assume @@ -2370,8 +2392,8 @@ static void brcmf_sdbrcm_dpc(struct brcmf_sdio *bus) err = r_sdreg32(bus, &newstatus, offsetof(struct sdpcmd_regs, intstatus)); bus->sdcnt.f1regdata += 2; - bus->fcstate = - !!(newstatus & (I_HMB_FC_STATE | I_HMB_FC_CHANGE)); + atomic_set(&bus->fcstate, + !!(newstatus & (I_HMB_FC_STATE | I_HMB_FC_CHANGE))); intstatus |= (newstatus & bus->hostintmask); } @@ -2416,7 +2438,10 @@ static void brcmf_sdbrcm_dpc(struct brcmf_sdio *bus) } /* Keep still-pending events for next scheduling */ - bus->intstatus = intstatus; + if (intstatus) { + for_each_set_bit(n, &intstatus, 32) + set_bit(n, (unsigned long *)&bus->intstatus.counter); + } brcmf_sdbrcm_clrintr(bus); @@ -2461,7 +2486,7 @@ static void brcmf_sdbrcm_dpc(struct brcmf_sdio *bus) brcmf_sdbrcm_wait_event_wakeup(bus); } /* Send queued frames (limit 1 if rx may still be pending) */ - else if ((bus->clkstate == CLK_AVAIL) && !bus->fcstate && + else if ((bus->clkstate == CLK_AVAIL) && !atomic_read(&bus->fcstate) && brcmu_pktq_mlen(&bus->txq, ~bus->flowcontrol) && txlimit && data_ok(bus)) { framecnt = rxdone ? txlimit : min(txlimit, bus->txminmax); @@ -2472,10 +2497,12 @@ static void brcmf_sdbrcm_dpc(struct brcmf_sdio *bus) if ((bus->sdiodev->bus_if->state == BRCMF_BUS_DOWN) || (err != 0)) { brcmf_dbg(ERROR, "failed backplane access over SDIO, halting operation\n"); bus->sdiodev->bus_if->state = BRCMF_BUS_DOWN; - bus->intstatus = 0; - } else if (bus->intstatus || atomic_read(&bus->ipend) > 0 || - (!bus->fcstate && brcmu_pktq_mlen(&bus->txq, ~bus->flowcontrol) - && data_ok(bus)) || PKT_AVAILABLE()) { + atomic_set(&bus->intstatus, 0); + } else if (atomic_read(&bus->intstatus) || + atomic_read(&bus->ipend) > 0 || + (!atomic_read(&bus->fcstate) && + brcmu_pktq_mlen(&bus->txq, ~bus->flowcontrol) && + data_ok(bus)) || PKT_AVAILABLE()) { brcmf_sdbrcm_adddpctsk(bus); } @@ -3640,7 +3667,13 @@ void brcmf_sdbrcm_isr(void *arg) } /* Count the interrupt call */ bus->sdcnt.intrcount++; - atomic_set(&bus->ipend, 1); + if (in_interrupt()) + atomic_set(&bus->ipend, 1); + else + if (brcmf_sdio_intr_rstatus(bus)) { + brcmf_dbg(ERROR, "failed backplane access\n"); + bus->sdiodev->bus_if->state = BRCMF_BUS_DOWN; + } /* Disable additional interrupts (is this needed now)? */ if (!bus->intr) diff --git a/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h b/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h index 29bf78d2..0d30afd 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h +++ b/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h @@ -174,6 +174,8 @@ extern void brcmf_sdio_regwb(struct brcmf_sdio_dev *sdiodev, u32 addr, u8 data, int *ret); extern void brcmf_sdio_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr, u32 data, int *ret); +extern int brcmf_sdio_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr, + void *data, bool write); /* Buffer transfer to/from device (client) core via cmd53. * fn: function number