2021-05-14 09:16:30

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/7] rtl8xxxu: add code to handle BSS_CHANGED_TXPOWER/IEEE80211_CONF_CHANGE_POWER

On Fri, 2021-05-14 at 04:04 +0200, Reto Schneider wrote:
> From: Chris Chiu <[email protected]>
>
> The 'iw set txpower' is not handled by the driver. Use the existing
> set_tx_power function to apply the tx power change

trivial notes:

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
[]
> @@ -1382,6 +1382,38 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
> ? }
> ?}
> ?
>
> +#define MAX_TXPWR_IDX_NMODE_92S 63
> +
> +u8
> +rtl8xxxu_gen1_dbm_to_txpwridx(struct rtl8xxxu_priv *priv, u16 mode, int dbm)
> +{
> + u8 txpwridx;
> + long offset;

why should offset be long when dbm is int?

> +
> + switch (mode) {
> + case WIRELESS_MODE_B:
> + offset = -7;
> + break;
> + case WIRELESS_MODE_G:
> + case WIRELESS_MODE_N_24G:
> + offset = -8;
> + break;
> + default:
> + offset = -8;
> + break;
> + }
> +
> + if ((dbm - offset) > 0)
> + txpwridx = (u8)((dbm - offset) * 2);

overflow of u8 when dbm >= 136?


> + else
> + txpwridx = 0;
> +
> + if (txpwridx > MAX_TXPWR_IDX_NMODE_92S)
> + txpwridx = MAX_TXPWR_IDX_NMODE_92S;
> +
> + return txpwridx;
> +}
> +
> ?void
> ?rtl8xxxu_gen1_set_tx_power(struct rtl8xxxu_priv *priv, int channel, bool ht40)
> ?{
> @@ -4508,6 +4540,55 @@ rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
> ? return network_type;
> ?}
> ?
>
> +static void rtl8xxxu_update_txpower(struct rtl8xxxu_priv *priv, int power)
> +{
> + bool ht40 = false;

unnecessary initializations.

> + struct ieee80211_hw *hw = priv->hw;
> + int channel = hw->conf.chandef.chan->hw_value;
> + u8 cck_txpwridx, ofdm_txpwridx;
> + int i, group;
> +
> + if (!priv->fops->dbm_to_txpwridx)
> + return;
> +
> + switch (hw->conf.chandef.width) {
> + case NL80211_CHAN_WIDTH_20_NOHT:
> + case NL80211_CHAN_WIDTH_20:
> + ht40 = false;
> + break;
> + case NL80211_CHAN_WIDTH_40:
> + ht40 = true;
> + break;
> + default:
> + return;
> + }