wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

[Martin Blumenstingl]

Hello Ping-Ke,

as a next step I want to add support for RTW_WCPU_11N SDIO cards to
the rtw88 driver.
Currently only one RTW_WCPU_11N chip is supported: RTL8723D by the
rtw88 (PCIe and USB HCIs).

My first question is very hopefully a simple one:
It seems that RTL8723DS has two SDIO IDs: 0xd723 and 0xd724.
Do these have the same name or is there some kind of "revision" (like
rev 2, revision B, ...) internally?

My second question is more abstract:
Based on my understanding of the vendor drivers for RTL8723BS and
RTL8723DS both seem very close in terms of registers. initialization
sequence, ...
So I am hoping that it's possible to add RTL8723BS support to the rtw88 driver.
There's two main differences that I found so far:
- rtw_dump_hw_feature() doesn't work because REG_C2HEVT has an
incorrect value (C2H_HW_FEATURE_REPORT is expected but it still
contains C2H_HW_FEATURE_DUMP). It seems that this is a firmware issue.
- The whole ltecoex support (my understanding is: this is part of the
Bluetooth 4.2 spec but RTL8723BS was originally developed for the
Bluetooth 4.0 spec) seems missing in terms of all required registers.
I'm not sure if this is a hardware or firmware topic.

Have you considered adding RTW8723B support to the rtw88 driver?
Do you have some initial suggestions on what would be needed to do so
(is my list from above complete, what do do about these points, ...)?

There's a huge number of boards with RTL8723BS out there and I think
they could benefit from the rtw88 driver (and so we can finally get
rid of drivers/staging/rtl8723bs/).


Best regards,
Martin

RE: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

[Ping-Ke Shih]

Hi Martin,

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Wednesday, May 17, 2023 12:40 AM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Larry Finger <Larry.Finger@lwfinger.net>; tony0620emma@gmail.com; jernej.skrabec@gmail.com
> Subject: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS
> 
> Hello Ping-Ke,
> 
> as a next step I want to add support for RTW_WCPU_11N SDIO cards to
> the rtw88 driver.
> Currently only one RTW_WCPU_11N chip is supported: RTL8723D by the
> rtw88 (PCIe and USB HCIs).

Originally, we intended to add 8723D to rtlwifi, because it is a 802.11n chip
and very similar to 8723BE. However, its PCIe HCI is changed to use circular
ring instead of own bit, like other chips implemented in rtw88.

> 
> My first question is very hopefully a simple one:
> It seems that RTL8723DS has two SDIO IDs: 0xd723 and 0xd724.
> Do these have the same name or is there some kind of "revision" (like
> rev 2, revision B, ...) internally?

0xd723 is dual antenna, and 0xd724 is single antenna. Since 8723D is a
1x1 WiFi BT combo chip, 0xd723 can have a dedicated antenna for BT.
The main difference will be BT-coex code.

> 
> My second question is more abstract:
> Based on my understanding of the vendor drivers for RTL8723BS and
> RTL8723DS both seem very close in terms of registers. initialization
> sequence, ...
> So I am hoping that it's possible to add RTL8723BS support to the rtw88 driver.
> There's two main differences that I found so far:
> - rtw_dump_hw_feature() doesn't work because REG_C2HEVT has an
> incorrect value (C2H_HW_FEATURE_REPORT is expected but it still
> contains C2H_HW_FEATURE_DUMP). It seems that this is a firmware issue.

Not an issue. At that moment of developing 8723B, applications are simple, so
we don't read feature from firmware, so just ignore this for 8723B.

> - The whole ltecoex support (my understanding is: this is part of the
> Bluetooth 4.2 spec but RTL8723BS was originally developed for the
> Bluetooth 4.0 spec) seems missing in terms of all required registers.
> I'm not sure if this is a hardware or firmware topic.

Seeing BT-coex of rtlwifi or vendor driver, you can find each chip has
its own BT-coex files of 1/2 antenna. When developing rtw88, we try to
integrate them together. So, you might take time to compare BT-coex code
of vendor driver between 8723B and 8723D, and then implement their
difference. I suggest you can pending this work as the last step.

