Linux wireless drivers development
 help / color / mirror / Atom feed
From: Ping-Ke Shih <pkshih@realtek.com>
To: Luka Gejak <luka.gejak@linux.dev>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"igor@listopad.email" <igor@listopad.email>
Subject: RE: [RFC] Adding RTL8189ES SDIO support to rtw88
Date: Wed, 11 Nov 2026 05:22:23 +0000	[thread overview]
Message-ID: <d34cd45d652f4b88a02c4795b0e1aaf2@realtek.com> (raw)
In-Reply-To: <977DC5C2-4EB7-45BD-9663-69E4094EE596@linux.dev>

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.



  reply	other threads:[~2026-11-11  5:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-11-11  8:20     ` igor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d34cd45d652f4b88a02c4795b0e1aaf2@realtek.com \
    --to=pkshih@realtek.com \
    --cc=igor@listopad.email \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luka.gejak@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox