Re: [PATCH net] wifi: ath12k: properly set single_chip_mlo_supp to true in ath12k_core_alloc()

[Baochen Qiang]

On 3/19/2025 5:12 PM, neil.armstrong@xxxxxxxxxx wrote:
> Hi,
> 
> On 19/03/2025 10:06, Vasanthakumar Thiagarajan wrote:
>>
>>
>> On 3/19/2025 1:34 PM, Neil Armstrong wrote:
>>> On 18/03/2025 17:35, Jeff Johnson wrote:
>>>> On 3/3/2025 7:00 AM, Neil Armstrong wrote:
>>>>> In commit 46d16f7e1d14 ("wifi: ath12k: rename mlo_capable_flags to
>>>>> single_chip_mlo_supp")
>>>>> the line:
>>>>>     ab->mlo_capable_flags = ATH12K_INTRA_DEVICE_MLO_SUPPORT;
>>>>> was incorrectly updated to:
>>>>>     ab->single_chip_mlo_supp = false;
>>>>> leading to always disabling INTRA_DEVICE_MLO even if the device supports it.
>>>>>
>>>>> The firmware "WLAN.HMT.1.1.c5-00156-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1"
>>>>> crashes on driver initialization with:
>>>>>   ath12k_pci 0000:01:00.0: chip_id 0x2 chip_family 0x4 board_id 0x3d soc_id 0x40170200
>>>>>   ath12k_pci 0000:01:00.0: fw_version 0x110f009c fw_build_timestamp 2024-05-30 11:35
>>>>> fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HMT.1.1.c5-00156-
>>>>> QCAHMTSWPL_V1.0_V2.0_SILICONZ-1

this FW version is not upstream yet, why are you testing with it?

Generally we only support upstrmea driver + upstream FW.


>>>>>   ath12k_pci 0000:01:00.0: ignore reset dev flags 0x200
>>>>>   ath12k_pci 0000:01:00.0: failed to receive wmi unified ready event: -110
>>>>>   ath12k_pci 0000:01:00.0: failed to start core: -110
>>>>>   failed to send QMI message
>>>>>   ath12k_pci 0000:01:00.0: qmi failed to send mode request, mode: 4, err = -5
>>>>>   ath12k_pci 0000:01:00.0: qmi failed to send wlan mode off
>>>>>
>>>>> With ab->single_chip_mlo_supp set to True, firmware loads nominally.
>>>>>
>>>>> Fixes: 46d16f7e1d14 ("wifi: ath12k: rename mlo_capable_flags to single_chip_mlo_supp")
>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
>>>>> ---
>>>>> Bisect log for reference:
>>>>> The bisect leaded to:
>>>>> git bisect start 'v6.14-rc4' 'v6.12'
>>>>> git bisect good 5757b31666277e2b177b406e48878dc48d587a46
>>>>> git bisect bad d78794d4f4dbeac0a39e15d2fbc8e917741b5b7c
>>>>> git bisect bad cf33d96f50903214226b379b3f10d1f262dae018
>>>>> git bisect good 12e070eb6964b341b41677fd260af5a305316a1f
>>>>> git bisect bad 6917d207b469ee81e6dc7f8ccca29c234a16916d
>>>>> git bisect good 4fefbc66dfb356145633e571475be2459d73ce16
>>>>> git bisect bad a6ac667467b642c94928c24ac2eb40d20110983c
>>>>> git bisect bad b05d30c2b6df7e2172b18bf1baee9b202f9c6b53
>>>>> git bisect good 56dcbf0b520796e26b2bbe5686bdd305ad924954
>>>>> git bisect bad d302ac65ac938516487f57ae20f11e9cf6327606
>>>>> git bisect good 8c2143702d0719a0357600bca0236900781ffc78
>>>>> git bisect good a5686ae820fa7ab03226a3b0ff529720b7bac599
>>>>> git bisect bad 6f245ea0ec6c29b90c8fa4fdf6e178c646125d7e
>>>>> git bisect bad 46d16f7e1d1413ad7ff99c1334d8874623717745
>>>>> ---
>>>>>   drivers/net/wireless/ath/ath12k/core.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/
>>>>> ath12k/core.c
>>>>> index
>>>>> 0606116d6b9c491b6ede401b2e1aedfb619339a8..33aba5fceec946fad5a47a11a4d86b7be96e682e
>>>>> 100644
>>>>> --- a/drivers/net/wireless/ath/ath12k/core.c
>>>>> +++ b/drivers/net/wireless/ath/ath12k/core.c
>>>>> @@ -1927,7 +1927,7 @@ struct ath12k_base *ath12k_core_alloc(struct device *dev,
>>>>> size_t priv_size,
>>>>>       ab->dev = dev;
>>>>>       ab->hif.bus = bus;
>>>>>       ab->qmi.num_radios = U8_MAX;
>>>>> -    ab->single_chip_mlo_supp = false;
>>>>> +    ab->single_chip_mlo_supp = true;
>>>>>       /* Device index used to identify the devices in a group.
>>>>>        *
>>>>>
>>>>> ---
>>>>> base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
>>>>> change-id: 20250303-topic-ath12k-fix-crash-49e9055c61a1
>>>>>
>>>>> Best regards,
>>>>
>>>> NAK since this will break QCN
>>>> There is a series under internal review to address MLO issues for WCN chipsets
>>>
>>> ???
>>>
>>> The original commit is wrong, this fixes the conversion, nothing else.
>>
>> Nope. Driver changes to enable MLO with WCN chipset are not there yet.
>> Setting the mlo capability flag without having required driver changes
>> for WCN chipset will likely result in firmware crash. So the recommendation
>> is to enable MLO (in WCN) only when all the necessary driver changes
>> (in development, public posting in near future) are in place.
> 
> Right, I understand clearly, _but_ before 46d16f7e1d14 the firmware
> was _not_ crashing, and 46d16f7e1d14 causes a regression because
> single_chip_mlo_supp was set to false instead of true.
> 
> So if you read the commit message, it clearly explains the regression,
> and the reason of the patch.
> 
> This has nothing to do with enabling MLO, it fixes a regression
> on mainline for current users.
> 
> #regzbot introduced: 46d16f7e1d14
> 
> Neil
> 
>>
>> Vasanth
> 
>