On Wed, Aug 17, 2022 at 12:42 AM Lorenzo Bianconi <[email protected]> wrote:
>
> > From: Sean Wang <[email protected]>
> >
> > The firmware can have the capability to manage the channel context
> > scheduling on multiple roles running on the device including Station,
> > AP and P2P GC/GO mode (will be extended based on the patchset) to help
> > users sharing the network with others on a single device.
> >
> > The firmware is able to support the channel chanctx up to 2 interface
> > simultaneously running on the different channels.
> >
> > Another thing to be noted is that before the driver is going sent out the
> > management frames, the driver has to get the privilege from the firmware
> > to occupy the current channel context until the frame handshake is
> > completed and then get the privilege back to the firmware.
> >
> > We temporarily disable the feature with a module parameter
> > mt7921_disable_cnm for a while until we can ensure the patchset doesn't
> > cause any regression.
> >
> > Co-developed-by: Deren Wu <[email protected]>
> > Signed-off-by: Deren Wu <[email protected]>
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > .../net/wireless/mediatek/mt76/mt7921/init.c | 40 ++++++-
> > .../net/wireless/mediatek/mt76/mt7921/main.c | 111 +++++++++++++++++-
> > .../wireless/mediatek/mt76/mt7921/mt7921.h | 1 +
> > 3 files changed, 145 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > index 1b7a18d42f5b..208a6117cb69 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > @@ -7,6 +7,10 @@
> > #include "mcu.h"
> > #include "eeprom.h"
> >
> > +static bool mt7921_disable_cnm = true;
> > +module_param_named(disable_cnm, mt7921_disable_cnm, bool, 0644);
> > +MODULE_PARM_DESC(disable_cnm, "disable concurrent network support");
>
> do we need it? I guess we can just leave it enabled by default and disable it
> through a debugfs node. What do you think?

The kernel parameter would be checked before registering ops to
mac80211, it seemed to me the way debugfs node doesn't work for that
moment.

