On Wed, 2022-05-18 at 10:23 +0200, Sascha Hauer wrote: > Add the common bits and pieces to add USB support to the RTW88 driver. > This is based on https://github.com/ulli-kroll/rtw88-usb.git which > itself is first written by Neo Jou. > > Signed-off-by: neo_jou <neo_jou@xxxxxxxxxxx> > Signed-off-by: Hans Ulli Kroll <linux@xxxxxxxxxxxxx> > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > --- > drivers/net/wireless/realtek/rtw88/Kconfig | 3 + > drivers/net/wireless/realtek/rtw88/Makefile | 2 + > drivers/net/wireless/realtek/rtw88/mac.c | 3 + > drivers/net/wireless/realtek/rtw88/main.c | 5 + > drivers/net/wireless/realtek/rtw88/main.h | 4 + > drivers/net/wireless/realtek/rtw88/reg.h | 1 + > drivers/net/wireless/realtek/rtw88/tx.h | 31 + > drivers/net/wireless/realtek/rtw88/usb.c | 1051 +++++++++++++++++++ > drivers/net/wireless/realtek/rtw88/usb.h | 109 ++ > 9 files changed, 1209 insertions(+) > create mode 100644 drivers/net/wireless/realtek/rtw88/usb.c > create mode 100644 drivers/net/wireless/realtek/rtw88/usb.h > [...] > diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h > index 84ba9ec489c37..a928899030863 100644 > --- a/drivers/net/wireless/realtek/rtw88/reg.h > +++ b/drivers/net/wireless/realtek/rtw88/reg.h > @@ -184,6 +184,7 @@ > #define BIT_TXDMA_VIQ_MAP(x) \ ^^^^^^^ replace 8 spaces by one tab > (((x) & BIT_MASK_TXDMA_VIQ_MAP) << BIT_SHIFT_TXDMA_VIQ_MAP) > #define REG_TXDMA_PQ_MAP 0x010C > +#define BIT_RXDMA_ARBBW_EN BIT(0) > #define BIT_SHIFT_TXDMA_BEQ_MAP 8 > #define BIT_MASK_TXDMA_BEQ_MAP 0x3 > #define BIT_TXDMA_BEQ_MAP(x) \ ^^^^^ use tab > diff --git a/drivers/net/wireless/realtek/rtw88/tx.h b/drivers/net/wireless/realtek/rtw88/tx.h > index 56371eff9f7ff..c02d7a15895c6 100644 > --- a/drivers/net/wireless/realtek/rtw88/tx.h > +++ b/drivers/net/wireless/realtek/rtw88/tx.h > @@ -67,6 +67,14 @@ > le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, BIT(15)) > #define SET_TX_DESC_BT_NULL(txdesc, value) \ > le32p_replace_bits((__le32 *)(txdesc) + 0x02, value, BIT(23)) > +#define SET_TX_DESC_TXDESC_CHECKSUM(txdesc, value) \ > + le32p_replace_bits((__le32 *)(txdesc) + 0x07, value, GENMASK(15, 0)) > +#define SET_TX_DESC_DMA_TXAGG_NUM(txdesc, value) \ > + le32p_replace_bits((__le32 *)(txdesc) + 0x07, value, GENMASK(31, 24)) > +#define GET_TX_DESC_PKT_OFFSET(txdesc) \ > + le32_get_bits(*((__le32 *)(txdesc) + 0x01), GENMASK(28, 24)) > +#define GET_TX_DESC_QSEL(txdesc) \ ^^^^ use tab I think you can run ./script/checkpatch.pl to find out these coding style issues. > + le32_get_bits(*((__le32 *)(txdesc) + 0x01), GENMASK(12, 8)) > > enum rtw_tx_desc_queue_select { > TX_DESC_QSEL_TID0 = 0, > @@ -119,4 +127,27 @@ rtw_tx_write_data_h2c_get(struct rtw_dev *rtwdev, > struct rtw_tx_pkt_info *pkt_info, > u8 *buf, u32 size); > > [...] > diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c > new file mode 100644 > index 0000000000000..7641ea6f6ad1a > --- /dev/null > +++ b/drivers/net/wireless/realtek/rtw88/usb.c > @@ -0,0 +1,1051 @@ > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > +/* Copyright(c) 2018-2019 Realtek Corporation > + */ > + > +#include <linux/module.h> > +#include <linux/usb.h> > +#include <linux/mutex.h> > +#include "main.h" > +#include "debug.h" > +#include "reg.h" > +#include "tx.h" > +#include "rx.h" > +#include "fw.h" > +#include "ps.h" > +#include "usb.h" > + > +#define RTW_USB_MAX_RXQ_LEN 128 > + > +struct rtw_usb_txcb { > + struct rtw_dev *rtwdev; > + struct sk_buff_head tx_ack_queue; > +}; > + > +static void rtw_usb_fill_tx_checksum(struct rtw_usb *rtwusb, > + struct sk_buff *skb, int agg_num) > +{ > + struct rtw_dev *rtwdev = rtwusb->rtwdev; > + struct rtw_tx_pkt_info pkt_info; > + > + SET_TX_DESC_DMA_TXAGG_NUM(skb->data, agg_num); > + pkt_info.pkt_offset = GET_TX_DESC_PKT_OFFSET(skb->data); > + rtw_tx_fill_txdesc_checksum(rtwdev, &pkt_info, skb->data); > +} > + > +static void usbctrl_async_callback(struct urb *urb) > +{ > + /* free dr */ > + kfree(urb->setup_packet); > + /* free databuf */ > + kfree(urb->transfer_buffer); > +} > + > +static int usbctrl_vendorreq_async_write(struct usb_device *udev, u8 request, > + u16 value, u16 index, void *pdata, > + u16 len) > +{ > + int rc; Normally, we use 'ret' as return code instead. > + unsigned int pipe; > + u8 reqtype; > + struct usb_ctrlrequest *dr; > + struct urb *urb; > + const u16 databuf_maxlen = RTW_USB_VENQT_MAX_BUF_SIZE; > + u8 *databuf; declare in reverse X'mas tree: const u16 databuf_maxlen = RTW_USB_VENQT_MAX_BUF_SIZE; struct usb_ctrlrequest *dr; unsigned int pipe; struct urb *urb; u8 *databuf; int ret; > + > + if (WARN_ON_ONCE(len > databuf_maxlen)) > + len = databuf_maxlen; > + > + pipe = usb_sndctrlpipe(udev, 0); /* write_out */ > + reqtype = RTW_USB_CMD_WRITE; > + > + dr = kzalloc(sizeof(*dr), GFP_ATOMIC); > + if (!dr) > + return -ENOMEM; > + > + databuf = kmemdup(pdata, len, GFP_ATOMIC); > + if (!databuf) { > + kfree(dr); > + return -ENOMEM; > + } > + > + urb = usb_alloc_urb(0, GFP_ATOMIC); > + if (!urb) { > + kfree(databuf); > + kfree(dr); > + return -ENOMEM; > + } > + > + dr->bRequestType = reqtype; > + dr->bRequest = request; > + dr->wValue = cpu_to_le16(value); > + dr->wIndex = cpu_to_le16(index); > + dr->wLength = cpu_to_le16(len); > + > + usb_fill_control_urb(urb, udev, pipe, > + (unsigned char *)dr, databuf, len, > + usbctrl_async_callback, NULL); > + rc = usb_submit_urb(urb, GFP_ATOMIC); > + if (rc < 0) { > + kfree(databuf); > + kfree(dr); > + } > + > + usb_free_urb(urb); > + > + return rc; > +} > + > +static u32 rtw_usb_read_sync(struct rtw_dev *rtwdev, u32 addr, u16 len) > +{ > + struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev); > + struct usb_device *udev = rtwusb->udev; > + __le32 *data; > + unsigned long flags; > + int ret; > + static int count; > + > + spin_lock_irqsave(&rtwusb->usb_lock, flags); > + > + if (++rtwusb->usb_data_index >= RTW_USB_MAX_RX_COUNT) > + rtwusb->usb_data_index = 0; > + data = &rtwusb->usb_data[rtwusb->usb_data_index]; > + > + spin_unlock_irqrestore(&rtwusb->usb_lock, flags); > + > + ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), > + RTW_USB_CMD_REQ, RTW_USB_CMD_READ, addr, ^^^^ align open parenthesis (codging style) checkpatch.pl can help this. > + RTW_USB_VENQT_CMD_IDX, data, len, 1000); > + if (ret < 0 && ret != -ENODEV && count++ < 4) > + rtw_err(rtwdev, "reg 0x%x, usbctrl_vendorreq failed with %d\n", > + addr, ret); > + > + return le32_to_cpu(*data); > +} > + [...] > + > +static void rtw_usb_rx_refill_work(struct work_struct *work) > +{ > + struct rtw_usb *rtwusb = container_of(work, struct rtw_usb, rx_refill_work); > + struct rtw_dev *rtwdev = rtwusb->rtwdev; > + struct rx_usb_ctrl_block *rxcb; > + unsigned long flags; > + int error; > + > + do { > + spin_lock_irqsave(&rtwusb->rx_data_list_lock, flags); > + > + rxcb = list_first_entry_or_null(&rtwusb->rx_data_free, > + struct rx_usb_ctrl_block, list); > + > + spin_unlock_irqrestore(&rtwusb->rx_data_list_lock, flags); > + if (!rxcb) > + return; > + > + rxcb->rx_skb = alloc_skb(RTW_USB_MAX_RECVBUF_SZ, GFP_KERNEL); > + if (!rxcb->rx_skb) { > + rtw_err(rtwdev, "could not allocate rx skbuff\n"); > + return; > + } > + > + usb_fill_bulk_urb(rxcb->rx_urb, rtwusb->udev, > + usb_rcvbulkpipe(rtwusb->udev, rtwusb->pipe_in), > + rxcb->rx_skb->data, RTW_USB_MAX_RECVBUF_SZ, > + rtw_usb_read_port_complete, rxcb); > + > + spin_lock_irqsave(&rtwusb->rx_data_list_lock, flags); > + list_move(&rxcb->list, &rtwusb->rx_data_used); > + spin_unlock_irqrestore(&rtwusb->rx_data_list_lock, flags); > + > + error = usb_submit_urb(rxcb->rx_urb, GFP_KERNEL); > + if (error) { > + kfree_skb(rxcb->rx_skb); > + if (error != -ENODEV) > + rtw_err(rtwdev, "Err sending rx data urb %d\n", > + error); > + rtw_usb_rx_data_put(rtwusb, rxcb); > + > + return; > + } > + } while (true); Can we have a limit of 'for(;<limit;)' insetad of 'while (true)'? > +} > + > +static void rtw_usb_cancel_rx_bufs(struct rtw_usb *rtwusb) > +{ > + struct rx_usb_ctrl_block *rxcb; > + unsigned long flags; > + > + spin_lock_irqsave(&rtwusb->rx_data_list_lock, flags); > + > + while (true) { > + rxcb = list_first_entry_or_null(&rtwusb->rx_data_used, > + struct rx_usb_ctrl_block, list); > + > + spin_unlock_irqrestore(&rtwusb->rx_data_list_lock, flags); > + > + if (!rxcb) > + break; > + > + usb_kill_urb(rxcb->rx_urb); > + > + spin_lock_irqsave(&rtwusb->rx_data_list_lock, flags); > + list_move(&rxcb->list, &rtwusb->rx_data_free); > + } > +} The spin_lock pairs are not intuitive. Can we change this chunk to while (true) { spin_lock(); rxcb = list_first_entry_or_null(); spin_unlock() if (!rxcb) return; usb_free_urb(); spin_lock(); list_del(); spin_unlock(); } The drawback is lock/unlock twice in single loop. rtw_usb_free_rx_bufs() has similar coding. > + > +static void rtw_usb_free_rx_bufs(struct rtw_usb *rtwusb) > +{ > + struct rx_usb_ctrl_block *rxcb; > + unsigned long flags; > + > + rtw_usb_cancel_rx_bufs(rtwusb); > + > + spin_lock_irqsave(&rtwusb->rx_data_list_lock, flags); > + > + while (true) { > + rxcb = list_first_entry_or_null(&rtwusb->rx_data_free, struct rx_usb_ctrl_block, > list); > + > + spin_unlock_irqrestore(&rtwusb->rx_data_list_lock, flags); > + > + if (!rxcb) > + break; > + > + usb_free_urb(rxcb->rx_urb); > + > + spin_lock_irqsave(&rtwusb->rx_data_list_lock, flags); > + list_del(&rxcb->list); > + } > diff --git a/drivers/net/wireless/realtek/rtw88/usb.h b/drivers/net/wireless/realtek/rtw88/usb.h > new file mode 100644 > index 0000000000000..4d714372f265c > --- /dev/null > +++ b/drivers/net/wireless/realtek/rtw88/usb.h > @@ -0,0 +1,109 @@ > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */ > +/* Copyright(c) 2018-2019 Realtek Corporation > + */ > + > +#ifndef __RTW_USB_H_ > +#define __RTW_USB_H_ > + > +#define FW_8192C_START_ADDRESS 0x1000 > +#define FW_8192C_END_ADDRESS 0x5FFF ^^^^^^^^^^ use tab > + > +#define RTW_USB_MAX_RX_COUNT 100 > +#define RTW_USB_VENQT_MAX_BUF_SIZE 254 > +#define MAX_USBCTRL_VENDORREQ_TIMES 10 > + > +#define RTW_USB_CMD_READ 0xc0 > +#define RTW_USB_CMD_WRITE 0x40 > +#define RTW_USB_CMD_REQ 0x05 > + > +#define RTW_USB_VENQT_CMD_IDX 0x00 ^^^^^ use space > + > +#define RTW_USB_SUPER_SPEED_BULK_SIZE 1024 > +#define RTW_USB_HIGH_SPEED_BULK_SIZE 512 > +#define RTW_USB_FULL_SPEED_BULK_SIZE 64 > + -- Ping-Ke