Re: [PATCH v5 2/4] Bluetooth: qca: Update firmware-name to support board specific nvm

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 23.12.2024 3:47 AM, Cheng Jiang (IOE) wrote:
> Hi Konrad,
> 
> On 12/20/2024 9:46 PM, Konrad Dybcio wrote:
>> On 13.12.2024 8:05 AM, Cheng Jiang (IOE) wrote:
>>
>> [...]
>>
>>>> /*
>>>>  * If the board-specific file is missing, try loading the default
>>>>  * one, unless that was attempted already
>>>>  */
>>>>
>>>> But, even more importantly:
>>>>
>>>> a) do we want to load the "incorrect" file?
>>> Normally, there is a default NVM file ending with .bin, which is suitable 
>>> for most boards. However, some boards have different configurations that 
>>> require a specific NVM. If a board-specific NVM is not found, a default 
>>> NVM is preferred over not loading any NVM.
>>
>> So, if one is specified, but not found, this should either be a loud error,
>> or a very loud warning & fallback. Otherwise, the device may provide subpar
>> user experience without the user getting a chance to know the reason.
>>
>> I think failing is better here, as that sends a clearer message, and would
>> only happen if the DT has a specific path (meaning the user put some
>> intentions behind that choice)
>>
> In the existing BT driver implementation, even if the rampatch/nvm are not found,
> BT still works with ROM code only. No fails, just a warning in the dmesg. For this
> new approach, we use the similar logic. 
> 
> The fallback to load a default nvm file is due to each board has a unique board
> id, it's not necessary to upstream all the board-specific nvm since most of them 
> may be the same, the default nvm file is suitable for them. But we can't set the 
> default nvm file name in the DT, since the platform can attach different 
> connectivity boards. This is a reasonable way to approach this. 

Okay, let's do it this way then.

>>>> b) why would we want to specify the .bin file if it's the default anyway?
>>> The default NVM directory is the root of qca. The 'firmware-name' property 
>>> can specify an NVM file in another directory. This can be either a default 
>>> NVM like 'QCA6698/hpnv21.bin' or a board-specific NVM like 'QCA6698/hpnv21.b205'.
>>
>> Do we expect QCA6698/hpnv21.bin and QCAabcd/hpnv21.bin to be compatible?
>>
> No. It may be different. 

That's a bit disappointing considering the filename implies it's suitable
for a family of chips.. But I guess there's nothing we can change here.

Konrad