Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1e366190-eb84-3cdb-6408-18dfa8da18c0@gmail.com>
Date:   Wed, 14 Dec 2022 13:23:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.3
Subject: Re: [PATCH v2 1/5] wifi: rtl8xxxu: Deduplicate the efuse dumping code
To:     Ping-Ke Shih <pkshih@realtek.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc:     Jes Sorensen <Jes.Sorensen@gmail.com>,
        Andrea Merello <andrea.merello@gmail.com>,
        Taehee Yoo <ap420073@gmail.com>
References: <c9619d20-ba6b-1611-dafb-9fe14617e1ee@gmail.com>
 <40b9a6949d22484493e49af0200ca65d@realtek.com>
Content-Language: en-US
From:   Bitterblue Smith <rtl8821cerfe2@gmail.com>
In-Reply-To: <40b9a6949d22484493e49af0200ca65d@realtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 14/12/2022 05:58, Ping-Ke Shih wrote:
> 
> 
>> -----Original Message-----
>> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> Sent: Wednesday, December 14, 2022 1:27 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>; Andrea Merello
>> <andrea.merello@gmail.com>; Taehee Yoo <ap420073@gmail.com>
>> Subject: [PATCH v2 1/5] wifi: rtl8xxxu: Deduplicate the efuse dumping code
>>
>> Every chip family except RTL8723AU has a copy of the efuse dumping
>> code. Remove this and dump the efuse from a single place using a new
>> function rtl8xxxu_dump_efuse().
>>
>> Also, use print_hex_dump() to print the efuse instead of a loop and
>> dev_info(). It shows the ASCII interpretation of the bytes, which is
>> nice.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>> v2:
>>  - Patch is new in v2.
>> ---
>>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c | 11 -----------
>>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c | 10 ----------
>>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c |  9 ---------
>>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 11 -----------
>>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 14 ++++++++++++++
>>  5 files changed, 14 insertions(+), 41 deletions(-)
>>
> 
> [...]
> 
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 3ed435401e57..827672ce953d 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -1813,6 +1813,17 @@ static int rtl8xxxu_read_efuse(struct rtl8xxxu_priv *priv)
>>  	return ret;
>>  }
>>
>> +static void rtl8xxxu_dump_efuse(struct rtl8xxxu_priv *priv)
>> +{
>> +	dev_info(&priv->udev->dev,
>> +		 "Dumping efuse for RTL%s (0x%02x bytes):\n",
>> +		 priv->chip_name, EFUSE_REAL_CONTENT_LEN_8723A);
>> +
>> +	print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 16, 1,
>> +		       priv->efuse_wifi.raw, EFUSE_REAL_CONTENT_LEN_8723A,
> 
> EFUSE_MAP_LEN seems to be more suitable, because:
> 
> 	union {
> 		u8 raw[EFUSE_MAP_LEN];
> 		...
> 	} efuse_wifi;
> 
> and, suffix _8723A is weird to me.
> 
Yes, that's better. I didn't notice EFUSE_MAP_LEN.

>> +		       true);
>> +}
>> +
>>  void rtl8xxxu_reset_8051(struct rtl8xxxu_priv *priv)
>>  {
>>  	u8 val8;
>> @@ -6839,6 +6850,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>>  		goto err_set_intfdata;
>>  	}
>>
>> +	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE)
>> +		rtl8xxxu_dump_efuse(priv);
>> +
>>  	rtl8xxxu_print_chipinfo(priv);
>>
>>  	ret = priv->fops->load_firmware(priv);
>> --
>> 2.38.0
>>
>> ------Please consider the environment before printing this e-mail.