Re: [PATCH 2/9] wifi: rtw88: Debug output for rtw8723x EFUSE

[Fiona Klute <fiona.klute@gmx.de>]

Am 05.02.24 um 03:17 schrieb Ping-Ke Shih:
>
>
>> -----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řej Jirman <megi@xff.cz>; Fiona Klute <fiona.klute@gmx.de>
>> 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@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.

Okay, I'll leave it where it is then.

>>
>>   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'

Okay, I'll change that.

>> +       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?

Right, otherwise the output could be ambiguous. I'll fix that.

>> +               (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?

I just followed the order of elements in the EFUSE data, as described in
the rtw8723x[eus]_efuse structs. I don't know why it is that way.