>
> > +
> > static const struct ieee80211_iface_limit if_limits[] = {
> > {
> > .max = MT7921_MAX_INTERFACES,
> > @@ -25,6 +29,27 @@ static const struct ieee80211_iface_combination if_comb[] = {
> > .max_interfaces = MT7921_MAX_INTERFACES,
> > .num_different_channels = 1,
> > .beacon_int_infra_match = true,
> > + },
> > +};
> > +
> > +static const struct ieee80211_iface_limit if_limits_chanctx[] = {
> > + {
> > + .max = 2,
> > + .types = BIT(NL80211_IFTYPE_STATION),
> > + },
> > + {
> > + .max = 1,
> > + .types = BIT(NL80211_IFTYPE_AP),
> > + }
> > +};
> > +
> > +static const struct ieee80211_iface_combination if_comb_chanctx[] = {
> > + {
> > + .limits = if_limits_chanctx,
> > + .n_limits = ARRAY_SIZE(if_limits_chanctx),
> > + .max_interfaces = 2,
> > + .num_different_channels = 2,
> > + .beacon_int_infra_match = false,
> > }
> > };
> >
> > @@ -63,11 +88,20 @@ static int mt7921_check_offload_capability(struct mt7921_dev *dev)
> > fw_can_roc = mktime64(year, mon, day, hour, min, sec) >=
> > mktime64(2022, 7, 15, 12, 1, 1);
> > out:
> > - if (!fw_can_roc) {
> > + if (!fw_can_roc || mt7921_disable_cnm) {
> > dev->ops->remain_on_channel = NULL;
> > dev->ops->cancel_remain_on_channel = NULL;
> > + dev->ops->add_chanctx = NULL;
> > + dev->ops->remove_chanctx = NULL;
> > + dev->ops->change_chanctx = NULL;
> > + dev->ops->assign_vif_chanctx = NULL;
> > + dev->ops->unassign_vif_chanctx = NULL;
> > + dev->ops->mgd_prepare_tx = NULL;
> > + dev->ops->mgd_complete_tx = NULL;
> >
> > wiphy->flags &= ~WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
> > + wiphy->iface_combinations = if_comb;
> > + wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
> > }
> >
> > return 0;
> > @@ -93,12 +127,12 @@ mt7921_init_wiphy(struct ieee80211_hw *hw)
> > hw->sta_data_size = sizeof(struct mt7921_sta);
> > hw->vif_data_size = sizeof(struct mt7921_vif);
> >
> > - wiphy->iface_combinations = if_comb;
> > + wiphy->iface_combinations = if_comb_chanctx;
> > wiphy->flags &= ~(WIPHY_FLAG_IBSS_RSN | WIPHY_FLAG_4ADDR_AP |
> > WIPHY_FLAG_4ADDR_STATION);
> > wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> > BIT(NL80211_IFTYPE_AP);
> > - wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
> > + wiphy->n_iface_combinations = ARRAY_SIZE(if_comb_chanctx);
> > wiphy->max_remain_on_channel_duration = 5000;
> > wiphy->max_scan_ie_len = MT76_CONNAC_SCAN_IE_LEN;
> > wiphy->max_scan_ssids = 4;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > index ae0aabe052e3..6386290ba71c 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > @@ -858,7 +858,7 @@ void mt7921_mac_sta_assoc(struct mt76_dev *mdev, struct ieee80211_vif *vif,
> >
> > if (vif->type == NL80211_IFTYPE_STATION && !sta->tdls)
> > mt76_connac_mcu_uni_add_bss(&dev->mphy, vif, &mvif->sta.wcid,
> > - true, NULL);
> > + true, mvif->ctx);
> >
> > mt7921_mac_wtbl_update(dev, msta->wcid.idx,
> > MT_WTBL_UPDATE_ADM_COUNT_CLEAR);
> > @@ -890,7 +890,7 @@ void mt7921_mac_sta_remove(struct mt76_dev *mdev, struct ieee80211_vif *vif,
> > if (!sta->tdls)
> > mt76_connac_mcu_uni_add_bss(&dev->mphy, vif,
> > &mvif->sta.wcid, false,
> > - NULL);
> > + mvif->ctx);
> > }
> >
> > spin_lock_bh(&dev->sta_poll_lock);
> > @@ -1640,7 +1640,7 @@ mt7921_start_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > mt7921_mutex_acquire(dev);
> >
> > err = mt76_connac_mcu_uni_add_bss(phy->mt76, vif, &mvif->sta.wcid,
> > - true, NULL);
> > + true, mvif->ctx);
> > if (err)
> > goto failed;
> >
> > @@ -1672,12 +1672,108 @@ mt7921_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > goto failed;
> >
> > mt76_connac_mcu_uni_add_bss(phy->mt76, vif, &mvif->sta.wcid, false,
> > - NULL);
> > + mvif->ctx);
> >
> > failed:
> > mt7921_mutex_release(dev);
> > }
> >
> > +static int
> > +mt7921_add_chanctx(struct ieee80211_hw *hw,
> > + struct ieee80211_chanctx_conf *ctx)
> > +{
> > + return 0;
> > +}
> > +
> > +static void
> > +mt7921_remove_chanctx(struct ieee80211_hw *hw,
> > + struct ieee80211_chanctx_conf *ctx)
> > +{
> > +}
> > +
> > +static void mt7921_ctx_iter(void *priv, u8 *mac,
> > + struct ieee80211_vif *vif)
> > +{
> > + struct mt7921_vif *mvif = (struct mt7921_vif *)vif->drv_priv;
> > + struct ieee80211_chanctx_conf *ctx = priv;
> > +
> > + if (ctx != mvif->ctx)
> > + return;
> > +
> > + mt76_connac_mcu_uni_set_chctx(mvif->phy->mt76, &mvif->mt76, ctx);
> > +}
> > +
> > +static void
> > +mt7921_change_chanctx(struct ieee80211_hw *hw,
> > + struct ieee80211_chanctx_conf *ctx,
> > + u32 changed)
> > +{
> > + struct mt7921_phy *phy = mt7921_hw_phy(hw);
> > +
> > + mt7921_mutex_acquire(phy->dev);
> > + ieee80211_iterate_active_interfaces(phy->mt76->hw,
> > + IEEE80211_IFACE_ITER_ACTIVE,
> > + mt7921_ctx_iter, ctx);
> > + mt7921_mutex_release(phy->dev);
> > +}
> > +
> > +static int
> > +mt7921_assign_vif_chanctx(struct ieee80211_hw *hw,
> > + struct ieee80211_vif *vif,
> > + struct ieee80211_bss_conf *link_conf,
> > + struct ieee80211_chanctx_conf *ctx)
> > +{
> > + struct mt7921_vif *mvif = (struct mt7921_vif *)vif->drv_priv;
> > + struct mt7921_dev *dev = mt7921_hw_dev(hw);
> > +
> > + mt7921_mutex_acquire(dev);
>
> I think in this case we can just grub the mutex without waking up the device.
> what do you think?

ack

>
> > + mvif->ctx = ctx;
> > + mt7921_mutex_release(dev);
> > +
> > + return 0;
> > +}
> > +
> > +static void
> > +mt7921_unassign_vif_chanctx(struct ieee80211_hw *hw,
> > + struct ieee80211_vif *vif,
> > + struct ieee80211_bss_conf *link_conf,
> > + struct ieee80211_chanctx_conf *ctx)
> > +{
> > + struct mt7921_vif *mvif = (struct mt7921_vif *)vif->drv_priv;
> > + struct mt7921_dev *dev = mt7921_hw_dev(hw);
> > +
> > + mt7921_mutex_acquire(dev);
>
> same here.

ack

>
> Regards,
> Lorenzo
>
> > + mvif->ctx = NULL;
> > + mt7921_mutex_release(dev);
> > +}
> > +
> > +static void mt7921_mgd_prepare_tx(struct ieee80211_hw *hw,
> > + struct ieee80211_vif *vif,
> > + struct ieee80211_prep_tx_info *info)
> > +{
> > + struct mt7921_vif *mvif = (struct mt7921_vif *)vif->drv_priv;
> > + struct mt7921_dev *dev = mt7921_hw_dev(hw);
> > + u16 duration = info->duration ? info->duration :
> > + jiffies_to_msecs(HZ);
> > +
> > + mt7921_mutex_acquire(dev);
> > + mt7921_set_roc(mvif->phy, mvif, mvif->ctx->def.chan, duration,
> > + MT7921_ROC_REQ_JOIN);
> > + mt7921_mutex_release(dev);
> > +}
> > +
> > +static void mt7921_mgd_complete_tx(struct ieee80211_hw *hw,
> > + struct ieee80211_vif *vif,
> > + struct ieee80211_prep_tx_info *info)
> > +{
> > + struct mt7921_vif *mvif = (struct mt7921_vif *)vif->drv_priv;
> > + struct mt7921_dev *dev = mt7921_hw_dev(hw);
> > +
> > + mt7921_mutex_acquire(dev);
> > + mt7921_abort_roc(mvif->phy, mvif);
> > + mt7921_mutex_release(dev);
> > +}
> > +
> > const struct ieee80211_ops mt7921_ops = {
> > .tx = mt7921_tx,
> > .start = mt7921_start,
> > @@ -1730,6 +1826,13 @@ const struct ieee80211_ops mt7921_ops = {
> > .set_sar_specs = mt7921_set_sar_specs,
> > .remain_on_channel = mt7921_remain_on_channel,
> > .cancel_remain_on_channel = mt7921_cancel_remain_on_channel,
> > + .add_chanctx = mt7921_add_chanctx,
> > + .remove_chanctx = mt7921_remove_chanctx,
> > + .change_chanctx = mt7921_change_chanctx,
> > + .assign_vif_chanctx = mt7921_assign_vif_chanctx,
> > + .unassign_vif_chanctx = mt7921_unassign_vif_chanctx,
> > + .mgd_prepare_tx = mt7921_mgd_prepare_tx,
> > + .mgd_complete_tx = mt7921_mgd_complete_tx,
> > };
> > EXPORT_SYMBOL_GPL(mt7921_ops);
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h b/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h
> > index 280605ffc4da..fda85252325c 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mt7921.h
> > @@ -156,6 +156,7 @@ struct mt7921_vif {
> > struct ewma_rssi rssi;
> >
> > struct ieee80211_tx_queue_params queue_params[IEEE80211_NUM_ACS];
> > + struct ieee80211_chanctx_conf *ctx;
> > };
> >
> > struct mib_stats {
> > --
> > 2.25.1
> >

2022-08-18 01:01:51

by Sean Wang

[permalink] [raw]

Subject: Re: [PATCH 6/9] wifi: mt76: mt7921: drop ieee80211_[start, stop]_queues in driver

HI Lorenzo,

<snip>
> > +
> > + dev_dbg(dev->mt76.dev, "BSS %d is %s\n", event->bss_idx,
> > + event->is_absent ? "Absent" : "Present");
>
> do we really need this? I guess we can just drop mt7921_mcu_bss_event entirely

That is not really necessary. That was just what I used to diagnose
firmware behavior. I think I will remove it in the next version.

Sean
>
> Regards,
> Lorenzo
>
> > }
> >
> > static void
> > --
> > 2.25.1
> >

2022-08-18 01:05:29

by Sean Wang

[permalink] [raw]

Subject: Re: [PATCH 8/9] wifi: mt76: mt7921: introduce remain_on_channel support

Hi Lorenzo,

On Wed, Aug 17, 2022 at 12:18 AM Lorenzo Bianconi <[email protected]> wrote:
>
> > From: Sean Wang <[email protected]>
> >
> > Introduce remain_on_channel support. Additionally, we add
> > mt7921_check_offload_capability to disable .remain_on_channel and
> > .cancel_remain_on_channel and related configuration because those
> > operations would rely on the fundamental MCU commands that will be only
> > supported with newer firmware.
> >
> > Co-developed-by: Deren Wu <[email protected]>
> > Signed-off-by: Deren Wu <[email protected]>
> > Signed-off-by: Sean Wang <[email protected]>
> > ---
> > .../net/wireless/mediatek/mt76/mt7921/init.c | 36 ++++++
> > .../net/wireless/mediatek/mt76/mt7921/main.c | 112 ++++++++++++++++++
> > .../net/wireless/mediatek/mt76/mt7921/mcu.c | 24 ++++
> > .../wireless/mediatek/mt76/mt7921/mt7921.h | 34 ++++++
> > .../net/wireless/mediatek/mt76/mt7921/pci.c | 13 +-
> > .../net/wireless/mediatek/mt76/mt7921/sdio.c | 11 +-
> > .../net/wireless/mediatek/mt76/mt7921/usb.c | 1 +
> > 7 files changed, 225 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > index cd960e23770f..1b7a18d42f5b 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > @@ -44,6 +44,35 @@ mt7921_regd_notifier(struct wiphy *wiphy,
> > mt7921_mutex_release(dev);
> > }
> >
> > +static int mt7921_check_offload_capability(struct mt7921_dev *dev)
> > +{
> > + struct ieee80211_hw *hw = mt76_hw(dev);
> > + int year, mon, day, hour, min, sec;
> > + struct wiphy *wiphy = hw->wiphy;
> > + bool fw_can_roc = false;
> > + int ret;
> > +
> > + ret = sscanf(dev->mt76.hw->wiphy->fw_version + 11, "%4d%2d%2d%2d%2d%2d",
> > + &year, &mon, &day, &hour, &min, &sec);
>
> does the fw have a differnt base version with respect to the previous ones?
> checking the date is a bit ugly.

I admitted that way was a bit ugly, but I have investigated for a
while, and that is the only way we can use to distinguish the version
in current mt7921 firmware.

>
> > + if (ret != 6)
> > + goto out;
> > +
> > + /* Old firmware cannot support remained on channel and channel
> > + * context management.
> > + */
> > + fw_can_roc = mktime64(year, mon, day, hour, min, sec) >=
> > + mktime64(2022, 7, 15, 12, 1, 1);
> > +out:
> > + if (!fw_can_roc) {
> > + dev->ops->remain_on_channel = NULL;
> > + dev->ops->cancel_remain_on_channel = NULL;
> > +
> > + wiphy->flags &= ~WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
> > + }
> > +
> > + return 0;
> > +}
> > +

<snip>

> > - mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), &mt7921_ops,
> > - &drv_ops);
> > + ops = devm_kmemdup(&pdev->dev, &mt7921_ops, sizeof(mt7921_ops),
> > + GFP_KERNEL);
>
> why do we need to copy mt7921_ops?
>