> 
> Have you considered adding RTW8723B support to the rtw88 driver?
> Do you have some initial suggestions on what would be needed to do so
> (is my list from above complete, what do do about these points, ...)?

As my comment above, 8723BE use different PCIe design, and rtlwifi
has been supported it, so I don't have plan to support 8723BE in rtw88.
But, to add 8723BS to rtw88 seems workable. 

To add 8723B, you can make a copy from 8723D and change settings along
with vendor driver. The main things you need to review are:
1. PCI probe when plugging wifi card
2. hardware initialization when interface up
3. RF calibration when starting connection (you may put this work later)
4. connection setting after connected
5. BT-coex you have mentioned 


To make rtlwifi support SDIO is another way. It seems like add a HCI is
easier than an new chip, and we can have a simple support list of rtlwifi
and rtw88 eventually, like

        rtlwifi       rtw88
8723BE    o
8723BS    o?
8723DE                  o
8723DS                  o

> 
> There's a huge number of boards with RTL8723BS out there and I think
> they could benefit from the rtw88 driver (and so we can finally get
> rid of drivers/staging/rtl8723bs/).

Agree. That will be helpful for Linux users.

Ping-Ke

Re: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

[Martin Blumenstingl]

Hi Ping-Ke,

On Wed, May 17, 2023 at 4:06 AM Ping-Ke Shih <pkshih@realtek.com> wrote:
>
> Hi Martin,
>
> > -----Original Message-----
> > From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > Sent: Wednesday, May 17, 2023 12:40 AM
> > To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> > Cc: Larry Finger <Larry.Finger@lwfinger.net>; tony0620emma@gmail.com; jernej.skrabec@gmail.com
> > Subject: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS
> >
> > Hello Ping-Ke,
> >
> > as a next step I want to add support for RTW_WCPU_11N SDIO cards to
> > the rtw88 driver.
> > Currently only one RTW_WCPU_11N chip is supported: RTL8723D by the
> > rtw88 (PCIe and USB HCIs).
>
> Originally, we intended to add 8723D to rtlwifi, because it is a 802.11n chip
> and very similar to 8723BE. However, its PCIe HCI is changed to use circular
> ring instead of own bit, like other chips implemented in rtw88.
As far as I can tell such differences are not present in the SDIO HCI

> >
> > My first question is very hopefully a simple one:
> > It seems that RTL8723DS has two SDIO IDs: 0xd723 and 0xd724.
> > Do these have the same name or is there some kind of "revision" (like
> > rev 2, revision B, ...) internally?
>
> 0xd723 is dual antenna, and 0xd724 is single antenna. Since 8723D is a
> 1x1 WiFi BT combo chip, 0xd723 can have a dedicated antenna for BT.
> The main difference will be BT-coex code.
Awesome, thanks for the explanation. I sent patches to add RTL8723DS
support to the rtw88 driver.

> >
> > My second question is more abstract:
> > Based on my understanding of the vendor drivers for RTL8723BS and
> > RTL8723DS both seem very close in terms of registers. initialization
> > sequence, ...
> > So I am hoping that it's possible to add RTL8723BS support to the rtw88 driver.
> > There's two main differences that I found so far:
> > - rtw_dump_hw_feature() doesn't work because REG_C2HEVT has an
> > incorrect value (C2H_HW_FEATURE_REPORT is expected but it still
> > contains C2H_HW_FEATURE_DUMP). It seems that this is a firmware issue.
>
> Not an issue. At that moment of developing 8723B, applications are simple, so
> we don't read feature from firmware, so just ignore this for 8723B.
Jernej also suggested this as a first step. So I guess it's the way to go.

