Re: [PATCH v4 06/19] rtw89: add files to download and communicate with firmware

[Pkshih <pkshih@realtek.com>]

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