[PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Alexander Wilhelm]

The ath12k_wmi_scan_req_arg structure defines the channel list in
CPU-native order, while wmi_start_scan_cmd expects the values in
little-endian format. The simple memcpy causes the hardware scan to fail on
big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
values to support both architectures correctly.

Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
index 65a05a9520ff..9e1d3c662852 100644
--- a/drivers/net/wireless/ath/ath12k/wmi.c
+++ b/drivers/net/wireless/ath/ath12k/wmi.c
@@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
 	struct wmi_tlv *tlv;
 	void *ptr;
 	int i, ret, len;
-	u32 *tmp_ptr, extraie_len_with_pad = 0;
+	__le32 *tmp_ptr;
+	u32 extraie_len_with_pad = 0;
 	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
 	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
 
@@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
 	tlv = ptr;
 	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
 	ptr += TLV_HDR_SIZE;
-	tmp_ptr = (u32 *)ptr;
+	tmp_ptr = (__le32 *)ptr;
 
-	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
+	for (i = 0; i < arg->num_chan; i++)
+		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
 
 	ptr += len;
 

---
base-commit: 702847e8cfd51856836a282db2073defd7cfd80c
change-id: 20260317-fix-channel-list-copy-cef5cad24fb6

Best regards,
-- 
Alexander Wilhelm <alexander.wilhelm@westermo.com>

Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Baochen Qiang]

On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
> The ath12k_wmi_scan_req_arg structure defines the channel list in
> CPU-native order, while wmi_start_scan_cmd expects the values in
> little-endian format. The simple memcpy causes the hardware scan to fail on
> big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
> values to support both architectures correctly.
> 
> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> ---
>  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 65a05a9520ff..9e1d3c662852 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>  	struct wmi_tlv *tlv;
>  	void *ptr;
>  	int i, ret, len;
> -	u32 *tmp_ptr, extraie_len_with_pad = 0;
> +	__le32 *tmp_ptr;
> +	u32 extraie_len_with_pad = 0;
>  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
>  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
>  
> @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>  	tlv = ptr;
>  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
>  	ptr += TLV_HDR_SIZE;
> -	tmp_ptr = (u32 *)ptr;
> +	tmp_ptr = (__le32 *)ptr;
>  
> -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
> +	for (i = 0; i < arg->num_chan; i++)
> +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
>  
>  	ptr += len;
>  
> 

seems hint_s_ssid and hint_bssid at the last also need the endian conversion?

> ---
> base-commit: 702847e8cfd51856836a282db2073defd7cfd80c
> change-id: 20260317-fix-channel-list-copy-cef5cad24fb6
> 
> Best regards,

Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Alexander Wilhelm]

On Thu, Jul 02, 2026 at 04:27:44PM +0800, Baochen Qiang wrote:
> 
> 
> On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
> > The ath12k_wmi_scan_req_arg structure defines the channel list in
> > CPU-native order, while wmi_start_scan_cmd expects the values in
> > little-endian format. The simple memcpy causes the hardware scan to fail on
> > big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
> > values to support both architectures correctly.
> > 
> > Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> > ---
> >  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> > index 65a05a9520ff..9e1d3c662852 100644
> > --- a/drivers/net/wireless/ath/ath12k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> > @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
> >  	struct wmi_tlv *tlv;
> >  	void *ptr;
> >  	int i, ret, len;
> > -	u32 *tmp_ptr, extraie_len_with_pad = 0;
> > +	__le32 *tmp_ptr;
> > +	u32 extraie_len_with_pad = 0;
> >  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
> >  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
> >  
> > @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
> >  	tlv = ptr;
> >  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
> >  	ptr += TLV_HDR_SIZE;
> > -	tmp_ptr = (u32 *)ptr;
> > +	tmp_ptr = (__le32 *)ptr;
> >  
> > -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
> > +	for (i = 0; i < arg->num_chan; i++)
> > +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
> >  
> >  	ptr += len;
> >  
> > 
> 
> seems hint_s_ssid and hint_bssid at the last also need the endian conversion?

Okay, I will investigate this further.


Best regards
Alexander Wilhelm

Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Alexander Wilhelm]

