Re: [PATCH v3 1/2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Luka Gejak]

Hi Alastair,

The split looks good to me.

One issue with acknowledging the request up front:

> +	rtw_sdio_disable_interrupt(rtwdev);
> +	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);

When dev_alloc_skb() fails, rtw_sdio_rxfifo_recv() returns without
reading the port:

	skb = dev_alloc_skb(bufsz);
	if (!skb)
	  return;

but the loop counts the bytes anyway:

		rtw_sdio_rxfifo_recv(rtwdev, rx_len);

		total_rx_bytes += rx_len;

so a few failed reads burn the 64K budget and the loop exits with the
FIFO still full. The request is already cleared by then, and only a new
packet sets it again, which may never come if the FIFO is full and the
sender is paused. 8051 does not have this problem because it leaves the
bit to hardware. Can rtw_sdio_rxfifo_recv() report the failure, so the
loop can stop and retry instead? The read error just below the allocation
leaves the FIFO in the same state.

Same function, the enable at the end:

> +	/* Unmasking HIMR re-asserts the IRQ line if new packets arrived */
> +	rtw_sdio_enable_interrupt(rtwdev);

It also runs when the device has been stopped. ksdioirqd can still run a
handler that was signalled before rtw_sdio_stop() released the host, and
stop only writes HIMR as zero:

	static void rtw_sdio_stop(struct rtw_dev *rtwdev)
	{
		rtw_sdio_disable_interrupt(rtwdev);
	}

rtwsdio->irq_mask still has RX_REQUEST and CPWM1, so this puts the mask
back and arms the stopped device again. PCI clears rtwpci->running in its
stop path and only re-enables while it is still set:

	if (rtwpci->running)
		rtw_pci_enable_interrupt(rtwdev, rtwpci, rx);

Can you add something like that here?

Best regards,
Luka Gejak