Linux wireless drivers development
 help / color / mirror / Atom feed
* [RFC] Adding RTL8189ES SDIO support to rtw88
@ 2026-11-10  4:50 igor
  2026-11-10  8:42 ` Luka Gejak
  0 siblings, 1 reply; 4+ messages in thread
From: igor @ 2026-11-10  4:50 UTC (permalink / raw)
  To: linux-wireless

Hi,

Would support for RTL8189ES SDIO devices, based on the RTL8188E T-series
(SDIO ID 024c:8179), be welcome in rtw88?

An implementation is available in lwfinger/rtw88 PR #465:
https://github.com/lwfinger/rtw88/pull/465

Following feedback there, I am contacting this list before preparing
a patch series for the kernel.

The implementation adds chip operations, register tables, firmware,
and an SDIO binding. It handles the chip's legacy TX/RX descriptors,
TX reports, host-side rate adaptation, calibration, power saving,
beacons, and reserved pages, while integrating with the existing
rtw88/mac80211 infrastructure.

All rtw88 modules have been built successfully. Hardware testing
covered STA, AP, concurrent AP+STA, WPA2 connectivity, bidirectional
traffic, multicast, scanning, rate control, power saving, calibration,
and interface/module lifecycle operations. Regression testing was
also performed on RTL8723BS.

Would this be an acceptable direction for rtw88? Are there architectural
concerns or requirements that should be addressed before submitting
the series for review?

Regards,
Igor Listopad

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] Adding RTL8189ES SDIO support to rtw88
  2026-11-10  4:50 [RFC] Adding RTL8189ES SDIO support to rtw88 igor
@ 2026-11-10  8:42 ` Luka Gejak
  2026-11-11  5:22   ` Ping-Ke Shih
  0 siblings, 1 reply; 4+ messages in thread
From: Luka Gejak @ 2026-11-10  8:42 UTC (permalink / raw)
  To: linux-wireless, igor

Hi Igor,

Thanks for asking before sending it. Ping-Ke has the
final word on this, so this is only my view after adding RTL8723BS to the
rtw88.

On November 10, 2026 5:50:38 AM GMT+01:00, igor@listopad.email wrote:

> Would support for RTL8189ES SDIO devices, based on the RTL8188E T-series
> (SDIO ID 024c:8179), be welcome in rtw88?

I think so. As far as I can see nothing in the kernel binds this SDIO ID
today. rtlwifi has RTL8188EE on PCIe and rtl8xxxu has RTL8188EU on USB,
while rtw88 already has the SDIO code and other 8051 based 802.11n chips
like RTL8703B and RTL8723B.

On my RTL8723BS series Ping-Ke said it isn't so good to maintain three
interfaces in the tree drivers, but he kept the existing ones and took the
new one. So I would expect the same split to be acceptable here.

Refusing cut I seems fine too. rtl8xxxu does the same for RTL8188EU.

[ ... ]

> The implementation adds chip operations, register tables, firmware,
> and an SDIO binding.

The firmware in the PR is byte for byte the same as
rtlwifi/rtl8188eufw.bin, which is already in the firmware repository and
which rtl8xxxu uses. So I don't think you need a new blob. You could
request that name, or add a Link: in WHENCE if Ping-Ke prefers an rtw88
path.

[ ... ]

> Would this be an acceptable direction for rtw88? Are there architectural
> concerns or requirements that should be addressed before submitting
> the series for review?

From my quick skim of the PR, I think the direction is fine. What I would
worry about is how much it changes the shared code. It needs an additional
20 callbacks in rtw_chip_ops and more than 20 checks for this chip in the
core and SDIO code. On my series Ping-Ke asked why the existing logic
didn't work for special cases like these, so I would expect each one to
need a reason.

A few examples:

rtw8188e.c casts rtwdev->priv to struct rtw_sdio, and nothing outside
sdio.c uses that struct today. Maybe those parts can become hci ops.

rtw_sdio_tx_handler() reads host->sdio_irq_pending, which as far as I can
see only the mmc core uses today. TX keeping the IRQ worker away from the
host also doesn't sound specific to this chip, so I think that wants a
fix in the common SDIO TX path.

rtw8188e_efuse_grant() and rtw8188e_false_alarm_statistics() look the same
as the rtw8723x.c helpers that RTL8723B uses. Maybe those can be shared
instead of copied.

A more thorough review will follow once the series is posted.

For the actual series, please base it on rtw-next instead of the lwfinger
tree. Several of the core files don't apply there, and the
LINUX_VERSION_CODE checks have to go. It also needs the Kconfig and
Makefile entries, the device ID in sdio_ids.h, and the fixes from your
later commits squashed into the patches they fix. The sign off should use
a real address instead of the GitHub noreply one.

You can also take a look at how the RTL8723B series went upstream for
reference, from the RFC to the version that was applied:

https://lore.kernel.org/linux-wireless/cover.1784047561.git.luka.gejak@linux.dev/
https://lore.kernel.org/linux-wireless/20261006130701.120460-1-luka.gejak@linux.dev/

Best regards,
Luka Gejak

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [RFC] Adding RTL8189ES SDIO support to rtw88
  2026-11-10  8:42 ` Luka Gejak