On Thu, Jul 02, 2026 at 10:44:46AM +0200, Alexander Wilhelm wrote:
> On Thu, Jul 02, 2026 at 04:27:44PM +0800, Baochen Qiang wrote:
> > 
> > 
> > On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
> > > The ath12k_wmi_scan_req_arg structure defines the channel list in
> > > CPU-native order, while wmi_start_scan_cmd expects the values in
> > > little-endian format. The simple memcpy causes the hardware scan to fail on
> > > big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
> > > values to support both architectures correctly.
> > > 
> > > Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> > > ---
> > >  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> > > index 65a05a9520ff..9e1d3c662852 100644
> > > --- a/drivers/net/wireless/ath/ath12k/wmi.c
> > > +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> > > @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
> > >  	struct wmi_tlv *tlv;
> > >  	void *ptr;
> > >  	int i, ret, len;
> > > -	u32 *tmp_ptr, extraie_len_with_pad = 0;
> > > +	__le32 *tmp_ptr;
> > > +	u32 extraie_len_with_pad = 0;
> > >  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
> > >  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
> > >  
> > > @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
> > >  	tlv = ptr;
> > >  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
> > >  	ptr += TLV_HDR_SIZE;
> > > -	tmp_ptr = (u32 *)ptr;
> > > +	tmp_ptr = (__le32 *)ptr;
> > >  
> > > -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
> > > +	for (i = 0; i < arg->num_chan; i++)
> > > +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
> > >  
> > >  	ptr += len;
> > >  
> > > 
> > 
> > seems hint_s_ssid and hint_bssid at the last also need the endian conversion?

`hist_s_ssid` and `hint_bssid` are both structs within
`ath12k_wmi_scan_req_arg`, and the remaining member variables are also stored in
CPU order. Therefore, it seems wrong to me to perform byte swapping at this
point. What I actually need to swap is the data written through `ptr`, not the
structure members themselves. For that reason, I could also use the `__le32
*tmp_ptr` approach. What do you think about that?

One thing I am still unsure about is the use of `ether_addr_copy()`. Can I
simply copy the bytes as-is here, or does the address also need to be
byte-swapped? I could not find any place where this address is being populated,
so I am not sure what byte order it is expected to be in.


Best regards
Alexander Wilhelm

Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Baochen Qiang]

On 7/2/2026 8:18 PM, Alexander Wilhelm wrote:
> On Thu, Jul 02, 2026 at 10:44:46AM +0200, Alexander Wilhelm wrote:
>> On Thu, Jul 02, 2026 at 04:27:44PM +0800, Baochen Qiang wrote:
>>>
>>>
>>> On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
>>>> The ath12k_wmi_scan_req_arg structure defines the channel list in
>>>> CPU-native order, while wmi_start_scan_cmd expects the values in
>>>> little-endian format. The simple memcpy causes the hardware scan to fail on
>>>> big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
>>>> values to support both architectures correctly.
>>>>
>>>> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
>>>> ---
>>>>  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
>>>> index 65a05a9520ff..9e1d3c662852 100644
>>>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
>>>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>>>> @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>>>  	struct wmi_tlv *tlv;
>>>>  	void *ptr;
>>>>  	int i, ret, len;
>>>> -	u32 *tmp_ptr, extraie_len_with_pad = 0;
>>>> +	__le32 *tmp_ptr;
>>>> +	u32 extraie_len_with_pad = 0;
>>>>  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
>>>>  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
>>>>  
>>>> @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>>>  	tlv = ptr;
>>>>  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
>>>>  	ptr += TLV_HDR_SIZE;
>>>> -	tmp_ptr = (u32 *)ptr;
>>>> +	tmp_ptr = (__le32 *)ptr;
>>>>  
>>>> -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
>>>> +	for (i = 0; i < arg->num_chan; i++)
>>>> +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
>>>>  
>>>>  	ptr += len;
>>>>  
>>>>
>>>
>>> seems hint_s_ssid and hint_bssid at the last also need the endian conversion?
> 
> `hist_s_ssid` and `hint_bssid` are both structs within
> `ath12k_wmi_scan_req_arg`, and the remaining member variables are also stored in
> CPU order. Therefore, it seems wrong to me to perform byte swapping at this
> point. What I actually need to swap is the data written through `ptr`, not the
> structure members themselves. For that reason, I could also use the `__le32
> *tmp_ptr` approach. What do you think about that?

Maybe I was not clear. I intended to mean the ptr:

