diff mbox series

[01/11] mt76: mt7915: add comments about rx descriptor parsing

Message ID 20211118164056.2965-1-greearb@candelatech.com (mailing list archive)
State Rejected
Delegated to: Felix Fietkau
Headers show
Series [01/11] mt76: mt7915: add comments about rx descriptor parsing | expand

Commit Message

Ben Greear Nov. 18, 2021, 4:40 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

This is a tricky beast to understand, so add some notes for
next time someone is looking at this code and trying to compare
against documents.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

txs and related patches rebased on top of 5.16

 .../net/wireless/mediatek/mt76/mt7915/mac.c   | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Lorenzo Bianconi Nov. 19, 2021, 4:33 p.m. UTC | #1 
> From: Ben Greear <greearb@candelatech.com>
> 
> This is a tricky beast to understand, so add some notes for
> next time someone is looking at this code and trying to compare
> against documents.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> 
> txs and related patches rebased on top of 5.16
> 
>  .../net/wireless/mediatek/mt76/mt7915/mac.c   | 23 +++++++++++++++----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
> index 8e5b87af2efb..6fbd58ae9c88 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
> @@ -410,6 +410,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>  	__le32 *rxd = (__le32 *)skb->data;
>  	__le32 *rxv = NULL;
>  	u32 mode = 0;
> +	/* table "PP -> HOST / X-CPU"  RX Format */
>  	u32 rxd0 = le32_to_cpu(rxd[0]);
>  	u32 rxd1 = le32_to_cpu(rxd[1]);
>  	u32 rxd2 = le32_to_cpu(rxd[2]);
> @@ -489,7 +490,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>  
>  	rxd += 6;
>  	if (rxd1 & MT_RXD1_NORMAL_GROUP_4) {
> -		u32 v0 = le32_to_cpu(rxd[0]);
> +		u32 v0 = le32_to_cpu(rxd[0]); /* DW6 */
>  		u32 v2 = le32_to_cpu(rxd[2]);
>  
>  		fc = cpu_to_le16(FIELD_GET(MT_RXD6_FRAME_CONTROL, v0));
> @@ -502,6 +503,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>  	}
>  
>  	if (rxd1 & MT_RXD1_NORMAL_GROUP_1) {
> +		/* DW10, assuming Group-4 enabled */
>  		u8 *data = (u8 *)rxd;
>  
>  		if (status->flag & RX_FLAG_DECRYPTED) {
> @@ -533,6 +535,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>  	}
>  
>  	if (rxd1 & MT_RXD1_NORMAL_GROUP_2) {
> +		/* DW14, assuming group-1,4 */
>  		status->timestamp = le32_to_cpu(rxd[0]);
>  		status->flag |= RX_FLAG_MACTIME_START;
>  
> @@ -558,18 +561,22 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>  	if (rxd1 & MT_RXD1_NORMAL_GROUP_3) {
>  		u32 v0, v1, v2;
>  
> -		rxv = rxd;
> +		rxv = rxd; /* DW16 assuming group 1,2,3,4 */
>  		rxd += 2;
>  		if ((u8 *)rxd - skb->data >= skb->len)
>  			return -EINVAL;
>  
> -		v0 = le32_to_cpu(rxv[0]);
> +		v0 = le32_to_cpu(rxv[0]);  /* DW16, P-VEC1 31:0 */
> +		/* DW17, RX_RCPI copied over P-VEC 64:32 Per RX Format doc. */

This seems not so useful for everybody.

>  		v1 = le32_to_cpu(rxv[1]);
> -		v2 = le32_to_cpu(rxv[2]);
> +		v2 = le32_to_cpu(rxv[2]); /* first DW of group-5, C-RXV */
>  
>  		if (v0 & MT_PRXV_HT_AD_CODE)
>  			status->enc_flags |= RX_ENC_FLAG_LDPC;
>  
> +		/* TODO:  When group-5 is enabled, use nss (and stbc) to
> +		 * calculate chains properly for this particular skb.
> +		 */
>  		status->chains = mphy->antenna_mask;
>  		status->chain_signal[0] = to_rssi(MT_PRXV_RCPI0, v1);
>  		status->chain_signal[1] = to_rssi(MT_PRXV_RCPI1, v1);
> @@ -581,12 +588,18 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>  			if (!(status->chains & BIT(i)))
>  				continue;
>  
> +			/* TODO:  Use db sum logic instead of max. */

This seems just a not for you :)

>  			status->signal = max(status->signal,
>  					     status->chain_signal[i]);
>  		}
>  
> -		/* RXD Group 5 - C-RXV */
> +		/* RXD Group 5 - C-RXV.
> +		 * Group 5 Not currently enabled for 7915 except in
> +		 * monitor mode.
> +		 *   See MT_DMA_DCR0_RXD_G5_EN
> +		 */
>  		if (rxd1 & MT_RXD1_NORMAL_GROUP_5) {
> +			/* See RXV document ... */

This is not so useful :)

>  			u8 stbc = FIELD_GET(MT_CRXV_HT_STBC, v2);
>  			u8 gi = FIELD_GET(MT_CRXV_HT_SHORT_GI, v2);
>  			bool cck = false;
> -- 
> 2.20.1
>
Ben Greear Nov. 19, 2021, 4:41 p.m. UTC | #2 
On 11/19/21 8:33 AM, Lorenzo Bianconi wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> This is a tricky beast to understand, so add some notes for
>> next time someone is looking at this code and trying to compare
>> against documents.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>
>> txs and related patches rebased on top of 5.16
>>
>>   .../net/wireless/mediatek/mt76/mt7915/mac.c   | 23 +++++++++++++++----
>>   1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
>> index 8e5b87af2efb..6fbd58ae9c88 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
>> @@ -410,6 +410,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>>   	__le32 *rxd = (__le32 *)skb->data;
>>   	__le32 *rxv = NULL;
>>   	u32 mode = 0;
>> +	/* table "PP -> HOST / X-CPU"  RX Format */
>>   	u32 rxd0 = le32_to_cpu(rxd[0]);
>>   	u32 rxd1 = le32_to_cpu(rxd[1]);
>>   	u32 rxd2 = le32_to_cpu(rxd[2]);
>> @@ -489,7 +490,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>>   
>>   	rxd += 6;
>>   	if (rxd1 & MT_RXD1_NORMAL_GROUP_4) {
>> -		u32 v0 = le32_to_cpu(rxd[0]);
>> +		u32 v0 = le32_to_cpu(rxd[0]); /* DW6 */
>>   		u32 v2 = le32_to_cpu(rxd[2]);
>>   
>>   		fc = cpu_to_le16(FIELD_GET(MT_RXD6_FRAME_CONTROL, v0));
>> @@ -502,6 +503,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>>   	}
>>   
>>   	if (rxd1 & MT_RXD1_NORMAL_GROUP_1) {
>> +		/* DW10, assuming Group-4 enabled */
>>   		u8 *data = (u8 *)rxd;
>>   
>>   		if (status->flag & RX_FLAG_DECRYPTED) {
>> @@ -533,6 +535,7 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>>   	}
>>   
>>   	if (rxd1 & MT_RXD1_NORMAL_GROUP_2) {
>> +		/* DW14, assuming group-1,4 */
>>   		status->timestamp = le32_to_cpu(rxd[0]);
>>   		status->flag |= RX_FLAG_MACTIME_START;
>>   
>> @@ -558,18 +561,22 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>>   	if (rxd1 & MT_RXD1_NORMAL_GROUP_3) {
>>   		u32 v0, v1, v2;
>>   
>> -		rxv = rxd;
>> +		rxv = rxd; /* DW16 assuming group 1,2,3,4 */
>>   		rxd += 2;
>>   		if ((u8 *)rxd - skb->data >= skb->len)
>>   			return -EINVAL;
>>   
>> -		v0 = le32_to_cpu(rxv[0]);
>> +		v0 = le32_to_cpu(rxv[0]);  /* DW16, P-VEC1 31:0 */
>> +		/* DW17, RX_RCPI copied over P-VEC 64:32 Per RX Format doc. */
> 
> This seems not so useful for everybody.

It was put in for folks doing dev work on this driver, so just a handful
of people in the whole world.  It doesn't need to be useful for lots
of people.

> 
>>   		v1 = le32_to_cpu(rxv[1]);
>> -		v2 = le32_to_cpu(rxv[2]);
>> +		v2 = le32_to_cpu(rxv[2]); /* first DW of group-5, C-RXV */
>>   
>>   		if (v0 & MT_PRXV_HT_AD_CODE)
>>   			status->enc_flags |= RX_ENC_FLAG_LDPC;
>>   
>> +		/* TODO:  When group-5 is enabled, use nss (and stbc) to
>> +		 * calculate chains properly for this particular skb.
>> +		 */
>>   		status->chains = mphy->antenna_mask;
>>   		status->chain_signal[0] = to_rssi(MT_PRXV_RCPI0, v1);
>>   		status->chain_signal[1] = to_rssi(MT_PRXV_RCPI1, v1);
>> @@ -581,12 +588,18 @@ mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
>>   			if (!(status->chains & BIT(i)))
>>   				continue;
>>   
>> +			/* TODO:  Use db sum logic instead of max. */
> 
> This seems just a not for you :)

Anyone wanting proper RSSI.  The current implementation is wrong
and could be off by as much as 6db on a 4x4 radio.

I will fix it eventually if no one beats me to it.

Anyway, please apply anything you think is helpful, leave out
the rest.

Thanks,
Ben
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
index 8e5b87af2efb..6fbd58ae9c88 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mac.c
@@ -410,6 +410,7 @@  mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
 	__le32 *rxd = (__le32 *)skb->data;
 	__le32 *rxv = NULL;
 	u32 mode = 0;
+	/* table "PP -> HOST / X-CPU"  RX Format */
 	u32 rxd0 = le32_to_cpu(rxd[0]);
 	u32 rxd1 = le32_to_cpu(rxd[1]);
 	u32 rxd2 = le32_to_cpu(rxd[2]);
@@ -489,7 +490,7 @@  mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
 
 	rxd += 6;
 	if (rxd1 & MT_RXD1_NORMAL_GROUP_4) {
-		u32 v0 = le32_to_cpu(rxd[0]);
+		u32 v0 = le32_to_cpu(rxd[0]); /* DW6 */
 		u32 v2 = le32_to_cpu(rxd[2]);
 
 		fc = cpu_to_le16(FIELD_GET(MT_RXD6_FRAME_CONTROL, v0));
@@ -502,6 +503,7 @@  mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
 	}
 
 	if (rxd1 & MT_RXD1_NORMAL_GROUP_1) {
+		/* DW10, assuming Group-4 enabled */
 		u8 *data = (u8 *)rxd;
 
 		if (status->flag & RX_FLAG_DECRYPTED) {
@@ -533,6 +535,7 @@  mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
 	}
 
 	if (rxd1 & MT_RXD1_NORMAL_GROUP_2) {
+		/* DW14, assuming group-1,4 */
 		status->timestamp = le32_to_cpu(rxd[0]);
 		status->flag |= RX_FLAG_MACTIME_START;
 
@@ -558,18 +561,22 @@  mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
 	if (rxd1 & MT_RXD1_NORMAL_GROUP_3) {
 		u32 v0, v1, v2;
 
-		rxv = rxd;
+		rxv = rxd; /* DW16 assuming group 1,2,3,4 */
 		rxd += 2;
 		if ((u8 *)rxd - skb->data >= skb->len)
 			return -EINVAL;
 
-		v0 = le32_to_cpu(rxv[0]);
+		v0 = le32_to_cpu(rxv[0]);  /* DW16, P-VEC1 31:0 */
+		/* DW17, RX_RCPI copied over P-VEC 64:32 Per RX Format doc. */
 		v1 = le32_to_cpu(rxv[1]);
-		v2 = le32_to_cpu(rxv[2]);
+		v2 = le32_to_cpu(rxv[2]); /* first DW of group-5, C-RXV */
 
 		if (v0 & MT_PRXV_HT_AD_CODE)
 			status->enc_flags |= RX_ENC_FLAG_LDPC;
 
+		/* TODO:  When group-5 is enabled, use nss (and stbc) to
+		 * calculate chains properly for this particular skb.
+		 */
 		status->chains = mphy->antenna_mask;
 		status->chain_signal[0] = to_rssi(MT_PRXV_RCPI0, v1);
 		status->chain_signal[1] = to_rssi(MT_PRXV_RCPI1, v1);
@@ -581,12 +588,18 @@  mt7915_mac_fill_rx(struct mt7915_dev *dev, struct sk_buff *skb)
 			if (!(status->chains & BIT(i)))
 				continue;
 
+			/* TODO:  Use db sum logic instead of max. */
 			status->signal = max(status->signal,
 					     status->chain_signal[i]);
 		}
 
-		/* RXD Group 5 - C-RXV */
+		/* RXD Group 5 - C-RXV.
+		 * Group 5 Not currently enabled for 7915 except in
+		 * monitor mode.
+		 *   See MT_DMA_DCR0_RXD_G5_EN
+		 */
 		if (rxd1 & MT_RXD1_NORMAL_GROUP_5) {
+			/* See RXV document ... */
 			u8 stbc = FIELD_GET(MT_CRXV_HT_STBC, v2);
 			u8 gi = FIELD_GET(MT_CRXV_HT_SHORT_GI, v2);
 			bool cck = false;