[05/11] rtw88: 8822c: modify rf protection setting
diff mbox series

Message ID 20191220092156.13443-6-yhchuang@realtek.com
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series
  • rtw88: some driver fixes
Related show

Commit Message

Tony Chuang Dec. 20, 2019, 9:21 a.m. UTC
From: Chien-Hsun Liao <ben.liao@realtek.com>

According to some experiments, the original rf protection
setting can not perfectly make sure that there is no hardware
pi write during the direct write. So, modify the setting so
that the hardware block of pi would be turned off during the
direct write.

Signed-off-by: Chien-Hsun Liao <ben.liao@realtek.com>
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/phy.c      | 10 ----------
 drivers/net/wireless/realtek/rtw88/rtw8822c.c | 15 +++++++++++++++
 drivers/net/wireless/realtek/rtw88/rtw8822c.h |  5 +++++
 3 files changed, 20 insertions(+), 10 deletions(-)

Comments

Chris Chiu Dec. 24, 2019, 7:39 a.m. UTC | #1
On Fri, Dec 20, 2019 at 5:22 PM <yhchuang@realtek.com> wrote:
>
> From: Chien-Hsun Liao <ben.liao@realtek.com>
>
> According to some experiments, the original rf protection
> setting can not perfectly make sure that there is no hardware
> pi write during the direct write. So, modify the setting so
> that the hardware block of pi would be turned off during the
> direct write.
>

Sorry, I don't really understand this part. Does it mean rtw8822c_rstb_3wire()
is to disable/enable the hardware block of PI? In this patch, I can only
see the code block of ENABLE_PI/DISABLE_PI been removed and some
rtw_write_rf()s been protected by new rtw8822c_rstb_3wire(). If the new
function is to replace the ENABLE_PI/DISABLE_PI, maybe they should be
removed in the reg.h. And It seems rtw8822c_rstb_3wire() is only for 8822c,
means there's no such problem for 8822b?

Chris

> Signed-off-by: Chien-Hsun Liao <ben.liao@realtek.com>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> ---
>  drivers/net/wireless/realtek/rtw88/phy.c      | 10 ----------
>  drivers/net/wireless/realtek/rtw88/rtw8822c.c | 15 +++++++++++++++
>  drivers/net/wireless/realtek/rtw88/rtw8822c.h |  5 +++++
>  3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
> index a3e1e9578b65..4b2f11be60cf 100644
> --- a/drivers/net/wireless/realtek/rtw88/phy.c
> +++ b/drivers/net/wireless/realtek/rtw88/phy.c
> @@ -749,20 +749,10 @@ bool rtw_phy_write_rf_reg(struct rtw_dev *rtwdev, enum rtw_rf_path rf_path,
>         direct_addr = base_addr[rf_path] + (addr << 2);
>         mask &= RFREG_MASK;
>
> -       if (addr == RF_CFGCH) {
> -               rtw_write32_mask(rtwdev, REG_RSV_CTRL, BITS_RFC_DIRECT, DISABLE_PI);
> -               rtw_write32_mask(rtwdev, REG_WLRF1, BITS_RFC_DIRECT, DISABLE_PI);
> -       }
> -
>         rtw_write32_mask(rtwdev, direct_addr, mask, data);
>
>         udelay(1);
>
> -       if (addr == RF_CFGCH) {
> -               rtw_write32_mask(rtwdev, REG_RSV_CTRL, BITS_RFC_DIRECT, ENABLE_PI);
> -               rtw_write32_mask(rtwdev, REG_WLRF1, BITS_RFC_DIRECT, ENABLE_PI);
> -       }
> -
>         return true;
>  }
>
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.c b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
> index 7c8db951a5bc..4231f94d515e 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
> @@ -1289,6 +1289,17 @@ static int rtw8822c_mac_init(struct rtw_dev *rtwdev)
>         return 0;
>  }
>
> +static void rtw8822c_rstb_3wire(struct rtw_dev *rtwdev, bool enable)
> +{
> +       if (enable) {
> +               rtw_write32_mask(rtwdev, REG_RSTB, BIT_RSTB_3WIRE, 0x1);
> +               rtw_write32_mask(rtwdev, REG_ANAPAR_A, BIT_ANAPAR_UPDATE, 0x1);
> +               rtw_write32_mask(rtwdev, REG_ANAPAR_B, BIT_ANAPAR_UPDATE, 0x1);
> +       } else {
> +               rtw_write32_mask(rtwdev, REG_RSTB, BIT_RSTB_3WIRE, 0x0);
> +       }
> +}
> +
>  static void rtw8822c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
>  {
>  #define RF18_BAND_MASK         (BIT(16) | BIT(9) | BIT(8))
> @@ -1337,6 +1348,8 @@ static void rtw8822c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
>                 break;
>         }
>
> +       rtw8822c_rstb_3wire(rtwdev, false);
> +
>         rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTWE2, 0x04, 0x01);
>         rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTWA, 0x1f, 0x12);
>         rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTWD0, 0xfffff, rf_rxbb);
> @@ -1349,6 +1362,8 @@ static void rtw8822c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
>
>         rtw_write_rf(rtwdev, RF_PATH_A, RF_CFGCH, RFREG_MASK, rf_reg18);
>         rtw_write_rf(rtwdev, RF_PATH_B, RF_CFGCH, RFREG_MASK, rf_reg18);
> +
> +       rtw8822c_rstb_3wire(rtwdev, true);
>  }
>
>  static void rtw8822c_toggle_igi(struct rtw_dev *rtwdev)
> --
> 2.17.1
>
Tony Chuang Dec. 24, 2019, 7:50 a.m. UTC | #2
> On Fri, Dec 20, 2019 at 5:22 PM <yhchuang@realtek.com> wrote:
> >
> > From: Chien-Hsun Liao <ben.liao@realtek.com>
> >
> > According to some experiments, the original rf protection
> > setting can not perfectly make sure that there is no hardware
> > pi write during the direct write. So, modify the setting so
> > that the hardware block of pi would be turned off during the
> > direct write.
> >
> 
> Sorry, I don't really understand this part. Does it mean rtw8822c_rstb_3wire()
> is to disable/enable the hardware block of PI? In this patch, I can only
> see the code block of ENABLE_PI/DISABLE_PI been removed and some
> rtw_write_rf()s been protected by new rtw8822c_rstb_3wire(). If the new
> function is to replace the ENABLE_PI/DISABLE_PI, maybe they should be
> removed in the reg.h. And It seems rtw8822c_rstb_3wire() is only for 8822c,
> means there's no such problem for 8822b?
> 