[...]
> > Have you considered adding RTW8723B support to the rtw88 driver?
> > Do you have some initial suggestions on what would be needed to do so
> > (is my list from above complete, what do do about these points, ...)?
>
> As my comment above, 8723BE use different PCIe design, and rtlwifi
> has been supported it, so I don't have plan to support 8723BE in rtw88.
> But, to add 8723BS to rtw88 seems workable.
>
> To add 8723B, you can make a copy from 8723D and change settings along
> with vendor driver. The main things you need to review are:
> 1. PCI probe when plugging wifi card
> 2. hardware initialization when interface up
> 3. RF calibration when starting connection (you may put this work later)
> 4. connection setting after connected
> 5. BT-coex you have mentioned
>
>
> To make rtlwifi support SDIO is another way. It seems like add a HCI is
> easier than an new chip, and we can have a simple support list of rtlwifi
> and rtw88 eventually, like
>
>         rtlwifi       rtw88
> 8723BE    o
> 8723BS    o?
> 8723DE                  o
> 8723DS                  o
Per chip support matrix is:
8723BE: rtlwifi
8723BU: rtl8xxxu
8723BS: staging driver which I'd like to replace
8723DE/U/S: rtw88

HCI support matrix:
rtlwifi: PCIe, USB
rtl8xxxu: USB
rtw88: PCIe, USB and SDIO

So it seems you are right: I should consider whether adding RTL8723BS
support to rtw88 is the right choice.


Thank you!
Martin

Re: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

[Jernej Škrabec]

Dne četrtek, 18. maj 2023 ob 19:00:14 CEST je Martin Blumenstingl napisal(a):
> Hi Ping-Ke,
> 
> On Wed, May 17, 2023 at 4:06 AM Ping-Ke Shih <pkshih@realtek.com> wrote:
> >
> > Hi Martin,
> >
> > > -----Original Message-----
> > > From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > > Sent: Wednesday, May 17, 2023 12:40 AM
> > > To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> > > Cc: Larry Finger <Larry.Finger@lwfinger.net>; tony0620emma@gmail.com; jernej.skrabec@gmail.com
> > > Subject: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS
> > >
> > > Hello Ping-Ke,
> > >
> > > as a next step I want to add support for RTW_WCPU_11N SDIO cards to
> > > the rtw88 driver.
> > > Currently only one RTW_WCPU_11N chip is supported: RTL8723D by the
> > > rtw88 (PCIe and USB HCIs).
> >
> > Originally, we intended to add 8723D to rtlwifi, because it is a 802.11n chip
> > and very similar to 8723BE. However, its PCIe HCI is changed to use circular
> > ring instead of own bit, like other chips implemented in rtw88.
> As far as I can tell such differences are not present in the SDIO HCI
> 
> > >
> > > My first question is very hopefully a simple one:
> > > It seems that RTL8723DS has two SDIO IDs: 0xd723 and 0xd724.
> > > Do these have the same name or is there some kind of "revision" (like
> > > rev 2, revision B, ...) internally?
> >
> > 0xd723 is dual antenna, and 0xd724 is single antenna. Since 8723D is a
> > 1x1 WiFi BT combo chip, 0xd723 can have a dedicated antenna for BT.
> > The main difference will be BT-coex code.
> Awesome, thanks for the explanation. I sent patches to add RTL8723DS
> support to the rtw88 driver.
> 
> > >
> > > My second question is more abstract:
> > > Based on my understanding of the vendor drivers for RTL8723BS and
> > > RTL8723DS both seem very close in terms of registers. initialization
> > > sequence, ...
> > > So I am hoping that it's possible to add RTL8723BS support to the rtw88 driver.
> > > There's two main differences that I found so far:
> > > - rtw_dump_hw_feature() doesn't work because REG_C2HEVT has an
> > > incorrect value (C2H_HW_FEATURE_REPORT is expected but it still
> > > contains C2H_HW_FEATURE_DUMP). It seems that this is a firmware issue.
> >
> > Not an issue. At that moment of developing 8723B, applications are simple, so
> > we don't read feature from firmware, so just ignore this for 8723B.
> Jernej also suggested this as a first step. So I guess it's the way to go.
> 
> [...]
> > > Have you considered adding RTW8723B support to the rtw88 driver?
> > > Do you have some initial suggestions on what would be needed to do so
> > > (is my list from above complete, what do do about these points, ...)?
> >
> > As my comment above, 8723BE use different PCIe design, and rtlwifi
> > has been supported it, so I don't have plan to support 8723BE in rtw88.
> > But, to add 8723BS to rtw88 seems workable.
> >
> > To add 8723B, you can make a copy from 8723D and change settings along
> > with vendor driver. The main things you need to review are:
> > 1. PCI probe when plugging wifi card
> > 2. hardware initialization when interface up
> > 3. RF calibration when starting connection (you may put this work later)
> > 4. connection setting after connected
> > 5. BT-coex you have mentioned
> >
> >
> > To make rtlwifi support SDIO is another way. It seems like add a HCI is
> > easier than an new chip, and we can have a simple support list of rtlwifi
> > and rtw88 eventually, like
> >
> >         rtlwifi       rtw88
> > 8723BE    o
> > 8723BS    o?
> > 8723DE                  o
> > 8723DS                  o
> Per chip support matrix is:
> 8723BE: rtlwifi
> 8723BU: rtl8xxxu
> 8723BS: staging driver which I'd like to replace
> 8723DE/U/S: rtw88
> 
> HCI support matrix:
> rtlwifi: PCIe, USB
> rtl8xxxu: USB
> rtw88: PCIe, USB and SDIO
> 
> So it seems you are right: I should consider whether adding RTL8723BS
> support to rtw88 is the right choice.

