Hi, So ... yeah. I shied away from even reviewing this because it's such a messy topic and we've had so many threads about this. Sorry about that. On Wed, 2023-03-15 at 18:58 +0530, Aditya Kumar Singh wrote: > 6 GHz introduces various power modes of operation. Currently, based > on the power mode, channel's Power Spectral Density (PSD) value, > Regulatory power value, as well as channel disabled flag can change. > For single interface, current implementation works just fine. But for > multi-interfaces, for example AP-STA concurrency, two different channel > context needs to be maintained. This is because, STA power mode also > depends on the AP's power mode it is going to associate with. Hence, > PSD value, regulatory power value and channel disabled flag might vary. > In this case, same channel context cannot be used for both AP and STA. > > Therefore, to support multiple channel space for each power mode, the > 6 GHz channels needs a separate storage space in data structure > ieee80211_supported_band. Because of this, the existing APIs to get the > channel/frequency from frequency/channel will not work for 6 GHz band. > > Add support to store all possible 6 GHz channel pools according to the > power mode as well as add API support for getting channel/frequency info > from the new struct ieee80211_6ghz_channel. > > > Why different channel pools and not array of varying member in the same channel?: I really don't understand. Why do you even need to set *from userspace* the power mode for a client? That ... doesn't make that much sense? I mean, you're explaining here the mechanics, which is all fine, but what's the "theory of operation"? Yes, I get that 6 GHz spectrum use introduced power modes, but why do we even need to handle it this way? None of this or the patches actually introduce any rationale? Also, I'll note that the patch subjects with "cfg80211" or "mac80211" are _really_ unclear. Sometimes you have "cfg80211" patches that mostly change mac80211, etc. It'd make reviewing easier if you actually did limit cfg80211 patches to touching cfg80211 only (with the exception of necessary API updates, of course), and mac80211 patches to implementing the new cfg80211 APIs. The first patch is probably neither, you can mark it as ieee80211. And then trivial things like - don't introduce a bug only to fix it in the next patch! I mean, really? I _still_ don't like "(A)" operation with different channel pointers - you talk about introducing bugs with (B) but at least those would be bugs in the new parts; (A) is almost certainly introducing bugs in existing code that compares pointers and you never even know about it. Also, related to that, is kind of a fundamental question - should we really think that two *channels* are different because you use different (TX) power control/scheme on them? That seems a bit strange, and you've not gotten into that at all, from a semantic POV, only from a code POV. I actually currently don't think that makes sense, but convince me otherwise? For a chandef, _maybe_? But also see the discussion around puncturing, I'm not sure that's actually the right solution either. johannes
