Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1660606893.git.objelf@gmail.com> <473fc7b169f288b7815a7736cf33ac0ec1599a09.1660606893.git.objelf@gmail.com>
 <edd45a1e732a0eef1894b9f6eda2d4e3c692359b.camel@sipsolutions.net>
 <CAGp9LzrqYsXAPsbZvTy-_3Tnu23oc11KHdxNs9WvYfe0JHQaDg@mail.gmail.com> <5b24421363048bff1a9f03174cb0223b3e226bf9.camel@sipsolutions.net>
In-Reply-To: <5b24421363048bff1a9f03174cb0223b3e226bf9.camel@sipsolutions.net>
From:   Sean Wang <sean.wang@kernel.org>
Date:   Wed, 17 Aug 2022 17:11:39 -0700
Message-ID: <CAGp9Lzrag1b3G+QfW0RCQPGLnR_WzGSg4k+Zdx4zi4b_oeunRQ@mail.gmail.com>
Subject: Re: [PATCH 1/9] wifi: mac80211: allow enabling chanctx until hw registration
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     =?UTF-8?B?U2VhbiBXYW5nICjnjovlv5fkupgp?= <sean.wang@mediatek.com>,
        Felix Fietkau <nbd@nbd.name>, lorenzo.bianconi@redhat.com,
        Soul.Huang@mediatek.com, YN.Chen@mediatek.com,
        Leon.Yen@mediatek.com, Eric-SY.Chang@mediatek.com,
        Deren Wu <Deren.Wu@mediatek.com>, km.lin@mediatek.com,
        jenhao.yang@mediatek.com, robin.chiu@mediatek.com,
        Eddie.Chen@mediatek.com, ch.yeh@mediatek.com,
        posh.sun@mediatek.com, ted.huang@mediatek.com,
        Stella.Chang@mediatek.com, Tom.Chou@mediatek.com,
        steve.lee@mediatek.com, jsiuda@google.com, frankgor@google.com,
        kuabhs@google.com, druth@google.com, abhishekpandit@google.com,
        shawnku@google.com, linux-wireless@vger.kernel.org,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Johannes,

On Wed, Aug 17, 2022 at 1:30 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Wed, 2022-08-17 at 01:28 -0700, Sean Wang wrote:
> > Based on mt76 driver logic, alloc_hw would be needed before loading firmware
> > because alloc_hw creates an instance of "struct mt76_dev*" the
> > firmware loading relies on,
> > and so the firmware capabilities cannot be decided before we alloc_hw
> > in mt76 driver.
> >
>
> I don't really see why you couldn't change that though? There's no
> fundamental reason you need to load the firmware before registering with
> mac80211?
>

It could be changed but it would break the consistency of the current
mt76 driver.

mt76 driver does the things in the following order
- ieee80211_alloc_hw (where an instance of "struct mt76_dev *" would be created)
- register bus operation into mt76 core (depending on struct mt76_dev
to provide an abstraction layer for mt76 core to access bus)
- read chip identifier (depending on bus operation)
- load the firmware capabilities (depending on chip identifier)
- initialize the hardware
....
-ieee80211_register_hw

If firmware capability is needed to load before ieee80211_alloc_hw, we
need to create kind of similar functions to read chip identifiers and
load firmware.
I know It may not a strong reason not to change, but if we can support
read firmware capabilities after alloc_hw, it provides flexibility to
the vendor driver
and helps mt7921 look more consistent and save a few changes across
different mt7921 bus drivers (mt7921 now supports SDIO, USB, PCIe type
driver).

> And fundamentally, I'm not even sure how you are achieving a change of

I kmemdup the const ops and ieee80211_alloc_hw with the duplicated ops
the duplicated ops would be updated by the actual firmware
capabilities before ieee80211_register_hw.

> the ops - you're meant to point those to a *const* ops, so you need two
> versions of the ops, one with and one without chanctx, and point to the
> correct one at allocation ...
>

If you don't like the reason and the way I proposed in the patch,
please let me know. I still can move on to the way you suggested here.

> johannes