From: Lorenzo Bianconi <lorenzo@kernel.org>
To: sean.wang@mediatek.com
Cc: 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@mediatek.com, km.lin@mediatek.com,
robin.chiu@mediatek.com, ch.yeh@mediatek.com,
posh.sun@mediatek.com, Eric.Liang@mediatek.com,
Stella.Chang@mediatek.com, jemele@google.com,
yenlinlai@google.com, linux-wireless@vger.kernel.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] mt76: mt7921: add .set_sar_specs support
Date: Sun, 20 Jun 2021 11:40:21 +0200 [thread overview]
Message-ID: <YM8NBUZp+/97kiIH@lore-desk> (raw)
In-Reply-To: <05d4551875d7ddf8de6679612ca3e650bded9926.1624167097.git.objelf@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 7211 bytes --]
> From: YN Chen <YN.Chen@mediatek.com>
>
> add .set_sar_specs to allow configuring SAR power limitations on the
> frequency ranges from the userland.
>
> Co-developed-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: YN Chen <YN.Chen@mediatek.com>
> ---
> drivers/net/wireless/mediatek/mt76/mt76.h | 3 ++
> .../net/wireless/mediatek/mt76/mt76_connac.h | 5 +++
> .../wireless/mediatek/mt76/mt76_connac_mcu.c | 24 +++++++++-
> .../net/wireless/mediatek/mt76/mt7921/init.c | 21 +++++++++
> .../net/wireless/mediatek/mt76/mt7921/main.c | 45 +++++++++++++++++++
> 5 files changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 25c5ceef5257..63ce5242c4d5 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -636,6 +636,9 @@ struct mt76_phy {
> struct sk_buff **tail;
> u16 seqno;
> } rx_amsdu[__MT_RXQ_MAX];
> +
> + void *frp_data;
> + u8 num_frps;
> };
>
> struct mt76_dev {
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac.h b/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> index f49d97d0a1c5..95c7d1cac7c4 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> @@ -83,6 +83,11 @@ struct mt76_connac_coredump {
> unsigned long last_activity;
> };
>
> +struct mt76_connac_freq_range_power {
> + const struct cfg80211_sar_freq_ranges *range;
> + s8 power;
> +};
> +
> extern const struct wiphy_wowlan_support mt76_connac_wowlan_support;
>
> static inline bool is_mt7921(struct mt76_dev *dev)
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> index 5c3a81e5f559..07f45cb4f500 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> @@ -1749,6 +1749,24 @@ mt76_connac_mcu_build_sku(struct mt76_dev *dev, s8 *sku,
> }
> }
>
> +static s8 mt76_connac_get_sar_power(struct mt76_phy *phy,
> + struct ieee80211_channel *chan,
> + s8 target_power)
> +{
> + struct mt76_connac_freq_range_power *frp = phy->frp_data;
> + int freq, i;
> +
> + freq = ieee80211_channel_to_frequency(chan->hw_value, chan->band);
> + for (i = 0 ; i < phy->num_frps; i++) {
> + if (frp[i].range &&
> + freq >= frp[i].range->start_freq &&
> + freq < frp[i].range->end_freq)
> + target_power = min_t(s8, frp[i].power, target_power);
can we have overlapping freq range? if not I guess you can add a break here.
> + }
> +
> + return target_power;
> +}
> +
> static int
> mt76_connac_mcu_rate_txpower_band(struct mt76_phy *phy,
> enum nl80211_band band)
> @@ -1816,9 +1834,13 @@ mt76_connac_mcu_rate_txpower_band(struct mt76_phy *phy,
> .hw_value = ch_list[idx],
> .band = band,
> };
> + s8 sar_power;
> +
> + sar_power = mt76_connac_get_sar_power(phy, &chan,
> + tx_power);
>
> mt76_get_rate_power_limits(phy, &chan, &limits,
> - tx_power);
> + sar_power);
>
> tx_power_tlv.last_msg = ch_list[idx] == last_ch;
> sku_tlbv.channel = ch_list[idx];
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> index a9ce10b98827..e7f66d988a49 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> @@ -24,6 +24,20 @@ static const struct ieee80211_iface_combination if_comb[] = {
> }
> };
>
> +const struct cfg80211_sar_freq_ranges mt7921_sar_freq_ranges[] = {
> + { .start_freq = 2402, .end_freq = 2494, },
> + { .start_freq = 5150, .end_freq = 5350, },
> + { .start_freq = 5350, .end_freq = 5470, },
> + { .start_freq = 5470, .end_freq = 5725, },
> + { .start_freq = 5725, .end_freq = 5950, },
> +};
> +
> +static const struct cfg80211_sar_capa mt7921_sar_capa = {
> + .type = NL80211_SAR_TYPE_POWER,
> + .num_freq_ranges = ARRAY_SIZE(mt7921_sar_freq_ranges),
> + .freq_ranges = &mt7921_sar_freq_ranges[0],
> +};
> +
> static void
> mt7921_regd_notifier(struct wiphy *wiphy,
> struct regulatory_request *request)
> @@ -75,6 +89,13 @@ mt7921_init_wiphy(struct ieee80211_hw *hw)
> wiphy->max_sched_scan_reqs = 1;
> wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;
> wiphy->reg_notifier = mt7921_regd_notifier;
> + wiphy->sar_capa = &mt7921_sar_capa;
> +
> + phy->mt76->frp_data = devm_kcalloc(dev->mt76.dev,
> + wiphy->sar_capa->num_freq_ranges,
> + sizeof(struct mt76_connac_freq_range_power),
> + GFP_KERNEL);
Here I guess we need to check if devm_kcalloc fails returning an int from
mt7921_init_wiphy()
> + phy->mt76->num_frps = wiphy->sar_capa->num_freq_ranges;
>
> wiphy->features |= NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR |
> NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> index 7fd21049ff5a..e9b5d79e139f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> @@ -1171,6 +1171,50 @@ static void mt7921_sta_set_decap_offload(struct ieee80211_hw *hw,
> MCU_UNI_CMD_STA_REC_UPDATE);
> }
>
> +static int mt7921_set_sar_specs(struct ieee80211_hw *hw,
> + const struct cfg80211_sar_specs *sar)
> +{
> + const struct cfg80211_sar_capa *capa = hw->wiphy->sar_capa;
> + struct mt76_connac_freq_range_power *frp_data, *frp;
> + struct mt7921_dev *dev = mt7921_hw_dev(hw);
> + struct mt76_phy *mphy = hw->priv;
> + int err;
> + u32 i;
> +
> + mt7921_mutex_acquire(dev);
> +
> + if (!sar || sar->type != NL80211_SAR_TYPE_POWER ||
> + sar->num_sub_specs == 0) {
> + err = -EINVAL;
> + goto out;
> + }
I guess you can move this out of the lock
> +
> + frp_data = mphy->frp_data;
> +
> + for (i = 0; i < sar->num_sub_specs; i++) {
> + u32 index = sar->sub_specs[i].freq_range_index;
> + /* SAR specifies power limitaton in 0.25dbm */
> + s32 power = sar->sub_specs[i].power >> 1;
> +
> + if (power > 127 || power < -127)
> + power = 127;
> +
> + if (index >= mphy->num_frps)
> + continue;
> +
> + frp = &frp_data[index];
> + frp->range = &capa->freq_ranges[index];
I guess there is an use-after-free here since sar is freed at the end of
nl80211_set_sar_specs, right?
Regards,
Lorenzo
> + frp->power = power;
> + }
> +
> + err = mt76_connac_mcu_set_rate_txpower(mphy);
> +
> +out:
> + mt7921_mutex_release(dev);
> +
> + return err;
> +}
> +
> const struct ieee80211_ops mt7921_ops = {
> .tx = mt7921_tx,
> .start = mt7921_start,
> @@ -1209,4 +1253,5 @@ const struct ieee80211_ops mt7921_ops = {
> .set_rekey_data = mt7921_set_rekey_data,
> #endif /* CONFIG_PM */
> .flush = mt7921_flush,
> + .set_sar_specs = mt7921_set_sar_specs,
> };
> --
> 2.25.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Bianconi <lorenzo@kernel.org>
To: sean.wang@mediatek.com
Cc: 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@mediatek.com, km.lin@mediatek.com,
robin.chiu@mediatek.com, ch.yeh@mediatek.com,
posh.sun@mediatek.com, Eric.Liang@mediatek.com,
Stella.Chang@mediatek.com, jemele@google.com,
yenlinlai@google.com, linux-wireless@vger.kernel.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] mt76: mt7921: add .set_sar_specs support
Date: Sun, 20 Jun 2021 11:40:21 +0200 [thread overview]
Message-ID: <YM8NBUZp+/97kiIH@lore-desk> (raw)
In-Reply-To: <05d4551875d7ddf8de6679612ca3e650bded9926.1624167097.git.objelf@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7211 bytes --]
> From: YN Chen <YN.Chen@mediatek.com>
>
> add .set_sar_specs to allow configuring SAR power limitations on the
> frequency ranges from the userland.
>
> Co-developed-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: YN Chen <YN.Chen@mediatek.com>
> ---
> drivers/net/wireless/mediatek/mt76/mt76.h | 3 ++
> .../net/wireless/mediatek/mt76/mt76_connac.h | 5 +++
> .../wireless/mediatek/mt76/mt76_connac_mcu.c | 24 +++++++++-
> .../net/wireless/mediatek/mt76/mt7921/init.c | 21 +++++++++
> .../net/wireless/mediatek/mt76/mt7921/main.c | 45 +++++++++++++++++++
> 5 files changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 25c5ceef5257..63ce5242c4d5 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -636,6 +636,9 @@ struct mt76_phy {
> struct sk_buff **tail;
> u16 seqno;
> } rx_amsdu[__MT_RXQ_MAX];
> +
> + void *frp_data;
> + u8 num_frps;
> };
>
> struct mt76_dev {
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac.h b/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> index f49d97d0a1c5..95c7d1cac7c4 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac.h
> @@ -83,6 +83,11 @@ struct mt76_connac_coredump {
> unsigned long last_activity;
> };
>
> +struct mt76_connac_freq_range_power {
> + const struct cfg80211_sar_freq_ranges *range;
> + s8 power;
> +};
> +
> extern const struct wiphy_wowlan_support mt76_connac_wowlan_support;
>
> static inline bool is_mt7921(struct mt76_dev *dev)
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> index 5c3a81e5f559..07f45cb4f500 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
> @@ -1749,6 +1749,24 @@ mt76_connac_mcu_build_sku(struct mt76_dev *dev, s8 *sku,
> }
> }
>
> +static s8 mt76_connac_get_sar_power(struct mt76_phy *phy,
> + struct ieee80211_channel *chan,
> + s8 target_power)
> +{
> + struct mt76_connac_freq_range_power *frp = phy->frp_data;
> + int freq, i;
> +
> + freq = ieee80211_channel_to_frequency(chan->hw_value, chan->band);
> + for (i = 0 ; i < phy->num_frps; i++) {
> + if (frp[i].range &&
> + freq >= frp[i].range->start_freq &&
> + freq < frp[i].range->end_freq)
> + target_power = min_t(s8, frp[i].power, target_power);
can we have overlapping freq range? if not I guess you can add a break here.
> + }
> +
> + return target_power;
> +}
> +
> static int
> mt76_connac_mcu_rate_txpower_band(struct mt76_phy *phy,
> enum nl80211_band band)
> @@ -1816,9 +1834,13 @@ mt76_connac_mcu_rate_txpower_band(struct mt76_phy *phy,
> .hw_value = ch_list[idx],
> .band = band,
> };
> + s8 sar_power;
> +
> + sar_power = mt76_connac_get_sar_power(phy, &chan,
> + tx_power);
>
> mt76_get_rate_power_limits(phy, &chan, &limits,
> - tx_power);
> + sar_power);
>
> tx_power_tlv.last_msg = ch_list[idx] == last_ch;
> sku_tlbv.channel = ch_list[idx];
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> index a9ce10b98827..e7f66d988a49 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> @@ -24,6 +24,20 @@ static const struct ieee80211_iface_combination if_comb[] = {
> }
> };
>
> +const struct cfg80211_sar_freq_ranges mt7921_sar_freq_ranges[] = {
> + { .start_freq = 2402, .end_freq = 2494, },
> + { .start_freq = 5150, .end_freq = 5350, },
> + { .start_freq = 5350, .end_freq = 5470, },
> + { .start_freq = 5470, .end_freq = 5725, },
> + { .start_freq = 5725, .end_freq = 5950, },
> +};
> +
> +static const struct cfg80211_sar_capa mt7921_sar_capa = {
> + .type = NL80211_SAR_TYPE_POWER,
> + .num_freq_ranges = ARRAY_SIZE(mt7921_sar_freq_ranges),
> + .freq_ranges = &mt7921_sar_freq_ranges[0],
> +};
> +
> static void
> mt7921_regd_notifier(struct wiphy *wiphy,
> struct regulatory_request *request)
> @@ -75,6 +89,13 @@ mt7921_init_wiphy(struct ieee80211_hw *hw)
> wiphy->max_sched_scan_reqs = 1;
> wiphy->flags |= WIPHY_FLAG_HAS_CHANNEL_SWITCH;
> wiphy->reg_notifier = mt7921_regd_notifier;
> + wiphy->sar_capa = &mt7921_sar_capa;
> +
> + phy->mt76->frp_data = devm_kcalloc(dev->mt76.dev,
> + wiphy->sar_capa->num_freq_ranges,
> + sizeof(struct mt76_connac_freq_range_power),
> + GFP_KERNEL);
Here I guess we need to check if devm_kcalloc fails returning an int from
mt7921_init_wiphy()
> + phy->mt76->num_frps = wiphy->sar_capa->num_freq_ranges;
>
> wiphy->features |= NL80211_FEATURE_SCHED_SCAN_RANDOM_MAC_ADDR |
> NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR;
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> index 7fd21049ff5a..e9b5d79e139f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> @@ -1171,6 +1171,50 @@ static void mt7921_sta_set_decap_offload(struct ieee80211_hw *hw,
> MCU_UNI_CMD_STA_REC_UPDATE);
> }
>
> +static int mt7921_set_sar_specs(struct ieee80211_hw *hw,
> + const struct cfg80211_sar_specs *sar)
> +{
> + const struct cfg80211_sar_capa *capa = hw->wiphy->sar_capa;
> + struct mt76_connac_freq_range_power *frp_data, *frp;
> + struct mt7921_dev *dev = mt7921_hw_dev(hw);
> + struct mt76_phy *mphy = hw->priv;
> + int err;
> + u32 i;
> +
> + mt7921_mutex_acquire(dev);
> +
> + if (!sar || sar->type != NL80211_SAR_TYPE_POWER ||
> + sar->num_sub_specs == 0) {
> + err = -EINVAL;
> + goto out;
> + }
I guess you can move this out of the lock
> +
> + frp_data = mphy->frp_data;
> +
> + for (i = 0; i < sar->num_sub_specs; i++) {
> + u32 index = sar->sub_specs[i].freq_range_index;
> + /* SAR specifies power limitaton in 0.25dbm */
> + s32 power = sar->sub_specs[i].power >> 1;
> +
> + if (power > 127 || power < -127)
> + power = 127;
> +
> + if (index >= mphy->num_frps)
> + continue;
> +
> + frp = &frp_data[index];
> + frp->range = &capa->freq_ranges[index];
I guess there is an use-after-free here since sar is freed at the end of
nl80211_set_sar_specs, right?
Regards,
Lorenzo
> + frp->power = power;
> + }
> +
> + err = mt76_connac_mcu_set_rate_txpower(mphy);
> +
> +out:
> + mt7921_mutex_release(dev);
> +
> + return err;
> +}
> +
> const struct ieee80211_ops mt7921_ops = {
> .tx = mt7921_tx,
> .start = mt7921_start,
> @@ -1209,4 +1253,5 @@ const struct ieee80211_ops mt7921_ops = {
> .set_rekey_data = mt7921_set_rekey_data,
> #endif /* CONFIG_PM */
> .flush = mt7921_flush,
> + .set_sar_specs = mt7921_set_sar_specs,
> };
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2021-06-20 9:40 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-20 5:39 [PATCH] mt76: mt7921: add .set_sar_specs support sean.wang
2021-06-20 5:39 ` sean.wang
2021-06-20 9:40 ` Lorenzo Bianconi [this message]
2021-06-20 9:40 ` Lorenzo Bianconi
[not found] <YM8NBUZp+/97kiIH@lore-desk--annotate>
2021-06-21 22:42 ` sean.wang
2021-06-21 22:42 ` sean.wang
2021-06-22 6:40 ` Lorenzo Bianconi
2021-06-22 6:40 ` Lorenzo Bianconi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YM8NBUZp+/97kiIH@lore-desk \
--to=lorenzo@kernel.org \
--cc=Deren.Wu@mediatek.com \
--cc=Eric-SY.Chang@mediatek.com \
--cc=Eric.Liang@mediatek.com \
--cc=Leon.Yen@mediatek.com \
--cc=Soul.Huang@mediatek.com \
--cc=Stella.Chang@mediatek.com \
--cc=YN.Chen@mediatek.com \
--cc=ch.yeh@mediatek.com \
--cc=jemele@google.com \
--cc=km.lin@mediatek.com \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lorenzo.bianconi@redhat.com \
--cc=nbd@nbd.name \
--cc=posh.sun@mediatek.com \
--cc=robin.chiu@mediatek.com \
--cc=sean.wang@mediatek.com \
--cc=yenlinlai@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.