| Message ID | 20210701163354.118403-1-chris.chiu@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | rtl8xxxu: disable interrupt_in transfer for 8188cu and 8192cu | expand |
chris.chiu@canonical.com writes: > From: Chris Chiu <chris.chiu@canonical.com> > > There will be crazy numbers of interrupts triggered by 8188cu and > 8192cu module, around 8000~10000 interrupts per second, on the usb > host controller. Compare with the vendor driver source code, it's > mapping to the configuration CONFIG_USB_INTERRUPT_IN_PIPE and it is > disabled by default. > > Since the interrupt transfer is neither used for TX/RX nor H2C > commands. Disable it to avoid the confusing interrupts for the > 8188cu and 8192cu module which I only have for verification. The last paragraph is not entirely clear for me, can you elaborate it more? What do you mean with "confusing interrupts"? And is this fixing an actual user visible bug or are you just reducing the number of interrupts?
On 01.07.21 18:33, chris.chiu@canonical.com wrote: > There will be crazy numbers of interrupts triggered by 8188cu and > 8192cu module, around 8000~10000 interrupts per second, on the usb > host controller. Compare with the vendor driver source code, it's > mapping to the configuration CONFIG_USB_INTERRUPT_IN_PIPE and it is > disabled by default. > > Since the interrupt transfer is neither used for TX/RX nor H2C > commands. Disable it to avoid the confusing interrupts for the > 8188cu and 8192cu module which I only have for verification. I tested the new code on the GARDENA smart gateway and it works as expected. Interrupts are greatly reduced while the same level of TX/RX performance could be measured as before the change: A (too) high percentage of retransmissions, but otherwise fine. Tested-by: reto.schneider@husqvarnagroup.com Kind regards, Reto
On Fri, Jul 2, 2021 at 4:42 PM Kalle Valo <kvalo@codeaurora.org> wrote: > > chris.chiu@canonical.com writes: > > > From: Chris Chiu <chris.chiu@canonical.com> > > > > There will be crazy numbers of interrupts triggered by 8188cu and > > 8192cu module, around 8000~10000 interrupts per second, on the usb > > host controller. Compare with the vendor driver source code, it's > > mapping to the configuration CONFIG_USB_INTERRUPT_IN_PIPE and it is > > disabled by default. > > > > Since the interrupt transfer is neither used for TX/RX nor H2C > > commands. Disable it to avoid the confusing interrupts for the > > 8188cu and 8192cu module which I only have for verification. > > The last paragraph is not entirely clear for me, can you elaborate it > more? What do you mean with "confusing interrupts"? And is this fixing > an actual user visible bug or are you just reducing the number of > interrupts? > It's confusing because there are 8000~9000 interrupts per second even though the association is not done yet and no traffic is pumped. It's also way too many even the reception of the beacon frames triggers the interrupt. This huge number overwhelms the normal interrupt we expected from the register setting (only < 100/sec if runs with rtlwif/rtl8192cu driver instead). It's difficult to judge where/why the interrupts come from and what possible overhead it could possibly incur. > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 7/1/21 12:33 PM, chris.chiu@canonical.com wrote: > From: Chris Chiu <chris.chiu@canonical.com> > > There will be crazy numbers of interrupts triggered by 8188cu and > 8192cu module, around 8000~10000 interrupts per second, on the usb > host controller. Compare with the vendor driver source code, it's > mapping to the configuration CONFIG_USB_INTERRUPT_IN_PIPE and it is > disabled by default. > > Since the interrupt transfer is neither used for TX/RX nor H2C > commands. Disable it to avoid the confusing interrupts for the > 8188cu and 8192cu module which I only have for verification. > > Signed-off-by: Chris Chiu <chris.chiu@canonical.com> I remember noticing this earlier but never had a chance to dig into it. Thanks for fixing this! Acked-by: Jes Sorensen <Jes.Sorensen@gmail.com> Jes
chris.chiu@canonical.com wrote: > From: Chris Chiu <chris.chiu@canonical.com> > > There will be crazy numbers of interrupts triggered by 8188cu and > 8192cu module, around 8000~10000 interrupts per second, on the usb > host controller. Compare with the vendor driver source code, it's > mapping to the configuration CONFIG_USB_INTERRUPT_IN_PIPE and it is > disabled by default. > > Since the interrupt transfer is neither used for TX/RX nor H2C > commands. Disable it to avoid the excessive amount of interrupts > for the 8188cu and 8192cu module which I only have for verification. > > Signed-off-by: Chris Chiu <chris.chiu@canonical.com> > Tested-by: reto.schneider@husqvarnagroup.com > Acked-by: Jes Sorensen <Jes.Sorensen@gmail.com> Patch applied to wireless-drivers-next.git, thanks. f62cdab7f5db rtl8xxxu: disable interrupt_in transfer for 8188cu and 8192cu -- https://patchwork.kernel.org/project/linux-wireless/patch/20210701163354.118403-1-chris.chiu@canonical.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 03c6ed7efe06..6a3dfa71b27f 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -1670,7 +1670,7 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv) priv->rf_paths = 2; priv->rx_paths = 2; priv->tx_paths = 2; - priv->usb_interrupts = 1; + priv->usb_interrupts = 0; priv->rtl_chip = RTL8192C; } priv->has_wifi = 1; @@ -1680,7 +1680,7 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv *priv) priv->rx_paths = 1; priv->tx_paths = 1; priv->rtl_chip = RTL8188C; - priv->usb_interrupts = 1; + priv->usb_interrupts = 0; priv->has_wifi = 1; }