Re: [PATCH wireless-next 14/35] wifi: mm81x: add mac.c

[Johannes Berg <johannes@sipsolutions.net>]

On Mon, 2026-03-09 at 15:43 +1100, Lachlan Hodges wrote:
> >  2) Are you going to incur the wrath of mm/ folks, where instances of
> >     'struct mm_struct' are commonly called 'mm'? I can find a few
> >     examples of others (struct drm_buddy *mm, struct mqd_manager *mm),
> >     but you'd double the instances.
> 
> This.. is definitely something I did not think of. I have no issue with
> renaming to something else.. maybe mx? I'm not sure.

Yeah I really don't know. There's no 'mm->lock' (any more? for some
reason _that_ was what caught my eye wrt. the naming) in mm/, and I
guess soon also not in your driver. I'll try to ask around, but it's
probably safer to rename, and shouldn't be _that_ hard with spatch I
guess. I guess 'mx' seems reasonable, 'mmx' is also confusing perhaps,
and 'mm81x' doesn't lend itself to obvious other abbreviations.


> > > +	/*
> > > +	 * mm81x only support changing/setting the channel
> > > +	 * when we create an interface.
> > > +	 */
> > > +	if (WARN_ON(changed & IEEE80211_CHANCTX_CHANGE_CHANNEL))
> > > +		mm81x_err(mm, "Changing channel via chanctx not supported");
> > 
> > Wait, what, why do you have chanctx support then? This seems highly
> > questionable, how do you not run into this all the time?
> > 
> > If it just has a single, wouldn't the chanctx emulation suit the driver
> > better, and that'd make this more obvious? Hmm, but you _do_ support
> > multiple vifs? I'm confused.
> 
> We originally used chanctx emulation.. but I suppose in an effort to
> be "modern" we use chanctx. It's probably best to switch back to the
> chanctx emulation anyway. As for why we don't run into this is due
> to no channel switch support yet, iirc mac80211 I think needs a minor
> tweak to work with S1G (which further reinforces the idea that we
> should just emulate chanctx)

I don't mind the emulation _that_ much to force drivers into some
unnatural scheme for them :) This seems even more confusing and
unexpected than the emulation perhaps.

But I don't want to impose here either.

> Thanks for the review. On the other thread [1] you mentioned sending a
> pull request once reviews settle down, as per the documentation in [2]
> (which I should have read earlier... :) ),

Heh, I didn't really know we had that document either, Kalle did all
that :)

> can we confirm that this means
> we are to submit subsequent patchset revisions in the same per-file
> format until everyone is happy with the driver, and then raise the PR?

I wouldn't necessarily way _everyone_, you can probably always find
someone willing to nitpick if you look hard enough ;-)

But yeah, I don't think you have a choice for how to post, the whole
driver as one patch would not really even load well in an email client I
guess, let alone make it possible to comment on easily.

As I said there, for the merge I'd prefer just a single commit as a pull
request.

Obviously I hope/expect you're going to continue to maintaining the
driver and we'll have to figure out the workflow for that - perhaps
depending on how much work you're planning to put into it.

johannes