On 26/09/2024 05:27, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>> On 25/09/2024 04:25, Ping-Ke Shih wrote:
>>> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>>>> On 23/09/2024 08:47, Ping-Ke Shih wrote:
>>>>> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>>>>>>>>>>>> +
>>>>>>>>>>>> +const struct rtw_chip_info rtw8812a_hw_spec = {
>>>>>>>>>>>
>>>>>>>>>>> Is it possible moving 8812a to individual file?
>>>>>>>>>>> Since you have rtw8812au.c and rtw8821au.c.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I think it is possible. But most of the code is common to both chips.
>>>>>>>>>> Only the IQ calibration could be moved.
>>>>>>>>>
>>>>>>>>> Yep, depend on how much IQK code echo chip has.
>>>>>>>>>
>>>>>>>>
>>>>>>>> The IQ calibration for RTL8812AU is about 700 lines.
>>>>>>>
>>>>>>> rtw8812au -----> (a) rtw8812a
>>>>>>> |
>>>>>>> v
>>>>>>> (b) rtw8821a_common (hard to give a name)
>>>>>>> ^
>>>>>>> |
>>>>>>> rtw8821au -----> (c) rtw8821a
>>>>>>>
>>>>>>> Put all common code to (b). IQK code in (a) or (c).
>>>>>>>
>>>>>>> I feel you have thought above picture already. What are problems we will encounter?
>>>>>>> Many export symbols? If so, how about below?
>>>>>>>
>>>>>>> rtw8812au -----> (1) rtw8812a
>>>>>>> +---------+
>>>>>>> +-> (2) rtw8821a_common (hard to give a name)
>>>>>>> +---------+
>>>>>>> rtw8821au -----> (3) rtw8821a
>>>>>>>
>>>>>>> Put rtw8812a_hw_spec and rtw8821a_hw_spec in (2). Only IQK code in (1) and (3)
>>>>>>> respectively, and export IQK entry only. Does it work?
>>>>>>>
>>>>>>>
>>>>>> For the name of the common module, I was thinking rtw88_88xxa.ko.
>>>>>>
>>>>>> I wonder, what is the goal? To put the code in separate kernel
>>>>>> modules, or just separate files?
>>>>>
>>>>> I would like to reduce runtime memory. As I asked, how many IQK code are different
>>>>> from them? If you have separated and compiled them, can you share size by the
>>>>> output of 'size' command?
>>>>>
>>>>
>>>> I separated the IQK code into two files (still just one module).
>>>> size says:
>>>>
>>>> text data bss dec hex filename
>>>> 7192 32 0 7224 1c38 rtw8821a-iqk.o
>>>> 12319 40 0 12359 3047 rtw8812a-iqk.o
>>>>
>>>> This is x86_64.
>>>>
>>>>>>
>>>>>> I think we can have rtw88xxa.c (common code), rtw8821a.c (IQK code,
>>>>>> rtw8821a_hw_spec, bluetooth stuff), and rtw8812a.c (IQK code,
>>>>>> rtw8812a_hw_spec, some efuse stuff, channel switching)... if these
>>>>>> three files compile into a single module, rtw88_88xxa.ko.
>>>>>>
>>>>>> If each file compiles into a module of its own, we have circular
>>>>>> dependencies: rtw8821a_hw_spec -> common code -> IQK code.
>>>>>> If *_hw_spec go in the common module, it still depends on both of
>>>>>> the other two modules, so what use is it?
>>>>>
>>>>> If we have dependency of common code -> IQK code, we can't save runtime
>>>>> memory, because common code reference to both IQK code. So I felt
>>>>> dependency of IQK code would be rtw8812au --> IQK code as above second
>>>>> diagram.
>>>>>
>>>>> But if the work is complicated and save not few runtime memory, we can
>>>>> use simple design as current did.
>>>>>
>>>>>
>>>>
>>>> The IQK code can be separated into different modules if I duplicate
>>>> rtw8821a_ops and rtw8821a_pwr_track, and rtw8821a_phy_pwrtrack takes
>>>> a pointer to the IQK function. Then your first diagram above can work.
>>>
>>> Not sure the "duplicate" you meant. If it only a struct, that would be fine.
>>> Not prefer duplicate of tables.
>>>
>>
>> Yes, it's a struct rtw_chip_ops.
>>
>>>>
>>>> The tables also take up a bit of space:
>>>>
>>>> text data bss dec hex filename
>>>> 16832 0 0 16832 41c0 rtw8821a_table.o
>>>> 21552 0 0 21552 5430 rtw8812a_table.o
>>>
>>> Good point.
>>>
>>>>
>>>> I don't know how many kilobytes is enough to make it worth
>>>> creating two more modules.
>>>
>>> I think we can list all *.o related to rtw8821a/8812a, and check the
>>> percentage to make decisions. I mean if it occupies 50%, I will prefer
>>> to have separated module. But I don't have an exact number now.
>>>
>>
>> text data bss dec hex filename
>> 12319 40 0 12359 3047 rtw8812a-iqk.o
>> 21552 0 0 21552 5430 rtw8812a_table.o
>> 7192 32 0 7224 1c38 rtw8821a-iqk.o
>> 16832 0 0 16832 41c0 rtw8821a_table.o
>> 29445 429 0 29874 74b2 rtw8821a.o
>> =========
>> 87340 total. So it's about 38% for 8812a and 27% for 8821a.
>> Maybe a bit more in the final version.
>
> chip separated(a) single one(b) increase rate(c)
> ----- ------------ ------------- ----------------
> 8812a 63,785 87,841 38%
> 8821a 53,930 87,841 63%
>
> * increase rate (c) = (b - a) / a
>
> Since increasing rate of 8821a is 63%, I feel separated case would be better.
>
>
All right.
