Received-SPF: pass (google.com: domain of linux-wireless+bounces-3118-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Ping-Ke Shih <pkshih@realtek.com>
To: Fiona Klute <fiona.klute@gmx.de>,
        "linux-wireless@vger.kernel.org"
	<linux-wireless@vger.kernel.org>
CC: Kalle Valo <kvalo@kernel.org>, Ulf Hansson <ulf.hansson@linaro.org>,
        "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
        Pavel Machek
	<pavel@ucw.cz>,
        =?iso-8859-2?Q?Ond=F8ej_Jirman?= <megi@xff.cz>
Subject: RE: [PATCH 2/9] wifi: rtw88: Debug output for rtw8723x EFUSE
Thread-Topic: [PATCH 2/9] wifi: rtw88: Debug output for rtw8723x EFUSE
Thread-Index: AQHaVdDuc4Tx2NyUOkComV3+akm5JLD7AKug
Date: Mon, 5 Feb 2024 02:17:27 +0000
Message-ID: <40d7f3f551254cb8b3cdfcba25052550@realtek.com>
References: <20240202121050.977223-1-fiona.klute@gmx.de>
 <20240202121050.977223-3-fiona.klute@gmx.de>
In-Reply-To: <20240202121050.977223-3-fiona.klute@gmx.de>
Accept-Language: en-US, zh-TW
Content-Language: zh-TW
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0


> -----Original Message-----
> From: Fiona Klute <fiona.klute@gmx.de>
> Sent: Friday, February 2, 2024 8:11 PM
> To: linux-wireless@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>
> Cc: Kalle Valo <kvalo@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; =
linux-mmc@vger.kernel.org; Pavel
> Machek <pavel@ucw.cz>; Ond=F8ej Jirman <megi@xff.cz>; Fiona Klute <fiona.=
klute@gmx.de>
> Subject: [PATCH 2/9] wifi: rtw88: Debug output for rtw8723x EFUSE
>=20
> Some 8703b chips contain invalid EFUSE data, getting detailed
> information is critical when analyzing issues caused by that.
>=20
> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
> ---
> The TX power table debug output function (rtw8723x_debug_txpwr_limit)
> isn't specific to the chip family. Should I move it to debug.c
> (e.g. as rtw_debug_txpwr_limit_2g)?

I think no need. My another thinking is to add an debugfs entry to read
these stuff out, but if failed to probe we can't get these information.=20

>=20
>  drivers/net/wireless/realtek/rtw88/rtw8723x.c | 159 ++++++++++++++++++
>  drivers/net/wireless/realtek/rtw88/rtw8723x.h |  11 ++
>  2 files changed, 170 insertions(+)
>=20
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723x.c
> b/drivers/net/wireless/realtek/rtw88/rtw8723x.c
> index 2b58064547..bca650c6bb 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8723x.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8723x.c
> @@ -63,6 +63,163 @@ static void __rtw8723x_lck(struct rtw_dev *rtwdev)
>                 rtw_write8(rtwdev, REG_TXPAUSE, 0x00);
>  }
>=20
> +#define DBG_EFUSE_VAL(map, name)                        \

I think we should not hide 'rtwdev'

> +       rtw_dbg(rtwdev, RTW_DBG_EFUSE, # name "=3D0x%x\n", \
> +               (map)->name)
> +#define DBG_EFUSE_2BYTE(map, name)                        \
> +       rtw_dbg(rtwdev, RTW_DBG_EFUSE, # name "=3D0x%x%x\n", \

Should format be '%02x%02x' for two bytes?

> +               (map)->name[0], (map)->name[1])
> +
> +static void rtw8723xe_efuse_debug(struct rtw_dev *rtwdev,
> +                                 struct rtw8723x_efuse *map)
> +{
> +       rtw_dbg(rtwdev, RTW_DBG_EFUSE, "mac_addr=3D%pM\n", map->e.mac_add=
r);
> +       DBG_EFUSE_2BYTE(map, e.vendor_id);
> +       DBG_EFUSE_2BYTE(map, e.device_id);
> +       DBG_EFUSE_2BYTE(map, e.sub_vendor_id);
> +       DBG_EFUSE_2BYTE(map, e.sub_device_id);
> +}
> +
> +static void rtw8723xu_efuse_debug(struct rtw_dev *rtwdev,
> +                                 struct rtw8723x_efuse *map)
> +{
> +       DBG_EFUSE_2BYTE(map, u.vendor_id);
> +       DBG_EFUSE_2BYTE(map, u.product_id);
> +       DBG_EFUSE_VAL(map, u.usb_option);
> +       rtw_dbg(rtwdev, RTW_DBG_EFUSE, "mac_addr=3D%pM\n", map->u.mac_add=
r);

Just curious why 'mac_addr' is the first one in rtw8723xe_efuse_debug(), bu=
t
the last one here?


> +}
> +

[...]