On 10/1/24 12:01, Johannes Berg wrote:
+ /*
+ * inform about the link info changed parameters after all
+ * stations are also added
+ */
I don't understand that comment - you're not doing anything with the
stations here? And per the commit log it's explicitly _not_ after doing
the AP station. I'm not sure we should set up everything before the AP
station?
Okay let me try to explain the situation here -
In the *if (add)* block, drivers are informed about the new links added
via drv_change_vif_links(). For drivers like ath12k, we know which link
is changing, but since the channel contexts for the new links aren’t
available yet, the driver can’t determine which link the firmware should
create the interface for. This is because we have more than one firmware
operating under this interface (grouped multiple hardware under single
wiphy). Therefore, this notification isn’t very helpful in creating the
link interface now.
Next, in the loop list_for_each_entry(sta, &local->sta_list, list),
drv_change_sta_links() is called to notify drivers that the links for a
given station (STA) have changed. Drivers use this callback to create
the link stations after the STA has moved to the authorized state.
At this point, the driver knows which ML STA and which link STA to
create. However, to create a link STA, the corresponding link interface
must exist first.
Currently, it doesn’t, so the driver can’t add the link STA.
Later, in the loop for_each_set_bit(link_id, &add,
IEEE80211_MLD_MAX_NUM_LINKS), channels are added. At this stage, the
driver will actually create the link on the interface at its own level.
Since here using the channel information, appropriate firmware can be
picked. For example 2 GHz or 5 GHz or 6 GHz firmware.
+ BSS_CHANGED_HE_BSS_COLOR);
+
+ }
You make it look like you just moved code but also snuck in a new blank
line :)
:) sorry about that. Will fix in next version.
+
for_each_set_bit(link_id, &rem, IEEE80211_MLD_MAX_NUM_LINKS) {
struct ieee80211_link_data *link;
I also think you put this code too early now - you're now first using
more channel contexts by way of _ieee80211_link_use_channel() before you
even release the ones from deactivated ("rem" bitmap in this code)
links. That doesn't seem like it could work correctly in general.
hmm... yeah true that. May be I will move this once the old links are
removed?
--
Aditya
