Re: [PATCH v2 2/2] wifi: ath11k: support board-specific firmware overrides

[Miaoqing Pan <quic_miaoqing@quicinc.com>]

On 10/28/2024 9:45 PM, Dmitry Baryshkov wrote:
> On Mon, Oct 28, 2024 at 06:32:58PM +0800, Miaoqing Pan wrote:
>>
>>
>> On 10/26/2024 10:31 AM, Miaoqing Pan wrote:
>>>
>>>
>>> On 10/25/2024 11:27 PM, Dmitry Baryshkov wrote:
>>>> On Fri, Oct 25, 2024 at 10:23:45PM +0800, Miaoqing Pan wrote:
>>>>>
>>>>>
>>>>> On 10/25/2024 10:01 PM, Dmitry Baryshkov wrote:
>>>>>> On Fri, Oct 25, 2024 at 09:43:04PM +0800, Miaoqing Pan wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 10/25/2024 8:21 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, 25 Oct 2024 at 15:03, Miaoqing Pan
>>>>>>>> <quic_miaoqing@quicinc.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 10/25/2024 6:20 PM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Fri, 25 Oct 2024 at 10:23, Miaoqing Pan
>>>>>>>>>> <quic_miaoqing@quicinc.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 10/25/2024 2:01 PM, Dmitry Baryshkov wrote:
>>>>>>>>>>>> On Fri, Oct 25, 2024 at 10:56:02AM +0800, Miaoqing Pan wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 10/25/2024 3:39 AM, Dmitry Baryshkov wrote:
>>>>>>>>>>>>>> On Thu, Oct 24, 2024 at 08:25:14AM +0800, Miaoqing Pan wrote:
>>>>>>>>>>>>>>> QCA6698AQ IP core is the
>>>>>>>>>>>>>>> same as WCN6855 hw2.1,
>>>>>>>>>>>>>>> but it has different RF,
>>>>>>>>>>>>>>> IPA, thermal, RAM size
>>>>>>>>>>>>>>> and etc, so new firmware
>>>>>>>>>>>>>>> files used. This change
>>>>>>>>>>>>>>> allows board DT files to
>>>>>>>>>>>>>>> override the subdir of
>>>>>>>>>>>>>>> the firmware directory
>>>>>>>>>>>>>>> used to lookup the amss.bin and m3.bin.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I have slight concerns
>>>>>>>>>>>>>> regarding the _board_ DT
>>>>>>>>>>>>>> files overriding the
>>>>>>>>>>>>>> subdir. This opens a can of
>>>>>>>>>>>>>> worms, allowing per-board
>>>>>>>>>>>>>> firmware sets,
>>>>>>>>>>>>>> which (as far as I
>>>>>>>>>>>>>> understand) is far from
>>>>>>>>>>>>>> being what driver
>>>>>>>>>>>>>> maintainers
>>>>>>>>>>>>>> would like to see. This was
>>>>>>>>>>>>>> required for ath10k-snoc
>>>>>>>>>>>>>> devices, since
>>>>>>>>>>>>>> firmware for those platforms
>>>>>>>>>>>>>> is signed by the vendor keys
>>>>>>>>>>>>>> and it is
>>>>>>>>>>>>>> limited to a particular SoC
>>>>>>>>>>>>>> or SoC family. For
>>>>>>>>>>>>>> ath11k-pci there is no
>>>>>>>>>>>>>> such limitation.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Would it be possible to use
>>>>>>>>>>>>>> PCI subvendor / subdev to
>>>>>>>>>>>>>> identify affected
>>>>>>>>>>>>>> cards? PCI Revision? Any
>>>>>>>>>>>>>> other way to identify the
>>>>>>>>>>>>>> device?  Please
>>>>>>>>>>>>>> provide lspci -nnvv for the
>>>>>>>>>>>>>> affected device kind. Is
>>>>>>>>>>>>>> there a way to
>>>>>>>>>>>>>> identify the RF part somehow?
>>>>>>>>>>>>>
>>>>>>>>>>>>> It's rather difficult, for
>>>>>>>>>>>>> WCN685x, there are multiple
>>>>>>>>>>>>> evolved subseries for
>>>>>>>>>>>>> customized products. e.g.
>>>>>>>>>>>>>
>>>>>>>>>>>>> QCA6698AQ/hw2.1
>>>>>>>>>>>>> QCA2066/hw2.1
>>>>>>>>>>>>> WCN6855/hw2.0/hw2.1
>>>>>>>>>>>>> WCN6856/hw2.1
>>>>>>>>>>>>>
>>>>>>>>>>>>> They have the same PCIe ID
>>>>>>>>>>>>> (17cb:1103), the commit
>>>>>>>>>>>>> 5dc9d1a55e95 ("wifi:
>>>>>>>>>>>>> ath11k: add support for
>>>>>>>>>>>>> QCA2066") reads
>>>>>>>>>>>>> TCSR_SOC_HW_SUB_VER to enumerate
>>>>>>>>>>>>> all
>>>>>>>>>>>>> QCA2066 cards, it lacks of
>>>>>>>>>>>>> flexibility, as the list will
>>>>>>>>>>>>> become longer and
>>>>>>>>>>>>> longer. But it's the only choice
>>>>>>>>>>>>> for QCA2066, as it's customized
>>>>>>>>>>>>> for X86
>>>>>>>>>>>>> platform which without DT files.
>>>>>>>>>>>>
>>>>>>>>>>>> I guess, this is closer to Kalle's
>>>>>>>>>>>> expectations: being able to detect
>>>>>>>>>>>> the hardware instead of adding DT properties.
>>>>>>>>>>>>
>>>>>>>>>>>>> So for MSM those have DT file
>>>>>>>>>>>>> platforms, like SA8775P-RIDE/
>>>>>>>>>>>>> QCS8300-RIDE both
>>>>>>>>>>>>> attached to QCA6698AQ, we can specify the correct firmware to
>>>>>>>>>>>>> 'ath11k/WCN6855/hw2.1/qca6698aq',
>>>>>>>>>>>>> so it's not per-board firmware,
>>>>>>>>>>>>> it depends
>>>>>>>>>>>>> on the type of the products(x86 windows, IoT products or AUTO).
>>>>>>>>>>>>
>>>>>>>>>>>> No-no-no and no. The firmware used
>>>>>>>>>>>> must not be specific to the product
>>>>>>>>>>>> type.  This is what everybody here is trying to avoid. Please try
>>>>>>>>>>>> following the QCA2066 approach
>>>>>>>>>>>> instead. And note that it could use
>>>>>>>>>>>> new
>>>>>>>>>>>> TLD as it perfectly shows itself as a different hardware kind.
>>>>>>>>>>>
>>>>>>>>>>> Actually, TCSR_SOC_HW_SUB_VER is not SOC register, it's a TLMM hw
>>>>>>>>>>> revision register in BAR0 space, it's hard to maintain the list.
>>>>>>>>>>
>>>>>>>>>> How is it so?
>>>>>>>>>
>>>>>>>>> I think QCA2066 approach is just a workaround.
>>>>>>>>> Different batches of chip
>>>>>>>>> manufacture has different value in TCSR_SOC_HW_SUB_VER.
>>>>>>>>
>>>>>>>> Ok. So, subvendor / subdevice?
>>>>>>>
>>>>>>> The 'subvendor' is fixed to 0x17cb, so it's useless. And
>>>>>>> I don't have enough
>>>>>>> samples to decide to use 'subdevice', it's a risk for
>>>>>>> existing devices.
>>>>>>
>>>>>> What kind of risk? If subvendor is fixed, then it's Qualcomm who
>>>>>> enumerates subdevices.
>>>>>
>>>>> It's risk for there is not enough sample card to verify.
>>>>> Subdevice is never
>>>>> used by ath1xk drivers.
>>>>
>>>> Oh, so it's just about development. I'd say, please discuss such risks
>>>> with your management, unless Kalle or Jeff disagree with using the
>>>> subdevice for identification.
>>>
>>> Kalle & Jeff, any idea to introduce subdevice ?
>>>
>>>
>>
>> Whether 'QCA2066 approach' or 'subdevice approach', all need to introduce
>> lots of redundant code, as they share the same IP code.
>>
>> 'DT approach' only needs minor change, brings great flexibility. It's only
>> for Snapdragon boards, will not affect default firmware for X86 platforms.
>>
>>>>
>>>>>
>>>>>>
>>>>>> I'm really reluctant to bringing more DT usage into the PCIe space.
>>>>>> Especially if the user is able to swap cards.
>>>>>
>>>>> Understand your concern, automatic adaptation is always the best
>>>>> choice. But
>>>>> it may not work for MSM boards, the PCIe card (non m.2) is
>>>>> customized, which
>>>>> has special PMU control. User can't swap cards. And that's why power
>>>>> sequencing module was introduced.
>>>>
>>>> I know. Still, it's better to have less unnecessary data there for
>>>> autodiscoverable devices.
>>>
>>
>> We discussed internally, we have no other choice to enable NFA765 for non
>> X86 boards. Could you please approve this 'DT' approach ?
> 
> If you can't use subdevice approach for some reason, then we have no
> other choice that I can imagine.
> 

A new patch was submitted: 
https://lore.kernel.org/linux-wireless/20241031000541.3331606-1-quic_miaoqing@quicinc.com/. 
This patch will add QCA6698AQ support, which follows the approach done 
in commit 5dc9d1a55e95 ("wifi: ath11k: add support for QCA2066"), 
enumerates the subversion number to identify the specific card.

But there is still a problem enabling NFA765 m.2 card for IoT platforms, 
which requires ath11k to support board-specific firmware overrides.