Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Fiona Klute <fiona.klute@xxxxxx>
> Sent: Tuesday, February 6, 2024 9:09 AM
> To: Ping-Ke Shih <pkshih@xxxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx
> Cc: Kalle Valo <kvalo@xxxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Pavel
> Machek <pavel@xxxxxx>; Ondřej Jirman <megi@xxxxxx>
> 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@xxxxxx>
> >> Sent: Friday, February 2, 2024 8:11 PM
> >> To: linux-wireless@xxxxxxxxxxxxxxx; Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> >> Cc: Kalle Valo <kvalo@xxxxxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx;
> Pavel
> >> Machek <pavel@xxxxxx>; Ondřej Jirman <megi@xxxxxx>; Fiona Klute <fiona.klute@xxxxxx>
> >> 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@xxxxxx>
> >> ---
> >>   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@xxxxxx> */
> >> +
> >> +#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. 





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux