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;
Message-ID: <bf93fce2-6669-1e0e-3117-c84e7e7b4de9@gmail.com>
Date:   Sun, 31 Oct 2021 15:05:02 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.2.1
Subject: Re: [PATCH] staging: rtl8712: remove the condition with no effect
Content-Language: en-GB
To:     Pavel Skripkin <paskripkin@gmail.com>, Larry.Finger@lwfinger.net,
        florian.c.schilhabel@googlemail.com, gregkh@linuxfoundation.org,
        alec@vc-celle.de, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
Cc:     saurav.girepunje@hotmail.com
References: <YXonpdnCatbuOo8H@Sauravs-MacBook-Air.local>
 <d4ee4591-0642-002b-ddd8-41693c0f1ccb@gmail.com>
From:   Saurav Girepunje <saurav.girepunje@gmail.com>
In-Reply-To: <d4ee4591-0642-002b-ddd8-41693c0f1ccb@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 28/10/21 4:56 pm, Pavel Skripkin wrote:
> On 10/28/21 07:31, Saurav Girepunje wrote:
>> Remove the if condition check and else code section in
>> query_rx_phy_status(). As variable cck_highpwr is assign a value of '0'
>> after this assignment, if condition check !cck_highpwr is always
>> going to be true. So remove the if condition check as well as else
>> section which will be never true.
>>
>> After removing the if condition there is no use of local variable
>> cck_highpwr. Remove the local variable cck_highpwr.
>>
>> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
>> ---
> 
> [code snip]
> 
>> +        report = pcck_buf->cck_agc_rpt & 0xc0;
>> +        report >>= 6;
>> +        switch (report) {
>> +        /* Modify the RF RNA gain value to -40, -20,
>> +         * -2, 14 by Jenyu's suggestion
>> +         * Note: different RF with the different
>> +         * RNA gain.
>> +         */
>> +        case 0x3:
>> +            rx_pwr_all = -40 - (pcck_buf->cck_agc_rpt &
>> +                     0x3e);
> 
> You can move "0x3e);" part on previous line, since it will fit even 80 characters boundary. It will improve readability, IMO
> 
> Same below.
> 
Yes, This is also can be done on same patch.
Thanks for review Pavel
>> +            break;
>> +        case 0x2:
>> +            rx_pwr_all = -20 - (pcck_buf->cck_agc_rpt &
>> +                     0x3e);
>> +            break;
>> +        case 0x1:
>> +            rx_pwr_all = -2 - (pcck_buf->cck_agc_rpt &
>> +                     0x3e);
>> +            break;
>> +        case 0x0:
>> +            rx_pwr_all = 14 - (pcck_buf->cck_agc_rpt &
>> +                     0x3e);
>> +            break;
>>           }
>> +
> 
> 
> 
> With regards,
> Pavel Skripkin

Regards,
Saurav