linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Fabio Aiuto <fabioaiuto83@gmail.com>
Cc: gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 12/18] staging: rtl8723bs: remove VHT dead code
Date: Tue, 22 Jun 2021 13:15:00 +0200	[thread overview]
Message-ID: <d80cf1a5-d70c-97c2-a6a2-af2f4209715d@redhat.com> (raw)
In-Reply-To: <20210622095757.GB1426@agape.jhs>

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


  reply	other threads:[~2021-06-22 11:15 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-19 10:47 [PATCH 00/18] staging: rtl8723bs: remove 5Ghz code Fabio Aiuto
2021-06-19 10:47 ` [PATCH 01/18] staging: rtl8723bs: remove all 5Ghz network types Fabio Aiuto
2021-06-21  9:40   ` Hans de Goede
2021-06-21 11:23     ` Fabio Aiuto
2021-06-19 10:47 ` [PATCH 02/18] staging: rtl8723bs: remove code related to unsupported channel bandwidth Fabio Aiuto
2021-06-19 10:47 ` [PATCH 03/18] staging: rtl8723bs: remove unused enum items related to channel bonding Fabio Aiuto
2021-06-19 10:47 ` [PATCH 04/18] staging: rtl8723bs: rename " Fabio Aiuto
2021-06-19 10:47 ` [PATCH 05/18] staging: rtl8723bs: remove 5Ghz field in struct registry_priv Fabio Aiuto
2021-06-19 10:47 ` [PATCH 06/18] staging: rtl8723bs: remove struct rt_channel_plan_5g Fabio Aiuto
2021-06-19 10:47 ` [PATCH 07/18] staging: rtl8723bs: remove all branchings between 2.4Ghz and 5Ghz band types Fabio Aiuto
2021-06-19 10:47 ` [PATCH 08/18] staging: rtl8723bs: beautify prototypes in include/hal_com_phycfg.h Fabio Aiuto
2021-06-19 10:47 ` [PATCH 09/18] staging: rtl8723bs: remove 5Ghz code related to channel plan definition Fabio Aiuto
2021-06-19 10:47 ` [PATCH 10/18] staging: rtl8723bs: remove some unused 5Ghz macro definitions Fabio Aiuto
2021-06-19 10:47 ` [PATCH 11/18] staging: rtl8723bs: remove 5Ghz code related to RF power calibration Fabio Aiuto
2021-06-19 10:47 ` [PATCH 12/18] staging: rtl8723bs: remove VHT dead code Fabio Aiuto
2021-06-21  9:45   ` Hans de Goede
2021-06-21 12:41     ` Fabio Aiuto
2021-06-21 12:45       ` Hans de Goede
2021-06-21 12:51         ` Fabio Aiuto
2021-06-22  9:16     ` Fabio Aiuto
2021-06-22  9:19       ` Hans de Goede
2021-06-22  9:57         ` Fabio Aiuto
2021-06-22 11:15           ` Hans de Goede [this message]
2021-06-19 10:47 ` [PATCH 13/18] staging: rtl8723bs: remove unused ODM_CMNINFO_BOARD_TYPE enum item Fabio Aiuto
2021-06-19 10:47 ` [PATCH 14/18] staging: rtl8723bs: fix macro value for 2.4Ghz only device Fabio Aiuto
2021-06-19 10:47 ` [PATCH 15/18] staging: rtl8723bs: remove register initializations tied to 802.11ac standard Fabio Aiuto
2021-06-21  9:48   ` Hans de Goede
2021-06-21 12:45     ` Fabio Aiuto
2021-06-19 10:47 ` [PATCH 16/18] staging: rtl8723bs: remove obsolete 5Ghz comments Fabio Aiuto
2021-06-19 10:47 ` [PATCH 17/18] staging: rtl8723bs: fix check allowing 5Ghz settings Fabio Aiuto
2021-06-19 10:47 ` [PATCH 18/18] staging: rtl8723bs: remove item from TODO list Fabio Aiuto
2021-06-21  9:49 ` [PATCH 00/18] staging: rtl8723bs: remove 5Ghz code Hans de Goede
2021-06-21 12:47   ` Fabio Aiuto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d80cf1a5-d70c-97c2-a6a2-af2f4209715d@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=fabioaiuto83@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).