@ 2026-11-11  5:22   ` Ping-Ke Shih
  2026-11-11  8:20     ` igor
  0 siblings, 1 reply; 4+ messages in thread
From: Ping-Ke Shih @ 2026-11-11  5:22 UTC (permalink / raw)
  To: Luka Gejak, linux-wireless@vger.kernel.org, igor@listopad.email

Luka Gejak <luka.gejak@linux.dev> wrote:
> Hi Igor,
> 
> Thanks for asking before sending it. Ping-Ke has the
> final word on this, so this is only my view after adding RTL8723BS to the
> rtw88.
> 
> On November 10, 2026 5:50:38 AM GMT+01:00, igor@listopad.email wrote:
> 
> > Would support for RTL8189ES SDIO devices, based on the RTL8188E T-series
> > (SDIO ID 024c:8179), be welcome in rtw88?
> 
> I think so. As far as I can see nothing in the kernel binds this SDIO ID
> today. rtlwifi has RTL8188EE on PCIe and rtl8xxxu has RTL8188EU on USB,
> while rtw88 already has the SDIO code and other 8051 based 802.11n chips
> like RTL8703B and RTL8723B.

Maybe one day people can move RTL8188EE to rtw88 as well. For now,
rtw88 is only one driver supporting Realtek WiFi SDIO, so let's add
RTL8189ES to rtw88. 

> 
> On my RTL8723BS series Ping-Ke said it isn't so good to maintain three
> interfaces in the tree drivers, but he kept the existing ones and took the
> new one. So I would expect the same split to be acceptable here.
> 
> Refusing cut I seems fine too. rtl8xxxu does the same for RTL8188EU.
> 
> [ ... ]
> 
> > The implementation adds chip operations, register tables, firmware,
> > and an SDIO binding.
> 
> The firmware in the PR is byte for byte the same as
> rtlwifi/rtl8188eufw.bin, which is already in the firmware repository and
> which rtl8xxxu uses. So I don't think you need a new blob. You could
> request that name, or add a Link: in WHENCE if Ping-Ke prefers an rtw88
> path.

I prefer a Link. One day rtw88 can support RTL8188EE, and remove RTL8188EE
from rtlwifi. It'd be easier to move firmware from rtlwifi/ to rtw88/

> 
> [ ... ]
> 
> > Would this be an acceptable direction for rtw88? Are there architectural
> > concerns or requirements that should be addressed before submitting
> > the series for review?
> 
> From my quick skim of the PR, I think the direction is fine. What I would
> worry about is how much it changes the shared code. It needs an additional
> 20 callbacks in rtw_chip_ops and more than 20 checks for this chip in the
> core and SDIO code. On my series Ping-Ke asked why the existing logic
> didn't work for special cases like these, so I would expect each one to
> need a reason.
> 
> A few examples:
> 
> rtw8188e.c casts rtwdev->priv to struct rtw_sdio, and nothing outside
> sdio.c uses that struct today. Maybe those parts can become hci ops.
> 
> rtw_sdio_tx_handler() reads host->sdio_irq_pending, which as far as I can
> see only the mmc core uses today. TX keeping the IRQ worker away from the
> host also doesn't sound specific to this chip, so I think that wants a
> fix in the common SDIO TX path.
> 
> rtw8188e_efuse_grant() and rtw8188e_false_alarm_statistics() look the same
> as the rtw8723x.c helpers that RTL8723B uses. Maybe those can be shared
> instead of copied.
> 
> A more thorough review will follow once the series is posted.
> 
> For the actual series, please base it on rtw-next instead of the lwfinger
> tree. Several of the core files don't apply there, and the
> LINUX_VERSION_CODE checks have to go. It also needs the Kconfig and
> Makefile entries, the device ID in sdio_ids.h, and the fixes from your
> later commits squashed into the patches they fix. The sign off should use
> a real address instead of the GitHub noreply one.
> 
> You can also take a look at how the RTL8723B series went upstream for
> reference, from the RFC to the version that was applied:
> 
> https://lore.kernel.org/linux-wireless/cover.1784047561.git.luka.gejak@linux.dev/
> https://lore.kernel.org/linux-wireless/20261006130701.120460-1-luka.gejak@linux.dev/

Agree. Luka, Thanks for sharing these experiences.



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] Adding RTL8189ES SDIO support to rtw88
  2026-11-11  5:22   ` Ping-Ke Shih
@ 2026-11-11  8:20     ` igor
  0 siblings, 0 replies; 4+ messages in thread
