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] staging: vt6656: Refactor the vnt_ofdm_min_rate function
To:     Stefano Brivio <sbrivio@redhat.com>,
        Oscar Carter <oscar.carter@gmx.com>
Cc:     Forest Bond <forest@alittletooquiet.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        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
References: <20200418134553.6415-1-oscar.carter@gmx.com>
 <20200419195534.1463ddcd@elisabeth>
From:   Malcolm Priestley <tvboxspy@gmail.com>
Message-ID: <1c118ba5-627e-17ee-7f21-100e2eb67cf1@gmail.com>
Date:   Sun, 19 Apr 2020 20:11:49 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
MIME-Version: 1.0
In-Reply-To: <20200419195534.1463ddcd@elisabeth>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi all

On 19/04/2020 18:55, Stefano Brivio wrote:
> 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.
<snip>

>>
>> -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.
> 
Yes only 1 or more needs to be true and it is false when none present.

I have run-time checked the patch and it does function as before.

Regards

Malcolm.