While rtw88 might not be best choice by above matrix, it seems to be most
featureful, stable and at least from what I saw, easiest to add new variants.

I'll do a bit of experiment with rtw88 to see how hard it would be...

Best regards,
Jernej

Re: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

[Larry Finger]

On 5/18/23 12:13, Jernej Škrabec wrote:
> Dne četrtek, 18. maj 2023 ob 19:00:14 CEST je Martin Blumenstingl napisal(a):
>> Hi Ping-Ke,
>>
>> On Wed, May 17, 2023 at 4:06 AM Ping-Ke Shih <pkshih@realtek.com> wrote:
>>>
>>> Hi Martin,
>>>
>>>> -----Original Message-----
>>>> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>>> Sent: Wednesday, May 17, 2023 12:40 AM
>>>> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
>>>> Cc: Larry Finger <Larry.Finger@lwfinger.net>; tony0620emma@gmail.com; jernej.skrabec@gmail.com
>>>> Subject: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS
>>>>
>>>> Hello Ping-Ke,
>>>>
>>>> as a next step I want to add support for RTW_WCPU_11N SDIO cards to
>>>> the rtw88 driver.
>>>> Currently only one RTW_WCPU_11N chip is supported: RTL8723D by the
>>>> rtw88 (PCIe and USB HCIs).
>>>
>>> Originally, we intended to add 8723D to rtlwifi, because it is a 802.11n chip
>>> and very similar to 8723BE. However, its PCIe HCI is changed to use circular
>>> ring instead of own bit, like other chips implemented in rtw88.
>> As far as I can tell such differences are not present in the SDIO HCI
>>
>>>>
>>>> My first question is very hopefully a simple one:
>>>> It seems that RTL8723DS has two SDIO IDs: 0xd723 and 0xd724.
>>>> Do these have the same name or is there some kind of "revision" (like
>>>> rev 2, revision B, ...) internally?
>>>
>>> 0xd723 is dual antenna, and 0xd724 is single antenna. Since 8723D is a
>>> 1x1 WiFi BT combo chip, 0xd723 can have a dedicated antenna for BT.
>>> The main difference will be BT-coex code.
>> Awesome, thanks for the explanation. I sent patches to add RTL8723DS
>> support to the rtw88 driver.
>>
>>>>
>>>> My second question is more abstract:
>>>> Based on my understanding of the vendor drivers for RTL8723BS and
>>>> RTL8723DS both seem very close in terms of registers. initialization
>>>> sequence, ...
>>>> So I am hoping that it's possible to add RTL8723BS support to the rtw88 driver.
>>>> There's two main differences that I found so far:
>>>> - rtw_dump_hw_feature() doesn't work because REG_C2HEVT has an
>>>> incorrect value (C2H_HW_FEATURE_REPORT is expected but it still
>>>> contains C2H_HW_FEATURE_DUMP). It seems that this is a firmware issue.
>>>
>>> Not an issue. At that moment of developing 8723B, applications are simple, so
>>> we don't read feature from firmware, so just ignore this for 8723B.
>> Jernej also suggested this as a first step. So I guess it's the way to go.
>>
>> [...]
>>>> Have you considered adding RTW8723B support to the rtw88 driver?
>>>> Do you have some initial suggestions on what would be needed to do so
>>>> (is my list from above complete, what do do about these points, ...)?
>>>
>>> As my comment above, 8723BE use different PCIe design, and rtlwifi
>>> has been supported it, so I don't have plan to support 8723BE in rtw88.
>>> But, to add 8723BS to rtw88 seems workable.
>>>
>>> To add 8723B, you can make a copy from 8723D and change settings along
>>> with vendor driver. The main things you need to review are:
>>> 1. PCI probe when plugging wifi card
>>> 2. hardware initialization when interface up
>>> 3. RF calibration when starting connection (you may put this work later)
>>> 4. connection setting after connected
>>> 5. BT-coex you have mentioned
>>>
>>>
>>> To make rtlwifi support SDIO is another way. It seems like add a HCI is
>>> easier than an new chip, and we can have a simple support list of rtlwifi
>>> and rtw88 eventually, like
>>>
>>>          rtlwifi       rtw88
>>> 8723BE    o
>>> 8723BS    o?
>>> 8723DE                  o
>>> 8723DS                  o
>> Per chip support matrix is:
>> 8723BE: rtlwifi
>> 8723BU: rtl8xxxu
>> 8723BS: staging driver which I'd like to replace
>> 8723DE/U/S: rtw88
>>
>> HCI support matrix:
>> rtlwifi: PCIe, USB
>> rtl8xxxu: USB
>> rtw88: PCIe, USB and SDIO
>>
>> So it seems you are right: I should consider whether adding RTL8723BS
>> support to rtw88 is the right choice.
> 
> While rtw88 might not be best choice by above matrix, it seems to be most
> featureful, stable and at least from what I saw, easiest to add new variants.
> 
> I'll do a bit of experiment with rtw88 to see how hard it would be...
> 
> Best regards,
> Jernej

