The watchdog function is broken on rt2800 series SoCs. This patch
fixes the incorrect watchdog logic to make it work again.
1. Update current wdt queue index if it's not equal to the previous
index. Watchdog compares the current and previous queue index to
judge if the queue hung.
2. Make sure hung_{rx,tx} 'true' status won't be override by the
normal queue. Any queue hangs should trigger a reset action.
3. Clear the watchdog counter of all queues before resetting the
hardware. This change may help to avoid the reset loop.
4. Change hang check function return type to bool as we only need
to return two status, yes or no.
Signed-off-by: Shiji Yang <[email protected]>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index e65cc00fa..d39d87827 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1236,13 +1236,14 @@ void rt2800_txdone_nostatus(struct rt2x00_dev *rt2x00dev)
}
EXPORT_SYMBOL_GPL(rt2800_txdone_nostatus);
-static int rt2800_check_hung(struct data_queue *queue)
+static bool rt2800_check_hung(struct data_queue *queue)
{
unsigned int cur_idx = rt2800_drv_get_dma_done(queue);
- if (queue->wd_idx != cur_idx)
+ if (queue->wd_idx != cur_idx) {
+ queue->wd_idx = cur_idx;
queue->wd_count = 0;
- else
+ } else
queue->wd_count++;
return queue->wd_count > 16;
@@ -1279,7 +1280,7 @@ void rt2800_watchdog(struct rt2x00_dev *rt2x00dev)
case QID_MGMT:
if (rt2x00queue_empty(queue))
continue;
- hung_tx = rt2800_check_hung(queue);
+ hung_tx = hung_tx || rt2800_check_hung(queue);
break;
case QID_RX:
/* For station mode we should reactive at least
@@ -1288,7 +1289,7 @@ void rt2800_watchdog(struct rt2x00_dev *rt2x00dev)
*/
if (rt2x00dev->intf_sta_count == 0)
continue;
- hung_rx = rt2800_check_hung(queue);
+ hung_rx = hung_rx || rt2800_check_hung(queue);
break;
default:
break;
@@ -1301,8 +1302,12 @@ void rt2800_watchdog(struct rt2x00_dev *rt2x00dev)
if (hung_rx)
rt2x00_warn(rt2x00dev, "Watchdog RX hung detected\n");
- if (hung_tx || hung_rx)
+ if (hung_tx || hung_rx) {
+ queue_for_each(rt2x00dev, queue)
+ queue->wd_count = 0;
+
ieee80211_restart_hw(rt2x00dev->hw);
+ }
}
EXPORT_SYMBOL_GPL(rt2800_watchdog);
--
2.39.2
Shiji Yang <[email protected]> wrote:
> The watchdog function is broken on rt2800 series SoCs. This patch
> fixes the incorrect watchdog logic to make it work again.
>
> 1. Update current wdt queue index if it's not equal to the previous
> index. Watchdog compares the current and previous queue index to
> judge if the queue hung.
> 2. Make sure hung_{rx,tx} 'true' status won't be override by the
> normal queue. Any queue hangs should trigger a reset action.
> 3. Clear the watchdog counter of all queues before resetting the
> hardware. This change may help to avoid the reset loop.
> 4. Change hang check function return type to bool as we only need
> to return two status, yes or no.
>
> Signed-off-by: Shiji Yang <[email protected]>
Is this patch ok to take?
--
https://patchwork.kernel.org/project/linux-wireless/patch/TYAP286MB0315BC1D83D31154924F0D39BCD1A@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Thu, 19 Oct 2023 07:00:30 +0000, Kalle Valo wrote:
>Shiji Yang <[email protected]> wrote:
>
>> The watchdog function is broken on rt2800 series SoCs. This patch
>> fixes the incorrect watchdog logic to make it work again.
>>
>> 1. Update current wdt queue index if it's not equal to the previous
>> index. Watchdog compares the current and previous queue index to
>> judge if the queue hung.
>> 2. Make sure hung_{rx,tx} 'true' status won't be override by the
>> normal queue. Any queue hangs should trigger a reset action.
>> 3. Clear the watchdog counter of all queues before resetting the
>> hardware. This change may help to avoid the reset loop.
>> 4. Change hang check function return type to bool as we only need
>> to return two status, yes or no.
>>
>> Signed-off-by: Shiji Yang <[email protected]>
>
>Is this patch ok to take?
Hi! I think it's ready. I've been testing it for a few weeks.
>
>--
>https://patchwork.kernel.org/project/linux-wireless/patch/TYAP286MB0315BC1D83D31154924F0D39BCD1A@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM/
>
>https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>
Regards,
Shiji Yang
Shiji Yang <[email protected]> writes:
> On Thu, 19 Oct 2023 07:00:30 +0000, Kalle Valo wrote:
>
>>Shiji Yang <[email protected]> wrote:
>>
>>> The watchdog function is broken on rt2800 series SoCs. This patch
>>> fixes the incorrect watchdog logic to make it work again.
>>>
>>> 1. Update current wdt queue index if it's not equal to the previous
>>> index. Watchdog compares the current and previous queue index to
>>> judge if the queue hung.
>>> 2. Make sure hung_{rx,tx} 'true' status won't be override by the
>>> normal queue. Any queue hangs should trigger a reset action.
>>> 3. Clear the watchdog counter of all queues before resetting the
>>> hardware. This change may help to avoid the reset loop.
>>> 4. Change hang check function return type to bool as we only need
>>> to return two status, yes or no.
>>>
>>> Signed-off-by: Shiji Yang <[email protected]>
>>
>>Is this patch ok to take?
>
>
> Hi! I think it's ready. I've been testing it for a few weeks.
That question was more towards maintainers Stanislaw, Helmut and other
reviewers :)
That reminds me, is Helmut still maintaining this driver? I haven't seen
an ack from him since 2016. As we nowadays try to keep the MAINTAINERS
up-to-date so should we remove him?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Thu, Oct 19, 2023 at 04:33:02PM +0300, Kalle Valo wrote:
> Shiji Yang <[email protected]> writes:
>
> > On Thu, 19 Oct 2023 07:00:30 +0000, Kalle Valo wrote:
> >
> >>Shiji Yang <[email protected]> wrote:
> >>
> >>> The watchdog function is broken on rt2800 series SoCs. This patch
> >>> fixes the incorrect watchdog logic to make it work again.
> >>>
> >>> 1. Update current wdt queue index if it's not equal to the previous
> >>> index. Watchdog compares the current and previous queue index to
> >>> judge if the queue hung.
> >>> 2. Make sure hung_{rx,tx} 'true' status won't be override by the
> >>> normal queue. Any queue hangs should trigger a reset action.
> >>> 3. Clear the watchdog counter of all queues before resetting the
> >>> hardware. This change may help to avoid the reset loop.
> >>> 4. Change hang check function return type to bool as we only need
> >>> to return two status, yes or no.
> >>>
> >>> Signed-off-by: Shiji Yang <[email protected]>
Acked-by: Stanislaw Gruszka <[email protected]>
> >>Is this patch ok to take?
> >
> >
> > Hi! I think it's ready. I've been testing it for a few weeks.
>
> That question was more towards maintainers Stanislaw, Helmut and other
> reviewers :)
>
> That reminds me, is Helmut still maintaining this driver? I haven't seen
> an ack from him since 2016. As we nowadays try to keep the MAINTAINERS
> up-to-date so should we remove him?
Yeah, feel free to remove him. I can post a patch if needed.
Regards
Stanislaw
Stanislaw Gruszka <[email protected]> writes:
> On Thu, Oct 19, 2023 at 04:33:02PM +0300, Kalle Valo wrote:
>> Shiji Yang <[email protected]> writes:
>>
>> > On Thu, 19 Oct 2023 07:00:30 +0000, Kalle Valo wrote:
>> >
>> >>Shiji Yang <[email protected]> wrote:
>> >>
>> >>> The watchdog function is broken on rt2800 series SoCs. This patch
>> >>> fixes the incorrect watchdog logic to make it work again.
>> >>>
>> >>> 1. Update current wdt queue index if it's not equal to the previous
>> >>> index. Watchdog compares the current and previous queue index to
>> >>> judge if the queue hung.
>> >>> 2. Make sure hung_{rx,tx} 'true' status won't be override by the
>> >>> normal queue. Any queue hangs should trigger a reset action.
>> >>> 3. Clear the watchdog counter of all queues before resetting the
>> >>> hardware. This change may help to avoid the reset loop.
>> >>> 4. Change hang check function return type to bool as we only need
>> >>> to return two status, yes or no.
>> >>>
>> >>> Signed-off-by: Shiji Yang <[email protected]>
>
> Acked-by: Stanislaw Gruszka <[email protected]>
Thanks!
>> That reminds me, is Helmut still maintaining this driver? I haven't seen
>> an ack from him since 2016. As we nowadays try to keep the MAINTAINERS
>> up-to-date so should we remove him?
>
> Yeah, feel free to remove him. I can post a patch if needed.
It would help if you can send a patch, thanks.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Shiji Yang <[email protected]> wrote:
> The watchdog function is broken on rt2800 series SoCs. This patch
> fixes the incorrect watchdog logic to make it work again.
>
> 1. Update current wdt queue index if it's not equal to the previous
> index. Watchdog compares the current and previous queue index to
> judge if the queue hung.
> 2. Make sure hung_{rx,tx} 'true' status won't be override by the
> normal queue. Any queue hangs should trigger a reset action.
> 3. Clear the watchdog counter of all queues before resetting the
> hardware. This change may help to avoid the reset loop.
> 4. Change hang check function return type to bool as we only need
> to return two status, yes or no.
>
> Signed-off-by: Shiji Yang <[email protected]>
> Acked-by: Stanislaw Gruszka <[email protected]>
Patch applied to wireless-next.git, thanks.
69708fbb2c69 wifi: rt2x00: fix rt2800 watchdog function
--
https://patchwork.kernel.org/project/linux-wireless/patch/TYAP286MB0315BC1D83D31154924F0D39BCD1A@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches