linux-kernel - RE: [PATCH] wifi: rtl8xxxu: retry firmware download on error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9552269787c444de931b3f4476a41d71@realtek.com>
Date: Thu, 23 Jan 2025 02:52:34 +0000
From: Ping-Ke Shih <pkshih@...ltek.com>
To: Soeren Moch <smoch@....de>
CC: Kalle Valo <kvalo@...nel.org>, Jes Sorensen <Jes.Sorensen@...il.com>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] wifi: rtl8xxxu: retry firmware download on error

Soeren Moch <smoch@....de> wrote:
> > Also, this message
> > isn't so important to user, so dev_dbg() is more suitable, but already printing
> > "%s: download_firmware %i\n" for every retry is enough? Or move it out of loop?
> I think this message is important since there is the "failed to write block"
> error message before:
> 
> [    6.239205] usb 2-1.1: RTL8188EU rev D (TSMC) romver 0, 1T1R, TX
> queues 2, WiFi=1, BT=0, GPS=0, HI PA=0
> [    6.248793] usb 2-1.1: RTL8188EU MAC: 00:33:0e:00:15:8e
> [    6.254218] usb 2-1.1: rtl8xxxu: Loading firmware rtlwifi/rtl8188eufw.bin
> [    6.343753] usb 2-1.1: Firmware revision 11.1 (signature 0x88e1)
> [    6.374659] usb 2-1.1: rtl8xxxu_writeN: Failed to write block at
> addr: 1c80 size: 0080
> [    6.382894] usb 2-1.1: retry firmware download
> [    7.221505] usbcore: registered new interface driver rtl8xxxu
> 
> So the retry message after the error makes clear that the error will
> hopefully
> be corrected.
> 
> Since rtl8xxxu_writeN is only used for firmware download, an other option
> would be to downgrade both the error and the retry message to dev_dbg().
> 
> What do you think?
> 

It looks like rtl8xxxu uses this kind of debug pattern for writing: 

	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_WRITE)
		dev_info(&udev->dev, "%s(%04x) = 0x%08x\n",
			 __func__, addr, val);


Maybe rtl8xxxu_writeN() can use the same pattern.
Then, no "rtl8xxxu_writeN: Failed to write block at..." to users. 

Ping-Ke