Received-SPF: pass (google.com: domain of linux-wireless+bounces-551-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Content-Type: text/plain; charset="utf-8"
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH 4/5] wifi: rtw89: 8922a: dump MAC registers when SER
 occurs
From: Kalle Valo <kvalo@kernel.org>
In-Reply-To: <20231204080751.15354-5-pkshih@realtek.com>
References: <20231204080751.15354-5-pkshih@realtek.com>
To: Ping-Ke Shih <pkshih@realtek.com>
Cc: <leo.li@realtek.com>, <kevin_yang@realtek.com>,
 <linux-wireless@vger.kernel.org>
User-Agent: pwcli/0.1.1-git (https://github.com/kvalo/pwcli/) Python/3.11.2
Message-ID: <170196608744.2897000.6455822302893464057.kvalo@kernel.org>
Date: Thu,  7 Dec 2023 16:21:29 +0000 (UTC)

Ping-Ke Shih <pkshih@realtek.com> wrote:

> From: Chia-Yuan Li <leo.li@realtek.com>
> 
> To diagnose the reason why firmware or hardware get abnormal, add to dump
> MAC registers related to counters and interrupt masks. With these values,
> people can classify problems and check if registers values are unexpected,
> and then correct them. However, it could possible false alarm because
> firmware triggers this SER event by wrong conditions that we should
> correct it at firmware or register settings.
> 
> In field, SER might happen under special conditions, and very hard to
> happen again, so dump lots of registers to provide rich information to
> catch the problem.
> 
> Signed-off-by: Chia-Yuan Li <leo.li@realtek.com>
> Signed-off-by: Zong-Zhe Yang <kevin_yang@realtek.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

I can understand printing few essential firmware/hardware information to dmesg
but from quickly looking at the patch it looks like rtw89 is dumping A LOT of
values during a firmware crash. I think that's too extensive and other means
should be considered, for example providing the data using coredump framework.

This is not a blocker but more like a comment for future design decisions.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20231204080751.15354-5-pkshih@realtek.com/

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