| Message ID | d9bd4949-6e92-4f35-8b60-3b45f9ad74ab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b06439c6687443e6b99dbcf746042067ff0e9ba9 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] wifi: rtlwifi: Speed up firmware loading for USB | expand |
> -----Original Message----- > From: Bitterblue Smith <rtl8821cerfe2@gmail.com> > Sent: Thursday, January 18, 2024 4:12 AM > To: linux-wireless@vger.kernel.org > Cc: Ping-Ke Shih <pkshih@realtek.com>; Larry Finger <Larry.Finger@lwfinger.net> > Subject: [PATCH v2] wifi: rtlwifi: Speed up firmware loading for USB > > Currently it takes almost 6 seconds to upload the firmware for RTL8192CU > (and 11 seconds for RTL8192DU). That's because the firmware is uploaded > one byte at a time. > > Also, after plugging the device, the firmware gets uploaded three times > before a connection to the AP is established. > > Maybe this is fine for most users, but when testing changes to the > driver it's really annoying to wait so long. > > Speed up the firmware upload by writing chunks of 64 bytes at a time. > This way it takes about 110 ms for RTL8192CU (and about 210 ms for > RTL8192DU). > > PCI devices could upload it in chunks of 4 bytes, but I don't have any > to test and commit 89d32c9071aa ("rtlwifi: Download firmware as bytes > rather than as dwords") decided otherwise anyway. > > Allocate memory for the firmware image with kmalloc instead of vzalloc > because this memory is passed directly to usb_control_msg(), which > can't take memory allocated by vmalloc. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> Acked-by: Ping-Ke Shih <pkshih@realtek.com> [...] > diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c > index 07a7e6fa46af..1fc480fe18ad 100644 > --- a/drivers/net/wireless/realtek/rtlwifi/usb.c > +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c > @@ -125,6 +125,14 @@ static void _usb_write32_sync(struct rtl_priv *rtlpriv, u32 addr, u32 val) > _usb_write_sync(rtlpriv, addr, val, 4); > } > > +static void _usb_write_chunk_sync(struct rtl_priv *rtlpriv, u32 addr, > + u32 length, u8 *data) > +{ > + struct usb_device *udev = to_usb_device(rtlpriv->io.dev); > + > + _usbctrl_vendorreq_sync(udev, REALTEK_USB_VENQT_WRITE, addr, data, length); Just curious. Originally, it uses 1/2/4 as length for write8/16/32, and this patch additionally uses 8/64 as length. Any limitation of argument 'length' of this function? Is arbitrary number disallowed? > +} > +
On 18/01/2024 02:37, Ping-Ke Shih wrote: > > >> -----Original Message----- >> From: Bitterblue Smith <rtl8821cerfe2@gmail.com> >> Sent: Thursday, January 18, 2024 4:12 AM >> To: linux-wireless@vger.kernel.org >> Cc: Ping-Ke Shih <pkshih@realtek.com>; Larry Finger <Larry.Finger@lwfinger.net> >> Subject: [PATCH v2] wifi: rtlwifi: Speed up firmware loading for USB >> >> Currently it takes almost 6 seconds to upload the firmware for RTL8192CU >> (and 11 seconds for RTL8192DU). That's because the firmware is uploaded >> one byte at a time. >> >> Also, after plugging the device, the firmware gets uploaded three times >> before a connection to the AP is established. >> >> Maybe this is fine for most users, but when testing changes to the >> driver it's really annoying to wait so long. >> >> Speed up the firmware upload by writing chunks of 64 bytes at a time. >> This way it takes about 110 ms for RTL8192CU (and about 210 ms for >> RTL8192DU). >> >> PCI devices could upload it in chunks of 4 bytes, but I don't have any >> to test and commit 89d32c9071aa ("rtlwifi: Download firmware as bytes >> rather than as dwords") decided otherwise anyway. >> >> Allocate memory for the firmware image with kmalloc instead of vzalloc >> because this memory is passed directly to usb_control_msg(), which >> can't take memory allocated by vmalloc. >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > > Acked-by: Ping-Ke Shih <pkshih@realtek.com> > > [...] > >> diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c >> index 07a7e6fa46af..1fc480fe18ad 100644 >> --- a/drivers/net/wireless/realtek/rtlwifi/usb.c >> +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c >> @@ -125,6 +125,14 @@ static void _usb_write32_sync(struct rtl_priv *rtlpriv, u32 addr, u32 val) >> _usb_write_sync(rtlpriv, addr, val, 4); >> } >> >> +static void _usb_write_chunk_sync(struct rtl_priv *rtlpriv, u32 addr, >> + u32 length, u8 *data) >> +{ >> + struct usb_device *udev = to_usb_device(rtlpriv->io.dev); >> + >> + _usbctrl_vendorreq_sync(udev, REALTEK_USB_VENQT_WRITE, addr, data, length); > > Just curious. Originally, it uses 1/2/4 as length for write8/16/32, and this > patch additionally uses 8/64 as length. Any limitation of argument 'length' of > this function? Is arbitrary number disallowed? > I didn't find anything in the usb_control_msg() documentation. I only found this issue, where some people say 0xffff is fine, but older hardware may have a limit of 4096: https://github.com/libusb/libusb/issues/125 rtl8xxxu uses lengths of 32, 94, 124, 126, 128, 156, 254 bytes. Some other Realtek wifi drivers use 196 bytes.
> -----Original Message----- > From: Bitterblue Smith <rtl8821cerfe2@gmail.com> > Sent: Thursday, January 18, 2024 11:56 PM > To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org > Cc: Larry Finger <Larry.Finger@lwfinger.net> > Subject: Re: [PATCH v2] wifi: rtlwifi: Speed up firmware loading for USB > > > On 18/01/2024 02:37, Ping-Ke Shih wrote: > > > > > >> -----Original Message----- > >> From: Bitterblue Smith <rtl8821cerfe2@gmail.com> > >> Sent: Thursday, January 18, 2024 4:12 AM > >> To: linux-wireless@vger.kernel.org > >> Cc: Ping-Ke Shih <pkshih@realtek.com>; Larry Finger <Larry.Finger@lwfinger.net> > >> Subject: [PATCH v2] wifi: rtlwifi: Speed up firmware loading for USB > >> > >> +static void _usb_write_chunk_sync(struct rtl_priv *rtlpriv, u32 addr, > >> + u32 length, u8 *data) > >> +{ > >> + struct usb_device *udev = to_usb_device(rtlpriv->io.dev); > >> + > >> + _usbctrl_vendorreq_sync(udev, REALTEK_USB_VENQT_WRITE, addr, data, length); > > > > Just curious. Originally, it uses 1/2/4 as length for write8/16/32, and this > > patch additionally uses 8/64 as length. Any limitation of argument 'length' of > > this function? Is arbitrary number disallowed? > > > > I didn't find anything in the usb_control_msg() documentation. > I only found this issue, where some people say 0xffff is fine, > but older hardware may have a limit of 4096: > https://github.com/libusb/libusb/issues/125 > > rtl8xxxu uses lengths of 32, 94, 124, 126, 128, 156, 254 bytes. > Some other Realtek wifi drivers use 196 bytes. Got it. Thanks for the information.
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > Currently it takes almost 6 seconds to upload the firmware for RTL8192CU > (and 11 seconds for RTL8192DU). That's because the firmware is uploaded > one byte at a time. > > Also, after plugging the device, the firmware gets uploaded three times > before a connection to the AP is established. > > Maybe this is fine for most users, but when testing changes to the > driver it's really annoying to wait so long. > > Speed up the firmware upload by writing chunks of 64 bytes at a time. > This way it takes about 110 ms for RTL8192CU (and about 210 ms for > RTL8192DU). > > PCI devices could upload it in chunks of 4 bytes, but I don't have any > to test and commit 89d32c9071aa ("rtlwifi: Download firmware as bytes > rather than as dwords") decided otherwise anyway. > > Allocate memory for the firmware image with kmalloc instead of vzalloc > because this memory is passed directly to usb_control_msg(), which > can't take memory allocated by vmalloc. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > Acked-by: Ping-Ke Shih <pkshih@realtek.com> Patch applied to wireless-next.git, thanks. b06439c66874 wifi: rtlwifi: Speed up firmware loading for USB
diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c index 2e945554ed6d..c1fbc29d5ca1 100644 --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c @@ -1287,18 +1287,44 @@ int rtl_get_hwinfo(struct ieee80211_hw *hw, struct rtl_priv *rtlpriv, } EXPORT_SYMBOL_GPL(rtl_get_hwinfo); -void rtl_fw_block_write(struct ieee80211_hw *hw, const u8 *buffer, u32 size) +static void _rtl_fw_block_write_usb(struct ieee80211_hw *hw, u8 *buffer, u32 size) +{ + struct rtl_priv *rtlpriv = rtl_priv(hw); + u32 start = START_ADDRESS; + u32 n; + + while (size > 0) { + if (size >= 64) + n = 64; + else if (size >= 8) + n = 8; + else + n = 1; + + rtl_write_chunk(rtlpriv, start, n, buffer); + + start += n; + buffer += n; + size -= n; + } +} + +void rtl_fw_block_write(struct ieee80211_hw *hw, u8 *buffer, u32 size) { struct rtl_priv *rtlpriv = rtl_priv(hw); - u8 *pu4byteptr = (u8 *)buffer; u32 i; - for (i = 0; i < size; i++) - rtl_write_byte(rtlpriv, (START_ADDRESS + i), *(pu4byteptr + i)); + if (rtlpriv->rtlhal.interface == INTF_PCI) { + for (i = 0; i < size; i++) + rtl_write_byte(rtlpriv, (START_ADDRESS + i), + *(buffer + i)); + } else if (rtlpriv->rtlhal.interface == INTF_USB) { + _rtl_fw_block_write_usb(hw, buffer, size); + } } EXPORT_SYMBOL_GPL(rtl_fw_block_write); -void rtl_fw_page_write(struct ieee80211_hw *hw, u32 page, const u8 *buffer, +void rtl_fw_page_write(struct ieee80211_hw *hw, u32 page, u8 *buffer, u32 size) { struct rtl_priv *rtlpriv = rtl_priv(hw); diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.h b/drivers/net/wireless/realtek/rtlwifi/efuse.h index 1ec59f439382..4821625ad1e5 100644 --- a/drivers/net/wireless/realtek/rtlwifi/efuse.h +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.h @@ -91,8 +91,8 @@ void efuse_power_switch(struct ieee80211_hw *hw, u8 write, u8 pwrstate); int rtl_get_hwinfo(struct ieee80211_hw *hw, struct rtl_priv *rtlpriv, int max_size, u8 *hwinfo, int *params); void rtl_fill_dummy(u8 *pfwbuf, u32 *pfwlen); -void rtl_fw_page_write(struct ieee80211_hw *hw, u32 page, const u8 *buffer, +void rtl_fw_page_write(struct ieee80211_hw *hw, u32 page, u8 *buffer, u32 size); -void rtl_fw_block_write(struct ieee80211_hw *hw, const u8 *buffer, u32 size); +void rtl_fw_block_write(struct ieee80211_hw *hw, u8 *buffer, u32 size); void rtl_efuse_ops_init(struct ieee80211_hw *hw); #endif diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c index 20b4aac69642..9f4cf09090d6 100644 --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/sw.c @@ -40,7 +40,7 @@ static int rtl92cu_init_sw_vars(struct ieee80211_hw *hw) rtlpriv->dm.thermalvalue = 0; /* for firmware buf */ - rtlpriv->rtlhal.pfirmware = vzalloc(0x4000); + rtlpriv->rtlhal.pfirmware = kmalloc(0x4000, GFP_KERNEL); if (!rtlpriv->rtlhal.pfirmware) { pr_err("Can't alloc buffer for fw\n"); return 1; @@ -61,7 +61,7 @@ static int rtl92cu_init_sw_vars(struct ieee80211_hw *hw) fw_name, rtlpriv->io.dev, GFP_KERNEL, hw, rtl_fw_cb); if (err) { - vfree(rtlpriv->rtlhal.pfirmware); + kfree(rtlpriv->rtlhal.pfirmware); rtlpriv->rtlhal.pfirmware = NULL; } return err; @@ -72,7 +72,7 @@ static void rtl92cu_deinit_sw_vars(struct ieee80211_hw *hw) struct rtl_priv *rtlpriv = rtl_priv(hw); if (rtlpriv->rtlhal.pfirmware) { - vfree(rtlpriv->rtlhal.pfirmware); + kfree(rtlpriv->rtlhal.pfirmware); rtlpriv->rtlhal.pfirmware = NULL; } } diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c index 07a7e6fa46af..1fc480fe18ad 100644 --- a/drivers/net/wireless/realtek/rtlwifi/usb.c +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c @@ -125,6 +125,14 @@ static void _usb_write32_sync(struct rtl_priv *rtlpriv, u32 addr, u32 val) _usb_write_sync(rtlpriv, addr, val, 4); } +static void _usb_write_chunk_sync(struct rtl_priv *rtlpriv, u32 addr, + u32 length, u8 *data) +{ + struct usb_device *udev = to_usb_device(rtlpriv->io.dev); + + _usbctrl_vendorreq_sync(udev, REALTEK_USB_VENQT_WRITE, addr, data, length); +} + static void _rtl_usb_io_handler_init(struct device *dev, struct ieee80211_hw *hw) { @@ -135,6 +143,7 @@ static void _rtl_usb_io_handler_init(struct device *dev, rtlpriv->io.write8 = _usb_write8_sync; rtlpriv->io.write16 = _usb_write16_sync; rtlpriv->io.write32 = _usb_write32_sync; + rtlpriv->io.write_chunk = _usb_write_chunk_sync; rtlpriv->io.read8 = _usb_read8_sync; rtlpriv->io.read16 = _usb_read16_sync; rtlpriv->io.read32 = _usb_read32_sync; diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h index 53af324f3807..3821f6e31447 100644 --- a/drivers/net/wireless/realtek/rtlwifi/wifi.h +++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h @@ -1450,6 +1450,8 @@ struct rtl_io { void (*write8)(struct rtl_priv *rtlpriv, u32 addr, u8 val); void (*write16)(struct rtl_priv *rtlpriv, u32 addr, u16 val); void (*write32)(struct rtl_priv *rtlpriv, u32 addr, u32 val); + void (*write_chunk)(struct rtl_priv *rtlpriv, u32 addr, u32 length, + u8 *data); u8 (*read8)(struct rtl_priv *rtlpriv, u32 addr); u16 (*read16)(struct rtl_priv *rtlpriv, u32 addr); @@ -2962,6 +2964,12 @@ static inline void rtl_write_dword(struct rtl_priv *rtlpriv, rtlpriv->io.read32(rtlpriv, addr); } +static inline void rtl_write_chunk(struct rtl_priv *rtlpriv, + u32 addr, u32 length, u8 *data) +{ + rtlpriv->io.write_chunk(rtlpriv, addr, length, data); +} + static inline u32 rtl_get_bbreg(struct ieee80211_hw *hw, u32 regaddr, u32 bitmask) {
Currently it takes almost 6 seconds to upload the firmware for RTL8192CU (and 11 seconds for RTL8192DU). That's because the firmware is uploaded one byte at a time. Also, after plugging the device, the firmware gets uploaded three times before a connection to the AP is established. Maybe this is fine for most users, but when testing changes to the driver it's really annoying to wait so long. Speed up the firmware upload by writing chunks of 64 bytes at a time. This way it takes about 110 ms for RTL8192CU (and about 210 ms for RTL8192DU). PCI devices could upload it in chunks of 4 bytes, but I don't have any to test and commit 89d32c9071aa ("rtlwifi: Download firmware as bytes rather than as dwords") decided otherwise anyway. Allocate memory for the firmware image with kmalloc instead of vzalloc because this memory is passed directly to usb_control_msg(), which can't take memory allocated by vmalloc. Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- v2: - Simplify _rtl_fw_block_write_usb(). - Explain kmalloc in the commit message. --- drivers/net/wireless/realtek/rtlwifi/efuse.c | 36 ++++++++++++++++--- drivers/net/wireless/realtek/rtlwifi/efuse.h | 4 +-- .../wireless/realtek/rtlwifi/rtl8192cu/sw.c | 6 ++-- drivers/net/wireless/realtek/rtlwifi/usb.c | 9 +++++ drivers/net/wireless/realtek/rtlwifi/wifi.h | 8 +++++ 5 files changed, 53 insertions(+), 10 deletions(-)