Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2dd49e4e-6628-eb2e-1168-0cc926c4d1fb@gmail.com>
Date:   Sun, 2 Apr 2023 00:24:10 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 1/2] wifi: rtl8xxxu: Clean up some messy ifs
Content-Language: en-US
To:     Julian Calaby <julian.calaby@gmail.com>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        Jes Sorensen <Jes.Sorensen@gmail.com>,
        Ping-Ke Shih <pkshih@realtek.com>
References: <eb152b5b-fe65-3783-a3d9-71c9cb7ef9d3@gmail.com>
 <CAGRGNgUoEoN9Tm8=1LRtznbwm2KqiArtUHjXXZht0Nz1ZPunow@mail.gmail.com>
From:   Bitterblue Smith <rtl8821cerfe2@gmail.com>
In-Reply-To: <CAGRGNgUoEoN9Tm8=1LRtznbwm2KqiArtUHjXXZht0Nz1ZPunow@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 01/04/2023 07:23, Julian Calaby wrote:
> Hi Bitterblue,
> 
> On Sat, Apr 1, 2023 at 7:18 AM Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>
>> Add some new members to rtl8xxxu_fileops and use them instead of
>> checking priv->rtl_chip.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  5 ++++
>>  .../realtek/rtl8xxxu/rtl8xxxu_8188e.c         |  1 +
>>  .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  5 ++++
>>  .../realtek/rtl8xxxu/rtl8xxxu_8192e.c         |  1 +
>>  .../realtek/rtl8xxxu/rtl8xxxu_8710b.c         |  9 +++++++
>>  .../realtek/rtl8xxxu/rtl8xxxu_8723b.c         |  3 +++
>>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++--------------
>>  7 files changed, 31 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
>> index 6a82ec47568e..af8436070ba7 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
>> @@ -1883,6 +1883,7 @@ struct rtl8xxxu_fileops rtl8188eu_fops = {
>>         .rx_desc_size = sizeof(struct rtl8xxxu_rxdesc16),
>>         .tx_desc_size = sizeof(struct rtl8xxxu_txdesc32),
>>         .has_tx_report = 1,
>> +       .init_reg_pkt_life_time = 1,
> 
> I'm sure it's safe, but the fops structs that don't set the
> ampdu_max_time and ustime_tsf_edca values feel odd.
> 
They don't set those because they don't use them. Maybe one day
I will initialise all the members -- I read somewhere that it's
good practice -- but that's not what this patch is about.

>>         .gen2_thermal_meter = 1,
>>         .adda_1t_init = 0x0b1b25a0,
>>         .adda_1t_path_on = 0x0bdb25a0,
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> index 82dee1fed477..dfb250adb168 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> @@ -1746,6 +1746,11 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>>         .has_tx_report = 1,
>>         .gen2_thermal_meter = 1,
>>         .needs_full_init = 1,
>> +       .init_reg_rxfltmap = 1,
>> +       .init_reg_pkt_life_time = 1,
>> +       .init_reg_hmtfr = 1,
>> +       .ampdu_max_time = 0x70,
>> +       .ustime_tsf_edca = 0x28,
> 
> The original code had comments for why the 8188fu had different values
> for ampdu_max_time and ustime_tsf_edca. Should they be copied here?
> 
I don't know if those comments are that useful.

>>         .adda_1t_init = 0x03c00014,
>>         .adda_1t_path_on = 0x03c00014,
>>         .trxff_boundary = 0x3f7f,
> 
> Thanks,
>