@@ -2728,8 +2728,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
                ptr += TLV_HDR_SIZE;
                s_ssid = ptr;
                for (i = 0; i < arg->num_hint_s_ssid; ++i) {
-                       s_ssid->freq_flags = arg->hint_s_ssid[i].freq_flags;
-                       s_ssid->short_ssid = arg->hint_s_ssid[i].short_ssid;
+                       s_ssid->freq_flags = cpu_to_le32(arg->hint_s_ssid[i].freq_flags);
+                       s_ssid->short_ssid = cpu_to_le32(arg->hint_s_ssid[i].short_ssid);
                        s_ssid++;
                }
                ptr += len;
@@ -2743,7 +2743,7 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
                hint_bssid = ptr;
                for (i = 0; i < arg->num_hint_bssid; ++i) {
                        hint_bssid->freq_flags =
-                               arg->hint_bssid[i].freq_flags;
+                               cpu_to_le32(arg->hint_bssid[i].freq_flags);
                        ether_addr_copy(&arg->hint_bssid[i].bssid.addr[0],
                                        &hint_bssid->bssid.addr[0]);

> 
> One thing I am still unsure about is the use of `ether_addr_copy()`. Can I
> simply copy the bytes as-is here, or does the address also need to be
> byte-swapped? I could not find any place where this address is being populated,
> so I am not sure what byte order it is expected to be in.

I think you are talking about ath12k_wmi_hint_bssid_arg::bssid, right? For now ath12k does
not populate hint_bssid or hint_s_ssid members, so bssid always remain empty. But if we
are going to populate it I think the address originates from userspace and there it is
passed in byte steams, so byte-swapping not required I think.

> 
> 
> Best regards
> Alexander Wilhelm

Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Alexander Wilhelm]

