diff mbox series

[4/9] wifi: rtw88: Add rtw8703b.h

Message ID 20240202121050.977223-5-fiona.klute@gmx.de (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series rtw88: Add support for RTL8723CS/RTL8703B | expand

Commit Message

Fiona Klute Feb. 2, 2024, 12:10 p.m. UTC
This is the main header for the new rtw88_8703b chip driver.

Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
---
 drivers/net/wireless/realtek/rtw88/rtw8703b.h | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8703b.h

--
2.43.0

Comments

Ping-Ke Shih Feb. 5, 2024, 2:24 a.m. UTC | #1
> -----Original Message-----
> From: Fiona Klute <fiona.klute@gmx.de>
> Sent: Friday, February 2, 2024 8:11 PM
> To: linux-wireless@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>
> Cc: Kalle Valo <kvalo@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; Pavel
> Machek <pavel@ucw.cz>; Ondřej Jirman <megi@xff.cz>; Fiona Klute <fiona.klute@gmx.de>
> Subject: [PATCH 4/9] wifi: rtw88: Add rtw8703b.h
> 
> This is the main header for the new rtw88_8703b chip driver.
> 
> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
> ---
>  drivers/net/wireless/realtek/rtw88/rtw8703b.h | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8703b.h
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> new file mode 100644
> index 0000000000..f5ff23f2ee
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
> +/* Copyright Fiona Klute <fiona.klute@gmx.de> */
> +
> +#ifndef __RTW8703B_H__
> +#define __RTW8703B_H__
> +
> +extern const struct rtw_chip_info rtw8703b_hw_spec;
> +
> +/* phy status parsing */
> +#define GET_PHY_STAT_AGC_GAIN_A(phy_stat)                                   \
> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x00), GENMASK(6, 0)))

We are planning to use struct and le32_get_bits() directly, so don't introduce
this old style anymore. An example is

struct rtw8703b_phy_stat {
	__le32 w0;
	__le32 w1;
	...
};

#define RTW8703B_PHY_STAT_W0_AGC_GAIN_A GENMASK(6, 0)

val_s8 = le32_get_bits(stat->w0, RTW8703B_PHY_STAT_W0_AGC_GAIN_A);

> +
> +#define GET_PHY_STAT_PWDB(phy_stat)                                         \
> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(7, 0))
> +#define GET_PHY_STAT_VGA(phy_stat)                                          \
> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(12, 8))
> +#define GET_PHY_STAT_LNA_L(phy_stat)                                        \
> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(15, 13))
> +/* the high LNA stat bit if 4 bit format is used */
> +#define GET_PHY_STAT_LNA_H(phy_stat)                                        \
> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), BIT(23))
> +#define BIT_LNA_H_MASK BIT(3)
> +#define BIT_LNA_L_MASK GENMASK(2, 0)
> +
> +#define GET_PHY_STAT_CFO_TAIL_A(phy_stat)                                   \
> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x02), GENMASK(15, 8)))
> +#define GET_PHY_STAT_RXEVM_A(phy_stat)                                      \
> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x03), GENMASK(15, 8)))
> +#define GET_PHY_STAT_RXSNR_A(phy_stat)                                      \
> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x03), GENMASK(31, 24)))
> +
> +/* Baseband registers */
> +#define REG_BB_PWR_SAV5_11N 0x0818
> +/* BIT(11) should be 1 for 8703B *and* 8723D, which means LNA uses 4
> + * bit for CCK rates in report, not 3. Vendor driver logs a warning if
> + * it's 0, but handles the case.
> + *
> + * Purpose of other parts of this register is unknown, 8723cs driver
> + * code indicates some other chips use certain bits for antenna
> + * diversity.
> + */
> +#define REG_BB_AMP 0x0950
> +#define BIT_MASK_RX_LNA (BIT(11))
> +
> +/* 0xaXX: 40MHz channel settings */
> +#define REG_CCK_TXSF2 0x0a24  /* CCK TX filter 2 */
> +#define REG_CCK_DBG 0x0a28  /* debug port */
> +#define REG_OFDM0_A_TX_AFE 0x0c84
> +#define REG_TXIQK_MATRIXB_LSB2_11N 0x0c9c
> +#define REG_OFDM0_TX_PSD_NOISE 0x0ce4  /* TX pseudo noise weighting */
> +/* is != 0 when IQK is done */

Is this comment for 0x0e90? move to rear of the line?

