diff mbox series

ath11k: Reset PCIE_GPIO_CFG_REG register when power on

Message ID 20220207093023.10605-1-quic_bqiang@quicinc.com
State New
Headers show
Series ath11k: Reset PCIE_GPIO_CFG_REG register when power on | expand

Commit Message

Baochen Qiang Feb. 7, 2022, 9:30 a.m. UTC
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(+)

Comments

Abhishek Kumar Feb. 10, 2022, 1:05 a.m. UTC | #1
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
Baochen Qiang Feb. 10, 2022, 2:45 a.m. UTC | #2
> -----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 mbox series

Patch

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;