Re: [PATCH v3 3/6] wifi: rtw88: 8723b: add the RTL8723B chip driver

["Luka Gejak" <luka.gejak@linux.dev>]

September 29, 2026 at 04:04, "Ping-Ke Shih" <pkshih@realtek.com mailto:pkshih@realtek.com?to=%22Ping-Ke%20Shih%22%20%3Cpkshih%40realtek.com%3E > wrote:


> 
> Luka Gejak <luka.gejak@linux.dev> wrote:
> 
> > 
> > Hi Ping-Ke,
> >  
> >  September 24, 2026 at 06:11, "Ping-Ke Shih" <pkshih@realtek.com> wrote:
> > 
> >  Luka Gejak <luka.gejak@linux.dev> wrote:
> >  
> >  Answers in the order of your mail. The MAINTAINERS point is in a
> >  separate reply.
> >  
> >  Can you also review other RTL8723B specific functions? I didn't review
> >  them one by one by v3, but I wonder why it needs specific functions,
> >  not common flow. If any of them is necessary, please point out reasons.
> > 
> >  By the way, I didn't only mention these three functions. At here there
> >  are many specific functions. Please analyze them.
> >  (Honestly, I don't fully re-examinate your analysis in detail, and
> >  believe your results.)
> >  
> >  I went through the whole file. The part that is already common flow can
> >  be listed exactly, because those ops point at shared code:
> >  
> >  power_on, power_off rtw_power_on, rtw_power_off
> >  mac_postinit rtw8723x_mac_postinit
> >  set_tx_power_index rtw8723x_set_tx_power_index
> >  false_alarm_statistics rtw8723x_false_alarm_statistics
> >  read_rf, write_rf rtw_phy_read_rf_sipi,
> >  rtw_phy_write_rf_reg_sipi
> >  read_efuse rtw8723x_read_efuse, plus the hardware
> >  capability, because this chip has no
> >  hardware feature report: the firmware
> >  reports id 0xfd instead of the C2H
> >  efuse_grant rtw8723x_efuse_grant, plus the 0x6b BT
> >  power cut and output isolation write that
> >  the vendor efuse path does
> > 
> I think you have checked them. Please reconsider to rewrite them. 
> 
> As Johannes mentioned, LLM is a tool, but please not fully believe
> and rely on it. LLM can generate a lot of stuff, but I read by my
> eyes and then think and type by my hands. To understand and rework
> stuff generated by LLM is submitter's business.
> 
> > 
> > I think the better way is to assign proper rtwdev->hal.rcr per chip
> >  in rtw_core_init().
> > 
> [..]
> 
> > 
> > If you prefer, I can put the core change in a small patch before the
> >  chip series rather than in patch 3, so the chip series stays free of
> >  core changes.
> > 
> Yes, that'd be good. 
> 
> The 8723BS specific stuff should do in the kind of rewriting. 

Understood, already did while addressing Bitterblue's comments.

> 
> > 
> > Can you reuse the existing since they are the same?
> > 
> I think I can refer a common rule... 
> 
> > 
> > (Please quote the code; to reply to this, I need to switch to your
> >  patches again).
> >  On your last sentence, that the reason is hard to explain later if it is
> >  not written down: whatever we change will carry its reason in the code
> >  or in the commit message, not only in this mail. The same goes for the
> >  two things I am asking to keep, cfg_ldo25() and the RESP_SIFS writes,
> >  which will say why they are there.
> > 
> It is still hard to me to recall what I wrote at last sentence...
> 
> Let's follow Bitterblue's comments on v4, and move to v5.
> 
> I noted the copyright aren't all consistent. Check them yourself.

I noticed that too, and fixed it in v5. I will send it together with
other replys to Bitterblue's comments today.

> 
> Ping-Ke
>