Received-SPF: pass (google.com: domain of linux-wireless+bounces-4308-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <64286e79-4612-9601-138d-e08aa8217857@candelatech.com>
Date: Fri, 1 Mar 2024 12:12:23 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: ***Spam*** [PATCH 2/8] wifi: mt76: mt7996: add txpower setting
 support
Content-Language: en-US
From: Ben Greear <greearb@candelatech.com>
To: shayne.chen@mediatek.com, "nbd@nbd.name" <nbd@nbd.name>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
 =?UTF-8?B?QWxsZW4gWWUgKOiRieiKt+WLsyk=?= <Allen.Ye@mediatek.com>,
 "linux-mediatek@lists.infradead.org" <linux-mediatek@lists.infradead.org>,
 =?UTF-8?B?RXZlbHluIFRzYWkgKOiUoeePiumIuik=?= <Evelyn.Tsai@mediatek.com>,
 "lorenzo@kernel.org" <lorenzo@kernel.org>, Ryder Lee
 <Ryder.Lee@mediatek.com>, =?UTF-8?B?U3RhbmxleVlQIFdhbmcgKOeOi+S+kemCpik=?=
 <StanleyYP.Wang@mediatek.com>
References: <20231102100302.22160-1-shayne.chen@mediatek.com>
 <20231102100302.22160-2-shayne.chen@mediatek.com>
 <bbf8fe7e-1098-5b33-8ed6-dee38329edf6@candelatech.com>
 <8c5fbe88bcc5b55e6d80247144e4f890fd68ef27.camel@mediatek.com>
 <487ef0bb-5930-e998-7daa-6f57d09d682c@candelatech.com>
Organization: Candela Technologies
In-Reply-To: <487ef0bb-5930-e998-7daa-6f57d09d682c@candelatech.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 12/7/23 13:01, Ben Greear wrote:
> On 12/7/23 09:15, shayne.chen@mediatek.com wrote:
>> On Fri, 2023-12-01 at 15:40 -0800, Ben Greear wrote:
>>>
>>> External email : Please do not click links or open attachments until
>>> you have verified the sender or the content.
>>>   On 11/2/23 03:02, Shayne Chen wrote:
>>>> Add support for setting txpower from upper layer and configuring
>>> per-rate
>>>> txpower limit table.
>>>
>>> Hello Shayne,
>>
>> Hi Ben,
>>>
>>>   From what I can tell, this patch causes STA vdevs to no longer send
>>> probe
>>> requests when trying to associate.  I bisected to this in Felix's
>>> tree that holds
>>> this patch.
>>>
>>> Tested on x86-64, mtk7996 radio.  Specifically
>>> debugged on 2.4Ghz radio, but I think it affects 5Ghz too.
>>>
>> I have done some tests but didn't face this issue. Could you also use
>> the newer firmware to test if it still happens?

Hello Shayne,

I rebased on 6.8, and started adding some debugging around this issue.

I am using the MTK 7996 developer radio board on this system, in an x86-64 pc.  It is not able
to find the 'node', so the per mode power tables are not populated and left at
0x0.  Then, when this 0x0 data is sent to the firmware, the firmware gives
back a failure response 0xc00000bb

[ 8658.340984] mt7996e 0000:03:00.0: get-rate-power-limits:  Could not find node.
[ 8658.340988] mt7996e 0000:03:00.0: set-txpower-sku, tx_power: 36
[ 8658.341127] mt7996e 0000:03:00.0: mcu-parse-response, firmware returned failure code: 0xc00000bb.

Do we need to add code to not even send the mcu command if the per-mode txpower tables
cannot be found?

Or can we calculate the values to some useful default in that case?

For reference, here is my debug patch:


diff --git a/drivers/net/wireless/mediatek/mt76/eeprom.c b/drivers/net/wireless/mediatek/mt76/eeprom.c
index 0c5c664de6b3..cf48073783b8 100644
--- a/drivers/net/wireless/mediatek/mt76/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt76/eeprom.c
@@ -419,12 +422,16 @@ s8 mt76_get_rate_power_limits(struct mt76_phy *phy,

         memset(dest, target_power, sizeof(*dest));

-       if (!IS_ENABLED(CONFIG_OF))
+       if (!IS_ENABLED(CONFIG_OF)) {
+               mtk_dbg(dev, CFG, "get-rate-power-limits:  CONFIG_OF not enabled.\n");
                 return target_power;
+       }

         np = mt76_find_power_limits_node(dev);
-       if (!np)
+       if (!np) {
+               mtk_dbg(dev, CFG, "get-rate-power-limits:  Could not find node.\n");
                 return target_power;
+       }

         switch (chan->band) {
         case NL80211_BAND_2GHZ:
@@ -442,12 +449,17 @@ s8 mt76_get_rate_power_limits(struct mt76_phy *phy,

         snprintf(name, sizeof(name), "txpower-%cg", band);
         np = of_get_child_by_name(np, name);
-       if (!np)
+       if (!np) {
+               mtk_dbg(dev, CFG, "get-rate-power-limits:  Could not find band node: %s\n",
+                       name);
                 return target_power;
+       }

         np = mt76_find_channel_node(np, chan);
-       if (!np)
+       if (!np) {
+               mtk_dbg(dev, CFG, "get-rate-power-limits:  Could not find chan node\n");
                 return target_power;
+       }

         txs_delta = mt76_get_txs_delta(np, hweight16(phy->chainmask));

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 2c19b537feec..8f4774c4b1d5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -1114,6 +1114,7 @@ enum MTK_DEUBG {
         MTK_DEBUG_FATAL         = 0x00000004,
         MTK_DEBUG_WRN           = 0x00000008,
         MTK_DEBUG_MSG           = 0x00000010, /* messages to/from firmware */
+       MTK_DEBUG_CFG           = 0x00000020, /* Configuration related */
         MTK_DEBUG_ANY           = 0xffffffff
  };

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/main.c b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
index e89a06464651..c7042c3a905b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
@@ -422,8 +422,11 @@ static int mt7996_config(struct ieee80211_hw *hw, u32 changed)
         if (changed & (IEEE80211_CONF_CHANGE_POWER |
                        IEEE80211_CONF_CHANGE_CHANNEL)) {
                 ret = mt7996_mcu_set_txpower_sku(phy);
-               if (ret)
-                       return ret;
+               if (ret) {
+                       dev_err(dev->mt76.dev,
+                               "config:  Failed to set txpower: %d\n", ret);
+                       //return ret;
+               }
         }

         mutex_lock(&dev->mt76.mutex);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
index d709d26eacd1..9dc896cd04e6 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
@@ -281,6 +281,10 @@ mt7996_mcu_parse_response(struct mt76_dev *mdev, int cmd,
                 /* skip invalid event */
                 if (mcu_cmd != event->cid)
                         ret = -EAGAIN;
+               if (ret) {
+                       mtk_dbg(mdev, CFG, "mcu-parse-response, firmware returned failure code: 0x%x.\n",
+                               ret);
+               }
         } else {
                 skb_pull(skb, sizeof(struct mt7996_mcu_rxd));
         }
@@ -4571,6 +4575,7 @@ int mt7996_mcu_set_txpower_sku(struct mt7996_phy *phy)
         tx_power = mt76_get_rate_power_limits(mphy, mphy->chandef.chan,
                                               &la, tx_power);
         mphy->txpower_cur = tx_power;
+       mtk_dbg(&dev->mt76, CFG, "set-txpower-sku, tx_power: %d\n", tx_power);

         skb = mt76_mcu_msg_alloc(&dev->mt76, NULL,
                                  sizeof(req) + MT7996_SKU_RATE_NUM);

Thanks,
Ben