[PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Nicolas Escande]

Since inception there is an obvious typo laying around in
ath12k_hal_tx_cmd_ext_desc_setup(). Instead of intitializing + adding
flags to tcl_ext_cmd->info1, we initialize + override. This is needed to
make broadcast frames work with ethernet encapsulation.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.4.1-00199-QCAHKSWPL_SILICONZ-1

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
---
 drivers/net/wireless/ath/ath12k/dp_tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c
index e0b85f959cd4a..00437736e0322 100644
--- a/drivers/net/wireless/ath/ath12k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_tx.c
@@ -117,7 +117,7 @@ static void ath12k_hal_tx_cmd_ext_desc_setup(struct ath12k_base *ab,
 			       le32_encode_bits(ti->data_len,
 						HAL_TX_MSDU_EXT_INFO1_BUF_LEN);
 
-	tcl_ext_cmd->info1 = le32_encode_bits(1, HAL_TX_MSDU_EXT_INFO1_EXTN_OVERRIDE) |
+	tcl_ext_cmd->info1 |= le32_encode_bits(1, HAL_TX_MSDU_EXT_INFO1_EXTN_OVERRIDE) |
 				le32_encode_bits(ti->encap_type,
 						 HAL_TX_MSDU_EXT_INFO1_ENCAP_TYPE) |
 				le32_encode_bits(ti->encrypt_type,
-- 
2.48.1

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Karthikeyan Periyasamy]

On 1/24/2025 5:03 PM, Nicolas Escande wrote:
> Since inception there is an obvious typo laying around in
> ath12k_hal_tx_cmd_ext_desc_setup(). Instead of intitializing + adding
> flags to tcl_ext_cmd->info1, we initialize + override. This is needed to
> make broadcast frames work with ethernet encapsulation.
> 

SUPPORTS_TX_ENCAP_OFFLOAD not yet advertised by ath12k. How it work with 
ethernet encapsulation ?


-- 
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Nicolas Escande]

On Fri Jan 24, 2025 at 4:31 PM CET, Karthikeyan Periyasamy wrote:
> On 1/24/2025 5:03 PM, Nicolas Escande wrote:
>> Since inception there is an obvious typo laying around in
>> ath12k_hal_tx_cmd_ext_desc_setup(). Instead of intitializing + adding
>> flags to tcl_ext_cmd->info1, we initialize + override. This is needed to
>> make broadcast frames work with ethernet encapsulation.
>> 
>
> SUPPORTS_TX_ENCAP_OFFLOAD not yet advertised by ath12k. How it work with 
> ethernet encapsulation ?

Yes indeed, I made a small patch that adds the right hw features via a module
param like one of your CLO patches did:

+       if (ath12k_frame_mode == ATH12K_HW_TXRX_ETHERNET) {
+               ieee80211_hw_set(hw, SUPPORTS_TX_ENCAP_OFFLOAD);
+               ieee80211_hw_set(hw, SUPPORTS_RX_DECAP_OFFLOAD);
+       }
+

But maybe as this feature is not yet enabled upstream I need remove the fixes
tag ? After all It seems that in native wifi this bug causes no harm...

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Vasanthakumar Thiagarajan]

On 1/25/2025 6:23 PM, Nicolas Escande wrote:
> On Fri Jan 24, 2025 at 4:31 PM CET, Karthikeyan Periyasamy wrote:
>> On 1/24/2025 5:03 PM, Nicolas Escande wrote:
>>> Since inception there is an obvious typo laying around in
>>> ath12k_hal_tx_cmd_ext_desc_setup(). Instead of intitializing + adding
>>> flags to tcl_ext_cmd->info1, we initialize + override. This is needed to
>>> make broadcast frames work with ethernet encapsulation.
>>>
>>
>> SUPPORTS_TX_ENCAP_OFFLOAD not yet advertised by ath12k. How it work with
>> ethernet encapsulation ?
> 
> Yes indeed, I made a small patch that adds the right hw features via a module
> param like one of your CLO patches did:
> 
> +       if (ath12k_frame_mode == ATH12K_HW_TXRX_ETHERNET) {
> +               ieee80211_hw_set(hw, SUPPORTS_TX_ENCAP_OFFLOAD);
> +               ieee80211_hw_set(hw, SUPPORTS_RX_DECAP_OFFLOAD);
> +       }
> +
> 
> But maybe as this feature is not yet enabled upstream I need remove the fixes
> tag ? After all It seems that in native wifi this bug causes no harm...
> 

That's right, also you may want to remove fbx prefix in the patch title
as this change has nothing to do with any particular platform/hardware.

Vasanth

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Nicolas Escande]

