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

[Ben Greear <greearb@xxxxxxxxxxxxxxx>]

On 11/19/21 8:33 AM, Lorenzo Bianconi wrote:
> > From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
> > ---
> >
> > 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



--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc  http://www.candelatech.com