diff mbox series

[1/3] ieee80211: add trigger frame definition

Message ID 20220601013917.18814-2-pkshih@realtek.com
State Superseded
Headers show
Series rtw89: use count of trigger frames as a clue to accelerate CFO tracking | expand

Commit Message

Ping-Ke Shih June 1, 2022, 1:39 a.m. UTC
From: Po Hao Huang <phhuang@realtek.com>

Define trigger stype of control frame, and its checking function, struct
and trigger type within common_info of trigger.

Signed-off-by: Po Hao Huang <phhuang@realtek.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 include/linux/ieee80211.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Kalle Valo June 8, 2022, 8:06 a.m. UTC | #1
Ping-Ke Shih <pkshih@realtek.com> writes:

> From: Po Hao Huang <phhuang@realtek.com>
>
> Define trigger stype of control frame, and its checking function, struct
> and trigger type within common_info of trigger.
>
> Signed-off-by: Po Hao Huang <phhuang@realtek.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  include/linux/ieee80211.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> index 75d40acb60c1c..f3b2d5b56d643 100644
> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -76,6 +76,7 @@
>  #define IEEE80211_STYPE_ACTION		0x00D0
>  
>  /* control */
> +#define IEEE80211_STYPE_TRIGGER		0x0020
>  #define IEEE80211_STYPE_CTL_EXT		0x0060
>  #define IEEE80211_STYPE_BACK_REQ	0x0080
>  #define IEEE80211_STYPE_BACK		0x0090
> @@ -295,6 +296,17 @@ static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
>  
>  #define IEEE80211_HT_CTL_LEN		4
>  
> +/* trigger type within common_info of trigger frame */
> +#define IEEE80211_TRIGGER_TYPE_MASK		0xfULL
> +#define IEEE80211_TRIGGER_TYPE_BASIC		0x0
> +#define IEEE80211_TRIGGER_TYPE_BFRP		0x1
> +#define IEEE80211_TRIGGER_TYPE_MU_BAR		0x2
> +#define IEEE80211_TRIGGER_TYPE_MU_RTS		0x3
> +#define IEEE80211_TRIGGER_TYPE_BSRP		0x4
> +#define IEEE80211_TRIGGER_TYPE_GCR_MU_BAR	0x5
> +#define IEEE80211_TRIGGER_TYPE_BQRP		0x6
> +#define IEEE80211_TRIGGER_TYPE_NFRP		0x7

Why ULL in the mask? I don't see it used anywhere else in the file.
Ping-Ke Shih June 8, 2022, 8:13 a.m. UTC | #2
> -----Original Message-----
> From: Kalle Valo <kvalo@kernel.org>
> Sent: Wednesday, June 8, 2022 4:07 PM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: johannes@sipsolutions.net; linux-wireless@vger.kernel.org; Eric Huang <echuang@realtek.com>; Bernie
> Huang <phhuang@realtek.com>
> Subject: Re: [PATCH 1/3] ieee80211: add trigger frame definition
> 
> Ping-Ke Shih <pkshih@realtek.com> writes:
> 
> > From: Po Hao Huang <phhuang@realtek.com>
> >
> > Define trigger stype of control frame, and its checking function, struct
> > and trigger type within common_info of trigger.
> >
> > Signed-off-by: Po Hao Huang <phhuang@realtek.com>
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > ---
> >  include/linux/ieee80211.h | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >
> > diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> > index 75d40acb60c1c..f3b2d5b56d643 100644
> > --- a/include/linux/ieee80211.h
> > +++ b/include/linux/ieee80211.h
> > @@ -76,6 +76,7 @@
> >  #define IEEE80211_STYPE_ACTION		0x00D0
> >
> >  /* control */
> > +#define IEEE80211_STYPE_TRIGGER		0x0020
> >  #define IEEE80211_STYPE_CTL_EXT		0x0060
> >  #define IEEE80211_STYPE_BACK_REQ	0x0080
> >  #define IEEE80211_STYPE_BACK		0x0090
> > @@ -295,6 +296,17 @@ static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
> >
> >  #define IEEE80211_HT_CTL_LEN		4
> >
> > +/* trigger type within common_info of trigger frame */
> > +#define IEEE80211_TRIGGER_TYPE_MASK		0xfULL
> > +#define IEEE80211_TRIGGER_TYPE_BASIC		0x0
> > +#define IEEE80211_TRIGGER_TYPE_BFRP		0x1
> > +#define IEEE80211_TRIGGER_TYPE_MU_BAR		0x2
> > +#define IEEE80211_TRIGGER_TYPE_MU_RTS		0x3
> > +#define IEEE80211_TRIGGER_TYPE_BSRP		0x4
> > +#define IEEE80211_TRIGGER_TYPE_GCR_MU_BAR	0x5
> > +#define IEEE80211_TRIGGER_TYPE_BQRP		0x6
> > +#define IEEE80211_TRIGGER_TYPE_NFRP		0x7
> 
> Why ULL in the mask? I don't see it used anywhere else in the file.
> 