> +#define REG_IQK_RDY 0x0e90
> +
> +/* RF registers */
> +#define RF_RCK1 0x1E
> +
> +#define AGG_BURST_NUM 3
> +#define AGG_BURST_SIZE 0 /* 1K */
> +#define BIT_MASK_AGG_BURST_NUM (GENMASK(3, 2))
> +#define BIT_MASK_AGG_BURST_SIZE (GENMASK(5, 4))
> +
> +#endif /* __RTW8703B_H__ */
> --
> 2.43.0
>
Fiona Klute Feb. 6, 2024, 1:08 a.m. UTC | #2
Am 05.02.24 um 03:24 schrieb Ping-Ke Shih:
>> -----Original Message-----
>> From: Fiona Klute <fiona.klute@gmx.de>
>> Sent: Friday, February 2, 2024 8:11 PM
>> To: linux-wireless@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>
>> Cc: Kalle Valo <kvalo@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; Pavel
>> Machek <pavel@ucw.cz>; Ondřej Jirman <megi@xff.cz>; Fiona Klute <fiona.klute@gmx.de>
>> Subject: [PATCH 4/9] wifi: rtw88: Add rtw8703b.h
>>
>> This is the main header for the new rtw88_8703b chip driver.
>>
>> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
>> ---
>>   drivers/net/wireless/realtek/rtw88/rtw8703b.h | 62 +++++++++++++++++++
>>   1 file changed, 62 insertions(+)
>>   create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8703b.h
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.h
>> b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
>> new file mode 100644
>> index 0000000000..f5ff23f2ee
>> --- /dev/null
>> +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
>> @@ -0,0 +1,62 @@
>> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
>> +/* Copyright Fiona Klute <fiona.klute@gmx.de> */
>> +
>> +#ifndef __RTW8703B_H__
>> +#define __RTW8703B_H__
>> +
>> +extern const struct rtw_chip_info rtw8703b_hw_spec;
>> +
>> +/* phy status parsing */
>> +#define GET_PHY_STAT_AGC_GAIN_A(phy_stat)                                   \
>> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x00), GENMASK(6, 0)))
>
> We are planning to use struct and le32_get_bits() directly, so don't introduce
> this old style anymore. An example is
>
> struct rtw8703b_phy_stat {
> 	__le32 w0;
> 	__le32 w1;
> 	...
> };
>
> #define RTW8703B_PHY_STAT_W0_AGC_GAIN_A GENMASK(6, 0)
>
> val_s8 = le32_get_bits(stat->w0, RTW8703B_PHY_STAT_W0_AGC_GAIN_A);

Sorry, of all your mails this one got stuck in the spam filter. This
answers my question about what you meant by struct style, I hadn't
thought of using __le types. I assume you'd still want to use
appropriately sized types/arrays for elements that are multiples of one
byte?

>> +
>> +#define GET_PHY_STAT_PWDB(phy_stat)                                         \
>> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(7, 0))
>> +#define GET_PHY_STAT_VGA(phy_stat)                                          \
>> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(12, 8))
>> +#define GET_PHY_STAT_LNA_L(phy_stat)                                        \
>> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(15, 13))
>> +/* the high LNA stat bit if 4 bit format is used */
>> +#define GET_PHY_STAT_LNA_H(phy_stat)                                        \
>> +       le32_get_bits(*((__le32 *)(phy_stat) + 0x01), BIT(23))
>> +#define BIT_LNA_H_MASK BIT(3)
>> +#define BIT_LNA_L_MASK GENMASK(2, 0)
>> +
>> +#define GET_PHY_STAT_CFO_TAIL_A(phy_stat)                                   \
>> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x02), GENMASK(15, 8)))
>> +#define GET_PHY_STAT_RXEVM_A(phy_stat)                                      \
>> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x03), GENMASK(15, 8)))
>> +#define GET_PHY_STAT_RXSNR_A(phy_stat)                                      \
>> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x03), GENMASK(31, 24)))
>> +
>> +/* Baseband registers */
>> +#define REG_BB_PWR_SAV5_11N 0x0818
>> +/* BIT(11) should be 1 for 8703B *and* 8723D, which means LNA uses 4
>> + * bit for CCK rates in report, not 3. Vendor driver logs a warning if
>> + * it's 0, but handles the case.
>> + *
>> + * Purpose of other parts of this register is unknown, 8723cs driver
>> + * code indicates some other chips use certain bits for antenna
>> + * diversity.
>> + */
>> +#define REG_BB_AMP 0x0950
>> +#define BIT_MASK_RX_LNA (BIT(11))
>> +
>> +/* 0xaXX: 40MHz channel settings */
>> +#define REG_CCK_TXSF2 0x0a24  /* CCK TX filter 2 */
>> +#define REG_CCK_DBG 0x0a28  /* debug port */
>> +#define REG_OFDM0_A_TX_AFE 0x0c84
>> +#define REG_TXIQK_MATRIXB_LSB2_11N 0x0c9c
>> +#define REG_OFDM0_TX_PSD_NOISE 0x0ce4  /* TX pseudo noise weighting */
>> +/* is != 0 when IQK is done */
>
> Is this comment for 0x0e90? move to rear of the line?

