Received-SPF: pass (google.com: domain of linux-wireless+bounces-4284-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Ping-Ke Shih <pkshih@realtek.com>
To: Fiona Klute <fiona.klute@gmx.de>,
        "linux-wireless@vger.kernel.org"
	<linux-wireless@vger.kernel.org>
CC: "kvalo@kernel.org" <kvalo@kernel.org>,
        "ulf.hansson@linaro.org"
	<ulf.hansson@linaro.org>,
        "linux-mmc@vger.kernel.org"
	<linux-mmc@vger.kernel.org>,
        "pavel@ucw.cz" <pavel@ucw.cz>, "megi@xff.cz"
	<megi@xff.cz>
Subject: RE: [PATCH v2 4/9] wifi: rtw88: Add rtw8703b.h
Thread-Topic: [PATCH v2 4/9] wifi: rtw88: Add rtw8703b.h
Thread-Index: AQHaadiMK/BK89PGSkKvgAdLkkf/lLEiJPZA
Date: Fri, 1 Mar 2024 02:09:25 +0000
Message-ID: <368ebc04c2ec436791a27b5661189783@realtek.com>
References: <20240227235507.781615-1-fiona.klute@gmx.de>
 <20240227235507.781615-5-fiona.klute@gmx.de>
In-Reply-To: <20240227235507.781615-5-fiona.klute@gmx.de>
Accept-Language: en-US, zh-TW
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0


> -----Original Message-----
> From: Fiona Klute <fiona.klute@gmx.de>
> Sent: Wednesday, February 28, 2024 7:55 AM
> To: linux-wireless@vger.kernel.org; Ping-Ke Shih <pkshih@realtek.com>
> Cc: Fiona Klute <fiona.klute@gmx.de>; kvalo@kernel.org; ulf.hansson@linar=
o.org; linux-mmc@vger.kernel.org;
> pavel@ucw.cz; megi@xff.cz
> Subject: [PATCH v2 4/9] wifi: rtw88: Add rtw8703b.h
>=20
> This is the main header for the new rtw88_8703b chip driver.
>=20
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
> ---
>  drivers/net/wireless/realtek/rtw88/rtw8703b.h | 103 ++++++++++++++++++
>  1 file changed, 103 insertions(+)
>  create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8703b.h
>=20
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> new file mode 100644
> index 00000000000..69dac101d33
> --- /dev/null
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.h
> @@ -0,0 +1,103 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
> +/* Copyright Fiona Klute <fiona.klute@gmx.de> */
> +
> +#ifndef __RTW8703B_H__
> +#define __RTW8703B_H__
> +
> +#include <linux/types.h>
> +#include <linux/compiler_attributes.h>

Removing these two headers can still compiled pass in my side. Please check=
 why
you need them.

By the way, rtw8723d.h does
   #include "rtw8723x.h"

Can we use the same stuff?


> +
> +extern const struct rtw_chip_info rtw8703b_hw_spec;
> +
> +/* phy status parsing */
> +#define VGA_BITS GENMASK(4, 0)
> +#define LNA_L_BITS GENMASK(7, 5)
> +#define LNA_H_BIT BIT(7)
> +/* masks for assembling LNA index from high and low bits */
> +#define BIT_LNA_H_MASK BIT(3)
> +#define BIT_LNA_L_MASK GENMASK(2, 0)
> +
> +struct phy_rx_agc_info {
> +#ifdef __LITTLE_ENDIAN
> +       u8 gain: 7;
> +       u8 trsw: 1;
> +#else
> +       u8 trsw: 1;
> +       u8 gain: 7;
> +#endif
> +} __packed;

This struct is quite simple, or I will suggest to define MASK and access th=
em
by u8_get_bits(), like

#define RX_AGC_GAIN GENMASK(6, 0)
gain =3D u8_get_bits(RX_AGC_GAIN, raw);

Then,=20
struct phy_status_8703b {
	u8 path_agc[2];
	...

Just for reference. You can decide if changing to this style.=20

> +
> +/* This struct is called phy_status_rpt_8192cd in the vendor driver,
> + * there might be potential to share it with drivers for other chips
> + * of the same generation.
> + */
> +struct phy_status_8703b {
> +       struct phy_rx_agc_info path_agc[2];
> +       u8 ch_corr[2];
> +       u8 cck_sig_qual_ofdm_pwdb_all;
> +       /* for CCK: bits 0:4: VGA index, bits 5:7: LNA index (low) */
> +       u8 cck_agc_rpt_ofdm_cfosho_a;
> +       /* for CCK: bit 7 is high bit of LNA index if long report type */
> +       u8 cck_rpt_b_ofdm_cfosho_b;
> +       u8 reserved_1;
> +       u8 noise_power_db_msb;
> +       s8 path_cfotail[2];
> +       u8 pcts_mask[2];
> +       s8 stream_rxevm[2];
> +       u8 path_rxsnr[2];
> +       u8 noise_power_db_lsb;
> +       u8 reserved_2[3];
> +       u8 stream_csi[2];
> +       u8 stream_target_csi[2];
> +       s8 sig_evm;
> +       u8 reserved_3;
> +
> +#ifdef __LITTLE_ENDIAN
> +       u8 antsel_rx_keep_2: 1;
> +       u8 sgi_en: 1;
> +       u8 rxsc: 2;
> +       u8 idle_long: 1;
> +       u8 r_ant_train_en: 1;
> +       u8 ant_sel_b: 1;
> +       u8 ant_sel: 1;
> +#else /* __BIG_ENDIAN */
> +       u8 ant_sel: 1;
> +       u8 ant_sel_b: 1;
> +       u8 r_ant_train_en: 1;
> +       u8 idle_long: 1;
> +       u8 rxsc: 2;
> +       u8 sgi_en: 1;
> +       u8 antsel_rx_keep_2: 1;
> +#endif
> +} __packed;
> +
> +/* 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 */
> +#define REG_IQK_RDY 0x0e90  /* is !=3D 0 when IQK is done */
> +
> +/* 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
>=20