> -----Original Message-----
> From: Fiona Klute <fiona.klute@xxxxxx>
> Sent: Friday, February 2, 2024 8:11 PM
> To: linux-wireless@xxxxxxxxxxxxxxx; Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> Cc: Kalle Valo <kvalo@xxxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Pavel
> Machek <pavel@xxxxxx>; Ondřej Jirman <megi@xxxxxx>; Fiona Klute <fiona.klute@xxxxxx>
> Subject: [PATCH 2/9] wifi: rtw88: Debug output for rtw8723x EFUSE
>
> Some 8703b chips contain invalid EFUSE data, getting detailed
> information is critical when analyzing issues caused by that.
>
> Signed-off-by: Fiona Klute <fiona.klute@xxxxxx>
> ---
> 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.
>
> drivers/net/wireless/realtek/rtw88/rtw8723x.c | 159 ++++++++++++++++++
> drivers/net/wireless/realtek/rtw88/rtw8723x.h | 11 ++
> 2 files changed, 170 insertions(+)
>
> 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);
> }
>
> +#define DBG_EFUSE_VAL(map, name) \
I think we should not hide 'rtwdev'
> + rtw_dbg(rtwdev, RTW_DBG_EFUSE, # name "=0x%x\n", \
> + (map)->name)
> +#define DBG_EFUSE_2BYTE(map, name) \
> + rtw_dbg(rtwdev, RTW_DBG_EFUSE, # name "=0x%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=%pM\n", map->e.mac_addr);
> + 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=%pM\n", map->u.mac_addr);
Just curious why 'mac_addr' is the first one in rtw8723xe_efuse_debug(), but
the last one here?
> +}
> +
[...]
