Larry Finger <Larry.Finger@xxxxxxxxxxxx> writes: > On 10/2/18 9:57 PM, Tony Chuang wrote: >> >> >>> -----Original Message----- >>> From: Larry Finger [mailto:larry.finger@xxxxxxxxx] On Behalf Of Larry Finger >>> Sent: Tuesday, October 02, 2018 11:24 PM >>> To: Stanislaw Gruszka; Tony Chuang >>> Cc: kvalo@xxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; Pkshih; Andy >>> Huang >>> Subject: Re: [PATCH 01/12] rtwlan: main files >>> >>> On 10/2/18 5:29 AM, Stanislaw Gruszka wrote: >>>> On Fri, Sep 28, 2018 at 11:32:41AM +0000, Tony Chuang wrote: >>>>>> if (rtw_hci_tx(rtwdev, &pkt_info, skb)) >>>>>> dev_kfree_skb_any(skb) >>>>>> >>>>>> just to remove 'return;' and out label. >>>>> >>>>> >>>>> OK, but why not use ieee80211_free_txskb, should it be better for >>> mac80211? >>>> >>>> Yes, it is better as it also do some extra thing for dropped frame. >>>> >>>>>>> OK, but I think this is needed, our tables have different forms .... >>>>>> >>>>>> Not sure if that is better solution, but could the tables be pre-prarsed >>>>>> by user-space program and then embed in the driver in ready to send >>>>>> to the hardware from ? >>>>>> >>>>>> Also there are lot of redundancy in those tables, for example: >>>>>> >>>>>> + 0x81C, 0xFF000003, >>>>>> + 0x81C, 0xF5000003, >>>>>> + 0x81C, 0xF4020003, >>>>>> + 0x81C, 0xF3040003, >>>>>> + 0x81C, 0xF2060003, >>>>>> + 0x81C, 0xF1080003, >>>>>> + 0x81C, 0xF00A0003, >>>>>> + 0x81C, 0xEF0C0003, >>>>>> + 0x81C, 0xEE0E0003, >>>>>> + 0x81C, 0xED100003, >>>>>> + 0x81C, 0xEC120003, >>>>>> + 0x81C, 0xEB140003, >>>>>> + 0x81C, 0xEA160003, >>>>>> + 0x81C, 0xE9180003, >>>>>> + 0x81C, 0xE81A0003, >>>>>> + 0x81C, 0xE71C0003, >>>>>> + 0x81C, 0xE61E0003, >>>>>> + 0x81C, 0xE5200003, >>>>>> >>>>>> 0x81C and 0003 repeats in many lines. >>>>>> >>>>>> This seems to be parse data, not that we have to write 0x81C >>>>>> register many times. Would be possible to remove the redundancy? >>>>> >>>>> >>>>> No, they cannot be removed, the sequence matters. >>>>> And it is really writing to 0x81C ... >>>>> It is really magic, I cannot believe to this, too. >>>> >>>> This is contradiction for what I asked you before, i.e. doing parsing >>>> in user space, but since we have this parsing mechanism in the driver >>>> perhaps the tables can be coded in some more compact way, for example: >>>> >>>> { prefix, suffix, len, {data} } >>>> >>>> { 0x81C, 0x0003, N , >>>> { 0xFF02 , 0xF500 , .... , 0xE520 } } >>>> >>>> The rtw8822b_table.c file is quite big. >>> >>> You might also consider having these tables as a configuration file read from >>> the firmware directory. >>> >> >> Hi Larry, >> >> Can we put the configuration file in the firmware directory? >> Should we package them into binary files? Or just put the raw data. >> >> We can test the performance for it. After we got the result, we will make a decision >> about it. And if we decide to put them in the firmware directory, will send a patch. >> For now, I think we can just leave them in the .c. > > Yes, you could put the configuration files in the firmware directory. > I would put them in binary form, not as text files. That way the size > would be smaller, and it would not be possible to alter them, > particularly if the binary file is checksummed. > > It would likely be OK if only the agc table was stored in this way. > That would take away about half of the lines in the 8822b table file. So what's the worry here? The lines of source code, binary size or what? .../net/wireless/realtek/rtw88/rtw8822b_table.c | 20783 +++++++++++++++++++ Looking at the diffstat rtw8822b_table.c seems to be 20 kLOC, IMHO it's not that bad as it's just data. But of course I might be missing something as I haven't checked patches yet. -- Kalle Valo
