| 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 |
> -----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 >
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 >> >
> -----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 --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__ */
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