| Message ID | Zd1Muyd8mtA1Hih7@ns.kevlo.org |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath11k: adjust a comment to reflect reality | expand |
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...
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 --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);
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> ---