| Message ID | 20220207093023.10605-1-quic_bqiang@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath11k: Reset PCIE_GPIO_CFG_REG register when power on | expand |
On Mon, Feb 7, 2022 at 1:41 AM Baochen Qiang <quic_bqiang@quicinc.com> wrote: > > On some modules, PCIE_GPIO_CFG_REG is not initialized to right value, > this will cause WCN6855 hardware fails to be enumerated. > > Fix it by force writing the correct value to this register when power > on. > > Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1 Can you elaborate how you tested this ? I can see due to this patch shows resource temporarily unavailable after running simulated wifi crash in a loop. > > Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/pci.c | 11 +++++++++++ > drivers/net/wireless/ath/ath11k/pci.h | 3 +++ > 2 files changed, 14 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c > index d73b522a0081..06968ad488b0 100644 > --- a/drivers/net/wireless/ath/ath11k/pci.c > +++ b/drivers/net/wireless/ath/ath11k/pci.c > @@ -445,6 +445,16 @@ static void ath11k_pci_force_wake(struct ath11k_base *ab) > mdelay(5); > } > > +static void ath11k_pci_gpio_reset(struct ath11k_base *ab) > +{ > + int val; > + > + ath11k_pci_write32(ab, PCIE_GPIO_CFG_REG, PCIE_GPIO_RESET_VAL); > + mdelay(10); > + val = ath11k_pci_read32(ab, PCIE_GPIO_CFG_REG); > + ath11k_dbg(ab, ATH11K_DBG_PCI, "gpio cfg: 0x%x\n", val); > +} Looks like you have added delay before reading which gets printed as a debug log. In this case, I don't think you should add the unconditional delay and read the register unconditionally but rather should do only if debug is enabled. Thought ? > + > static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on) > { > mdelay(100); > @@ -461,6 +471,7 @@ static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on) > ath11k_pci_clear_dbg_registers(ab); > ath11k_pci_soc_global_reset(ab); > ath11k_mhi_set_mhictrl_reset(ab); > + ath11k_pci_gpio_reset(ab); > } > > int ath11k_pci_get_msi_irq(struct device *dev, unsigned int vector) > diff --git a/drivers/net/wireless/ath/ath11k/pci.h b/drivers/net/wireless/ath/ath11k/pci.h > index 61d67b20a0eb..2716fc7745d6 100644 > --- a/drivers/net/wireless/ath/ath11k/pci.h > +++ b/drivers/net/wireless/ath/ath11k/pci.h > @@ -52,6 +52,9 @@ > #define WLAON_QFPROM_PWR_CTRL_REG 0x01f8031c > #define QFPROM_PWR_CTRL_VDD4BLOW_MASK 0x4 > > +#define PCIE_GPIO_CFG_REG 0x0180e000 > +#define PCIE_GPIO_RESET_VAL 0xc5 > + > struct ath11k_msi_user { > char *name; > int num_vectors; > -- > 2.25.1 > Thanks Abhishek
> -----Original Message----- > From: Abhishek Kumar <kuabhs@chromium.org> > Sent: Thursday, February 10, 2022 9:05 AM > To: Baochen Qiang (QUIC) <quic_bqiang@quicinc.com> > Cc: ath11k@lists.infradead.org; linux-wireless@vger.kernel.org; Abhishek > Kumar <kuabhs@chromium.org> > Subject: Re: [PATCH] ath11k: Reset PCIE_GPIO_CFG_REG register when power > on > > On Mon, Feb 7, 2022 at 1:41 AM Baochen Qiang <quic_bqiang@quicinc.com> > wrote: > > > > On some modules, PCIE_GPIO_CFG_REG is not initialized to right value, > > this will cause WCN6855 hardware fails to be enumerated. > > > > Fix it by force writing the correct value to this register when power > > on. > > > > Tested-on: WCN6855 hw2.0 PCI > > WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1 > Can you elaborate how you tested this ? I can see due to this patch shows > resource temporarily unavailable after running simulated wifi crash in a loop. > > Could you please share build info? kernel logs? Your test steps etc.? > > Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> > > --- > > drivers/net/wireless/ath/ath11k/pci.c | 11 +++++++++++ > > drivers/net/wireless/ath/ath11k/pci.h | 3 +++ > > 2 files changed, 14 insertions(+) > > > > diff --git a/drivers/net/wireless/ath/ath11k/pci.c > > b/drivers/net/wireless/ath/ath11k/pci.c > > index d73b522a0081..06968ad488b0 100644 > > --- a/drivers/net/wireless/ath/ath11k/pci.c > > +++ b/drivers/net/wireless/ath/ath11k/pci.c > > @@ -445,6 +445,16 @@ static void ath11k_pci_force_wake(struct > ath11k_base *ab) > > mdelay(5); > > } > > > > +static void ath11k_pci_gpio_reset(struct ath11k_base *ab) { > > + int val; > > + > > + ath11k_pci_write32(ab, PCIE_GPIO_CFG_REG, PCIE_GPIO_RESET_VAL); > > + mdelay(10); > > + val = ath11k_pci_read32(ab, PCIE_GPIO_CFG_REG); > > + ath11k_dbg(ab, ATH11K_DBG_PCI, "gpio cfg: 0x%x\n", val); } > Looks like you have added delay before reading which gets printed as a debug > log. In this case, I don't think you should add the unconditional delay and read > the register unconditionally but rather should do only if debug is enabled. > Thought ? > > + > > static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool > > power_on) { > > mdelay(100); > > @@ -461,6 +471,7 @@ static void ath11k_pci_sw_reset(struct ath11k_base > *ab, bool power_on) > > ath11k_pci_clear_dbg_registers(ab); > > ath11k_pci_soc_global_reset(ab); > > ath11k_mhi_set_mhictrl_reset(ab); > > + ath11k_pci_gpio_reset(ab); > > } > > > > int ath11k_pci_get_msi_irq(struct device *dev, unsigned int vector) > > diff --git a/drivers/net/wireless/ath/ath11k/pci.h > > b/drivers/net/wireless/ath/ath11k/pci.h > > index 61d67b20a0eb..2716fc7745d6 100644 > > --- a/drivers/net/wireless/ath/ath11k/pci.h > > +++ b/drivers/net/wireless/ath/ath11k/pci.h > > @@ -52,6 +52,9 @@ > > #define WLAON_QFPROM_PWR_CTRL_REG 0x01f8031c > > #define QFPROM_PWR_CTRL_VDD4BLOW_MASK 0x4 > > > > +#define PCIE_GPIO_CFG_REG 0x0180e000 > > +#define PCIE_GPIO_RESET_VAL 0xc5 > > + > > struct ath11k_msi_user { > > char *name; > > int num_vectors; > > -- > > 2.25.1 > > > > Thanks > Abhishek
diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c index d73b522a0081..06968ad488b0 100644 --- a/drivers/net/wireless/ath/ath11k/pci.c +++ b/drivers/net/wireless/ath/ath11k/pci.c @@ -445,6 +445,16 @@ static void ath11k_pci_force_wake(struct ath11k_base *ab) mdelay(5); } +static void ath11k_pci_gpio_reset(struct ath11k_base *ab) +{ + int val; + + ath11k_pci_write32(ab, PCIE_GPIO_CFG_REG, PCIE_GPIO_RESET_VAL); + mdelay(10); + val = ath11k_pci_read32(ab, PCIE_GPIO_CFG_REG); + ath11k_dbg(ab, ATH11K_DBG_PCI, "gpio cfg: 0x%x\n", val); +} + static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on) { mdelay(100); @@ -461,6 +471,7 @@ static void ath11k_pci_sw_reset(struct ath11k_base *ab, bool power_on) ath11k_pci_clear_dbg_registers(ab); ath11k_pci_soc_global_reset(ab); ath11k_mhi_set_mhictrl_reset(ab); + ath11k_pci_gpio_reset(ab); } int ath11k_pci_get_msi_irq(struct device *dev, unsigned int vector) diff --git a/drivers/net/wireless/ath/ath11k/pci.h b/drivers/net/wireless/ath/ath11k/pci.h index 61d67b20a0eb..2716fc7745d6 100644 --- a/drivers/net/wireless/ath/ath11k/pci.h +++ b/drivers/net/wireless/ath/ath11k/pci.h @@ -52,6 +52,9 @@ #define WLAON_QFPROM_PWR_CTRL_REG 0x01f8031c #define QFPROM_PWR_CTRL_VDD4BLOW_MASK 0x4 +#define PCIE_GPIO_CFG_REG 0x0180e000 +#define PCIE_GPIO_RESET_VAL 0xc5 + struct ath11k_msi_user { char *name; int num_vectors;
On some modules, PCIE_GPIO_CFG_REG is not initialized to right value, this will cause WCN6855 hardware fails to be enumerated. Fix it by force writing the correct value to this register when power on. Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1 Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com> --- drivers/net/wireless/ath/ath11k/pci.c | 11 +++++++++++ drivers/net/wireless/ath/ath11k/pci.h | 3 +++ 2 files changed, 14 insertions(+)