Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 20 May 2023 13:26:31 +0300
From:   Dan Carpenter <dan.carpenter@linaro.org>
To:     pozega.tomislav@gmail.com
Cc:     linux-wireless@vger.kernel.org
Subject: [bug report] wifi: rt2x00: add TX LOFT calibration for MT7620
Message-ID: <a5e29cc3-b56b-4233-94f6-0fb82b53dd61@kili.mountain>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hello Tomislav Požega,

The patch dab902fe1d29: "wifi: rt2x00: add TX LOFT calibration for
MT7620" from Sep 17, 2022, leads to the following Smatch static
checker warning:

	drivers/net/wireless/ralink/rt2x00/rt2800lib.c:9681 rt2800_loft_iq_calibration()
	warn: unsigned 'vga_gain[ch_idx]' is never less than zero.

drivers/net/wireless/ralink/rt2x00/rt2800lib.c
    9540 static void rt2800_loft_iq_calibration(struct rt2x00_dev *rt2x00dev)
    9541 {
    9542         struct rf_reg_pair rf_store[CHAIN_NUM][13];
    9543         u32 macorg1 = 0;
    9544         u32 macorg2 = 0;
    9545         u32 macorg3 = 0;
    9546         u32 macorg4 = 0;
    9547         u32 macorg5 = 0;
    9548         u32 orig528 = 0;
    9549         u32 orig52c = 0;
    9550 
    9551         u32 savemacsysctrl = 0;
    9552         u32 macvalue = 0;
    9553         u32 mac13b8 = 0;
    9554         u32 p0 = 0, p1 = 0;
    9555         u32 p0_idx10 = 0, p1_idx10 = 0;
    9556 
    9557         u8 rfvalue;
    9558         u8 loft_dc_search_result[CHAIN_NUM][RF_ALC_NUM][2];
    9559         u8 ger[CHAIN_NUM], per[CHAIN_NUM];
    9560 
    9561         u8 vga_gain[] = {14, 14};
                 ^^^^^^^^^^^^^

    9562         u8 bbp = 0, ch_idx = 0, rf_alc_idx = 0, idx = 0;
    9563         u8 bbpr30, rfb0r39, rfb0r42;
    9564         u8 bbpr1;
    9565         u8 bbpr4;
    9566         u8 bbpr241, bbpr242;
    9567         u8 count_step;
    9568 
    9569         static const u8 rf_gain[] = {0x00, 0x01, 0x02, 0x04, 0x08, 0x0c};
    9570         static const u8 rfvga_gain_table[] = {0x24, 0x25, 0x26, 0x27, 0x28, 0x2c, 0x2d, 0x2e, 0x2f, 0x30,
    9571                                               0x31, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3F};
    9572         static const u8 bbp_2324gain[] = {0x16, 0x14, 0x12, 0x10, 0x0c, 0x08};
    9573 
    9574         savemacsysctrl = rt2800_register_read(rt2x00dev, MAC_SYS_CTRL);
    9575         macorg1 = rt2800_register_read(rt2x00dev, TX_PIN_CFG);
    9576         macorg2 = rt2800_register_read(rt2x00dev, RF_CONTROL0);
    9577         macorg3 = rt2800_register_read(rt2x00dev, RF_BYPASS0);
    9578         macorg4 = rt2800_register_read(rt2x00dev, RF_CONTROL3);
    9579         macorg5 = rt2800_register_read(rt2x00dev, RF_BYPASS3);
    9580         mac13b8 = rt2800_register_read(rt2x00dev, 0x13b8);
    9581         orig528 = rt2800_register_read(rt2x00dev, RF_CONTROL2);
    9582         orig52c = rt2800_register_read(rt2x00dev, RF_BYPASS2);
    9583 
    9584         macvalue = rt2800_register_read(rt2x00dev, MAC_SYS_CTRL);
    9585         macvalue &= (~0x04);
    9586         rt2800_register_write(rt2x00dev, MAC_SYS_CTRL, macvalue);
    9587 
    9588         if (unlikely(rt2800_wait_bbp_rf_ready(rt2x00dev, MAC_STATUS_CFG_BBP_RF_BUSY_TX)))
    9589                 rt2x00_warn(rt2x00dev, "RF TX busy in LOFT IQ calibration\n");
    9590 
    9591         macvalue = rt2800_register_read(rt2x00dev, MAC_SYS_CTRL);
    9592         macvalue &= (~0x08);
    9593         rt2800_register_write(rt2x00dev, MAC_SYS_CTRL, macvalue);
    9594 
    9595         if (unlikely(rt2800_wait_bbp_rf_ready(rt2x00dev, MAC_STATUS_CFG_BBP_RF_BUSY_RX)))
    9596                 rt2x00_warn(rt2x00dev, "RF RX busy in LOFT IQ calibration\n");
    9597 
    9598         for (ch_idx = 0; ch_idx < 2; ch_idx++)
    9599                 rt2800_rf_configstore(rt2x00dev, rf_store, ch_idx);
    9600 
    9601         bbpr30 = rt2800_bbp_read(rt2x00dev, 30);
    9602         rfb0r39 = rt2800_rfcsr_read_bank(rt2x00dev, 0, 39);
    9603         rfb0r42 = rt2800_rfcsr_read_bank(rt2x00dev, 0, 42);
    9604 
    9605         rt2800_bbp_write(rt2x00dev, 30, 0x1F);
    9606         rt2800_rfcsr_write_bank(rt2x00dev, 0, 39, 0x80);
    9607         rt2800_rfcsr_write_bank(rt2x00dev, 0, 42, 0x5B);
    9608 
    9609         rt2800_bbp_write(rt2x00dev, 23, 0x00);
    9610         rt2800_bbp_write(rt2x00dev, 24, 0x00);
    9611 
    9612         rt2800_setbbptonegenerator(rt2x00dev);
    9613 
    9614         for (ch_idx = 0; ch_idx < 2; ch_idx++) {
    9615                 rt2800_bbp_write(rt2x00dev, 23, 0x00);
    9616                 rt2800_bbp_write(rt2x00dev, 24, 0x00);
    9617                 rt2800_register_write(rt2x00dev, MAC_SYS_CTRL, 0x00);
    9618                 rt2800_register_write(rt2x00dev, TX_PIN_CFG, 0x0000000F);
    9619                 rt2800_register_write(rt2x00dev, RF_CONTROL0, 0x00000004);
    9620                 rt2800_register_write(rt2x00dev, RF_BYPASS0, 0x00003306);
    9621                 rt2800_register_write(rt2x00dev, 0x13b8, 0x10);
    9622                 udelay(1);
    9623 
    9624                 if (ch_idx == 0)
    9625                         rt2800_rf_aux_tx0_loopback(rt2x00dev);
    9626                 else
    9627                         rt2800_rf_aux_tx1_loopback(rt2x00dev);
    9628 
    9629                 udelay(1);
    9630 
    9631                 if (ch_idx == 0)
    9632                         rt2800_register_write(rt2x00dev, RF_CONTROL0, 0x00001004);
    9633                 else
    9634                         rt2800_register_write(rt2x00dev, RF_CONTROL0, 0x00002004);
    9635 
    9636                 rt2800_bbp_write(rt2x00dev, 158, 0x05);
    9637                 rt2800_bbp_write(rt2x00dev, 159, 0x00);
    9638 
    9639                 rt2800_bbp_write(rt2x00dev, 158, 0x01);
    9640                 if (ch_idx == 0)
    9641                         rt2800_bbp_write(rt2x00dev, 159, 0x00);
    9642                 else
    9643                         rt2800_bbp_write(rt2x00dev, 159, 0x01);
    9644 
    9645                 vga_gain[ch_idx] = 18;
                                            ^^

    9646                 for (rf_alc_idx = 0; rf_alc_idx < 3; rf_alc_idx++) {
    9647                         rt2800_bbp_write(rt2x00dev, 23, bbp_2324gain[rf_alc_idx]);
    9648                         rt2800_bbp_write(rt2x00dev, 24, bbp_2324gain[rf_alc_idx]);
    9649 
    9650                         macvalue = rt2800_register_read(rt2x00dev, RF_CONTROL3);
    9651                         macvalue &= (~0x0000F1F1);
    9652                         macvalue |= (rf_gain[rf_alc_idx] << 4);
    9653                         macvalue |= (rf_gain[rf_alc_idx] << 12);
    9654                         rt2800_register_write(rt2x00dev, RF_CONTROL3, macvalue);
    9655                         macvalue = (0x0000F1F1);
    9656                         rt2800_register_write(rt2x00dev, RF_BYPASS3, macvalue);
    9657 
    9658                         if (rf_alc_idx == 0) {
    9659                                 rt2800_write_dc(rt2x00dev, ch_idx, 0, 1, 0x21);
    9660                                 for (; vga_gain[ch_idx] > 0;
    9661                                      vga_gain[ch_idx] = vga_gain[ch_idx] - 2) {
                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18 is an even number so this can't actually underflow.


    9662                                         rfvalue = rfvga_gain_table[vga_gain[ch_idx]];
    9663                                         rt2800_rfcsr_write_dccal(rt2x00dev, 3, rfvalue);
    9664                                         rt2800_rfcsr_write_dccal(rt2x00dev, 4, rfvalue);
    9665                                         rt2800_write_dc(rt2x00dev, ch_idx, 0, 1, 0x00);
    9666                                         rt2800_write_dc(rt2x00dev, ch_idx, 0, 0, 0x00);
    9667                                         p0 = rt2800_do_fft_accumulation(rt2x00dev, 0x0A, 0);
    9668                                         rt2800_write_dc(rt2x00dev, ch_idx, 0, 0, 0x21);
    9669                                         p1 = rt2800_do_fft_accumulation(rt2x00dev, 0x0A, 0);
    9670                                         rt2x00_dbg(rt2x00dev, "LOFT AGC %d %d\n", p0, p1);
    9671                                         if ((p0 < 7000 * 7000) && (p1 < (7000 * 7000)))
    9672                                                 break;
    9673                                 }
    9674 
    9675                                 rt2800_write_dc(rt2x00dev, ch_idx, 0, 0, 0x00);
    9676                                 rt2800_write_dc(rt2x00dev, ch_idx, 0, 1, 0x00);
    9677 
    9678                                 rt2x00_dbg(rt2x00dev, "Used VGA %d %x\n", vga_gain[ch_idx],
    9679                                            rfvga_gain_table[vga_gain[ch_idx]]);
    9680 
--> 9681                                 if (vga_gain[ch_idx] < 0)
                                             ^^^^^^^^^^^^^^^^^^^^
But if it could then this test would not work because the type is u8.

    9682                                         vga_gain[ch_idx] = 0;
    9683                         }
    9684 
    9685                         rfvalue = rfvga_gain_table[vga_gain[ch_idx]];
    9686 
    9687                         rt2800_rfcsr_write_dccal(rt2x00dev, 3, rfvalue);
    9688                         rt2800_rfcsr_write_dccal(rt2x00dev, 4, rfvalue);
    9689 
    9690                         rt2800_loft_search(rt2x00dev, ch_idx, rf_alc_idx, loft_dc_search_result);
    9691                 }
    9692         }

regards,
dan carpenter