DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 45E1C2098A
Subject: Re: [PATCH 01/12] rtwlan: main files
To:     Tony Chuang <yhchuang@realtek.com>,
        Stanislaw Gruszka <sgruszka@redhat.com>
Cc:     "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        Pkshih <pkshih@realtek.com>, Andy Huang <tehuang@realtek.com>
References: <1537509847-21087-1-git-send-email-yhchuang@realtek.com>
 <1537509847-21087-2-git-send-email-yhchuang@realtek.com>
 <20180927135040.GA4712@redhat.com>
 <F7CD281DE3E379468C6D07993EA72F84D08D9F56@RTITMBSVM01.realtek.com.tw>
 <20180928092918.GC8323@redhat.com>
 <F7CD281DE3E379468C6D07993EA72F84D08DB7E3@RTITMBSVM01.realtek.com.tw>
 <20181002102957.GB25116@redhat.com>
 <a54dec4e-1164-f9c5-c791-3396a239eeba@lwfinger.net>
 <F7CD281DE3E379468C6D07993EA72F84D1728978@RTITMBSVM01.realtek.com.tw>
From:   Larry Finger <Larry.Finger@lwfinger.net>
Message-ID: <4b50e06f-2fd0-9378-d283-1ca6d02f9586@lwfinger.net>
Date:   Wed, 3 Oct 2018 00:40:06 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.0
MIME-Version: 1.0
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D1728978@RTITMBSVM01.realtek.com.tw>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 10/2/18 9:57 PM, Tony Chuang wrote:
> 
> 
>> -----Original Message-----
>> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger
>> Sent: Tuesday, October 02, 2018 11:24 PM
>> To: Stanislaw Gruszka; Tony Chuang
>> Cc: kvalo@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