Yes, rtw8822c_rstb_3wire() is to disable/enable PI. Only 8822c uses mix mode.
That means, 8822c uses direct write for RF registers except for register 0x0.
And 8822b uses sipi write (indirect). So 8822b doesn't have such problem.

Yan-Hsuan
Chris Chiu Dec. 24, 2019, 8:17 a.m. UTC | #3
On Tue, Dec 24, 2019 at 3:50 PM Tony Chuang <yhchuang@realtek.com> wrote:
>
> > On Fri, Dec 20, 2019 at 5:22 PM <yhchuang@realtek.com> wrote:
> > >
> > > From: Chien-Hsun Liao <ben.liao@realtek.com>
> > >
> > > According to some experiments, the original rf protection
> > > setting can not perfectly make sure that there is no hardware
> > > pi write during the direct write. So, modify the setting so
> > > that the hardware block of pi would be turned off during the
> > > direct write.
> > >
> >
> > Sorry, I don't really understand this part. Does it mean rtw8822c_rstb_3wire()
> > is to disable/enable the hardware block of PI? In this patch, I can only
> > see the code block of ENABLE_PI/DISABLE_PI been removed and some
> > rtw_write_rf()s been protected by new rtw8822c_rstb_3wire(). If the new
> > function is to replace the ENABLE_PI/DISABLE_PI, maybe they should be
> > removed in the reg.h. And It seems rtw8822c_rstb_3wire() is only for 8822c,
> > means there's no such problem for 8822b?
> >
>
> Yes, rtw8822c_rstb_3wire() is to disable/enable PI. Only 8822c uses mix mode.
> That means, 8822c uses direct write for RF registers except for register 0x0.
> And 8822b uses sipi write (indirect). So 8822b doesn't have such problem.
>
> Yan-Hsuan

Got it. Could you also explain this in the commit message so that we
won't misread
the code? Thanks.

Chris

Patch
diff mbox series

diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index a3e1e9578b65..4b2f11be60cf 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/phy.c
@@ -749,20 +749,10 @@  bool rtw_phy_write_rf_reg(struct rtw_dev *rtwdev, enum rtw_rf_path rf_path,
 	direct_addr = base_addr[rf_path] + (addr << 2);
 	mask &= RFREG_MASK;
 
-	if (addr == RF_CFGCH) {
-		rtw_write32_mask(rtwdev, REG_RSV_CTRL, BITS_RFC_DIRECT, DISABLE_PI);
-		rtw_write32_mask(rtwdev, REG_WLRF1, BITS_RFC_DIRECT, DISABLE_PI);
-	}
-
 	rtw_write32_mask(rtwdev, direct_addr, mask, data);
 
 	udelay(1);
 
-	if (addr == RF_CFGCH) {
-		rtw_write32_mask(rtwdev, REG_RSV_CTRL, BITS_RFC_DIRECT, ENABLE_PI);
-		rtw_write32_mask(rtwdev, REG_WLRF1, BITS_RFC_DIRECT, ENABLE_PI);
-	}
-
 	return true;
 }
 
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.c b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
index 7c8db951a5bc..4231f94d515e 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
@@ -1289,6 +1289,17 @@  static int rtw8822c_mac_init(struct rtw_dev *rtwdev)
 	return 0;
 }
 
+static void rtw8822c_rstb_3wire(struct rtw_dev *rtwdev, bool enable)
+{
+	if (enable) {
+		rtw_write32_mask(rtwdev, REG_RSTB, BIT_RSTB_3WIRE, 0x1);
+		rtw_write32_mask(rtwdev, REG_ANAPAR_A, BIT_ANAPAR_UPDATE, 0x1);
+		rtw_write32_mask(rtwdev, REG_ANAPAR_B, BIT_ANAPAR_UPDATE, 0x1);
+	} else {
+		rtw_write32_mask(rtwdev, REG_RSTB, BIT_RSTB_3WIRE, 0x0);
+	}
+}
+
 static void rtw8822c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
 {
 #define RF18_BAND_MASK		(BIT(16) | BIT(9) | BIT(8))
@@ -1337,6 +1348,8 @@  static void rtw8822c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
 		break;
 	}
 
+	rtw8822c_rstb_3wire(rtwdev, false);
+
 	rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTWE2, 0x04, 0x01);
 	rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTWA, 0x1f, 0x12);
 	rtw_write_rf(rtwdev, RF_PATH_A, RF_LUTWD0, 0xfffff, rf_rxbb);
@@ -1349,6 +1362,8 @@  static void rtw8822c_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
 
 	rtw_write_rf(rtwdev, RF_PATH_A, RF_CFGCH, RFREG_MASK, rf_reg18);
 	rtw_write_rf(rtwdev, RF_PATH_B, RF_CFGCH, RFREG_MASK, rf_reg18);
+
+	rtw8822c_rstb_3wire(rtwdev, true);
 }
 
 static void rtw8822c_toggle_igi(struct rtw_dev *rtwdev)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.h b/drivers/net/wireless/realtek/rtw88/rtw8822c.h
index abd9f300bedd..dfd8662a0c0e 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822c.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.h
@@ -190,6 +190,8 @@  const struct rtw_table name ## _tbl = {			\
 #define BIT_3WIRE_TX_EN		BIT(0)
 #define BIT_3WIRE_RX_EN		BIT(1)
 #define BIT_3WIRE_PI_ON		BIT(28)
+#define REG_ANAPAR_A	0x1830
+#define BIT_ANAPAR_UPDATE	BIT(29)
 #define REG_RXAGCCTL0	0x18ac
 #define BITS_RXAGC_CCK		GENMASK(15, 12)
 #define BITS_RXAGC_OFDM		GENMASK(8, 4)
@@ -223,6 +225,8 @@  const struct rtw_table name ## _tbl = {			\
 #define BIT_CCK_BLK_EN		BIT(1)
 #define BIT_CCK_OFDM_BLK_EN	(BIT(0) | BIT(1))
 #define REG_CCAMSK	0x1c80
+#define REG_RSTB	0x1c90
+#define BIT_RSTB_3WIRE		BIT(8)
 #define REG_RX_BREAK	0x1d2c
 #define BIT_COM_RX_GCK_EN	BIT(31)
 #define REG_RXFNCTL	0x1d30
@@ -243,6 +247,7 @@  const struct rtw_table name ## _tbl = {			\
 #define REG_OFDM_TXCNT	0x2de0
 #define REG_ORITXCODE2	0x4100
 #define REG_3WIRE2	0x410c
+#define REG_ANAPAR_B	0x4130
 #define REG_RXAGCCTL	0x41ac
 #define REG_DCKB_I_0	0x41bc
 #define REG_DCKB_I_1	0x41c0