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

[Kalle Valo <kvalo@codeaurora.org>]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> 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.

Configuration files for upstream drivers is something I would like to
avoid until we have a generic solution. That seems to come up now and
then but there does not seem to be any consensus how it should be
implemented.

-- 
Kalle Valo