2023-04-28 02:35:33

by Kevin Lo

[permalink] [raw]
Subject: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts

There's no need to initialize interrupts for RTL8188FTV like the vendor driver.

Signed-off-by: Kevin Lo <[email protected]>
---
v2:
- Fix my email address.
---
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 620a5cc2bfdd..7f620dc47ee0 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4023,9 +4023,6 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
if (priv->rtl_chip == RTL8192E) {
rtl8xxxu_write32(priv, REG_HIMR0, 0x00);
rtl8xxxu_write32(priv, REG_HIMR1, 0x00);
- } else if (priv->rtl_chip == RTL8188F) {
- rtl8xxxu_write32(priv, REG_HISR0, 0xffffffff);
- rtl8xxxu_write32(priv, REG_HISR1, 0xffffffff);
} else if (priv->rtl_chip == RTL8188E) {
rtl8xxxu_write32(priv, REG_HISR0, 0xffffffff);
val32 = IMR0_PSTIMEOUT | IMR0_TBDER | IMR0_CPWM | IMR0_CPWM2;
@@ -4035,7 +4032,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
val8 = rtl8xxxu_read8(priv, REG_USB_SPECIAL_OPTION);
val8 |= USB_SPEC_INT_BULK_SELECT;
rtl8xxxu_write8(priv, REG_USB_SPECIAL_OPTION, val8);
- } else {
+ } else if (priv->rtl_chip != RTL8188F) {
/*
* Enable all interrupts - not obvious USB needs to do this
*/


2023-04-28 05:52:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts

Kevin Lo <[email protected]> writes:

> There's no need to initialize interrupts for RTL8188FTV like the vendor driver.
>
> Signed-off-by: Kevin Lo <[email protected]>

Why do this, what's the benefit? Or is this just a theoretical fix. The
commit log should explain this.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2023-04-28 06:44:52

by Kevin Lo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts

On Fri, Apr 28, 2023 at 08:40:34AM +0300, Kalle Valo wrote:
>
> Kevin Lo <[email protected]> writes:
>
> > There's no need to initialize interrupts for RTL8188FTV like the vendor driver.
> >
> > Signed-off-by: Kevin Lo <[email protected]>
>
> Why do this, what's the benefit? Or is this just a theoretical fix. The
> commit log should explain this.

If I understand correctly, just like rtw88 usb devices, rtl8xxxu has no
hardware interrupt.

2023-05-02 09:16:29

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts



> -----Original Message-----
> From: Kevin Lo <[email protected]>
> Sent: Friday, April 28, 2023 2:35 PM
> To: Kalle Valo <[email protected]>
> Cc: [email protected]; Bitterblue Smith <[email protected]>; Jes Sorensen
> <[email protected]>; Ping-Ke Shih <[email protected]>
> Subject: Re: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts
>
> On Fri, Apr 28, 2023 at 08:40:34AM +0300, Kalle Valo wrote:
> >
> > Kevin Lo <[email protected]> writes:
> >
> > > There's no need to initialize interrupts for RTL8188FTV like the vendor driver.
> > >
> > > Signed-off-by: Kevin Lo <[email protected]>
> >
> > Why do this, what's the benefit? Or is this just a theoretical fix. The
> > commit log should explain this.
>
> If I understand correctly, just like rtw88 usb devices, rtl8xxxu has no
> hardware interrupt.
>

With/without hardware interrupt, it depends on IMR (interrupt mask register)
setting. If it is enabled, driver can receive C2H, PS indicators, and beacon
TBTT events via certain USB endpoint. But, normally vendor driver doesn't
use them neither.

Why it initializes interrupt in this patch is to reset ISR (interrupt status
register) to avoid unexpected interrupts are received. So, if it doesn't
really enable interrupt by IMR, I suppose it should work well even without
this patch.

Kevin, could I know the real problem you met?

Ping-Ke

2023-05-05 05:29:43

by Kevin Lo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts

On Tue, May 02, 2023 at 09:10:04AM +0000, Ping-Ke Shih wrote:
>
>
>
> > -----Original Message-----
> > From: Kevin Lo <[email protected]>
> > Sent: Friday, April 28, 2023 2:35 PM
> > To: Kalle Valo <[email protected]>
> > Cc: [email protected]; Bitterblue Smith <[email protected]>; Jes Sorensen
> > <[email protected]>; Ping-Ke Shih <[email protected]>
> > Subject: Re: [PATCH v2] wifi: rtl8xxxu: 8188f: no need to initialize interrupts
> >
> > On Fri, Apr 28, 2023 at 08:40:34AM +0300, Kalle Valo wrote:
> > >
> > > Kevin Lo <[email protected]> writes:
> > >
> > > > There's no need to initialize interrupts for RTL8188FTV like the vendor driver.
> > > >
> > > > Signed-off-by: Kevin Lo <[email protected]>
> > >
> > > Why do this, what's the benefit? Or is this just a theoretical fix. The
> > > commit log should explain this.
> >
> > If I understand correctly, just like rtw88 usb devices, rtl8xxxu has no
> > hardware interrupt.
> >
>
> With/without hardware interrupt, it depends on IMR (interrupt mask register)
> setting. If it is enabled, driver can receive C2H, PS indicators, and beacon
> TBTT events via certain USB endpoint. But, normally vendor driver doesn't
> use them neither.
>
> Why it initializes interrupt in this patch is to reset ISR (interrupt status
> register) to avoid unexpected interrupts are received. So, if it doesn't
> really enable interrupt by IMR, I suppose it should work well even without
> this patch.
>
> Kevin, could I know the real problem you met?

Ping-Ke, many thanks for the clear explanation.

I didn't encounter any problems. As you mentioned, while the vendor driver
doesn't have IMR settings, there is no harm in initializing interrupts.
Please drop this patch, thanks.

> Ping-Ke

Regards,
Kevin