On Fri, 2021-04-30 at 01:10 +0000, Brian Norris wrote: > Small review note, since I was looking through the PS code due to > another bug in this patchset: > > On Thu, Apr 29, 2021 at 04:01:36PM +0800, Ping-Ke Shih wrote: > > --- /dev/null > > +++ b/drivers/net/wireless/realtek/rtw89/fw.c > > + > > +#define H2C_LPS_PARM_LEN 8 > > +int rtw89_fw_h2c_lps_parm(struct rtw89_dev *rtwdev, u8 macid) > > You're not actually using the macid param from this function. Instead, > you're implicitly passing data to this function via > rtwdev->lps_parm...except that it looks like you only set and use it > directly in this callchain, and you don't actually need to save it in > your driver structure. > > IIUC, I believe this would be clearer and less error-prone if you just > pass a 'struct rtw89_lps_parm' arg (here, and in > rtw89_fw_h2c_general_pkt()), and drop 'struct rtw89_lps_parm' from > rtwdev. > You're right. Will follow your suggestion. -- Ping-Ke
