Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
> On 13/05/2025 09:12, Ping-Ke Shih wrote:
> > 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?
> >
>
> Maybe not. I just copied this from the vendor driver. How can I be
> sure only one thread runs?
For rtw89, currently all ieee80211_ops take wiphy_lock except to TX related
ops. The works forked by rtw89 use wiphy works basically. Some works still
use ieee80211 works only if they only set a simple flags or so.
Here, I would like to avoid adding unnecessary mutex at development stage,
because it is hard to remove a mutex with simple review. You can see only
one existing mutex is 'struct mutex rf_mutex'. I want to remove it, but
I'm still afraid that I miss something by review.
>
> I added this above the mutex_lock() today:
>
> if (mutex_is_locked(&rtwusb->vendor_req_mutex))
> pr_err("mutex already locked elsewhere\n");
>
> So far it hasn't printed the message.
Not sure if this function depends on lock debugging of kernel options.
By the experiments, this mutex seems to be unnecessary, right?
