diff mbox series

wifi: ath11k: adjust a comment to reflect reality

Message ID Zd1Muyd8mtA1Hih7@ns.kevlo.org
State New
Headers show
Series wifi: ath11k: adjust a comment to reflect reality | expand

Commit Message

Kevin Lo Feb. 27, 2024, 2:45 a.m. UTC
In ath11k_mhi_set_mhictrl_reset(), I observed on QCA6390/QCN9074/WCN6855,
MHISTATUS has SYSERR bit always been set after SOC_GLOBAL_RESET.

Signed-off-by: Kevin Lo <kevlo@kevlo.org>
---

Comments

Jeff Johnson March 1, 2024, 4:16 p.m. UTC | #1
On 2/26/2024 6:45 PM, Kevin Lo wrote:
> In ath11k_mhi_set_mhictrl_reset(), I observed on QCA6390/QCN9074/WCN6855,
> MHISTATUS has SYSERR bit always been set after SOC_GLOBAL_RESET.
> 
> Signed-off-by: Kevin Lo <kevlo@kevlo.org>
> ---
> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> index 3de7fa6f88d0..1198e80d9dff 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -158,8 +158,8 @@ void ath11k_mhi_set_mhictrl_reset(struct ath11k_base *ab)
>  
>  	ath11k_dbg(ab, ATH11K_DBG_PCI, "mhistatus 0x%x\n", val);
>  
> -	/* Observed on QCA6390 that after SOC_GLOBAL_RESET, MHISTATUS
> -	 * has SYSERR bit set and thus need to set MHICTRL_RESET
> +	/* After SOC_GLOBAL_RESET, MHISTATUS has SYSERR bit
> +	 * always been set and thus need to set MHICTRL_RESET
>  	 * to clear SYSERR.
>  	 */
>  	ath11k_pcic_write32(ab, MHICTRL, MHICTRL_RESET_MASK);

Unless you've verified this is always true for every supported chipset
I'd rather more accurately say something like:
After SOC_GLOBAL_RESET, MHISTATUS may still have SYSERR bit set and thus...
Kevin Lo March 4, 2024, 7:57 a.m. UTC | #2
On Fri, Mar 01, 2024 at 08:16:16AM -0800, Jeff Johnson wrote:
> 
> On 2/26/2024 6:45 PM, Kevin Lo wrote:
> > In ath11k_mhi_set_mhictrl_reset(), I observed on QCA6390/QCN9074/WCN6855,
> > MHISTATUS has SYSERR bit always been set after SOC_GLOBAL_RESET.
> > 
> > Signed-off-by: Kevin Lo <kevlo@kevlo.org>
> > ---
> > diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> > index 3de7fa6f88d0..1198e80d9dff 100644
> > --- a/drivers/net/wireless/ath/ath11k/mhi.c
> > +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> > @@ -158,8 +158,8 @@ void ath11k_mhi_set_mhictrl_reset(struct ath11k_base *ab)
> >  
> >  	ath11k_dbg(ab, ATH11K_DBG_PCI, "mhistatus 0x%x\n", val);
> >  
> > -	/* Observed on QCA6390 that after SOC_GLOBAL_RESET, MHISTATUS
> > -	 * has SYSERR bit set and thus need to set MHICTRL_RESET
> > +	/* After SOC_GLOBAL_RESET, MHISTATUS has SYSERR bit
> > +	 * always been set and thus need to set MHICTRL_RESET
> >  	 * to clear SYSERR.
> >  	 */
> >  	ath11k_pcic_write32(ab, MHICTRL, MHICTRL_RESET_MASK);
> 
> Unless you've verified this is always true for every supported chipset

Only PCIe devices trigger the call to ath11k_mhi_set_mhictrl_reset(),
and I've verified this on the QCA6390, QCN9074, and WCN6855 chips.

> I'd rather more accurately say something like:
> After SOC_GLOBAL_RESET, MHISTATUS may still have SYSERR bit set and thus...

Okay, I'll update the comments as per your suggestion, thanks.
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index 3de7fa6f88d0..1198e80d9dff 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -158,8 +158,8 @@  void ath11k_mhi_set_mhictrl_reset(struct ath11k_base *ab)
 
 	ath11k_dbg(ab, ATH11K_DBG_PCI, "mhistatus 0x%x\n", val);
 
-	/* Observed on QCA6390 that after SOC_GLOBAL_RESET, MHISTATUS
-	 * has SYSERR bit set and thus need to set MHICTRL_RESET
+	/* After SOC_GLOBAL_RESET, MHISTATUS has SYSERR bit
+	 * always been set and thus need to set MHICTRL_RESET
 	 * to clear SYSERR.
 	 */
 	ath11k_pcic_write32(ab, MHICTRL, MHICTRL_RESET_MASK);