This is because common_info in ieee80211_trigger is __le64:

+struct ieee80211_trigger {
	...
+	__le64 common_info;
	...
+}

Then, 
	type = le64_get_bits(tf->common_info, IEEE80211_TRIGGER_TYPE_MASK);
is used to access trigger type of common_info in patch 2/3.

Ping-Ke
Johannes Berg June 8, 2022, 8:14 a.m. UTC | #3
On Wed, 2022-06-08 at 08:13 +0000, Ping-Ke Shih wrote:
> 
> 
> This is because common_info in ieee80211_trigger is __le64:
> 
> +struct ieee80211_trigger {
> 	...
> +	__le64 common_info;
> 	...
> +}
> 
> Then, 
> 	type = le64_get_bits(tf->common_info,
> IEEE80211_TRIGGER_TYPE_MASK);
> is used to access trigger type of common_info in patch 2/3.

Yep, but you have (after macro expansion)

static __always_inline u64 le64_get_bits(__le64 v, u64 field)
{
	return ...;
}


so you can easily pass any number as the 'field' argument, it doesn't
need to already be ULL :-)

johannes
diff mbox series

Patch

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 75d40acb60c1c..f3b2d5b56d643 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -76,6 +76,7 @@ 
 #define IEEE80211_STYPE_ACTION		0x00D0
 
 /* control */
+#define IEEE80211_STYPE_TRIGGER		0x0020
 #define IEEE80211_STYPE_CTL_EXT		0x0060
 #define IEEE80211_STYPE_BACK_REQ	0x0080
 #define IEEE80211_STYPE_BACK		0x0090
@@ -295,6 +296,17 @@  static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
 
 #define IEEE80211_HT_CTL_LEN		4
 
+/* trigger type within common_info of trigger frame */
+#define IEEE80211_TRIGGER_TYPE_MASK		0xfULL
+#define IEEE80211_TRIGGER_TYPE_BASIC		0x0
+#define IEEE80211_TRIGGER_TYPE_BFRP		0x1
+#define IEEE80211_TRIGGER_TYPE_MU_BAR		0x2
+#define IEEE80211_TRIGGER_TYPE_MU_RTS		0x3
+#define IEEE80211_TRIGGER_TYPE_BSRP		0x4
+#define IEEE80211_TRIGGER_TYPE_GCR_MU_BAR	0x5
+#define IEEE80211_TRIGGER_TYPE_BQRP		0x6
+#define IEEE80211_TRIGGER_TYPE_NFRP		0x7
+
 struct ieee80211_hdr {
 	__le16 frame_control;
 	__le16 duration_id;
@@ -324,6 +336,15 @@  struct ieee80211_qos_hdr {
 	__le16 qos_ctrl;
 } __packed __aligned(2);
 
+struct ieee80211_trigger {
+	__le16 frame_control;
+	__le16 duration;
+	u8 ra[ETH_ALEN];
+	u8 ta[ETH_ALEN];
+	__le64 common_info;
+	u8 variable[];
+} __packed __aligned(2);
+
 /**
  * ieee80211_has_tods - check if IEEE80211_FCTL_TODS is set
  * @fc: frame control bytes in little-endian byteorder
@@ -729,6 +750,16 @@  static inline bool ieee80211_is_qos_nullfunc(__le16 fc)
 	       cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_QOS_NULLFUNC);
 }
 
+/**
+ * ieee80211_is_trigger - check if frame is trigger frame
+ * @fc: frame control field in little-endian byteorder
+ */
+static inline bool ieee80211_is_trigger(__le16 fc)
+{
+	return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | IEEE80211_FCTL_STYPE)) ==
+	       cpu_to_le16(IEEE80211_FTYPE_CTL | IEEE80211_STYPE_TRIGGER);
+}
+
 /**
  * ieee80211_is_any_nullfunc - check if frame is regular or QoS nullfunc frame
  * @fc: frame control bytes in little-endian byteorder