On 26/05/2025 06:18, Ping-Ke Shih wrote:
> 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.
>
I checked, mutex_is_locked() works with my Arch Linux kernel.
> By the experiments, this mutex seems to be unnecessary, right?
>
Yes, it looks unnecessary.
