| Message ID | 1563423855-32397-1-git-send-email-vthiagar@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | df3ae4c41c13b8aed7fdff49d28f56bc71f924be |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/2] ath11k/hal: Fix few bugs in ath11k_hal_srng_dst_num_free() | expand |
Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> writes: > The logic to compute the number of available buffers in destination > ring is wrong. It should be just the different between head and > tail pointers in terms of the entry size. This functions currently > unused, this is fixed to make use of this function in follow-up > patches. Also make destination ring head pointer volatile because > it is independently updated by HW. > > Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> [...] > --- a/drivers/net/wireless/ath/ath11k/hal.h > +++ b/drivers/net/wireless/ath/ath11k/hal.h > @@ -538,7 +538,7 @@ struct hal_srng { > u32 tp; > > /* Shadow head pointer location to be updated by HW */ > - u32 *hp_addr; > + volatile u32 *hp_addr; What about tp_addr, shouldn't we make that also volatile to remove the ugly cast? Can someone send another patch to fix that, please?
On 2019-07-18 17:07, Kalle Valo wrote: > Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> writes: > >> The logic to compute the number of available buffers in destination >> ring is wrong. It should be just the different between head and >> tail pointers in terms of the entry size. This functions currently >> unused, this is fixed to make use of this function in follow-up >> patches. Also make destination ring head pointer volatile because >> it is independently updated by HW. >> >> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> > > [...] > >> --- a/drivers/net/wireless/ath/ath11k/hal.h >> +++ b/drivers/net/wireless/ath/ath11k/hal.h >> @@ -538,7 +538,7 @@ struct hal_srng { >> u32 tp; >> >> /* Shadow head pointer location to be updated by HW */ >> - u32 *hp_addr; >> + volatile u32 *hp_addr; > > What about tp_addr, shouldn't we make that also volatile to remove the > ugly cast? Can someone send another patch to fix that, please? As mentioned, i'll address the other case separately. Vasanth
Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> wrote: > The logic to compute the number of available buffers in destination > ring is wrong. It should be just the different between head and > tail pointers in terms of the entry size. This functions currently > unused, this is fixed to make use of this function in follow-up > patches. Also make destination ring head pointer volatile because > it is independently updated by HW. > > Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> 2 patches applied to ath11k-bringup branch of ath.git, thanks. df3ae4c41c13 ath11k/hal: Fix few bugs in ath11k_hal_srng_dst_num_free() b5ca4711131d ath11k/dp_rx: Fix possible REO ring desc overwrite
diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c index 7da42a1..9eac311 100644 --- a/drivers/net/wireless/ath/ath11k/hal.c +++ b/drivers/net/wireless/ath/ath11k/hal.c @@ -745,9 +745,9 @@ int ath11k_hal_srng_dst_num_free(struct ath11k_base *ab, struct hal_srng *srng, } if (hp >= tp) - return ((hp - tp) / srng->entry_size) - 1; + return (hp - tp) / srng->entry_size; else - return ((srng->ring_size - tp + hp) / srng->entry_size) - 1; + return (srng->ring_size - tp + hp) / srng->entry_size; } /* Returns number of available entries in src ring */ @@ -862,8 +862,7 @@ void ath11k_hal_srng_access_begin(struct ath11k_base *ab, struct hal_srng *srng) srng->u.src_ring.cached_tp = *(volatile u32 *)srng->u.src_ring.tp_addr; else - srng->u.dst_ring.cached_hp = - *(volatile u32 *)srng->u.dst_ring.hp_addr; + srng->u.dst_ring.cached_hp = *srng->u.dst_ring.hp_addr; } /* Update cached ring head/tail pointers to HW. ath11k_hal_srng_access_begin() diff --git a/drivers/net/wireless/ath/ath11k/hal.h b/drivers/net/wireless/ath/ath11k/hal.h index a1e917e..d580acf 100644 --- a/drivers/net/wireless/ath/ath11k/hal.h +++ b/drivers/net/wireless/ath/ath11k/hal.h @@ -538,7 +538,7 @@ struct hal_srng { u32 tp; /* Shadow head pointer location to be updated by HW */ - u32 *hp_addr; + volatile u32 *hp_addr; /* Cached head pointer */ u32 cached_hp;
The logic to compute the number of available buffers in destination ring is wrong. It should be just the different between head and tail pointers in terms of the entry size. This functions currently unused, this is fixed to make use of this function in follow-up patches. Also make destination ring head pointer volatile because it is independently updated by HW. Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@codeaurora.org> --- drivers/net/wireless/ath/ath11k/hal.c | 7 +++---- drivers/net/wireless/ath/ath11k/hal.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-)