Re: [PATCH 01/12] rtwlan: main files

[Larry Finger]

On 10/2/18 9:57 PM, Tony Chuang wrote:


-----Original Message-----
From: Larry Finger [mailto:larry.fin...@gmail.com] On Behalf Of Larry Finger
Sent: Tuesday, October 02, 2018 11:24 PM
To: Stanislaw Gruszka; Tony Chuang
Cc: kv...@codeaurora.org; linux-wireless@vger.kernel.org; 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.

Larry