On 2/16/24 12:42, Johannes Berg wrote:
On Fri, 2024-02-16 at 08:45 +0530, Aditya Kumar Singh wrote:+ for (i = 0; i < n_vifs; i++) { + hwsim_check_chanctx_magic(vifs[i].old_ctx); + wiphy_dbg(hw->wiphy, + "switch vif channel context: %d MHz/width: %d/cfreqs:%d/%d MHz -> %d MHz/width: %d/cfreqs:%d/%d MHz\n", + vifs[i].old_ctx->def.chan->center_freq, + vifs[i].old_ctx->def.width, + vifs[i].old_ctx->def.center_freq1, + vifs[i].old_ctx->def.center_freq2, + vifs[i].new_ctx->def.chan->center_freq, + vifs[i].new_ctx->def.width, + vifs[i].new_ctx->def.center_freq1, + vifs[i].new_ctx->def.center_freq2); + hwsim_set_chanctx_magic(vifs[i].new_ctx);hwsim_set_chanctx_magic() is only partially correct, I think, this depends on the mode? For CHANCTX_SWMODE_REASSIGN_VIF the chanctx should already exist as well, so should also be hwsim_check_chanctx_magic() in that case?Oh yeah missed the mode dependency here. Thanks for pointing it out. So it should be something like - ... /* old already exist so check magic */ hwsim_check_chanctx_magic(vifs[i].old_ctx); if (mode == CHANCTX_SWMODE_REASSIGN_VIF) { /* Reassign then new would also exist, just interface is * switching */ hwsim_check_chanctx_magic(vifs[i].new_ctx); } else { /* SWAP_* then new context does not exist hence set magic. * Also old will no longer exist so clear the magic */ hwsim_set_chanctx_magic(vifs[i].new_ctx); hwsim_clear_chanctx_magic(vifs[i].old_ctx); } ... One thing, in patch should I keep those comments or those are noisy? Seems noisy to me (if at least mode documentation is referred then things are clear already)?I'm not sure I care all that much, but I'd say even with the reference to the modes, it's fairly easy to figure out at least by looking at the mode docs?
Yes correct. I will drop inline comments. Not required tbh.
I'd probably go for a switch () statement on the modes and even WARN on invalid mode, while at it? hwsim is a test vehicle, after all.
Yes, good point :). Thanks will incorporate and send v2 soon.
