| Message ID | 1c6684f6-d3a8-4eaa-842d-c21fa2dd81c1@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] wifi: brcm80211: brcmfmac: Prevent sdio bus going to sleep while transfering data | expand |
On July 7, 2024 2:22:49 PM Nikolay Nikolov <dobrev666@gmail.com> wrote: > Use mutex to prevent sdio bus to be put to sleep by the sdio_bus_watchdog > while sdio dataworker handles sdio_dpc data transfers. Any reason for sending 3 identical patches within the hour. As to the patch itself I would like to know if there is a reported issue being fixed here. What is the motivation behind this patch. Looking at the code I do not think the mutex is needed so please elaborate. Regards, Arend > Signed-off-by: Nikolay Nikolov <nikolay.nikolov@bench.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 ++++++ > 1 file changed, 6 insertions(+)
Nikolay Nikolov <dobrev666@gmail.com> writes: > I am really sorry for the spamming ! > I have not sent a patch to the Linux kernel mailing list for more than > 20 years and mail clients do not behave as I expect. My first email > was rejected from the mailing lists as it contained HTML. Indentation > is not correct in the second one. I hope third one is correct. BTW I recommend reading the documentation from our wiki, link below. That should save both your and our time.
On Tue, Jul 9, 2024 at 12:12 PM Kalle Valo <kvalo@kernel.org> wrote: > > Nikolay Nikolov <dobrev666@gmail.com> writes: > > > I am really sorry for the spamming ! > > I have not sent a patch to the Linux kernel mailing list for more than > > 20 years and mail clients do not behave as I expect. My first email > > was rejected from the mailing lists as it contained HTML. Indentation > > is not correct in the second one. I hope third one is correct. > > BTW I recommend reading the documentation from our wiki, link below. > That should save both your and our time. > I was reading for several days this and the kernel mailing list wiki before sending the patch. The problem was the email clients. Anyway I want to mention something which is not present here. There are some memory barriers around this dpc_running which I think are supposed to prevent such an issue. But looks like this do not help much. With the usage of mutex those memory barriers could be removed as well. I am already using such a patch in our embedded system. Please, let me know if I should submit this patch. I promise I will do my best not to spam anymore. Regards, Nikolay Nikolov > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 6b38d9de71af..26d0bce6cedc 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -524,6 +524,7 @@ struct brcmf_sdio { bool txglom; /* host tx glomming enable flag */ u16 head_align; /* buffer pointer alignment */ u16 sgentry_align; /* scatter-gather buffer alignment */ + struct mutex dpc_mutex; }; /* clkstate */ @@ -3724,6 +3725,7 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus) } #endif /* DEBUG */ + mutex_lock(&bus->dpc_mutex); /* On idle timeout clear activity flag and/or turn off clock */ if (!bus->dpc_triggered) { rmb(); @@ -3748,6 +3750,7 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus) } else { bus->idlecount = 0; } + mutex_unlock(&bus->dpc_mutex); } static void brcmf_sdio_dataworker(struct work_struct *work) @@ -3755,6 +3758,7 @@ static void brcmf_sdio_dataworker(struct work_struct *work) struct brcmf_sdio *bus = container_of(work, struct brcmf_sdio, datawork); + mutex_lock(&bus->dpc_mutex); bus->dpc_running = true; wmb(); while (READ_ONCE(bus->dpc_triggered)) { @@ -3763,6 +3767,7 @@ static void brcmf_sdio_dataworker(struct work_struct *work) bus->idlecount = 0; } bus->dpc_running = false; + mutex_unlock(&bus->dpc_mutex); if (brcmf_sdiod_freezing(bus->sdiodev)) { brcmf_sdiod_change_state(bus->sdiodev, BRCMF_SDIOD_DOWN); brcmf_sdiod_try_freeze(bus->sdiodev); @@ -4525,6 +4530,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev) /* SR state */ bus->sr_enabled = false; + mutex_init(&bus->dpc_mutex); brcmf_dbg(INFO, "completed!!\n");
Use mutex to prevent sdio bus to be put to sleep by the sdio_bus_watchdog while sdio dataworker handles sdio_dpc data transfers. Signed-off-by: Nikolay Nikolov <nikolay.nikolov@bench.com> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 ++++++ 1 file changed, 6 insertions(+) ---