On Sat Jan 25, 2025 at 3:54 PM CET, Vasanthakumar Thiagarajan wrote:
>
>
> On 1/25/2025 6:23 PM, Nicolas Escande wrote:
>> On Fri Jan 24, 2025 at 4:31 PM CET, Karthikeyan Periyasamy wrote:
>>> On 1/24/2025 5:03 PM, Nicolas Escande wrote:
>>>> Since inception there is an obvious typo laying around in
>>>> ath12k_hal_tx_cmd_ext_desc_setup(). Instead of intitializing + adding
>>>> flags to tcl_ext_cmd->info1, we initialize + override. This is needed to
>>>> make broadcast frames work with ethernet encapsulation.
>>>>
>>>
>>> SUPPORTS_TX_ENCAP_OFFLOAD not yet advertised by ath12k. How it work with
>>> ethernet encapsulation ?
>> 
>> Yes indeed, I made a small patch that adds the right hw features via a module
>> param like one of your CLO patches did:
>> 
>> +       if (ath12k_frame_mode == ATH12K_HW_TXRX_ETHERNET) {
>> +               ieee80211_hw_set(hw, SUPPORTS_TX_ENCAP_OFFLOAD);
>> +               ieee80211_hw_set(hw, SUPPORTS_RX_DECAP_OFFLOAD);
>> +       }
>> +
>> 
>> But maybe as this feature is not yet enabled upstream I need remove the fixes
>> tag ? After all It seems that in native wifi this bug causes no harm...
>> 
>
> That's right, also you may want to remove fbx prefix in the patch title
> as this change has nothing to do with any particular platform/hardware.
Yep indeed that fbx thing is an oversight on my part. I'll send an updated v2.

So You guys prefer I remove the fixes tag then ?

>
> Vasanth

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Karthikeyan Periyasamy]

On 1/26/2025 1:51 AM, Nicolas Escande wrote:
> On Sat Jan 25, 2025 at 3:54 PM CET, Vasanthakumar Thiagarajan wrote:
>>
>>
>> On 1/25/2025 6:23 PM, Nicolas Escande wrote:
>>> On Fri Jan 24, 2025 at 4:31 PM CET, Karthikeyan Periyasamy wrote:
>>>> On 1/24/2025 5:03 PM, Nicolas Escande wrote:
>>>>> Since inception there is an obvious typo laying around in
>>>>> ath12k_hal_tx_cmd_ext_desc_setup(). Instead of intitializing + adding
>>>>> flags to tcl_ext_cmd->info1, we initialize + override. This is needed to
>>>>> make broadcast frames work with ethernet encapsulation.
>>>>>
>>>>
>>>> SUPPORTS_TX_ENCAP_OFFLOAD not yet advertised by ath12k. How it work with
>>>> ethernet encapsulation ?
>>>
>>> Yes indeed, I made a small patch that adds the right hw features via a module
>>> param like one of your CLO patches did:
>>>
>>> +       if (ath12k_frame_mode == ATH12K_HW_TXRX_ETHERNET) {
>>> +               ieee80211_hw_set(hw, SUPPORTS_TX_ENCAP_OFFLOAD);
>>> +               ieee80211_hw_set(hw, SUPPORTS_RX_DECAP_OFFLOAD);
>>> +       }
>>> +
>>>
>>> But maybe as this feature is not yet enabled upstream I need remove the fixes
>>> tag ? After all It seems that in native wifi this bug causes no harm...
>>>
>>
>> That's right, also you may want to remove fbx prefix in the patch title
>> as this change has nothing to do with any particular platform/hardware.
> Yep indeed that fbx thing is an oversight on my part. I'll send an updated v2.
> 
> So You guys prefer I remove the fixes tag then ?
> 

Sure also you need to mention "ethernet encapsulation enabled in future"

-- 
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Jeff Johnson]

On 1/25/2025 12:21 PM, Nicolas Escande wrote:
> So You guys prefer I remove the fixes tag then ?

Yes, please don't add a Fixes: tag if no current upstream functionality is
broken. This may cause the 'stable' team to spend unnecessary time backporting
the change for no reason.

/jeff

Re: [PATCH] wifi: ath12k: fbx: fix ath12k_hal_tx_cmd_ext_desc_setup() info1 override

[Nicolas Escande]

On Sun Jan 26, 2025 at 7:16 PM CET, Jeff Johnson wrote:
> On 1/25/2025 12:21 PM, Nicolas Escande wrote:
>> So You guys prefer I remove the fixes tag then ?
>
> Yes, please don't add a Fixes: tag if no current upstream functionality is
> broken. This may cause the 'stable' team to spend unnecessary time backporting
> the change for no reason.
>
> /jeff

Sure, no problem, I'll send an updated v2.