[PATCH v2 1/2] wcn36xx: Fix Antenna Diversity Switching
Kalle Valo
kvalo at codeaurora.org
Tue Sep 21 06:33:53 PDT 2021
Bryan O'Donoghue <bryan.odonoghue at linaro.org> wrote:
> We have been tracking a strange bug with Antenna Diversity Switching (ADS)
> on wcn3680b for a while.
>
> ADS is configured like this:
> A. Via a firmware configuration table baked into the NV area.
> 1. Defines if ADS is enabled.
> 2. Defines which GPIOs are connected to which antenna enable pin.
> 3. Defines which antenna/GPIO is primary and which is secondary.
>
> B. WCN36XX_CFG_VAL(ANTENNA_DIVERSITY, N)
> N is a bitmask of available antenna.
>
> Setting N to 3 indicates a bitmask of enabled antenna (1 | 2).
>
> Obviously then we can set N to 1 or N to 2 to fix to a particular
> antenna and disable antenna diversity.
>
> C. WCN36XX_CFG_VAL(ASD_PROBE_INTERVAL, XX)
> XX is the number of beacons between each antenna RSSI check.
> Setting this value to 50 means, every 50 received beacons, run the
> ADS algorithm.
>
> D. WCN36XX_CFG_VAL(ASD_TRIGGER_THRESHOLD, YY)
> YY is a two's complement integer which specifies the RSSI decibel
> threshold below which ADS will run.
> We default to -60db here, meaning a measured RSSI <= -60db will
> trigger an ADS probe.
>
> E. WCN36XX_CFG_VAL(ASD_RTT_RSSI_HYST_THRESHOLD, Z)
> Z is a hysteresis value, indicating a delta which the RSSI must
> exceed for the antenna switch to be valid.
>
> For example if HYST_THRESHOLD == 3 AntennaId1-RSSI == -60db and
> AntennaId-2-RSSI == -58db then firmware will not switch antenna.
> The threshold needs to be -57db or better to satisfy the criteria.
>
> F. A firmware feature bit also exists ANTENNA_DIVERSITY_SELECTION.
> This feature bit is used by the firmware to report if
> ANTENNA_DIVERSITY_SELECTION is supported. The host is not required to
> toggle this bit to enable or disable ADS.
>
> ADS works like this:
>
> A. Every XX beacons the firmware switches to or remains on the primary
> antenna.
>
> B. The firmware then sends a Request-To-Send (RTS) packet to the AP.
>
> C. The firmware waits for a Clear-To-Send (CTS) response from the AP.
>
> D. The firmware then notes the received RSSI on the CTS packet.
>
> E. The firmware then repeats steps A-D on the secondary antenna.
>
> F. Subsequently if the RSSI on the measured antenna is better than
> ASD_TRIGGER_THRESHOLD + the active antenna's RSSI then the
> measured antenna becomes the active antenna.
>
> G. If RSSI rises past ASD_TRIGGER_THRESHOLD then ADS doesn't run at
> all even if there is a substantially better RSSI on the alternative
> antenna.
>
> What we have been observing is that the RTS packet is being sent but the
> MAC address is a byte-swapped version of the target MAC. The ADS/RTS MAC is
> corrupted only when the link is encrypted, if the AP is open the RTS MAC is
> correct. Similarly if we configure the firmware to an RTS/CTS sequence for
> regular data - the transmitted RTS MAC is correctly formatted.
>
> Internally the wcn36xx firmware uses the indexes in the SMD commands to
> populate and extract data from specific entries in an STA lookup table. The
> AP's MAC appears a number of times in different indexes within this lookup
> table, so the MAC address extracted for the data-transmit RTS and the MAC
> address extracted for the ADS/RTS packet are not the same STA table index.
>
> Our analysis indicates the relevant firmware STA table index is
> "bssSelfStaIdx".
>
> There is an STA populate function responsible for formatting the MAC
> address of the bssSelfStaIdx including byte-swapping the MAC address.
>
> Its clear then that the required STA populate command did not run for
> bssSelfStaIdx.
>
> So taking a look at the sequence of SMD commands sent to the firmware we
> see the following downstream when moving from an unencrypted to encrypted
> BSS setup.
>
> - WLAN_HAL_CONFIG_BSS_REQ
> - WLAN_HAL_CONFIG_STA_REQ
> - WLAN_HAL_SET_STAKEY_REQ
>
> Upstream in wcn36xx we have
>
> - WLAN_HAL_CONFIG_BSS_REQ
> - WLAN_HAL_SET_STAKEY_REQ
>
> The solution then is to add the missing WLAN_HAL_CONFIG_STA_REQ between
> WLAN_HAL_CONFIG_BSS_REQ and WLAN_HAL_SET_STAKEY_REQ.
>
> No surprise WLAN_HAL_CONFIG_STA_REQ is the routine responsible for
> populating the STA lookup table in the firmware and once done the MAC sent
> by the ADS routine is in the correct byte-order.
>
> This bug is apparent with ADS but it is also the case that any other
> firmware routine that depends on the "bssSelfStaIdx" would retrieve
> malformed data on an encrypted link.
>
> Fixes: 3e977c5c523d ("wcn36xx: Define wcn3680 specific firmware parameters")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
> Tested-by: Benjamin Li <benl at squareup.com>
> Reviewed-by: Loic Poulain <loic.poulain at linaro.org>
> Signed-off-by: Kalle Valo <kvalo at codeaurora.org>
2 patches applied to ath-next branch of ath.git, thanks.
701668d3bfa0 wcn36xx: Fix Antenna Diversity Switching
c0c2eb20c79e wcn36xx: Add ability for wcn36xx_smd_dump_cmd_req to pass two's complement
--
https://patchwork.kernel.org/project/linux-wireless/patch/20210909144428.2564650-2-bryan.odonoghue@linaro.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
More information about the wcn36xx
mailing list