On Fri, Jul 03, 2026 at 10:48:11AM +0800, Baochen Qiang wrote:
> 
> 
> On 7/2/2026 8:18 PM, Alexander Wilhelm wrote:
> > On Thu, Jul 02, 2026 at 10:44:46AM +0200, Alexander Wilhelm wrote:
> >> On Thu, Jul 02, 2026 at 04:27:44PM +0800, Baochen Qiang wrote:
> >>>
> >>>
> >>> On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
> >>>> The ath12k_wmi_scan_req_arg structure defines the channel list in
> >>>> CPU-native order, while wmi_start_scan_cmd expects the values in
> >>>> little-endian format. The simple memcpy causes the hardware scan to fail on
> >>>> big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
> >>>> values to support both architectures correctly.
> >>>>
> >>>> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> >>>> ---
> >>>>  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
> >>>>  1 file changed, 5 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> >>>> index 65a05a9520ff..9e1d3c662852 100644
> >>>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> >>>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> >>>> @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
> >>>>  	struct wmi_tlv *tlv;
> >>>>  	void *ptr;
> >>>>  	int i, ret, len;
> >>>> -	u32 *tmp_ptr, extraie_len_with_pad = 0;
> >>>> +	__le32 *tmp_ptr;
> >>>> +	u32 extraie_len_with_pad = 0;
> >>>>  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
> >>>>  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
> >>>>  
> >>>> @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
> >>>>  	tlv = ptr;
> >>>>  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
> >>>>  	ptr += TLV_HDR_SIZE;
> >>>> -	tmp_ptr = (u32 *)ptr;
> >>>> +	tmp_ptr = (__le32 *)ptr;
> >>>>  
> >>>> -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
> >>>> +	for (i = 0; i < arg->num_chan; i++)
> >>>> +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
> >>>>  
> >>>>  	ptr += len;
> >>>>  
> >>>>
> >>>
> >>> seems hint_s_ssid and hint_bssid at the last also need the endian conversion?
> > 
> > `hist_s_ssid` and `hint_bssid` are both structs within
> > `ath12k_wmi_scan_req_arg`, and the remaining member variables are also stored in
> > CPU order. Therefore, it seems wrong to me to perform byte swapping at this
> > point. What I actually need to swap is the data written through `ptr`, not the
> > structure members themselves. For that reason, I could also use the `__le32
> > *tmp_ptr` approach. What do you think about that?
> 
> Maybe I was not clear. I intended to mean the ptr:
> 
> @@ -2728,8 +2728,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>                 ptr += TLV_HDR_SIZE;
>                 s_ssid = ptr;
>                 for (i = 0; i < arg->num_hint_s_ssid; ++i) {
> -                       s_ssid->freq_flags = arg->hint_s_ssid[i].freq_flags;
> -                       s_ssid->short_ssid = arg->hint_s_ssid[i].short_ssid;
> +                       s_ssid->freq_flags = cpu_to_le32(arg->hint_s_ssid[i].freq_flags);
> +                       s_ssid->short_ssid = cpu_to_le32(arg->hint_s_ssid[i].short_ssid);

No, I understood what you meant. The issue is that the upper two lines will
trigger sparse warnings. The member variables `freq_flags` and `short_ssid` are
used on both the left-hand side and the right-hand side as members of the same
`struct ath12k_wmi_hint_short_ssid_arg`. I would keep these fields as `u32` and
use `tmp_ptr` on the left-hand side instead of `s_ssid` to keep things
consistent. Alternatively, I could introduce a separate structure specifically
for this use case.

>                         s_ssid++;
>                 }
>                 ptr += len;
> @@ -2743,7 +2743,7 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>                 hint_bssid = ptr;
>                 for (i = 0; i < arg->num_hint_bssid; ++i) {
>                         hint_bssid->freq_flags =
> -                               arg->hint_bssid[i].freq_flags;
> +                               cpu_to_le32(arg->hint_bssid[i].freq_flags);
>                         ether_addr_copy(&arg->hint_bssid[i].bssid.addr[0],
>                                         &hint_bssid->bssid.addr[0]);
> 
> > 
> > One thing I am still unsure about is the use of `ether_addr_copy()`. Can I
> > simply copy the bytes as-is here, or does the address also need to be
> > byte-swapped? I could not find any place where this address is being populated,
> > so I am not sure what byte order it is expected to be in.
> 
> I think you are talking about ath12k_wmi_hint_bssid_arg::bssid, right? For now ath12k does
> not populate hint_bssid or hint_s_ssid members, so bssid always remain empty. But if we
> are going to populate it I think the address originates from userspace and there it is
> passed in byte steams, so byte-swapping not required I think.

Okay, sounds good. I'll keep the `memcpy()` at this location and prepare the
next patch version.


Best regards
Alexander Wilhelm

Re: [PATCH RESEND] wifi: ath12k: fix channel list copy on big endian

[Baochen Qiang]

On 7/3/2026 1:49 PM, Alexander Wilhelm wrote:
> On Fri, Jul 03, 2026 at 10:48:11AM +0800, Baochen Qiang wrote:
>>
>>
>> On 7/2/2026 8:18 PM, Alexander Wilhelm wrote:
>>> On Thu, Jul 02, 2026 at 10:44:46AM +0200, Alexander Wilhelm wrote:
>>>> On Thu, Jul 02, 2026 at 04:27:44PM +0800, Baochen Qiang wrote:
>>>>>
>>>>>
>>>>> On 6/29/2026 3:46 PM, Alexander Wilhelm wrote:
>>>>>> The ath12k_wmi_scan_req_arg structure defines the channel list in
>>>>>> CPU-native order, while wmi_start_scan_cmd expects the values in
>>>>>> little-endian format. The simple memcpy causes the hardware scan to fail on
>>>>>> big-endian architectures. Set __le32* type for the tmp_ptr and swap channel
>>>>>> values to support both architectures correctly.
>>>>>>
>>>>>> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
>>>>>> ---
>>>>>>  drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++---
>>>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
>>>>>> index 65a05a9520ff..9e1d3c662852 100644
>>>>>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
>>>>>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>>>>>> @@ -2571,7 +2571,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>>>>>  	struct wmi_tlv *tlv;
>>>>>>  	void *ptr;
>>>>>>  	int i, ret, len;
>>>>>> -	u32 *tmp_ptr, extraie_len_with_pad = 0;
>>>>>> +	__le32 *tmp_ptr;
>>>>>> +	u32 extraie_len_with_pad = 0;
>>>>>>  	struct ath12k_wmi_hint_short_ssid_arg *s_ssid = NULL;
>>>>>>  	struct ath12k_wmi_hint_bssid_arg *hint_bssid = NULL;
>>>>>>  
>>>>>> @@ -2656,9 +2657,10 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>>>>>  	tlv = ptr;
>>>>>>  	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_UINT32, len);
>>>>>>  	ptr += TLV_HDR_SIZE;
>>>>>> -	tmp_ptr = (u32 *)ptr;
>>>>>> +	tmp_ptr = (__le32 *)ptr;
>>>>>>  
>>>>>> -	memcpy(tmp_ptr, arg->chan_list, arg->num_chan * 4);
>>>>>> +	for (i = 0; i < arg->num_chan; i++)
>>>>>> +		tmp_ptr[i] = cpu_to_le32(arg->chan_list[i]);
>>>>>>  
>>>>>>  	ptr += len;
>>>>>>  
>>>>>>
>>>>>
>>>>> seems hint_s_ssid and hint_bssid at the last also need the endian conversion?
>>>
>>> `hist_s_ssid` and `hint_bssid` are both structs within
>>> `ath12k_wmi_scan_req_arg`, and the remaining member variables are also stored in
>>> CPU order. Therefore, it seems wrong to me to perform byte swapping at this
>>> point. What I actually need to swap is the data written through `ptr`, not the
>>> structure members themselves. For that reason, I could also use the `__le32
>>> *tmp_ptr` approach. What do you think about that?
>>
>> Maybe I was not clear. I intended to mean the ptr:
>>
>> @@ -2728,8 +2728,8 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>                 ptr += TLV_HDR_SIZE;
>>                 s_ssid = ptr;
>>                 for (i = 0; i < arg->num_hint_s_ssid; ++i) {
>> -                       s_ssid->freq_flags = arg->hint_s_ssid[i].freq_flags;
>> -                       s_ssid->short_ssid = arg->hint_s_ssid[i].short_ssid;
>> +                       s_ssid->freq_flags = cpu_to_le32(arg->hint_s_ssid[i].freq_flags);
>> +                       s_ssid->short_ssid = cpu_to_le32(arg->hint_s_ssid[i].short_ssid);
> 
> No, I understood what you meant. The issue is that the upper two lines will
> trigger sparse warnings. The member variables `freq_flags` and `short_ssid` are
> used on both the left-hand side and the right-hand side as members of the same
> `struct ath12k_wmi_hint_short_ssid_arg`. I would keep these fields as `u32` and
> use `tmp_ptr` on the left-hand side instead of `s_ssid` to keep things
> consistent. Alternatively, I could introduce a separate structure specifically
> for this use case.

Ah, I get your point. You are right, the issue is both the WMI cmd member and the arg use
the same definition.

Please introduce new structures for this. Note ath12k has a guidance on WMI interface
naming: for those interfacing with firmware, use the _params as the suffix:

e.g.:
	struct ath12k_wmi_hint_short_ssid_params
	struct ath12k_wmi_hint_bssid_params

for those host-used only use the _arg suffix, like the existing ones:

	struct ath12k_wmi_hint_short_ssid_arg;
	struct ath12k_wmi_hint_bssid_arg;

> 
>>                         s_ssid++;
>>                 }
>>                 ptr += len;
>> @@ -2743,7 +2743,7 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>>                 hint_bssid = ptr;
>>                 for (i = 0; i < arg->num_hint_bssid; ++i) {
>>                         hint_bssid->freq_flags =
>> -                               arg->hint_bssid[i].freq_flags;
>> +                               cpu_to_le32(arg->hint_bssid[i].freq_flags);
>>                         ether_addr_copy(&arg->hint_bssid[i].bssid.addr[0],
>>                                         &hint_bssid->bssid.addr[0]);
>>
>>>
>>> One thing I am still unsure about is the use of `ether_addr_copy()`. Can I
>>> simply copy the bytes as-is here, or does the address also need to be
>>> byte-swapped? I could not find any place where this address is being populated,
>>> so I am not sure what byte order it is expected to be in.
>>
>> I think you are talking about ath12k_wmi_hint_bssid_arg::bssid, right? For now ath12k does
>> not populate hint_bssid or hint_s_ssid members, so bssid always remain empty. But if we
>> are going to populate it I think the address originates from userspace and there it is
>> passed in byte steams, so byte-swapping not required I think.
> 
> Okay, sounds good. I'll keep the `memcpy()` at this location and prepare the
> next patch version.
> 
> 
> Best regards
> Alexander Wilhelm