Received-SPF: pass (google.com: domain of linux-wireless+bounces-4709-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <1cfe8151-0989-4853-961d-e14daf293744@gmail.com>
Date: Thu, 14 Mar 2024 00:45:47 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
Subject: Re: [PATCH 3/3] wifi: rtlwifi: Add new rtl8192du driver
To: Ping-Ke Shih <pkshih@realtek.com>,
 "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc: "Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>,
 "s.l-h@gmx.de" <s.l-h@gmx.de>, "chewitt@libreelec.tv" <chewitt@libreelec.tv>
References: <e1922019-21eb-4013-a35f-0077167e92cf@gmail.com>
 <303a7402-43f0-4f98-9aa1-b4eb660eead0@gmail.com>
 <9e5485592f792339593d0b03b86794e5e010337a.camel@realtek.com>
Content-Language: en-US
In-Reply-To: <9e5485592f792339593d0b03b86794e5e010337a.camel@realtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 13/03/2024 07:49, Ping-Ke Shih wrote:
> On Wed, 2024-03-13 at 00:23 +0200, Bitterblue Smith wrote:
>>
>> The RTL8192DU is an older Wifi 4 dual band chip. It comes in two
>> flavours: single MAC single PHY (like most Realtek Wifi 4 USB devices),
>> and dual MAC dual PHY.
>>
>> The single MAC single PHY version is 2T2R and can work either in the
>> 2.4 GHz band or the 5 GHz band.
>>
>> The dual MAC dual PHY version has two USB interfaces and appears to the
>> system as two separate 1T1R Wifi devices, one working in the 2.4 GHz
>> band, the other in the 5 GHz band.
>>
>> This was tested only with a single MAC single PHY device.
> 
> 
> Please run checkpatch.pl that reports:
>   "total: 0 errors, 11 warnings, 27 checks, 8045 lines checked"
> 
> (Sorry, I wrongly posted above result on patch 2/3)> 

I did run checkpatch, these are the things I decided to ignore. :)
I will have another look.

>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>>
>> Should I split this into multiple patches, one for each new file,
>> to make reviewing easier?
>>
>> ---
>>  drivers/net/wireless/realtek/rtlwifi/Kconfig  |   12 +
>>  drivers/net/wireless/realtek/rtlwifi/Makefile |    1 +
>>  .../realtek/rtlwifi/rtl8192du/Makefile        |   13 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/dm.c   |  788 +++++
>>  .../wireless/realtek/rtlwifi/rtl8192du/dm.h   |   10 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/fw.c   |  109 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/fw.h   |    9 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/hw.c   | 1238 +++++++
>>  .../wireless/realtek/rtlwifi/rtl8192du/hw.h   |   24 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/led.c  |   10 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/led.h  |    9 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/phy.c  | 3064 +++++++++++++++++
>>  .../wireless/realtek/rtlwifi/rtl8192du/phy.h  |   32 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/rf.c   |  236 ++
>>  .../wireless/realtek/rtlwifi/rtl8192du/rf.h   |   11 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/sw.c   |  310 ++
>>  .../wireless/realtek/rtlwifi/rtl8192du/sw.h   |   12 +
>>  .../realtek/rtlwifi/rtl8192du/table.c         | 1675 +++++++++
>>  .../realtek/rtlwifi/rtl8192du/table.h         |   30 +
>>  .../wireless/realtek/rtlwifi/rtl8192du/trx.c  |  380 ++
>>  .../wireless/realtek/rtlwifi/rtl8192du/trx.h  |   60 +
>>  21 files changed, 8033 insertions(+)
> 
> Please split this patch to small ones. Like make dm.c/dm.h as a patch,
> hw.c/hw.h as another patch and etc. For smaller files such as led.c/led.h,
> it is fine to combine with others. 
> 

Okay.

> 
> Though you have done 8192DU in rtlwifi, I still want to know how you decide
> to support 8192DU in rtlwifi instead of rtl8xxxu. IIRC most USB users of
> Realtek WiFi use rtl8xxxu. 
> 
> Ping-Ke
> 

Originally I worked on rtl8xxxu. I didn't like how many changes
it needed to support this chip. And then the first test with
rtl8xxxu produced no scan results. At that point I gave up and
modified rtl8192de instead. (The first test with the modified
rtl8192de also produced no scan results... but rtlwifi has a lot
of debugging output, which helped.)