As the old fw cannot support the roc and chanctx and considering
backward compatibility, I need to copy the mt7921_ops here and
disable related operations for old fw before registering the hw to mac80211.

Sean

> Regards,
> Lorenzo
>
> > + if (!ops) {
> > + ret = -ENOMEM;
> > + goto err_free_pci_vec;
> > + }
> > +
> > + mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), ops, &drv_ops);
> > if (!mdev) {
> > ret = -ENOMEM;
> > goto err_free_pci_vec;
> > @@ -286,7 +293,7 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
> >
> > dev = container_of(mdev, struct mt7921_dev, mt76);
> > dev->hif_ops = &mt7921_pcie_ops;
> > -
> > + dev->ops = ops;
> > mt76_mmio_init(&dev->mt76, pcim_iomap_table(pdev)[0]);
> > tasklet_init(&dev->irq_tasklet, mt7921_irq_tasklet, (unsigned long)dev);
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > index 487acd6e2be8..6d27875f41b8 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > @@ -120,18 +120,23 @@ static int mt7921s_probe(struct sdio_func *func,
> > .fw_own = mt7921s_mcu_fw_pmctrl,
> > };
> >
> > + struct ieee80211_ops *ops;
> > struct mt7921_dev *dev;
> > struct mt76_dev *mdev;
> > int ret;
> >
> > - mdev = mt76_alloc_device(&func->dev, sizeof(*dev), &mt7921_ops,
> > - &drv_ops);
> > + ops = devm_kmemdup(&func->dev, &mt7921_ops, sizeof(mt7921_ops),
> > + GFP_KERNEL);
> > + if (!ops)
> > + return -ENOMEM;
> > +
> > + mdev = mt76_alloc_device(&func->dev, sizeof(*dev), ops, &drv_ops);
> > if (!mdev)
> > return -ENOMEM;
> >
> > dev = container_of(mdev, struct mt7921_dev, mt76);
> > dev->hif_ops = &mt7921_sdio_ops;
> > -
> > + dev->ops = ops;
> > sdio_set_drvdata(func, dev);
> >
> > ret = mt76s_init(mdev, func, &mt7921s_ops);
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
> > index d06cee386acd..cf3ec59a4270 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
> > @@ -217,6 +217,7 @@ static int mt7921u_probe(struct usb_interface *usb_intf,
> >
> > dev = container_of(mdev, struct mt7921_dev, mt76);
> > dev->hif_ops = &hif_ops;
> > + dev->ops = ops;
> >
> > udev = usb_get_dev(udev);
> > usb_reset_device(udev);
> > --
> > 2.25.1
> >

2022-08-18 07:43:22

by Lorenzo Bianconi

[permalink] [raw]

Subject: Re: [PATCH 8/9] wifi: mt76: mt7921: introduce remain_on_channel support

> Hi Lorenzo,
>
> On Wed, Aug 17, 2022 at 12:18 AM Lorenzo Bianconi <[email protected]> wrote:
> >
> > > From: Sean Wang <[email protected]>
> > >
> > > Introduce remain_on_channel support. Additionally, we add
> > > mt7921_check_offload_capability to disable .remain_on_channel and
> > > .cancel_remain_on_channel and related configuration because those
> > > operations would rely on the fundamental MCU commands that will be only
> > > supported with newer firmware.
> > >
> > > Co-developed-by: Deren Wu <[email protected]>
> > > Signed-off-by: Deren Wu <[email protected]>
> > > Signed-off-by: Sean Wang <[email protected]>
> > > ---
> > > .../net/wireless/mediatek/mt76/mt7921/init.c | 36 ++++++
> > > .../net/wireless/mediatek/mt76/mt7921/main.c | 112 ++++++++++++++++++
> > > .../net/wireless/mediatek/mt76/mt7921/mcu.c | 24 ++++
> > > .../wireless/mediatek/mt76/mt7921/mt7921.h | 34 ++++++
> > > .../net/wireless/mediatek/mt76/mt7921/pci.c | 13 +-
> > > .../net/wireless/mediatek/mt76/mt7921/sdio.c | 11 +-
> > > .../net/wireless/mediatek/mt76/mt7921/usb.c | 1 +
> > > 7 files changed, 225 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > > index cd960e23770f..1b7a18d42f5b 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > > @@ -44,6 +44,35 @@ mt7921_regd_notifier(struct wiphy *wiphy,
> > > mt7921_mutex_release(dev);
> > > }
> > >
> > > +static int mt7921_check_offload_capability(struct mt7921_dev *dev)
> > > +{
> > > + struct ieee80211_hw *hw = mt76_hw(dev);
> > > + int year, mon, day, hour, min, sec;
> > > + struct wiphy *wiphy = hw->wiphy;
> > > + bool fw_can_roc = false;
> > > + int ret;
> > > +
> > > + ret = sscanf(dev->mt76.hw->wiphy->fw_version + 11, "%4d%2d%2d%2d%2d%2d",
> > > + &year, &mon, &day, &hour, &min, &sec);
> >
> > does the fw have a differnt base version with respect to the previous ones?
> > checking the date is a bit ugly.
>
> I admitted that way was a bit ugly, but I have investigated for a
> while, and that is the only way we can use to distinguish the version
> in current mt7921 firmware.

the fw seems pretty new (2022/7/15), is it already available in linux-firmware
git tree? If not I guess you can increment fw version in a more evident way.
For the future please remember to do it for major fw changes.

>
> >
> > > + if (ret != 6)
> > > + goto out;
> > > +
> > > + /* Old firmware cannot support remained on channel and channel
> > > + * context management.
> > > + */
> > > + fw_can_roc = mktime64(year, mon, day, hour, min, sec) >=
> > > + mktime64(2022, 7, 15, 12, 1, 1);
> > > +out:
> > > + if (!fw_can_roc) {
> > > + dev->ops->remain_on_channel = NULL;
> > > + dev->ops->cancel_remain_on_channel = NULL;
> > > +
> > > + wiphy->flags &= ~WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
>
> <snip>
>
> > > - mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), &mt7921_ops,
> > > - &drv_ops);
> > > + ops = devm_kmemdup(&pdev->dev, &mt7921_ops, sizeof(mt7921_ops),
> > > + GFP_KERNEL);
> >
> > why do we need to copy mt7921_ops?
> >
>
> As the old fw cannot support the roc and chanctx and considering
> backward compatibility, I need to copy the mt7921_ops here and
> disable related operations for old fw before registering the hw to mac80211.

ack, right.

Regards,
Lorenzo

>
> Sean
>
> > Regards,
> > Lorenzo
> >
> > > + if (!ops) {
> > > + ret = -ENOMEM;
> > > + goto err_free_pci_vec;
> > > + }
> > > +
> > > + mdev = mt76_alloc_device(&pdev->dev, sizeof(*dev), ops, &drv_ops);
> > > if (!mdev) {
> > > ret = -ENOMEM;
> > > goto err_free_pci_vec;
> > > @@ -286,7 +293,7 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
> > >
> > > dev = container_of(mdev, struct mt7921_dev, mt76);
> > > dev->hif_ops = &mt7921_pcie_ops;
> > > -
> > > + dev->ops = ops;
> > > mt76_mmio_init(&dev->mt76, pcim_iomap_table(pdev)[0]);
> > > tasklet_init(&dev->irq_tasklet, mt7921_irq_tasklet, (unsigned long)dev);
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > > index 487acd6e2be8..6d27875f41b8 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> > > @@ -120,18 +120,23 @@ static int mt7921s_probe(struct sdio_func *func,
> > > .fw_own = mt7921s_mcu_fw_pmctrl,
> > > };
> > >
> > > + struct ieee80211_ops *ops;
> > > struct mt7921_dev *dev;
> > > struct mt76_dev *mdev;
> > > int ret;
> > >
> > > - mdev = mt76_alloc_device(&func->dev, sizeof(*dev), &mt7921_ops,
> > > - &drv_ops);
> > > + ops = devm_kmemdup(&func->dev, &mt7921_ops, sizeof(mt7921_ops),
> > > + GFP_KERNEL);
> > > + if (!ops)
> > > + return -ENOMEM;
> > > +
> > > + mdev = mt76_alloc_device(&func->dev, sizeof(*dev), ops, &drv_ops);
> > > if (!mdev)
> > > return -ENOMEM;
> > >
> > > dev = container_of(mdev, struct mt7921_dev, mt76);
> > > dev->hif_ops = &mt7921_sdio_ops;
> > > -
> > > + dev->ops = ops;
> > > sdio_set_drvdata(func, dev);
> > >
> > > ret = mt76s_init(mdev, func, &mt7921s_ops);
> > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
> > > index d06cee386acd..cf3ec59a4270 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/usb.c
> > > @@ -217,6 +217,7 @@ static int mt7921u_probe(struct usb_interface *usb_intf,
> > >
> > > dev = container_of(mdev, struct mt7921_dev, mt76);
> > > dev->hif_ops = &hif_ops;
> > > + dev->ops = ops;
> > >
> > > udev = usb_get_dev(udev);
> > > usb_reset_device(udev);
> > > --
> > > 2.25.1
> > >

Attachments:

(No filename) (6.36 kB)
signature.asc (235.00 B)
Download all attachments

2022-08-18 07:46:40

Sean Wang <[email protected]> writes:

>> > > > +static int mt7921_check_offload_capability(struct mt7921_dev *dev)
>> > > > +{
>> > > > + struct ieee80211_hw *hw = mt76_hw(dev);
>> > > > + int year, mon, day, hour, min, sec;
>> > > > + struct wiphy *wiphy = hw->wiphy;
>> > > > + bool fw_can_roc = false;
>> > > > + int ret;
>> > > > +
>> > > > + ret = sscanf(dev->mt76.hw->wiphy->fw_version + 11, "%4d%2d%2d%2d%2d%2d",
>> > > > + &year, &mon, &day, &hour, &min, &sec);
>> > >
>> > > does the fw have a differnt base version with respect to the previous ones?
>> > > checking the date is a bit ugly.
>> >
>> > I admitted that way was a bit ugly, but I have investigated for a
>> > while, and that is the only way we can use to distinguish the version
>> > in current mt7921 firmware.
>>
>> the fw seems pretty new (2022/7/15), is it already available in linux-firmware
>> git tree? If not I guess you can increment fw version in a more evident way.
>> For the future please remember to do it for major fw changes.
>
> The new FW is not released in linux-firmware. I will try to figure out
> a better way to recognize the FW can support the feature and add it in
> v2.

It would be a lot better and more future proof to use some kind feature
advertising instead of guessing from a firmware version. For example,
ath11k uses WMI service bits and other upstream drivers have something
similar.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches