On 1/10/2024 8:50 PM, Sriram R wrote: ... > @@ -7553,10 +7553,12 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar, > > static u16 ath12k_mac_get_ifmodes(struct ath12k_hw *ah) > { > - struct ath12k *ar = ath12k_ah_to_ar(ah); > + struct ath12k *ar; > + int i; > u16 interface_modes = U16_MAX; > > - interface_modes &= ar->ab->hw_params->interface_modes; > + for_each_ar(i, ah, ar) > + interface_modes &= ar->ab->hw_params->interface_modes; This logic sets interface_modes to be the modes that are supported on every radio (starts with all modes supported and then masks off the ones that are not supported on a given radio). That means that if radio 0 supports a mode that radio 1 does not support, then we don't advertise support for that mode. Just want to verify that is the correct logic, and that instead we don't want to start with no modes supported and then OR in the ones that are supported so that we get the list of modes that are supported on ANY radio instead of ALL radios. > > return interface_modes == U16_MAX ? 0 : interface_modes; > } > @@ -7564,15 +7566,19 @@ static u16 ath12k_mac_get_ifmodes(struct ath12k_hw *ah) > static bool ath12k_mac_is_iface_mode_enable(struct ath12k_hw *ah, > enum nl80211_iftype type) > { > - struct ath12k *ar = ath12k_ah_to_ar(ah); > + struct ath12k *ar; > + int i; > u16 interface_modes, mode; > bool is_enable = true; > > mode = BIT(type); > - > - interface_modes = ar->ab->hw_params->interface_modes; > - if (!(interface_modes & mode)) > - is_enable = false; > + for_each_ar(i, ah, ar) { > + interface_modes = ar->ab->hw_params->interface_modes; > + if (!(interface_modes & mode)) { > + is_enable = false; note that in the example I mentioned above, this logic could say a given mode is enabled even though the bit isn't set in the interface_modes listed above > + break; > + } > + } > > return is_enable;