Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 12/18] staging: rtl8723bs: remove VHT dead code
To:     Fabio Aiuto <fabioaiuto83@gmail.com>
Cc:     gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
References: <cover.1624099125.git.fabioaiuto83@gmail.com>
 <d3f79570088ad339bd80fe9d6b24604ac91ea8d3.1624099126.git.fabioaiuto83@gmail.com>
 <dd0332d7-5d9f-49e5-3fc6-8ae3e623f29c@redhat.com>
 <20210622091602.GA1426@agape.jhs>
 <4f128bd2-b84b-132e-c75f-0030701a8a9b@redhat.com>
 <20210622095757.GB1426@agape.jhs>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <d80cf1a5-d70c-97c2-a6a2-af2f4209715d@redhat.com>
Date:   Tue, 22 Jun 2021 13:15:00 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210622095757.GB1426@agape.jhs>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Fabio,

On 6/22/21 11:57 AM, Fabio Aiuto wrote:
> On Tue, Jun 22, 2021 at 11:19:36AM +0200, Hans de Goede wrote:
> 
> Hi Hans,
> 
>> Hi Fabio,
>>
>>> Moreover I have been quite conservative, for I left untouched HT indexes above
>>> 7 which rtl8723bs doesn't support.
>>>
>>> So IMO I think this patch is fine as is...
>>>> Perhaps this entire block can never be executed ?
>>>
>>> the block is executed but there's no register write happening. Just
>>> updating of values which will never be fetched.
>>
>> Ack, my bad I was under the impression that phy_SetTxPowerByRateBase()
>> would actually do a register write, but I checked and it just updates
>> some unused table values, so dropping this code is fine and you can
>> keep this patch for v2 of the patch set.
>>
>> Regards,
>>
>> Hans
>>
> 
> thank you, what do you think about what I replied about patch 1,

I somehow did not receive your reply, so I've just read it on the archives.

> shall
> I remove the '> 14 if block' or leave it as is?

I think it would be best to keep the '> 14 if block' for now and
remove all of them in a later patch-series (I assume there will
be more of them).

> Do you think is necessary
> to keep the conditions inside the block and pack them?

You could also remove the condition and just set
the band to WIRELESS_INVALID unconditionally as you
suggest, that is fine.

But if you keep the condition, like you did in v1 of the
patch, then you must pack the 2 masks together.

Regards,

Hans