Yes, I'll do that.

>> +#define REG_IQK_RDY 0x0e90
>> +
>> +/* RF registers */
>> +#define RF_RCK1 0x1E
>> +
>> +#define AGG_BURST_NUM 3
>> +#define AGG_BURST_SIZE 0 /* 1K */
>> +#define BIT_MASK_AGG_BURST_NUM (GENMASK(3, 2))
>> +#define BIT_MASK_AGG_BURST_SIZE (GENMASK(5, 4))
>> +
>> +#endif /* __RTW8703B_H__ */
>> --
>> 2.43.0
>>
>
Ping-Ke Shih Feb. 6, 2024, 2:02 a.m. UTC | #3
> -----Original Message-----
> From: Fiona Klute <fiona.klute@gmx.de>
> Sent: Tuesday, February 6, 2024 9:09 AM
> To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org
> Cc: Kalle Valo <kvalo@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org; Pavel
> Machek <pavel@ucw.cz>; Ondřej Jirman <megi@xff.cz>
> Subject: Re: [PATCH 4/9] wifi: rtw88: Add rtw8703b.h
> 
> Am 05.02.24 um 03:24 schrieb Ping-Ke Shih:
> >> -----Original Message-----
> >> From: Fiona Klute <fiona.klute@gmx.de>
> >> Sent: Friday, February 2, 2024 8:11 PM
> >> To: linux-wireless@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>
> >> Cc: Kalle Valo <kvalo@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org;
> Pavel
> >> Machek <pavel@ucw.cz>; Ondřej Jirman <megi@xff.cz>; Fiona Klute <fiona.klute@gmx.de>
> >> Subject: [PATCH 4/9] wifi: rtw88: Add rtw8703b.h
> >>
> >> This is the main header for the new rtw88_8703b chip driver.
> >>
> >> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
> >> ---
> >>   drivers/net/wireless/realtek/rtw88/rtw8703b.h | 62 +++++++++++++++++++
> >>   1 file changed, 62 insertions(+)
> >>   create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8703b.h
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> >> b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> >> new file mode 100644
> >> index 0000000000..f5ff23f2ee
> >> --- /dev/null
> >> +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> >> @@ -0,0 +1,62 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
> >> +/* Copyright Fiona Klute <fiona.klute@gmx.de> */
> >> +
> >> +#ifndef __RTW8703B_H__
> >> +#define __RTW8703B_H__
> >> +
> >> +extern const struct rtw_chip_info rtw8703b_hw_spec;
> >> +
> >> +/* phy status parsing */
> >> +#define GET_PHY_STAT_AGC_GAIN_A(phy_stat)                                   \
> >> +       (le32_get_bits(*((__le32 *)(phy_stat) + 0x00), GENMASK(6, 0)))
> >
> > We are planning to use struct and le32_get_bits() directly, so don't introduce
> > this old style anymore. An example is
> >
> > struct rtw8703b_phy_stat {
> >       __le32 w0;
> >       __le32 w1;
> >       ...
> > };
> >
> > #define RTW8703B_PHY_STAT_W0_AGC_GAIN_A GENMASK(6, 0)
> >
> > val_s8 = le32_get_bits(stat->w0, RTW8703B_PHY_STAT_W0_AGC_GAIN_A);
> 
> Sorry, of all your mails this one got stuck in the spam filter. This
> answers my question about what you meant by struct style, I hadn't
> thought of using __le types. I assume you'd still want to use
> appropriately sized types/arrays for elements that are multiples of one
> byte?

Not sure "multiples of one byte" means. I guess you want to use something like
u8 or __le16 for the elements that aren't required bit access, right? 
I'd say it is hard to define single one rule for all cases. 

Example 1-1 (fake): 
struct rtw8703b_phy_stat {
	u8 mac_id;
	u8 rssi;
	u8 evm;
	u8 evm_2;
	...
} __packed;

Example 1-2 (fake): 
struct rtw8703b_phy_stat {
	__le32 w0;
	...
} __packed;

#define PHY_STAT_W0_MACID GENMASK(7, 0)
#define PHY_STAT_W0_RSSI GENMASK(15, 8)
#define PHY_STAT_W0_EVM GENMASK(23, 16)
#define PHY_STAT_W0_EVM_2 GENMASK(31,24)

