* [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses
@ 2025-11-18 7:21 Alexander Wilhelm
2025-11-18 9:35 ` Baochen Qiang
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Wilhelm @ 2025-11-18 7:21 UTC (permalink / raw)
To: Jeff Johnson; +Cc: linux-wireless, ath12k, linux-kernel
The SRNG head and tail ring pointers are stored in device memory as
little-endian values. On big-endian systems, direct dereferencing of these
pointers leads to incorrect values being read or written, causing ring
management issues and potentially breaking data flow.
This patch ensures all accesses to SRNG ring pointers use the appropriate
endianness conversions. This affects both read and write paths for source
and destination rings, as well as debug output. The changes guarantee
correct operation on both little- and big-endian architectures.
Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
drivers/net/wireless/ath/ath12k/hal.c | 35 +++++++++++++++------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
index 6406fcf5d69f..bd4d1de9eb1a 100644
--- a/drivers/net/wireless/ath/ath12k/hal.c
+++ b/drivers/net/wireless/ath/ath12k/hal.c
@@ -2007,7 +2007,7 @@ int ath12k_hal_srng_dst_num_free(struct ath12k_base *ab, struct hal_srng *srng,
tp = srng->u.dst_ring.tp;
if (sync_hw_ptr) {
- hp = *srng->u.dst_ring.hp_addr;
+ hp = le32_to_cpu(*srng->u.dst_ring.hp_addr);
srng->u.dst_ring.cached_hp = hp;
} else {
hp = srng->u.dst_ring.cached_hp;
@@ -2030,7 +2030,7 @@ int ath12k_hal_srng_src_num_free(struct ath12k_base *ab, struct hal_srng *srng,
hp = srng->u.src_ring.hp;
if (sync_hw_ptr) {
- tp = *srng->u.src_ring.tp_addr;
+ tp = le32_to_cpu(*srng->u.src_ring.tp_addr);
srng->u.src_ring.cached_tp = tp;
} else {
tp = srng->u.src_ring.cached_tp;
@@ -2149,9 +2149,9 @@ void ath12k_hal_srng_access_begin(struct ath12k_base *ab, struct hal_srng *srng)
if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
srng->u.src_ring.cached_tp =
- *(volatile u32 *)srng->u.src_ring.tp_addr;
+ le32_to_cpu(*(volatile u32 *)srng->u.src_ring.tp_addr);
} else {
- hp = READ_ONCE(*srng->u.dst_ring.hp_addr);
+ hp = le32_to_cpu(READ_ONCE(*srng->u.dst_ring.hp_addr));
if (hp != srng->u.dst_ring.cached_hp) {
srng->u.dst_ring.cached_hp = hp;
@@ -2175,25 +2175,28 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
* hence written to a shared memory location that is read by FW
*/
if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
- srng->u.src_ring.last_tp =
- *(volatile u32 *)srng->u.src_ring.tp_addr;
+ srng->u.src_ring.last_tp = le32_to_cpu(
+ *(volatile u32 *)srng->u.src_ring.tp_addr);
/* Make sure descriptor is written before updating the
* head pointer.
*/
dma_wmb();
- WRITE_ONCE(*srng->u.src_ring.hp_addr, srng->u.src_ring.hp);
+ WRITE_ONCE(*srng->u.src_ring.hp_addr,
+ cpu_to_le32(srng->u.src_ring.hp));
} else {
- srng->u.dst_ring.last_hp = *srng->u.dst_ring.hp_addr;
+ srng->u.dst_ring.last_hp =
+ le32_to_cpu(*srng->u.dst_ring.hp_addr);
/* Make sure descriptor is read before updating the
* tail pointer.
*/
dma_mb();
- WRITE_ONCE(*srng->u.dst_ring.tp_addr, srng->u.dst_ring.tp);
+ WRITE_ONCE(*srng->u.dst_ring.tp_addr,
+ cpu_to_le32(srng->u.dst_ring.tp));
}
} else {
if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
- srng->u.src_ring.last_tp =
- *(volatile u32 *)srng->u.src_ring.tp_addr;
+ srng->u.src_ring.last_tp = le32_to_cpu(
+ *(volatile u32 *)srng->u.src_ring.tp_addr);
/* Assume implementation use an MMIO write accessor
* which has the required wmb() so that the descriptor
* is written before the updating the head pointer.
@@ -2203,7 +2206,8 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
(unsigned long)ab->mem,
srng->u.src_ring.hp);
} else {
- srng->u.dst_ring.last_hp = *srng->u.dst_ring.hp_addr;
+ srng->u.dst_ring.last_hp =
+ le32_to_cpu(*srng->u.dst_ring.hp_addr);
/* Make sure descriptor is read before updating the
* tail pointer.
*/
@@ -2547,7 +2551,7 @@ void ath12k_hal_srng_shadow_update_hp_tp(struct ath12k_base *ab,
* HP only when then ring isn't' empty.
*/
if (srng->ring_dir == HAL_SRNG_DIR_SRC &&
- *srng->u.src_ring.tp_addr != srng->u.src_ring.hp)
+ le32_to_cpu(*srng->u.src_ring.tp_addr) != srng->u.src_ring.hp)
ath12k_hal_srng_access_end(ab, srng);
}
@@ -2648,14 +2652,15 @@ void ath12k_hal_dump_srng_stats(struct ath12k_base *ab)
"src srng id %u hp %u, reap_hp %u, cur tp %u, cached tp %u last tp %u napi processed before %ums\n",
srng->ring_id, srng->u.src_ring.hp,
srng->u.src_ring.reap_hp,
- *srng->u.src_ring.tp_addr, srng->u.src_ring.cached_tp,
+ __le32_to_cpu(*srng->u.src_ring.tp_addr),
+ srng->u.src_ring.cached_tp,
srng->u.src_ring.last_tp,
jiffies_to_msecs(jiffies - srng->timestamp));
else if (srng->ring_dir == HAL_SRNG_DIR_DST)
ath12k_err(ab,
"dst srng id %u tp %u, cur hp %u, cached hp %u last hp %u napi processed before %ums\n",
srng->ring_id, srng->u.dst_ring.tp,
- *srng->u.dst_ring.hp_addr,
+ __le32_to_cpu(*srng->u.dst_ring.hp_addr),
srng->u.dst_ring.cached_hp,
srng->u.dst_ring.last_hp,
jiffies_to_msecs(jiffies - srng->timestamp));
base-commit: be83ff7549057d184b693a85cafc10fbd520f3d7
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses
2025-11-18 7:21 [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses Alexander Wilhelm
@ 2025-11-18 9:35 ` Baochen Qiang
2025-11-18 9:46 ` Alexander Wilhelm
0 siblings, 1 reply; 5+ messages in thread
From: Baochen Qiang @ 2025-11-18 9:35 UTC (permalink / raw)
To: Alexander Wilhelm, Jeff Johnson; +Cc: linux-wireless, ath12k, linux-kernel
On 11/18/2025 3:21 PM, Alexander Wilhelm wrote:
> The SRNG head and tail ring pointers are stored in device memory as
> little-endian values. On big-endian systems, direct dereferencing of these
> pointers leads to incorrect values being read or written, causing ring
> management issues and potentially breaking data flow.
>
> This patch ensures all accesses to SRNG ring pointers use the appropriate
> endianness conversions. This affects both read and write paths for source
> and destination rings, as well as debug output. The changes guarantee
> correct operation on both little- and big-endian architectures.
>
> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> ---
> drivers/net/wireless/ath/ath12k/hal.c | 35 +++++++++++++++------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> index 6406fcf5d69f..bd4d1de9eb1a 100644
> --- a/drivers/net/wireless/ath/ath12k/hal.c
> +++ b/drivers/net/wireless/ath/ath12k/hal.c
> @@ -2007,7 +2007,7 @@ int ath12k_hal_srng_dst_num_free(struct ath12k_base *ab, struct hal_srng *srng,
> tp = srng->u.dst_ring.tp;
>
> if (sync_hw_ptr) {
> - hp = *srng->u.dst_ring.hp_addr;
> + hp = le32_to_cpu(*srng->u.dst_ring.hp_addr);
should we also need to change its type to '__le32 *'?
> srng->u.dst_ring.cached_hp = hp;
> } else {
> hp = srng->u.dst_ring.cached_hp;
> @@ -2030,7 +2030,7 @@ int ath12k_hal_srng_src_num_free(struct ath12k_base *ab, struct hal_srng *srng,
> hp = srng->u.src_ring.hp;
>
> if (sync_hw_ptr) {
> - tp = *srng->u.src_ring.tp_addr;
> + tp = le32_to_cpu(*srng->u.src_ring.tp_addr);
> srng->u.src_ring.cached_tp = tp;
> } else {
> tp = srng->u.src_ring.cached_tp;
> @@ -2149,9 +2149,9 @@ void ath12k_hal_srng_access_begin(struct ath12k_base *ab, struct hal_srng *srng)
>
> if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
> srng->u.src_ring.cached_tp =
> - *(volatile u32 *)srng->u.src_ring.tp_addr;
> + le32_to_cpu(*(volatile u32 *)srng->u.src_ring.tp_addr);
> } else {
> - hp = READ_ONCE(*srng->u.dst_ring.hp_addr);
> + hp = le32_to_cpu(READ_ONCE(*srng->u.dst_ring.hp_addr));
>
> if (hp != srng->u.dst_ring.cached_hp) {
> srng->u.dst_ring.cached_hp = hp;
> @@ -2175,25 +2175,28 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
> * hence written to a shared memory location that is read by FW
> */
> if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
> - srng->u.src_ring.last_tp =
> - *(volatile u32 *)srng->u.src_ring.tp_addr;
> + srng->u.src_ring.last_tp = le32_to_cpu(
> + *(volatile u32 *)srng->u.src_ring.tp_addr);
> /* Make sure descriptor is written before updating the
> * head pointer.
> */
> dma_wmb();
> - WRITE_ONCE(*srng->u.src_ring.hp_addr, srng->u.src_ring.hp);
> + WRITE_ONCE(*srng->u.src_ring.hp_addr,
> + cpu_to_le32(srng->u.src_ring.hp));
> } else {
> - srng->u.dst_ring.last_hp = *srng->u.dst_ring.hp_addr;
> + srng->u.dst_ring.last_hp =
> + le32_to_cpu(*srng->u.dst_ring.hp_addr);
> /* Make sure descriptor is read before updating the
> * tail pointer.
> */
> dma_mb();
> - WRITE_ONCE(*srng->u.dst_ring.tp_addr, srng->u.dst_ring.tp);
> + WRITE_ONCE(*srng->u.dst_ring.tp_addr,
> + cpu_to_le32(srng->u.dst_ring.tp));
> }
> } else {
> if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
> - srng->u.src_ring.last_tp =
> - *(volatile u32 *)srng->u.src_ring.tp_addr;
> + srng->u.src_ring.last_tp = le32_to_cpu(
> + *(volatile u32 *)srng->u.src_ring.tp_addr);
> /* Assume implementation use an MMIO write accessor
> * which has the required wmb() so that the descriptor
> * is written before the updating the head pointer.
> @@ -2203,7 +2206,8 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
> (unsigned long)ab->mem,
> srng->u.src_ring.hp);
> } else {
> - srng->u.dst_ring.last_hp = *srng->u.dst_ring.hp_addr;
> + srng->u.dst_ring.last_hp =
> + le32_to_cpu(*srng->u.dst_ring.hp_addr);
> /* Make sure descriptor is read before updating the
> * tail pointer.
> */
> @@ -2547,7 +2551,7 @@ void ath12k_hal_srng_shadow_update_hp_tp(struct ath12k_base *ab,
> * HP only when then ring isn't' empty.
> */
> if (srng->ring_dir == HAL_SRNG_DIR_SRC &&
> - *srng->u.src_ring.tp_addr != srng->u.src_ring.hp)
> + le32_to_cpu(*srng->u.src_ring.tp_addr) != srng->u.src_ring.hp)
> ath12k_hal_srng_access_end(ab, srng);
> }
>
> @@ -2648,14 +2652,15 @@ void ath12k_hal_dump_srng_stats(struct ath12k_base *ab)
> "src srng id %u hp %u, reap_hp %u, cur tp %u, cached tp %u last tp %u napi processed before %ums\n",
> srng->ring_id, srng->u.src_ring.hp,
> srng->u.src_ring.reap_hp,
> - *srng->u.src_ring.tp_addr, srng->u.src_ring.cached_tp,
> + __le32_to_cpu(*srng->u.src_ring.tp_addr),
> + srng->u.src_ring.cached_tp,
> srng->u.src_ring.last_tp,
> jiffies_to_msecs(jiffies - srng->timestamp));
> else if (srng->ring_dir == HAL_SRNG_DIR_DST)
> ath12k_err(ab,
> "dst srng id %u tp %u, cur hp %u, cached hp %u last hp %u napi processed before %ums\n",
> srng->ring_id, srng->u.dst_ring.tp,
> - *srng->u.dst_ring.hp_addr,
> + __le32_to_cpu(*srng->u.dst_ring.hp_addr),
why __le32_to_cpu() only in logging, while le32_to_cpu() elsewhere?
> srng->u.dst_ring.cached_hp,
> srng->u.dst_ring.last_hp,
> jiffies_to_msecs(jiffies - srng->timestamp));
>
> base-commit: be83ff7549057d184b693a85cafc10fbd520f3d7
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses
2025-11-18 9:35 ` Baochen Qiang
@ 2025-11-18 9:46 ` Alexander Wilhelm
2025-11-18 9:53 ` Baochen Qiang
0 siblings, 1 reply; 5+ messages in thread
From: Alexander Wilhelm @ 2025-11-18 9:46 UTC (permalink / raw)
To: Baochen Qiang; +Cc: Jeff Johnson, linux-wireless, ath12k, linux-kernel
On Tue, Nov 18, 2025 at 05:35:09PM +0800, Baochen Qiang wrote:
>
>
> On 11/18/2025 3:21 PM, Alexander Wilhelm wrote:
> > The SRNG head and tail ring pointers are stored in device memory as
> > little-endian values. On big-endian systems, direct dereferencing of these
> > pointers leads to incorrect values being read or written, causing ring
> > management issues and potentially breaking data flow.
> >
> > This patch ensures all accesses to SRNG ring pointers use the appropriate
> > endianness conversions. This affects both read and write paths for source
> > and destination rings, as well as debug output. The changes guarantee
> > correct operation on both little- and big-endian architectures.
> >
> > Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> > ---
> > drivers/net/wireless/ath/ath12k/hal.c | 35 +++++++++++++++------------
> > 1 file changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> > index 6406fcf5d69f..bd4d1de9eb1a 100644
> > --- a/drivers/net/wireless/ath/ath12k/hal.c
> > +++ b/drivers/net/wireless/ath/ath12k/hal.c
> > @@ -2007,7 +2007,7 @@ int ath12k_hal_srng_dst_num_free(struct ath12k_base *ab, struct hal_srng *srng,
> > tp = srng->u.dst_ring.tp;
> >
> > if (sync_hw_ptr) {
> > - hp = *srng->u.dst_ring.hp_addr;
> > + hp = le32_to_cpu(*srng->u.dst_ring.hp_addr);
>
> should we also need to change its type to '__le32 *'?
I saw that in the 'wifi: ath12k: fix endianness handling while accessing wmi
service bit' patch where '__le32' was used? Which one should I preferably use?
I'll change that in the next version.
Best regards
Alexander Wilhelm
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses
2025-11-18 9:46 ` Alexander Wilhelm
@ 2025-11-18 9:53 ` Baochen Qiang
2025-11-18 10:09 ` Alexander Wilhelm
0 siblings, 1 reply; 5+ messages in thread
From: Baochen Qiang @ 2025-11-18 9:53 UTC (permalink / raw)
To: Alexander Wilhelm; +Cc: Jeff Johnson, linux-wireless, ath12k, linux-kernel
On 11/18/2025 5:46 PM, Alexander Wilhelm wrote:
> On Tue, Nov 18, 2025 at 05:35:09PM +0800, Baochen Qiang wrote:
>>
>>
>> On 11/18/2025 3:21 PM, Alexander Wilhelm wrote:
>>> The SRNG head and tail ring pointers are stored in device memory as
>>> little-endian values. On big-endian systems, direct dereferencing of these
>>> pointers leads to incorrect values being read or written, causing ring
>>> management issues and potentially breaking data flow.
>>>
>>> This patch ensures all accesses to SRNG ring pointers use the appropriate
>>> endianness conversions. This affects both read and write paths for source
>>> and destination rings, as well as debug output. The changes guarantee
>>> correct operation on both little- and big-endian architectures.
>>>
>>> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
>>> ---
>>> drivers/net/wireless/ath/ath12k/hal.c | 35 +++++++++++++++------------
>>> 1 file changed, 20 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
>>> index 6406fcf5d69f..bd4d1de9eb1a 100644
>>> --- a/drivers/net/wireless/ath/ath12k/hal.c
>>> +++ b/drivers/net/wireless/ath/ath12k/hal.c
>>> @@ -2007,7 +2007,7 @@ int ath12k_hal_srng_dst_num_free(struct ath12k_base *ab, struct hal_srng *srng,
>>> tp = srng->u.dst_ring.tp;
>>>
>>> if (sync_hw_ptr) {
>>> - hp = *srng->u.dst_ring.hp_addr;
>>> + hp = le32_to_cpu(*srng->u.dst_ring.hp_addr);
>>
>> should we also need to change its type to '__le32 *'?
>
> I saw that in the 'wifi: ath12k: fix endianness handling while accessing wmi
> service bit' patch where '__le32' was used? Which one should I preferably use?
I mean, should hp_addr in hal_srng structure be declared as '__le32 *' ?
> I'll change that in the next version.
>
>
> Best regards
> Alexander Wilhelm
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses
2025-11-18 9:53 ` Baochen Qiang
@ 2025-11-18 10:09 ` Alexander Wilhelm
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Wilhelm @ 2025-11-18 10:09 UTC (permalink / raw)
To: Baochen Qiang; +Cc: Jeff Johnson, linux-wireless, ath12k, linux-kernel
On Tue, Nov 18, 2025 at 05:53:52PM +0800, Baochen Qiang wrote:
>
>
> On 11/18/2025 5:46 PM, Alexander Wilhelm wrote:
> > On Tue, Nov 18, 2025 at 05:35:09PM +0800, Baochen Qiang wrote:
> >>
> >>
> >> On 11/18/2025 3:21 PM, Alexander Wilhelm wrote:
> >>> The SRNG head and tail ring pointers are stored in device memory as
> >>> little-endian values. On big-endian systems, direct dereferencing of these
> >>> pointers leads to incorrect values being read or written, causing ring
> >>> management issues and potentially breaking data flow.
> >>>
> >>> This patch ensures all accesses to SRNG ring pointers use the appropriate
> >>> endianness conversions. This affects both read and write paths for source
> >>> and destination rings, as well as debug output. The changes guarantee
> >>> correct operation on both little- and big-endian architectures.
> >>>
> >>> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
> >>> ---
> >>> drivers/net/wireless/ath/ath12k/hal.c | 35 +++++++++++++++------------
> >>> 1 file changed, 20 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> >>> index 6406fcf5d69f..bd4d1de9eb1a 100644
> >>> --- a/drivers/net/wireless/ath/ath12k/hal.c
> >>> +++ b/drivers/net/wireless/ath/ath12k/hal.c
> >>> @@ -2007,7 +2007,7 @@ int ath12k_hal_srng_dst_num_free(struct ath12k_base *ab, struct hal_srng *srng,
> >>> tp = srng->u.dst_ring.tp;
> >>>
> >>> if (sync_hw_ptr) {
> >>> - hp = *srng->u.dst_ring.hp_addr;
> >>> + hp = le32_to_cpu(*srng->u.dst_ring.hp_addr);
> >>
> >> should we also need to change its type to '__le32 *'?
> >
> > I saw that in the 'wifi: ath12k: fix endianness handling while accessing wmi
> > service bit' patch where '__le32' was used? Which one should I preferably use?
>
> I mean, should hp_addr in hal_srng structure be declared as '__le32 *' ?
Oh, you're right. I'll do that in v2. I'll set '__le32 *' for 'hp_addr/tp_addr'
for both 'dst_ring' and 'src_ring'. Thank you for the found issue.
Best regards
Alexander Wilhelm
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-18 10:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 7:21 [PATCH] wifi: ath12k: fix endianness handling for SRNG ring pointer accesses Alexander Wilhelm
2025-11-18 9:35 ` Baochen Qiang
2025-11-18 9:46 ` Alexander Wilhelm
2025-11-18 9:53 ` Baochen Qiang
2025-11-18 10:09 ` Alexander Wilhelm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).