2022-02-09 05:24:52

by Baochen Qiang

[permalink] [raw]
Subject: [PATCH] ath11k: Reset PCIE_GPIO_CFG_REG register when power on

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 <[email protected]>
---
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);
+}
+
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



2022-02-10 01:31:45

by Abhishek Kumar

[permalink] [raw]
Subject: Re: [PATCH] ath11k: Reset PCIE_GPIO_CFG_REG register when power on

On Mon, Feb 7, 2022 at 1:41 AM Baochen Qiang <[email protected]> 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 <[email protected]>
> ---
> 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

2022-02-10 05:52:38

by Baochen Qiang

[permalink] [raw]
Subject: RE: [PATCH] ath11k: Reset PCIE_GPIO_CFG_REG register when power on


> -----Original Message-----
> From: Abhishek Kumar <[email protected]>
> Sent: Thursday, February 10, 2022 9:05 AM
> To: Baochen Qiang (QUIC) <[email protected]>
> Cc: [email protected]; [email protected]; Abhishek
> Kumar <[email protected]>
> Subject: Re: [PATCH] ath11k: Reset PCIE_GPIO_CFG_REG register when power
> on
>
> On Mon, Feb 7, 2022 at 1:41 AM Baochen Qiang <[email protected]>
> 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 <[email protected]>
> > ---
> > 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