It is clear that example 1-1 is better than 1-2. 

Example 2-1 (fake): 
struct rtw8703b_phy_stat {
	u8 mac_id;
	u8 rssi;
	__le16 phy_st;		// evm: 7, evm_2: 7, rsvd :2
	...
} __packed;

#define PHY_ST_EVM GENMASK(6, 0)
#define PHY_ST_EVM_2 GENMASK(13, 7)


Example 2-2 (fake): 
struct rtw8703b_phy_stat {
	__le32 w0;
	...
} __packed;

#define PHY_STAT_W0_MACID GENMASK(7, 0)
#define PHY_STAT_W0_RSSI GENMASK(15, 8)
#define PHY_STAT_W0_EVM GENMASK(22, 16)
#define PHY_STAT_W0_EVM_2 GENMASK(29, 23)


Compare 2-1 and 2-2, it would be hard to say which one is better, because 2-1
mixes u8 and bit field. This is just a simple example, fields of real struct
are much more, so normally I use 1-2 and 2-2 styles.
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.h b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
new file mode 100644
index 0000000000..f5ff23f2ee
--- /dev/null
+++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
@@ -0,0 +1,62 @@ 
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
+/* Copyright Fiona Klute <fiona.klute@gmx.de> */
+
+#ifndef __RTW8703B_H__
+#define __RTW8703B_H__
+
+extern const struct rtw_chip_info rtw8703b_hw_spec;
+
+/* phy status parsing */
+#define GET_PHY_STAT_AGC_GAIN_A(phy_stat)                                   \
+	(le32_get_bits(*((__le32 *)(phy_stat) + 0x00), GENMASK(6, 0)))
+
+#define GET_PHY_STAT_PWDB(phy_stat)                                         \
+	le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(7, 0))
+#define GET_PHY_STAT_VGA(phy_stat)                                          \
+	le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(12, 8))
+#define GET_PHY_STAT_LNA_L(phy_stat)                                        \
+	le32_get_bits(*((__le32 *)(phy_stat) + 0x01), GENMASK(15, 13))
+/* the high LNA stat bit if 4 bit format is used */
+#define GET_PHY_STAT_LNA_H(phy_stat)                                        \
+	le32_get_bits(*((__le32 *)(phy_stat) + 0x01), BIT(23))
+#define BIT_LNA_H_MASK BIT(3)
+#define BIT_LNA_L_MASK GENMASK(2, 0)
+
+#define GET_PHY_STAT_CFO_TAIL_A(phy_stat)                                   \
+	(le32_get_bits(*((__le32 *)(phy_stat) + 0x02), GENMASK(15, 8)))
+#define GET_PHY_STAT_RXEVM_A(phy_stat)                                      \
+	(le32_get_bits(*((__le32 *)(phy_stat) + 0x03), GENMASK(15, 8)))
+#define GET_PHY_STAT_RXSNR_A(phy_stat)                                      \
+	(le32_get_bits(*((__le32 *)(phy_stat) + 0x03), GENMASK(31, 24)))
+
+/* Baseband registers */
+#define REG_BB_PWR_SAV5_11N 0x0818
+/* BIT(11) should be 1 for 8703B *and* 8723D, which means LNA uses 4
+ * bit for CCK rates in report, not 3. Vendor driver logs a warning if
+ * it's 0, but handles the case.
+ *
+ * Purpose of other parts of this register is unknown, 8723cs driver
+ * code indicates some other chips use certain bits for antenna
+ * diversity.
+ */
+#define REG_BB_AMP 0x0950
+#define BIT_MASK_RX_LNA (BIT(11))
+
+/* 0xaXX: 40MHz channel settings */
+#define REG_CCK_TXSF2 0x0a24  /* CCK TX filter 2 */
+#define REG_CCK_DBG 0x0a28  /* debug port */
+#define REG_OFDM0_A_TX_AFE 0x0c84
+#define REG_TXIQK_MATRIXB_LSB2_11N 0x0c9c
+#define REG_OFDM0_TX_PSD_NOISE 0x0ce4  /* TX pseudo noise weighting */
+/* is != 0 when IQK is done */
+#define REG_IQK_RDY 0x0e90
+
+/* RF registers */
+#define RF_RCK1 0x1E
+
+#define AGG_BURST_NUM 3
+#define AGG_BURST_SIZE 0 /* 1K */
+#define BIT_MASK_AGG_BURST_NUM (GENMASK(3, 2))
+#define BIT_MASK_AGG_BURST_SIZE (GENMASK(5, 4))
+
+#endif /* __RTW8703B_H__ */