Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
> Add very basic USB support. No TX/RX aggregation, no TX queues, no
> switching to USB 3 mode.
>
> RTL8851BU and RTL8832BU work.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
> ---
> drivers/net/wireless/realtek/rtw89/usb.c | 1030 ++++++++++++++++++++++
> drivers/net/wireless/realtek/rtw89/usb.h | 61 ++
> 2 files changed, 1091 insertions(+)
> create mode 100644 drivers/net/wireless/realtek/rtw89/usb.c
> create mode 100644 drivers/net/wireless/realtek/rtw89/usb.h
>
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c
> new file mode 100644
> index 000000000000..6e8a544b352c
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtw89/usb.c
> @@ -0,0 +1,1030 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +/* Copyright(c) 2025 Realtek Corporation
> + */
> +
> +#include <linux/usb.h>
> +#include "debug.h"
> +#include "mac.h"
> +#include "reg.h"
> +#include "txrx.h"
> +#include "usb.h"
> +
> +static void rtw89_usb_vendorreq(struct rtw89_dev *rtwdev, u32 addr,
> + void *data, u16 len, u8 reqtype)
> +{
> + struct rtw89_usb *rtwusb = rtw89_get_usb_priv(rtwdev);
> + struct usb_device *udev = rtwusb->udev;
> + unsigned int pipe;
> + u16 value, index;
> + int attempt, ret;
> +
> + value = addr & 0x0000ffff;
> + index = (addr & 0x00ff0000) >> 16;
u16_get_bits(addr, GENMASK(23, 16)) ?
> +
> + mutex_lock(&rtwusb->vendor_req_mutex);
rtw89 takes wiphy_lock for control path. Is there any case more than
one threads run at the same time?
> +
> + for (attempt = 0; attempt < 10; attempt++) {
> + *rtwusb->vendor_req_buf = 0;
> +
> + if (reqtype == RTW89_USB_VENQT_READ) {
> + pipe = usb_rcvctrlpipe(udev, 0);
> + } else { /* RTW89_USB_VENQT_WRITE */
> + pipe = usb_sndctrlpipe(udev, 0);
> +
> + memcpy(rtwusb->vendor_req_buf, data, len);
By "rtwusb->vendor_req_buf = kmalloc(sizeof(u32), GFP_KERNEL);", it seems like
buffer size of vendor_req_buf is only 4 bytes. Is it enough space to do
memcpy()? Also, why not just a local variable ?
> + }
> +
> + ret = usb_control_msg(udev, pipe, RTW89_USB_VENQT, reqtype,
> + value, index, rtwusb->vendor_req_buf,
> + len, 500);
> +
> + if (ret == len) { /* Success */
> + atomic_set(&rtwusb->continual_io_error, 0);
> +
> + if (reqtype == RTW89_USB_VENQT_READ)
> + memcpy(data, rtwusb->vendor_req_buf, len);
> +
> + break;
> + }
> +
> + if (ret == -ESHUTDOWN || ret == -ENODEV)
> + set_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags);
> + else if (ret < 0)
> + rtw89_warn(rtwdev,
> + "usb %s%u 0x%x fail ret=%d value=0x%x attempt=%d\n",
> + reqtype == RTW89_USB_VENQT_READ ? "read" : "write",
> + len * 8, addr, ret,
> + le32_to_cpup(rtwusb->vendor_req_buf),
> + attempt);
> + else if (ret > 0 && reqtype == RTW89_USB_VENQT_READ)
> + memcpy(data, rtwusb->vendor_req_buf, len);
> +
> + if (atomic_inc_return(&rtwusb->continual_io_error) > 4) {
> + set_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags);
> + break;
> + }
> + }
> +
> + mutex_unlock(&rtwusb->vendor_req_mutex);
> +}
> +
> +static u32 rtw89_usb_read_cmac(struct rtw89_dev *rtwdev, u32 addr)
> +{
> + u32 addr32, val32, shift;
> + __le32 data = 0;
> + int count;
> +
> + addr32 = addr & ~0x3;
> + shift = (addr & 0x3) * 8;
> +
> + for (count = 0; ; count++) {
> + rtw89_usb_vendorreq(rtwdev, addr32, &data, 4,
> + RTW89_USB_VENQT_READ);
> +
> + val32 = le32_to_cpu(data);
> +
no empty line between assignment and immediate checking.
> + if (val32 != RTW89_R32_DEAD)
> + break;
> +
> + if (count >= MAC_REG_POOL_COUNT) {
> + rtw89_warn(rtwdev, "%s: addr %#x = %#x\n",
> + __func__, addr32, val32);
> + val32 = RTW89_R32_DEAD;
> + break;
> + }
> +
> + rtw89_write32(rtwdev, R_AX_CK_EN, B_AX_CMAC_ALLCKEN);
> + }
> +
> + return val32 >> shift;
> +}
[...]
> +static void rtw89_usb_write_port_complete(struct urb *urb)
> +{
> + struct rtw89_usb_tx_ctrl_block *txcb = urb->context;
> + struct rtw89_dev *rtwdev = txcb->rtwdev;
> + struct ieee80211_tx_info *info;
> + struct sk_buff *skb;
> +
> + while (true) {
Is it possible that more than one skb are in txcb->tx_ack_queue?
With USB aggregation afterword, it will become possible?
> + skb = skb_dequeue(&txcb->tx_ack_queue);
> + if (!skb)
> + break;
> +
> + info = IEEE80211_SKB_CB(skb);
> + ieee80211_tx_info_clear_status(info);
> +
> + if (urb->status == 0) {
> + if (info->flags & IEEE80211_TX_CTL_NO_ACK)
> + info->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED;
> + else
> + info->flags |= IEEE80211_TX_STAT_ACK;
> + }
> +
> + ieee80211_tx_status_irqsafe(rtwdev->hw, skb);
> + }
> +
> + switch (urb->status) {
> + case 0:
> + case -EPIPE:
> + case -EPROTO:
> + case -EINPROGRESS:
> + case -ENOENT:
> + case -ECONNRESET:
> + break;
> + default:
> + set_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags);
> + break;
> + }
> +
> + kfree(txcb);
> + usb_free_urb(urb);
> +}
> +
[...]
> +
> +static void rtw89_usb_read_port_complete(struct urb *urb);
Move to beginning of this file.
> +
> +static void rtw89_usb_rx_resubmit(struct rtw89_usb *rtwusb,
> + struct rtw89_usb_rx_ctrl_block *rxcb,
> + gfp_t gfp)
> +{
> + struct rtw89_dev *rtwdev = rtwusb->rtwdev;
> + struct sk_buff *rx_skb;
> + int error;
Why not 'int ret' in commom?
[...]
> +
> +static void rtw89_usb_cancel_rx_bufs(struct rtw89_usb *rtwusb)
> +{
> + struct rtw89_usb_rx_ctrl_block *rxcb;
> + int i;
> +
> + for (i = 0; i < RTW89_USB_RXCB_NUM; i++) {
> + rxcb = &rtwusb->rx_cb[i];
> + usb_kill_urb(rxcb->rx_urb);
rtw89_usb_disconnect() calls rtw89_usb_cancel_rx_bufs() and then call
rtw89_usb_free_rx_bufs(). Is it a problem that usb_kill_urb() is called twice?
> + }
> +}
> +
> +static void rtw89_usb_free_rx_bufs(struct rtw89_usb *rtwusb)
> +{
> + struct rtw89_usb_rx_ctrl_block *rxcb;
> + int i;
> +
> + for (i = 0; i < RTW89_USB_RXCB_NUM; i++) {
> + rxcb = &rtwusb->rx_cb[i];
> +
> + usb_kill_urb(rxcb->rx_urb);
> + usb_free_urb(rxcb->rx_urb);
> + }
> +}
> +
[...]
> +
> +static int rtw89_usb_ops_mac_lv1_rcvy(struct rtw89_dev *rtwdev,
> + enum rtw89_lv1_rcvy_step step)
> +{
> + u32 reg, mask;
> +
> + switch (rtwdev->chip->chip_id) {
> + case RTL8851B:
> + case RTL8852A:
> + case RTL8852B:
> + reg = R_AX_USB_WLAN0_1;
> + mask = B_AX_USBRX_RST | B_AX_USBTX_RST;
> + break;
> + case RTL8852C:
> + reg = R_AX_USB_WLAN0_1_V1;
> + mask = B_AX_USBRX_RST_V1 | B_AX_USBTX_RST_V1;
> + break;
> + default:
> + rtw89_err(rtwdev, "%s: fix me\n", __func__);
> + return -EOPNOTSUPP;
> + }
> +
> + switch (step) {
> + case RTW89_LV1_RCVY_STEP_1:
> + rtw89_write32_set(rtwdev, reg, mask);
> +
> + msleep(30);
> +
maybe no need this empty line.
> + break;
> + case RTW89_LV1_RCVY_STEP_2:
> + rtw89_write32_clr(rtwdev, reg, mask);
> +
no need this empty line.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
[...]
> +static int rtw89_usb_intf_init(struct rtw89_dev *rtwdev,
> + struct usb_interface *intf)
> +{
> + struct rtw89_usb *rtwusb = rtw89_get_usb_priv(rtwdev);
> + int ret;
> +
> + ret = rtw89_usb_parse(rtwdev, intf);
> + if (ret)
> + return ret;
> +
> + rtwusb->vendor_req_buf = kmalloc(sizeof(u32), GFP_KERNEL);
I mentioned this allocation before.
> + if (!rtwusb->vendor_req_buf)
> + return -ENOMEM;
> +
> + rtwusb->udev = usb_get_dev(interface_to_usbdev(intf));
> +
> + usb_set_intfdata(intf, rtwdev->hw);
> +
> + SET_IEEE80211_DEV(rtwdev->hw, &intf->dev);
> +
> + mutex_init(&rtwusb->vendor_req_mutex);
> +
> + return 0;
> +}
> +
[...]
> diff --git a/drivers/net/wireless/realtek/rtw89/usb.h b/drivers/net/wireless/realtek/rtw89/usb.h
> new file mode 100644
> index 000000000000..86caae1b9d0b
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtw89/usb.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
> +/* Copyright(c) 2025 Realtek Corporation
> + */
> +
> +#ifndef __RTW89_USB_H__
> +#define __RTW89_USB_H__
> +
[...]
> +
> +static inline struct rtw89_usb *rtw89_get_usb_priv(struct rtw89_dev *rtwdev)
Not sure if it is worth of this wrapper. Or just rtw89_usb_priv()?
> +{
> + return (struct rtw89_usb *)rtwdev->priv;
> +}
> +
> +int rtw89_usb_probe(struct usb_interface *intf,
> + const struct usb_device_id *id);
> +void rtw89_usb_disconnect(struct usb_interface *intf);
> +
> +#endif
> --
> 2.49.0