From: igor @ 2026-11-11  8:20 UTC (permalink / raw)
  To: Ping-Ke Shih; +Cc: Luka Gejak, linux-wireless@vger.kernel.org

Hi Ping-Ke,

Thank you and Luka for the feedback. I am reworking the RTL8189ES support on top of rtw-next, taking your comments into account. I will send the patch series to linux-wireless when it is ready.

Best regards,
Igor

> 11 нояб. 2026 г., в 08:22, Ping-Ke Shih <pkshih@realtek.com> написал(а):
> 
> Luka Gejak <luka.gejak@linux.dev> wrote:
>> Hi Igor,
>> 
>> Thanks for asking before sending it. Ping-Ke has the
>> final word on this, so this is only my view after adding RTL8723BS to the
>> rtw88.
>> 
>> On November 10, 2026 5:50:38 AM GMT+01:00, igor@listopad.email wrote:
>> 
>>> Would support for RTL8189ES SDIO devices, based on the RTL8188E T-series
>>> (SDIO ID 024c:8179), be welcome in rtw88?
>> 
>> I think so. As far as I can see nothing in the kernel binds this SDIO ID
>> today. rtlwifi has RTL8188EE on PCIe and rtl8xxxu has RTL8188EU on USB,
>> while rtw88 already has the SDIO code and other 8051 based 802.11n chips
>> like RTL8703B and RTL8723B.
> 
> Maybe one day people can move RTL8188EE to rtw88 as well. For now,
> rtw88 is only one driver supporting Realtek WiFi SDIO, so let's add
> RTL8189ES to rtw88. 
> 
>> 
>> On my RTL8723BS series Ping-Ke said it isn't so good to maintain three
>> interfaces in the tree drivers, but he kept the existing ones and took the
>> new one. So I would expect the same split to be acceptable here.
>> 
>> Refusing cut I seems fine too. rtl8xxxu does the same for RTL8188EU.
>> 
>> [ ... ]
>> 
>>> The implementation adds chip operations, register tables, firmware,
>>> and an SDIO binding.
>> 
>> The firmware in the PR is byte for byte the same as
>> rtlwifi/rtl8188eufw.bin, which is already in the firmware repository and
>> which rtl8xxxu uses. So I don't think you need a new blob. You could
>> request that name, or add a Link: in WHENCE if Ping-Ke prefers an rtw88
>> path.
> 
> I prefer a Link. One day rtw88 can support RTL8188EE, and remove RTL8188EE
> from rtlwifi. It'd be easier to move firmware from rtlwifi/ to rtw88/
> 
>> 
>> [ ... ]
>> 
>>> Would this be an acceptable direction for rtw88? Are there architectural
>>> concerns or requirements that should be addressed before submitting
>>> the series for review?
>> 
>> From my quick skim of the PR, I think the direction is fine. What I would
>> worry about is how much it changes the shared code. It needs an additional
>> 20 callbacks in rtw_chip_ops and more than 20 checks for this chip in the
>> core and SDIO code. On my series Ping-Ke asked why the existing logic
>> didn't work for special cases like these, so I would expect each one to
>> need a reason.
>> 
>> A few examples:
>> 
>> rtw8188e.c casts rtwdev->priv to struct rtw_sdio, and nothing outside
>> sdio.c uses that struct today. Maybe those parts can become hci ops.
>> 
>> rtw_sdio_tx_handler() reads host->sdio_irq_pending, which as far as I can
>> see only the mmc core uses today. TX keeping the IRQ worker away from the
>> host also doesn't sound specific to this chip, so I think that wants a
>> fix in the common SDIO TX path.
>> 
>> rtw8188e_efuse_grant() and rtw8188e_false_alarm_statistics() look the same
>> as the rtw8723x.c helpers that RTL8723B uses. Maybe those can be shared
>> instead of copied.
>> 
>> A more thorough review will follow once the series is posted.
>> 
>> For the actual series, please base it on rtw-next instead of the lwfinger
>> tree. Several of the core files don't apply there, and the
>> LINUX_VERSION_CODE checks have to go. It also needs the Kconfig and
>> Makefile entries, the device ID in sdio_ids.h, and the fixes from your
>> later commits squashed into the patches they fix. The sign off should use
>> a real address instead of the GitHub noreply one.
>> 
>> You can also take a look at how the RTL8723B series went upstream for
>> reference, from the RFC to the version that was applied:
>> 
>> https://lore.kernel.org/linux-wireless/cover.1784047561.git.luka.gejak@linux.dev/
>> https://lore.kernel.org/linux-wireless/20261006130701.120460-1-luka.gejak@linux.dev/
> 
> Agree. Luka, Thanks for sharing these experiences.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-11  8:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10  4:50 [RFC] Adding RTL8189ES SDIO support to rtw88 igor
2026-11-10  8:42 ` Luka Gejak
2026-11-11  5:22   ` Ping-Ke Shih
2026-11-11  8:20     ` igor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox