Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: kvalo=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org D3A83C433C8
From:   Kalle Valo <kvalo@codeaurora.org>
To:     <tehuang@realtek.com>
Cc:     <yhchuang@realtek.com>, <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 2/5] rtw88: handle and recover when firmware crash
References: <20200925061219.23754-1-tehuang@realtek.com>
        <20200925061219.23754-3-tehuang@realtek.com>
Date:   Tue, 29 Sep 2020 11:20:21 +0300
In-Reply-To: <20200925061219.23754-3-tehuang@realtek.com> (tehuang's message
        of "Fri, 25 Sep 2020 14:12:16 +0800")
Message-ID: <87o8lpyoxm.fsf@tynnyri.adurom.net>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

<tehuang@realtek.com> writes:

> From: Tzu-En Huang <tehuang@realtek.com>
>
> This handles the situation when firmware crashes.
> When firmware crashes, it will send an interrupt, and driver will queue
> a work for recovery.
> In the work, driver will reset it's internal association state, which
> includes removing associated sta's macid, resetting vifs' states
> and removing keys. After resetting the driver's state, driver will call
> rtw_enter_ips() to force the chipset power off to reset the chip.
> Finally, driver calls ieee80211_restart_hw() to inform mac80211 stack
> to restart.
> Since only 8822c firmware supports this feature, the interrupt will only
> be triggered when 8822c chipset is loaded.
>
> Signed-off-by: Tzu-En Huang <tehuang@realtek.com>

[...]

> +int rtw_sta_add(struct rtw_dev *rtwdev, struct ieee80211_sta *sta,
> +		struct ieee80211_vif *vif)
> +{
> +	struct rtw_sta_info *si = (struct rtw_sta_info *)sta->drv_priv;
> +	int i;
> +
> +	si->mac_id = rtw_acquire_macid(rtwdev);
> +	if (si->mac_id >= RTW_MAX_MAC_ID_NUM)
> +		return -ENOSPC;
> +
> +	si->sta = sta;
> +	si->vif = vif;
> +	si->init_ra_lv = 1;
> +	ewma_rssi_init(&si->avg_rssi);
> +	for (i = 0; i < ARRAY_SIZE(sta->txq); i++)
> +		rtw_txq_init(rtwdev, sta->txq[i]);
> +
> +	rtw_update_sta_info(rtwdev, si);
> +	rtw_fw_media_status_report(rtwdev, si->mac_id, true);
> +
> +	rtwdev->sta_cnt++;
> +	rtw_info(rtwdev, "sta %pM joined with macid %d\n",
> +		 sta->addr, si->mac_id);

This was already in the driver before, but I'll comment still that I
find it bad practise to spam the log like this everytime a STA
associates or disassociates. So I would be happy to apply a patch to
change these messages to a debug message.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches