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;
Date:   Sun, 19 Apr 2020 19:55:34 +0200
From:   Stefano Brivio <sbrivio@redhat.com>
To:     Oscar Carter <oscar.carter@gmx.com>
Cc:     Forest Bond <forest@alittletooquiet.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Malcolm Priestley <tvboxspy@gmail.com>,
        Quentin Deslandes <quentin.deslandes@itdev.co.uk>,
        "John B . Wyatt IV" <jbwyatt4@gmail.com>,
        Colin Ian King <colin.king@canonical.com>,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: vt6656: Refactor the vnt_ofdm_min_rate
 function
Message-ID: <20200419195534.1463ddcd@elisabeth>
In-Reply-To: <20200418134553.6415-1-oscar.carter@gmx.com>
References: <20200418134553.6415-1-oscar.carter@gmx.com>
Organization: Red Hat
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Oscar,

On Sat, 18 Apr 2020 15:45:53 +0200
Oscar Carter <oscar.carter@gmx.com> wrote:

> Replace the for loop by a ternary operator whose condition is an AND
> bitmask against the priv->basic_rates variable.
> 
> The purpose of the for loop was to check if any of bits from RATE_54M to
> RATE_6M was set, but it's not necessary to check every individual bit.
> The same result can be achieved using only one single mask which
> comprises all the commented bits.
> 
> This way avoid the iteration over an unnecessary for loop.
> 
> Also change the return type to bool because it's the type that this
> function returns.
> 
> Signed-off-by: Oscar Carter <oscar.carter@gmx.com>
> ---
>  drivers/staging/vt6656/card.c | 11 ++---------
>  drivers/staging/vt6656/card.h |  2 +-
>  2 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/vt6656/card.c b/drivers/staging/vt6656/card.c
> index 9bd37e57c727..adaf93cf653a 100644
> --- a/drivers/staging/vt6656/card.c
> +++ b/drivers/staging/vt6656/card.c
> @@ -248,16 +248,9 @@ void vnt_update_top_rates(struct vnt_private *priv)
>  	priv->top_cck_basic_rate = top_cck;
>  }
> 
> -int vnt_ofdm_min_rate(struct vnt_private *priv)
> +bool vnt_ofdm_min_rate(struct vnt_private *priv)
>  {
> -	int ii;
> -
> -	for (ii = RATE_54M; ii >= RATE_6M; ii--) {
> -		if ((priv->basic_rates) & ((u16)BIT(ii)))
> -			return true;
> -	}
> -
> -	return false;
> +	return priv->basic_rates & GENMASK(RATE_54M, RATE_6M) ? true : false;

priv->basic_rates & GENMASK(RATE_54M, RATE_6M) is already true if
non-zero and false otherwise. Note that I haven't checked if the
rest is correct.

-- 
Stefano