Re: [PATCH v7] ath10k: fix crash in recent 3.5.3 9984 firmware due wrong handling of peer_bw_rxnss_override parameter

[Sebastian Gottschall <s.gottschall@xxxxxxxxxx>]

> > +    /* only 4x4 configuration do support 2x2 for VHT160, everything 
> > else must use 1x1 */
> > +    if (ar->cfg_rx_chainmask == 15)
> > +        nss160 = arg->peer_num_spatial_streams <= 2 ? 
> > arg->peer_num_spatial_streams : 2;
>
> If peer nss == 3, then nss160 must be 1x1.  That is why I previously 
> suggested the code that set nss160 to equal nss / 2
> (with special case to bump nss160 to 1x1 if nss == 1.
btw. it doesnt matter if the peer sends with 3x3 or even 4x4, i still 
can receive with 2x2. thats no conflict. switching back to 1x1 of the 
peer sends vht160 with 3x3 makes no real sense
i dont have to turn off a chain, if i'm able todo 2x2, no matter what 
the peer does. i just have to limit the maximum

> A 9984 peer with chainmask configured to 0x7 would hit this case I think.
>
> Overall this looks better than previous patches though.
>
> Thanks,
> Ben
>
> > +
> > +    /* in case if peer is connected with vht160 or vht80+80, we need 
> > to properly adjust rxnss parameters otherwise firmware will raise a 
> > assert */
> > +    switch(arg->peer_phymode) {
> > +    case MODE_11AC_VHT80_80:
> > +        arg->peer_bw_rxnss_override = BW_NSS_FWCONF_80_80(nss160);
> > +    /* fall through */
> > +    case MODE_11AC_VHT160:
> > +        arg->peer_bw_rxnss_override |= BW_NSS_FWCONF_160(nss160);
> > +    break;
> > +    default:
> > +    break;
> >       }
> > +
> > +    ath10k_dbg(ar, ATH10K_DBG_MAC, "mac vht peer %pM max_mpdu %d 
> > flags 0x%x peer_bw_rxnss_override 0x%x\n",
> > +           sta->addr, arg->peer_max_mpdu, arg->peer_flags, 
> > arg->peer_bw_rxnss_override);
> >   }
> >
> >   static void ath10k_peer_assoc_h_qos(struct ath10k *ar,
> > @@ -2696,9 +2700,9 @@ static int ath10k_peer_assoc_prepare(struct 
> > ath10k *ar,
> >       ath10k_peer_assoc_h_crypto(ar, vif, sta, arg);
> >       ath10k_peer_assoc_h_rates(ar, vif, sta, arg);
> >       ath10k_peer_assoc_h_ht(ar, vif, sta, arg);
> > +    ath10k_peer_assoc_h_phymode(ar, vif, sta, arg);
> >       ath10k_peer_assoc_h_vht(ar, vif, sta, arg);
> >       ath10k_peer_assoc_h_qos(ar, vif, sta, arg);
> > -    ath10k_peer_assoc_h_phymode(ar, vif, sta, arg);
> >
> >       return 0;
> >   }
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
> > b/drivers/net/wireless/ath/ath10k/wmi.c
> > index 2c36256a441d..3797dca317ff 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -7211,12 +7211,7 @@ ath10k_wmi_peer_assoc_fill_10_4(struct ath10k 
> > *ar, void *buf,
> >       struct wmi_10_4_peer_assoc_complete_cmd *cmd = buf;
> >
> >       ath10k_wmi_peer_assoc_fill_10_2(ar, buf, arg);
> > -    if (arg->peer_bw_rxnss_override)
> > -        cmd->peer_bw_rxnss_override =
> > -            __cpu_to_le32((arg->peer_bw_rxnss_override - 1) |
> > -                      BIT(PEER_BW_RXNSS_OVERRIDE_OFFSET));
> > -    else
> > -        cmd->peer_bw_rxnss_override = 0;
> > +    cmd->peer_bw_rxnss_override = 
> > __cpu_to_le32(arg->peer_bw_rxnss_override);
> >   }
> >
> >   static int
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.h 
> > b/drivers/net/wireless/ath/ath10k/wmi.h
> > index 46ae19bb2c92..1fe0aa5523a6 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.h
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> > @@ -6380,7 +6380,19 @@ struct wmi_10_2_peer_assoc_complete_cmd {
> >       __le32 info0; /* WMI_PEER_ASSOC_INFO0_ */
> >   } __packed;
> >
> > -#define PEER_BW_RXNSS_OVERRIDE_OFFSET  31
> > +#define BW_NSS_FWCONF_MAP_ENABLE             (1 << 31)
> > +#define BW_NSS_FWCONF_MAP_160MHZ_S           (0)
> > +#define BW_NSS_FWCONF_MAP_160MHZ_M           (0x00000007)
> > +#define BW_NSS_FWCONF_MAP_80_80MHZ_S         (3)
> > +#define BW_NSS_FWCONF_MAP_80_80MHZ_M         (0x00000038)
> > +#define BW_NSS_FWCONF_MAP_M                  (0x0000003F)
> > +
> > +#define GET_BW_NSS_FWCONF_160(x)             ((((x) & 
> > BW_NSS_FWCONF_MAP_160MHZ_M) >> BW_NSS_FWCONF_MAP_160MHZ_S) + 1)
> > +#define GET_BW_NSS_FWCONF_80_80(x)           ((((x) & 
> > BW_NSS_FWCONF_MAP_80_80MHZ_M) >> BW_NSS_FWCONF_MAP_80_80MHZ_S) + 1)
> > +
> > +/* Values defined to set 160 MHz Bandwidth NSS Mapping into FW*/
> > +#define BW_NSS_FWCONF_160(x)          (BW_NSS_FWCONF_MAP_ENABLE | 
> > (((x - 1) << BW_NSS_FWCONF_MAP_160MHZ_S) & BW_NSS_FWCONF_MAP_160MHZ_M))
> > +#define BW_NSS_FWCONF_80_80(x)        (BW_NSS_FWCONF_MAP_ENABLE | 
> > (((x - 1) << BW_NSS_FWCONF_MAP_80_80MHZ_S) & 
> > BW_NSS_FWCONF_MAP_80_80MHZ_M))
> >
> >   struct wmi_10_4_peer_assoc_complete_cmd {
> >       struct wmi_10_2_peer_assoc_complete_cmd cmd;

--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@xxxxxxxxxx
Tel.: +496251-582650 / Fax: +496251-5826565