linux-kernel - Re: [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for RTL8852C

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: <d0a6b711-f683-0104-a1c8-09130a160861@leemhuis.info>
Date:   Mon, 7 Aug 2023 12:40:58 +0200
From:   "Linux regression tracking (Thorsten Leemhuis)" 
        <regressions@...mhuis.info>
To:     Juerg Haefliger <juerg.haefliger@...onical.com>,
        max.chou@...ltek.com
Cc:     marcel@...tmann.org, johan.hedberg@...il.com, luiz.dentz@...il.com,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
        alex_lu@...lsil.com.cn, hildawu@...ltek.com, karenhsu@...ltek.com,
        kidman@...ltek.com, vicamo.yang@...onical.com, Riley.Kao@...l.com,
        stable@...r.kernel.org, jwboyer@...nel.org, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        regressions@...ts.linux.dev
Subject: Re: [PATCH v3] Bluetooth: btrtl: Load FW v2 otherwise FW v1 for
 RTL8852C

On 07.08.23 12:06, Juerg Haefliger wrote:
> On Mon, 7 Aug 2023 09:44:15 +0800
> <max.chou@...ltek.com> wrote:
> 
>> From: Max Chou <max.chou@...ltek.com>
>>
>> In the commit of linux-firmware project, rtl8852cu_fw.bin is updated as
>> FW v2 format[1]. Consider the case that if driver did not be updated for
>> FW v2 supported[2], it can not use FW v2.
>> By Canonical's suggestion, older driver should be able to load FW v1,
>> so rtl8852cu_fw.bin will be revert to the previous commit as FW v1 and
>> add rtl8852cu_fw_v2.bin as FW v2. This item will be started on
>> linux-firmware project.
>>
>> In this commit, the driver prefers to load FW v2 if available. Fallback to
>> FW v1 otherwise.
>>
>> Note that the driver has supported to extract the data for v1 and v2
>> since the commit[1].
>> The previous FW format of RTL8852C is v1. After the commit[2], the FW
>> format was changed to v2. Only RTL8852C suffered the different FW formats,
>> so we will use rtl8852cu_fw.bin for the original commit as FW v1 and
>> rtl8852cu_fw_v2.bin for the future maintained as FW v2. Other Realtek
>> chips will not been impacted by this patch.
>>
>> To do on linux-firmware project after this commit.
>> 1. revert '55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
>>    to 0x040D_7225")'
>>    => rtl_bt/rtl8852cu_fw.bin: FW v1 (stay at ver. 0xD7B8_FABF)  
>> 2. Add a new commit for rtl8852cu_fw_v2.bin
>>    =>rtl_bt/rtl8852cu_fw_v2.bin: FW v2 (to be maintained)  
>>
>> Reference:
>> [1]'9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")'
>> [2]'55e7448533e7 ("rtl_bt: Update RTL8852C BT USB firmware
>>     to 0x040D_7225")'
> 
> IMO this commit message is too confusing. I don't think you need nor should
> refer to linux-firmware commits and what happened in that repo. This commit
> simply fixes the v2 support by trying to load a v2 file for certain chips
> with a fall-back if fw is not found (or failed to load). That's all that
> needs to be mentioned in the commit message.

/me totally agrees that this in confusing the way it is and needs to be
simplified, but explaining things briefly with maybe a Link: to the
linux-firmware commit in question might nevertheless be a good idea (YMMV)

>> Fixes: '9a24ce5e29b ("Bluetooth: btrtl: Firmware format v2 support")'
> 
> No '' quotes.

Max Chou, you could have just picked the two tags I mentioned earlier
just without the space in front of it[1]. If you do so the 0-day robot
will also stop complaining, as you apparently misunderstood what I meant
with stable tag; [2] explains this.

Ciao, Thorsten

[1]
https://lore.kernel.org/all/f11ba0ef-0bb1-b925-3088-2ba7a8d17393@leemhuis.info/
[2] https://docs.kernel.org/process/stable-kernel-rules.html