> -----Original Message----- > From: kvalo=codeaurora.org@xxxxxxxxxxxxxxxxx <kvalo=codeaurora.org@xxxxxxxxxxxxxxxxx> On > Behalf Of Kalle Valo > Sent: Friday, October 1, 2021 2:19 PM > To: Pkshih <pkshih@xxxxxxxxxxx> > Cc: tony0620emma@xxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; Kevin Yang > <kevin_yang@xxxxxxxxxxx> > Subject: Re: [PATCH] rtw88: consider triggering state of simulating fw crash > > Ping-Ke Shih <pkshih@xxxxxxxxxxx> writes: > > > From: Zong-Zhe Yang <kevin_yang@xxxxxxxxxxx> > > > > In certain cases, triggering fw crash simulation via fw_crash debugfs > > will take a while. If the state is queried too early before restart > > begins processing, it may mistakenly think restart process has been > > done. If some tests are started at this time, something unexpected > > might happen due to the follow-up restart process. > > > > To avoid that, we consider the triggering state. > > > > Signed-off-by: Zong-Zhe Yang <kevin_yang@xxxxxxxxxxx> > > Signed-off-by: Ping-Ke Shih <pkshih@xxxxxxxxxxx> > > --- > > drivers/net/wireless/realtek/rtw88/debug.c | 5 ++++- > > drivers/net/wireless/realtek/rtw88/main.c | 1 + > > drivers/net/wireless/realtek/rtw88/main.h | 1 + > > 3 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/realtek/rtw88/debug.c > b/drivers/net/wireless/realtek/rtw88/debug.c > > index babf7fb238cc..682b23502e6e 100644 > > --- a/drivers/net/wireless/realtek/rtw88/debug.c > > +++ b/drivers/net/wireless/realtek/rtw88/debug.c > > @@ -886,6 +886,7 @@ static ssize_t rtw_debugfs_set_fw_crash(struct file *filp, > > > > mutex_lock(&rtwdev->mutex); > > rtw_leave_lps_deep(rtwdev); > > + set_bit(RTW_FLAG_RESTART_TRIGGERING, rtwdev->flags); > > rtw_write8(rtwdev, REG_HRCV_MSG, 1); > > mutex_unlock(&rtwdev->mutex); > > > > @@ -897,7 +898,9 @@ static int rtw_debugfs_get_fw_crash(struct seq_file *m, void *v) > > struct rtw_debugfs_priv *debugfs_priv = m->private; > > struct rtw_dev *rtwdev = debugfs_priv->rtwdev; > > > > - seq_printf(m, "%d\n", test_bit(RTW_FLAG_RESTARTING, rtwdev->flags)); > > + seq_printf(m, "%d\n", > > + test_bit(RTW_FLAG_RESTART_TRIGGERING, rtwdev->flags) || > > + test_bit(RTW_FLAG_RESTARTING, rtwdev->flags)); > > return 0; > > } > > You use the verb "consider" both in the title and the commit log, but > it's not really telling much (though I admit my english isn't very > good). From looking at the patch all I see is that it prints the state > of RTW_FLAG_RESTART_TRIGGERING flag. How is that "considering" anything > and how does that improve any of this? > > Can you improve the commit log and explain this is in detail? And what's > "it" in this case? > In order to make it clear, we change the title and provide the detail by v2 [1]. [1] https://lore.kernel.org/linux-wireless/20211001082301.4805-1-pkshih@xxxxxxxxxxx/T/#u -- Ping-Ke