Now that RTL8192CU is being handled, the USB part of rtlwifi is being considered 
for deletion. If you decide in terms of adding RTL8723BS to rtlwifi, please let 
me know.

Larry

Re: wifi: rtw88: questions about adding support for RTL8723DS/RTL8723BS

[Kalle Valo]

Larry Finger <Larry.Finger@lwfinger.net> writes:

>>>> To make rtlwifi support SDIO is another way. It seems like add a HCI is
>>>> easier than an new chip, and we can have a simple support list of rtlwifi
>>>> and rtw88 eventually, like
>>>>
>>>>          rtlwifi       rtw88
>>>> 8723BE    o
>>>> 8723BS    o?
>>>> 8723DE                  o
>>>> 8723DS                  o
>>> Per chip support matrix is:
>>> 8723BE: rtlwifi
>>> 8723BU: rtl8xxxu
>>> 8723BS: staging driver which I'd like to replace
>>> 8723DE/U/S: rtw88
>>>
>>> HCI support matrix:
>>> rtlwifi: PCIe, USB
>>> rtl8xxxu: USB
>>> rtw88: PCIe, USB and SDIO
>>>
>>> So it seems you are right: I should consider whether adding RTL8723BS
>>> support to rtw88 is the right choice.
>>
>> While rtw88 might not be best choice by above matrix, it seems to be most
>> featureful, stable and at least from what I saw, easiest to add new variants.
>>
>> I'll do a bit of experiment with rtw88 to see how hard it would be...
>>
>> Best regards,
>> Jernej
>
> Now that RTL8192CU is being handled, the USB part of rtlwifi is being
> considered for deletion. If you decide in terms of adding RTL8723BS to
> rtlwifi, please let me know.

From higher level point of view I would prefer rtlwifi going away in the
long run and the development focus be on rtl8xxxu, rtw88 and rtw89. So
if at all possible, please avoid adding new hardware support to rtlwifi.

But of course I'm not familiar with Realtek designs so please do correct
if I'm missing something :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches