* Re: [RFC PATCH v4] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Chris Chiu @ 2019-06-10 10:29 UTC (permalink / raw)
To: Jes Sorensen
Cc: Kalle Valo, David Miller, linux-wireless, netdev, Linux Kernel,
Linux Upstreaming Team
In-Reply-To: <CAB4CAwf3Mi2iuR7nAj1U4EoyU5ZnvY9xoLrv7QT2X-tc_1ex3g@mail.gmail.com>
On Wed, Jun 5, 2019 at 10:17 AM Chris Chiu <chiu@endlessm.com> wrote:
>
> On Tue, Jun 4, 2019 at 3:21 AM Jes Sorensen <jes.sorensen@gmail.com> wrote:
> >
> > On 5/31/19 5:12 AM, Chris Chiu wrote:
> > > We have 3 laptops which connect the wifi by the same RTL8723BU.
> > > The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
> > > They have the same problem with the in-kernel rtl8xxxu driver, the
> > > iperf (as a client to an ethernet-connected server) gets ~1Mbps.
> > > Nevertheless, the signal strength is reported as around -40dBm,
> > > which is quite good. From the wireshark capture, the tx rate for each
> > > data and qos data packet is only 1Mbps. Compare to the Realtek driver
> > > at https://github.com/lwfinger/rtl8723bu, the same iperf test gets
> > > ~12Mbps or better. The signal strength is reported similarly around
> > > -40dBm. That's why we want to improve.
> > >
> > > After reading the source code of the rtl8xxxu driver and Realtek's, the
> > > major difference is that Realtek's driver has a watchdog which will keep
> > > monitoring the signal quality and updating the rate mask just like the
> > > rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
> > > And this kind of watchdog also exists in rtlwifi driver of some specific
> > > chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
> > > the same member function named dm_watchdog and will invoke the
> > > corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
> > > mask.
> > >
> > > With this commit, the tx rate of each data and qos data packet will
> > > be 39Mbps (MCS4) with the 0xF00000 as the tx rate mask. The 20th bit
> > > to 23th bit means MCS4 to MCS7. It means that the firmware still picks
> > > the lowest rate from the rate mask and explains why the tx rate of
> > > data and qos data is always lowest 1Mbps because the default rate mask
> > > passed is always 0xFFFFFFF ranges from the basic CCK rate, OFDM rate,
> > > and MCS rate. However, with Realtek's driver, the tx rate observed from
> > > wireshark under the same condition is almost 65Mbps or 72Mbps.
> > >
> > > I believe the firmware of RTL8723BU may need fix. And I think we
> > > can still bring in the dm_watchdog as rtlwifi to improve from the
> > > driver side. Please leave precious comments for my commits and
> > > suggest what I can do better. Or suggest if there's any better idea
> > > to fix this. Thanks.
> > >
> > > Signed-off-by: Chris Chiu <chiu@endlessm.com>
> >
> > I am really pleased to see you're investigating some of these issues,
> > since I've been pretty swamped and not had time to work on this driver
> > for a long time.
> >
> > The firmware should allow for two rate modes, either firmware handled or
> > controlled by the driver. Ideally we would want the driver to handle it,
> > but I never was able to make that work reliable.
> >
> > This fix should at least improve the situation, and it may explain some
> > of the performance issues with the 8192eu as well?
> >
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > > index 8828baf26e7b..216f603827a8 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > > @@ -1195,6 +1195,44 @@ struct rtl8723bu_c2h {
> > >
> > > struct rtl8xxxu_fileops;
> > >
> > > +/*mlme related.*/
> > > +enum wireless_mode {
> > > + WIRELESS_MODE_UNKNOWN = 0,
> > > + /* Sub-Element */
> > > + WIRELESS_MODE_B = BIT(0),
> > > + WIRELESS_MODE_G = BIT(1),
> > > + WIRELESS_MODE_A = BIT(2),
> > > + WIRELESS_MODE_N_24G = BIT(3),
> > > + WIRELESS_MODE_N_5G = BIT(4),
> > > + WIRELESS_AUTO = BIT(5),
> > > + WIRELESS_MODE_AC = BIT(6),
> > > + WIRELESS_MODE_MAX = 0x7F,
> > > +};
> > > +
> > > +/* from rtlwifi/wifi.h */
> > > +enum ratr_table_mode_new {
> > > + RATEID_IDX_BGN_40M_2SS = 0,
> > > + RATEID_IDX_BGN_40M_1SS = 1,
> > > + RATEID_IDX_BGN_20M_2SS_BN = 2,
> > > + RATEID_IDX_BGN_20M_1SS_BN = 3,
> > > + RATEID_IDX_GN_N2SS = 4,
> > > + RATEID_IDX_GN_N1SS = 5,
> > > + RATEID_IDX_BG = 6,
> > > + RATEID_IDX_G = 7,
> > > + RATEID_IDX_B = 8,
> > > + RATEID_IDX_VHT_2SS = 9,
> > > + RATEID_IDX_VHT_1SS = 10,
> > > + RATEID_IDX_MIX1 = 11,
> > > + RATEID_IDX_MIX2 = 12,
> > > + RATEID_IDX_VHT_3SS = 13,
> > > + RATEID_IDX_BGN_3SS = 14,
> > > +};
> > > +
> > > +#define RTL8XXXU_RATR_STA_INIT 0
> > > +#define RTL8XXXU_RATR_STA_HIGH 1
> > > +#define RTL8XXXU_RATR_STA_MID 2
> > > +#define RTL8XXXU_RATR_STA_LOW 3
> > > +
> >
> > > extern struct rtl8xxxu_fileops rtl8192cu_fops;
> > > extern struct rtl8xxxu_fileops rtl8192eu_fops;
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > > index 26b674aca125..2071ab9fd001 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > > @@ -1645,6 +1645,148 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
> > > rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
> > > }
> > >
> > > +static u8 rtl8723b_signal_to_rssi(int signal)
> > > +{
> > > + if (signal < -95)
> > > + signal = -95;
> > > + return (u8)(signal + 95);
> > > +}
> >
> > Could you make this more generic so it can be used by the other sub-drivers?
> >
> Sure. I'll do that.
>
> > > +static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> > > + int signal, struct ieee80211_sta *sta)
> > > +{
> > > + struct ieee80211_hw *hw = priv->hw;
> > > + u16 wireless_mode;
> > > + u8 rssi_level, ratr_idx;
> > > + u8 txbw_40mhz;
> > > + u8 rssi, rssi_thresh_high, rssi_thresh_low;
> > > +
> > > + rssi_level = priv->rssi_level;
> > > + rssi = rtl8723b_signal_to_rssi(signal);
> > > + txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40) ? 1 : 0;
> > > +
> > > + switch (rssi_level) {
> > > + case RTL8XXXU_RATR_STA_HIGH:
> > > + rssi_thresh_high = 50;
> > > + rssi_thresh_low = 20;
> > > + break;
> > > + case RTL8XXXU_RATR_STA_MID:
> > > + rssi_thresh_high = 55;
> > > + rssi_thresh_low = 20;
> > > + break;
> > > + case RTL8XXXU_RATR_STA_LOW:
> > > + rssi_thresh_high = 60;
> > > + rssi_thresh_low = 25;
> > > + break;
> > > + default:
> > > + rssi_thresh_high = 50;
> > > + rssi_thresh_low = 20;
> > > + break;
> > > + }
> >
> > Can we make this use defined values with some explanation rather than
> > hard coded values?
> >
>
> I also thought about this. So I refer to the same refresh_rateadaotive_mask
> in rtlwifi/rtl8192se/dm.c, rtlwifi/rtl8723ae/dm.c, and rtl8188ee...etc. They
> don't give a better explanation. And I also don't know if these values can be
> generally applied to other subdrivers or specifically for 8723b series, for
> example, the rtl8192se use different values for the threshold. It maybe due
> to different noise floor for different chip? I'm not sure. I took these values
> from vendor driver and rtl8188ee. I can simply use defined values to replace
> but I have to admit it's hard to find a good explanation.
>
> > > + if (rssi > rssi_thresh_high)
> > > + rssi_level = RTL8XXXU_RATR_STA_HIGH;
> > > + else if (rssi > rssi_thresh_low)
> > > + rssi_level = RTL8XXXU_RATR_STA_MID;
> > > + else
> > > + rssi_level = RTL8XXXU_RATR_STA_LOW;
> > > +
> > > + if (rssi_level != priv->rssi_level) {
> > > + int sgi = 0;
> > > + u32 rate_bitmap = 0;
> > > +
> > > + rcu_read_lock();
> > > + rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> > > + (sta->ht_cap.mcs.rx_mask[0] << 12) |
> > > + (sta->ht_cap.mcs.rx_mask[1] << 20);
> > > + if (sta->ht_cap.cap &
> > > + (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> > > + sgi = 1;
> > > + rcu_read_unlock();
> > > +
> > > + wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> > > + switch (wireless_mode) {
> > > + case WIRELESS_MODE_B:
> > > + ratr_idx = RATEID_IDX_B;
> > > + if (rate_bitmap & 0x0000000c)
> > > + rate_bitmap &= 0x0000000d;
> > > + else
> > > + rate_bitmap &= 0x0000000f;
> > > + break;
> > > + case WIRELESS_MODE_A:
> > > + case WIRELESS_MODE_G:
> > > + ratr_idx = RATEID_IDX_G;
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > > + rate_bitmap &= 0x00000f00;
> > > + else
> > > + rate_bitmap &= 0x00000ff0;
> > > + break;
> > > + case (WIRELESS_MODE_B | WIRELESS_MODE_G):
> > > + ratr_idx = RATEID_IDX_BG;
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > > + rate_bitmap &= 0x00000f00;
> > > + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> > > + rate_bitmap &= 0x00000ff0;
> > > + else
> > > + rate_bitmap &= 0x00000ff5;
> > > + break;
> >
> > It would be nice as well to get all these masks into generic names.
> >
>
> I also take these mask values from the update_hal_rate_mask of the
> vendor driver and other realtek drivers under rtlwifi. I thought about to
> define the lower 12 bits like RTL8XXXU_BG_RATE_MASK, 13~20 bits
> as RTL8XXXU_MCS0_7_RATE_MASK. But it's still hard to express
> all the combinations here. So I just leave it as it is. I can try to add
> explanations for the rate mapping of each bit. It would be a lot easier.
>
> > > + case WIRELESS_MODE_N_24G:
> > > + case WIRELESS_MODE_N_5G:
> > > + case (WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> > > + case (WIRELESS_MODE_A | WIRELESS_MODE_N_5G):
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > > + ratr_idx = RATEID_IDX_GN_N2SS;
> > > + else
> > > + ratr_idx = RATEID_IDX_GN_N1SS;
> > > + case (WIRELESS_MODE_B | WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> > > + case (WIRELESS_MODE_B | WIRELESS_MODE_N_24G):
> > > + if (txbw_40mhz) {
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > > + ratr_idx = RATEID_IDX_BGN_40M_2SS;
> > > + else
> > > + ratr_idx = RATEID_IDX_BGN_40M_1SS;
> > > + } else {
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > > + ratr_idx = RATEID_IDX_BGN_20M_2SS_BN;
> > > + else
> > > + ratr_idx = RATEID_IDX_BGN_20M_1SS_BN;
> > > + }
> > > +
> > > + if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> > > + rate_bitmap &= 0x0f8f0000;
> > > + } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> > > + rate_bitmap &= 0x0f8ff000;
> > > + } else {
> > > + if (txbw_40mhz)
> > > + rate_bitmap &= 0x0f8ff015;
> > > + else
> > > + rate_bitmap &= 0x0f8ff005;
> > > + }
> > > + } else {
> > > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> > > + rate_bitmap &= 0x000f0000;
> > > + } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> > > + rate_bitmap &= 0x000ff000;
> > > + } else {
> > > + if (txbw_40mhz)
> > > + rate_bitmap &= 0x000ff015;
> > > + else
> > > + rate_bitmap &= 0x000ff005;
> > > + }
> > > + }
> > > + break;
> > > + default:
> > > + ratr_idx = RATEID_IDX_BGN_40M_2SS;
> > > + rate_bitmap &= 0x0fffffff;
> > > + break;
> > > + }
> > > +
> > > + priv->rssi_level = rssi_level;
> > > + priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi);
> > > + }
> > > +}
> > > +
> >
> > In general I think all of this should be fairly generic and the other
> > subdrivers should be able to benefit from it?
> >
> >
> I agree. Mabe separates the rssi level judgement function to be chip specific,
> and move the whole refresh_rate_mask thing generic?
>
> > > struct rtl8xxxu_fileops rtl8723bu_fops = {
> > > .parse_efuse = rtl8723bu_parse_efuse,
> > > .load_firmware = rtl8723bu_load_firmware,
> > > @@ -1665,6 +1807,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
> > > .usb_quirks = rtl8xxxu_gen2_usb_quirks,
> > > .set_tx_power = rtl8723b_set_tx_power,
> > > .update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> > > + .refresh_rate_mask = rtl8723b_refresh_rate_mask,
> > > .report_connect = rtl8xxxu_gen2_report_connect,
> > > .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
> > > .writeN_block_size = 1024,
> > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > index 039e5ca9d2e4..be322402ca01 100644
> > > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > > @@ -4311,7 +4311,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
> > > rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> > > }
> > >
> > > -void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> > > +void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> > > + u32 ramask, u8 rateid, int sgi)
> > > {
> > > struct h2c_cmd h2c;
> > >
> > > @@ -4331,7 +4332,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> > > }
> > >
> > > void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> > > - u32 ramask, int sgi)
> > > + u32 ramask, u8 rateid, int sgi)
> > > {
> > > struct h2c_cmd h2c;
> > > u8 bw = 0;
> > > @@ -4345,7 +4346,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> > > h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
> > >
> > > h2c.ramask.arg = 0x80;
> > > - h2c.b_macid_cfg.data1 = 0;
> > > + h2c.b_macid_cfg.data1 = rateid;
> > > if (sgi)
> > > h2c.b_macid_cfg.data1 |= BIT(7);
> > >
> > > @@ -4485,6 +4486,40 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
> > > rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx);
> > > }
> > >
> > > +u16
> > > +rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
> > > +{
> > > + u16 network_type = WIRELESS_MODE_UNKNOWN;
> > > + u32 rate_mask;
> > > +
> > > + rate_mask = (sta->supp_rates[0] & 0xfff) |
> > > + (sta->ht_cap.mcs.rx_mask[0] << 12) |
> > > + (sta->ht_cap.mcs.rx_mask[0] << 20);
> > > +
> > > + if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) {
> > > + if (sta->vht_cap.vht_supported)
> > > + network_type = WIRELESS_MODE_AC;
> > > + else if (sta->ht_cap.ht_supported)
> > > + network_type = WIRELESS_MODE_N_5G;
> > > +
> > > + network_type |= WIRELESS_MODE_A;
> > > + } else {
> > > + if (sta->vht_cap.vht_supported)
> > > + network_type = WIRELESS_MODE_AC;
> > > + else if (sta->ht_cap.ht_supported)
> > > + network_type = WIRELESS_MODE_N_24G;
> > > +
> > > + if (sta->supp_rates[0] <= 0xf)
> > > + network_type |= WIRELESS_MODE_B;
> > > + else if (sta->supp_rates[0] & 0xf)
> > > + network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
> > > + else
> > > + network_type |= WIRELESS_MODE_G;
> > > + }
> > > +
> > > + return network_type;
> > > +}
> >
> > I always hated the wireless_mode nonsense in the realtek driver, but
> > maybe we cannot avoid it :(
> >
> > Cheers,
> > Jes
Jes, look forward to any comments or suggestions from you. I would re-write a
patch with generic refresh_rate_mask for all rtl8xxxu series chips in
short time.
Chris
^ permalink raw reply
* Re: [PATCH v2 2/7] net/mac80211: move WEP handling to ARC4 library interface
From: Ard Biesheuvel @ 2019-06-10 10:58 UTC (permalink / raw)
To: Johannes Berg, Herbert Xu
Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Eric Biggers, <linux-wireless@vger.kernel.org>,
John W. Linville
In-Reply-To: <107dc7707e6c9d0110aa5535bd5baf4f6db7f6a5.camel@sipsolutions.net>
On Sun, 9 Jun 2019 at 22:09, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> Hi Ard,
>
> In general, I have no objections to this.
>
> However, with this
>
> > - select CRYPTO_ARC4
> > + select CRYPTO_LIB_ARC4
>
> and this
>
> > case WLAN_CIPHER_SUITE_WEP40:
> > case WLAN_CIPHER_SUITE_TKIP:
> > case WLAN_CIPHER_SUITE_WEP104:
> > - if (IS_ERR(local->wep_tx_tfm))
> > - return -EINVAL;
> > - break;
>
> there's one quirk that I worry about. Does this mean WEP is now *always*
> available/enabled?
>
> I had to dig in history a bit, but vaguely remembered this commit:
>
> commit 3473187d2459a078e00e5fac8aafc30af69c57fa
> Author: John W. Linville <linville@tuxdriver.com>
> Date: Wed Jul 7 15:07:49 2010 -0400
>
> mac80211: remove wep dependency
>
>
> Since you create the new CRYPTO_LIB_ARC4 in patch 1, I wonder if
> something is broken? I can't really seem to figure out how WEP is
> disallowed in FIPS mode to start with though.
>
>
> (This also is the reason for all the code that removes WEP/TKIP from the
> list of permitted cipher suites when ARC4 isn't available...)
>
Good point. And in the future, there may be other reasons besides FIPS
compliance to turn off WEP support, so it makes sense to retain this
logic.
I am still curious whether FIPS compliance actually requires us to do
this, or whether that code is simply there to work around the lack of
RC4 in the crypto API when FIPS support happens to be enabled.
^ permalink raw reply
* Re: [PATCH] carl9170: fix enum compare splat
From: Christian Lamparter @ 2019-06-10 11:45 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <87pnnlncll.fsf@codeaurora.org>
On Monday, June 10, 2019 9:06:30 AM CEST Kalle Valo wrote:
> Christian Lamparter <chunkeey@gmail.com> writes:
>
> > This patch fixes a noisy warning triggered by -Wenum-compare
> >
> > |main.c:1390:31: warning: comparison between ‘enum nl80211_ac’
> > | and ‘enum ar9170_txq’ [-Wenum-compare]
> > | BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ);
> > | ^
> > | [...]
> >
> > This is a little bit unfortunate, since the number of queues
> > (hence NL80211_NUM_ACS) is a constant based on the IEEE 802.11
> > (much like IEEE80211_NUM_ACS) and __AR9170_NUM_TXQ is more or
> > less defined by the AR9170 hardware.
>
> Is the warning enabled by default? TBH I'm not seeing how useful this
> warning is for kernel development.
It is included in the "-Wall" (which is coming from "KBUILD_CFLAGS"
in the main Makefile).
I tried debian's gcc starting from 4.6 to the lastest 8.3. They all
complain about it in various degrees.
https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Warning-Options.html#Warning-Options
> > --- a/drivers/net/wireless/ath/carl9170/main.c
> > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > @@ -1387,7 +1387,7 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw,
> > int ret;
> >
> > BUILD_BUG_ON(ARRAY_SIZE(ar9170_qmap) != __AR9170_NUM_TXQ);
> > - BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ);
> > + BUILD_BUG_ON((size_t)NL80211_NUM_ACS > (size_t)__AR9170_NUM_TXQ);
>
> IMHO this just makes the code worse. Does it make sense to workaround
> (stupid) compiler warnings like this?
True. What's worse: This isn't really code. The BUILD_BUG_ON Macro is there
to guard but it's getting compiled away. I could also just drop it.
^ permalink raw reply
* Re: [PATCH v2] carl9170: fix misuse of device driver API
From: Alan Stern @ 2019-06-10 14:12 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, linux-usb, Kalle Valo
In-Reply-To: <20190608144947.744-3-chunkeey@gmail.com>
On Sat, 8 Jun 2019, Christian Lamparter wrote:
> This patch follows Alan Stern's recent patch:
> "p54: Fix race between disconnect and firmware loading"
>
> that overhauled carl9170 buggy firmware loading and driver
> unbinding procedures.
>
> Since the carl9170 code was adapted from p54 it uses the
> same functions and is likely to have the same problem, but
> it's just that the syzbot hasn't reproduce them (yet).
>
> a summary from the changes (copied from the p54 patch):
> * Call usb_driver_release_interface() rather than
> device_release_driver().
>
> * Lock udev (the interface's parent) before unbinding the
> driver instead of locking udev->parent.
>
> * During the firmware loading process, take a reference
> to the USB interface instead of the USB device.
>
> * Don't take an unnecessary reference to the device during
> probe (and then don't drop it during disconnect).
>
> and
>
> * Make sure to prevent use-after-free bugs by explicitly
> setting the driver context to NULL after signaling the
> completion.
>
> Cc: <stable@vger.kernel.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> v2: Alan Stern's comments
> - fixed possible use-after-free
> ---
Acked-by: Alan Stern <stern@rowland.harvard.edu>
> drivers/net/wireless/ath/carl9170/usb.c | 39 +++++++++++--------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
> index e7c3f3b8457d..99f1897a775d 100644
> --- a/drivers/net/wireless/ath/carl9170/usb.c
> +++ b/drivers/net/wireless/ath/carl9170/usb.c
> @@ -128,6 +128,8 @@ static const struct usb_device_id carl9170_usb_ids[] = {
> };
> MODULE_DEVICE_TABLE(usb, carl9170_usb_ids);
>
> +static struct usb_driver carl9170_driver;
> +
> static void carl9170_usb_submit_data_urb(struct ar9170 *ar)
> {
> struct urb *urb;
> @@ -966,32 +968,28 @@ static int carl9170_usb_init_device(struct ar9170 *ar)
>
> static void carl9170_usb_firmware_failed(struct ar9170 *ar)
> {
> - struct device *parent = ar->udev->dev.parent;
> - struct usb_device *udev;
> -
> - /*
> - * Store a copy of the usb_device pointer locally.
> - * This is because device_release_driver initiates
> - * carl9170_usb_disconnect, which in turn frees our
> - * driver context (ar).
> + /* Store a copies of the usb_interface and usb_device pointer locally.
> + * This is because release_driver initiates carl9170_usb_disconnect,
> + * which in turn frees our driver context (ar).
> */
> - udev = ar->udev;
> + struct usb_interface *intf = ar->intf;
> + struct usb_device *udev = ar->udev;
>
> complete(&ar->fw_load_wait);
> + /* at this point 'ar' could be already freed. Don't use it anymore */
> + ar = NULL;
>
> /* unbind anything failed */
> - if (parent)
> - device_lock(parent);
> -
> - device_release_driver(&udev->dev);
> - if (parent)
> - device_unlock(parent);
> + usb_lock_device(udev);
> + usb_driver_release_interface(&carl9170_driver, intf);
> + usb_unlock_device(udev);
>
> - usb_put_dev(udev);
> + usb_put_intf(intf);
> }
>
> static void carl9170_usb_firmware_finish(struct ar9170 *ar)
> {
> + struct usb_interface *intf = ar->intf;
> int err;
>
> err = carl9170_parse_firmware(ar);
> @@ -1009,7 +1007,7 @@ static void carl9170_usb_firmware_finish(struct ar9170 *ar)
> goto err_unrx;
>
> complete(&ar->fw_load_wait);
> - usb_put_dev(ar->udev);
> + usb_put_intf(intf);
> return;
>
> err_unrx:
> @@ -1052,7 +1050,6 @@ static int carl9170_usb_probe(struct usb_interface *intf,
> return PTR_ERR(ar);
>
> udev = interface_to_usbdev(intf);
> - usb_get_dev(udev);
> ar->udev = udev;
> ar->intf = intf;
> ar->features = id->driver_info;
> @@ -1094,15 +1091,14 @@ static int carl9170_usb_probe(struct usb_interface *intf,
> atomic_set(&ar->rx_anch_urbs, 0);
> atomic_set(&ar->rx_pool_urbs, 0);
>
> - usb_get_dev(ar->udev);
> + usb_get_intf(intf);
>
> carl9170_set_state(ar, CARL9170_STOPPED);
>
> err = request_firmware_nowait(THIS_MODULE, 1, CARL9170FW_NAME,
> &ar->udev->dev, GFP_KERNEL, ar, carl9170_usb_firmware_step2);
> if (err) {
> - usb_put_dev(udev);
> - usb_put_dev(udev);
> + usb_put_intf(intf);
> carl9170_free(ar);
> }
> return err;
> @@ -1131,7 +1127,6 @@ static void carl9170_usb_disconnect(struct usb_interface *intf)
>
> carl9170_release_firmware(ar);
> carl9170_free(ar);
> - usb_put_dev(udev);
> }
>
> #ifdef CONFIG_PM
>
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-10 16:09 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Aaro Koskinen, Christian Zigotzky, Michael Ellerman, linux-kernel,
linux-wireless, linuxppc-dev
In-Reply-To: <20190610081825.GA16534@lst.de>
[-- Attachment #1: Type: text/plain, Size: 2404 bytes --]
On 6/10/19 3:18 AM, Christoph Hellwig wrote:
> On Sat, Jun 08, 2019 at 04:52:24PM -0500, Larry Finger wrote:
>> On 6/7/19 12:29 PM, Christoph Hellwig wrote:
>>> I don't think we should work around this in the driver, we need to fix
>>> it in the core. I'm curious why my previous patch didn't work. Can
>>> you throw in a few printks what failed? I.e. did dma_direct_supported
>>> return false? Did the actual allocation fail?
>>
>> Routine dma_direct_supported() returns true.
>>
>> The failure is in routine dma_set_mask() in the following if test:
>>
>> if (!dev->dma_mask || !dma_supported(dev, mask))
>> return -EIO;
>>
>> For b43legacy, dev->dma_mask is 0xc265684800000000.
>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x3fffffff, and
>> the routine returns -EIO.
>>
>> For b43, dev->dma_mask is 0xc265684800000001,
>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x77777777, and
>> the routine returns 0.
>
> I don't fully understand what values the above map to. Can you send
> me your actual debugging patch as well?
I do not understand why the if statement returns true as neither of the values
is zero. After seeing the x86 output shown below, I also do not understand all
the trailing zeros.
My entire patch is attached. That output came from this section:
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask,
dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
On a 32-bit x86 computer with 1GB of RAM, that same output was
For b43legacy, dev->dma_mask is 0x01f4029044.
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0x3fffffff, and
the routine returns 0. 30-bit DMA works.
For b43, dev->dma_mask is 0x01f4029044,
dma_supported(dev, mask) is 0x1ef37f7000, mask is 0xffffffff, and
the routine also returns 0. This card supports 32-bit DMA.
Larry
[-- Attachment #2: b43legacy_tests --]
[-- Type: text/plain, Size: 4133 bytes --]
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index b8286a2..7a367ce 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,6 +319,10 @@ extern void copy_user_page(void *to, void *from, unsigned long vaddr,
#endif /* __ASSEMBLY__ */
#include <asm/slice.h>
+#if 1 /* XXX: pmac? dynamic discovery? */
+#define ARCH_ZONE_DMA_BITS 30
+#else
#define ARCH_ZONE_DMA_BITS 31
+#endif
#endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index 09231ef..761d951 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -20,6 +20,8 @@
*/
static inline bool dma_iommu_alloc_bypass(struct device *dev)
{
+ pr_info("dev->archdata.iommu_bypass %d, !iommu_fixed_is_weak %d\n",
+ dev->archdata.iommu_bypass, !iommu_fixed_is_weak)
return dev->archdata.iommu_bypass && !iommu_fixed_is_weak &&
dma_direct_supported(dev, dev->coherent_dma_mask);
}
@@ -27,6 +29,8 @@ static inline bool dma_iommu_alloc_bypass(struct device *dev)
static inline bool dma_iommu_map_bypass(struct device *dev,
unsigned long attrs)
{
+ pr_info("(attrs & DMA_ATTR_WEAK_ORDERING) %d\n",
+ (attrs & DMA_ATTR_WEAK_ORDERING));
return dev->archdata.iommu_bypass &&
(!iommu_fixed_is_weak || (attrs & DMA_ATTR_WEAK_ORDERING));
}
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index cba2913..2540d3b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -248,7 +248,8 @@ void __init paging_init(void)
(long int)((top_of_ram - total_ram) >> 20));
#ifdef CONFIG_ZONE_DMA
- max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffffffUL >> PAGE_SHIFT);
+ max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
+ ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
#endif
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
#ifdef CONFIG_HIGHMEM
diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 806406a..e0270da 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -1053,6 +1053,7 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_info("dma_set_mask_and_coherent %d, mask 0x%llx\n", err, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c b/drivers/net/wireless/broadcom/b43legacy/dma.c
index 1cc25f4..c625ffc 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -794,6 +794,7 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
* lower mask, as we can always also support a lower one. */
while (1) {
err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+ pr_info("dma_set_mask_and_coherent %d, mask 0x%llx\n", err, mask);
if (!err)
break;
if (mask == DMA_BIT_MASK(64)) {
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e..b716e62 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -391,6 +391,8 @@ int dma_direct_supported(struct device *dev, u64 mask)
* use __phys_to_dma() here so that the SME encryption mask isn't
* part of the check.
*/
+ pr_info("min_mask 0x%x. max_pfn 0x%x, __phys_to_dma 0x%x, mask 0x%x\n", min_mask,
+ max_pfn, __phys_to_dma(dev, min_mask), mask);
return mask >= __phys_to_dma(dev, min_mask);
}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdad..ba2489d 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,9 +317,12 @@ int dma_supported(struct device *dev, u64 mask)
int dma_set_mask(struct device *dev, u64 mask)
{
+ pr_info("mask 0x%llx, dma_mask 0x%llx, dma_supported 0x%llx\n", mask, dev->dma_mask,
+ dma_supported(dev, mask));
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+ pr_info("Continuing in dma_set_mask()\n");
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
*dev->dma_mask = mask;
^ permalink raw reply related
* Re: [PATCH][next] qtnfmac: Use struct_size() in kzalloc()
From: Sergey Matyukevich @ 2019-06-10 16:14 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich, Kalle Valo,
David S. Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190607191745.GA19120@embeddedor>
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct ieee80211_regdomain {
> ...
> struct ieee80211_reg_rule reg_rules[];
> };
>
> instance = kzalloc(sizeof(*mac->rd) +
> sizeof(struct ieee80211_reg_rule) *
> count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, reg_rules, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Hi Gustavo,
Thanks for the patch !
Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
Regards,
Sergey
^ permalink raw reply
* Re: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Doug Anderson @ 2019-06-10 16:50 UTC (permalink / raw)
To: Hunter, Adrian
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel,
brcm80211-dev-list.pdl@broadcom.com,
linux-rockchip@lists.infradead.org, Double Lo,
briannorris@chromium.org, linux-wireless@vger.kernel.org,
Naveen Gupta, Madhan Mohan R, mka@chromium.org, Wright Feng,
Chi-Hsien Lin, netdev@vger.kernel.org,
brcm80211-dev-list@cypress.com, Franky Lin,
linux-kernel@vger.kernel.org, Hante Meuleman, YueHaibing,
David S. Miller
In-Reply-To: <363DA0ED52042842948283D2FC38E4649C52F8A0@IRSMSX106.ger.corp.intel.com>
Hi,
On Mon, Jun 10, 2019 at 1:56 AM Hunter, Adrian <adrian.hunter@intel.com> wrote:
>
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> > @@ -16,6 +16,7 @@
> > #include <linux/mmc/sdio_ids.h>
> > #include <linux/mmc/sdio_func.h>
> > #include <linux/mmc/card.h>
> > +#include <linux/mmc/core.h>
>
> SDIO function drivers should not really include linux/mmc/core.h
> (Also don't know why linux/mmc/card.h is included)
OK, so I guess you're requesting an extra level of "sdio_" wrappers
for all the functions I need to call. I don't think the wrappers buy
us a ton other than to abstract things a little bit and make it look
prettier. :-) ...but certainly I can code that up if that's what
everyone wants.
Just to make sure, I looked in "drivers/net/wireless/" and I do see
quite a few instances of "mmc_" functions being used. That doesn't
mean all these instances are correct but it does appear to be
commonplace. Selected examples:
drivers/net/wireless/ath/ath10k/sdio.c:
ret = mmc_hw_reset(ar_sdio->func->card->host);
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c:
mmc_set_data_timeout(md, func->card);
mmc_wait_for_req(func->card->host, mr);
drivers/net/wireless/marvell/mwifiex/sdio.c:
mmc_hw_reset(func->card->host);
drivers/net/wireless/rsi/rsi_91x_sdio.c:
err = mmc_wait_for_cmd(host, &cmd, 3);
...anyway, I'll give it a few days and if nobody else chimes in then
I'll assume you indeed want "sdio_" wrappers for things and I'll post
a v4. If patch #1 happens to land in the meantime then I won't
object. ;-)
-Doug
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Larry Finger @ 2019-06-10 18:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Aaro Koskinen, Christoph Hellwig,
Christian Zigotzky, Michael Ellerman
Cc: linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <7697a9d10777b28ae79fdffdde6d0985555f6310.camel@kernel.crashing.org>
On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
>
>> Please try the attached patch. I'm not really pleased with it and I will
>> continue to determine why the fallback to a 30-bit mask fails, but at least this
>> one works for me.
>
> Your patch only makes sense if the device is indeed capable of
> addressing 31-bits.
>
> So either the driver is buggy and asks for a too small mask in which
> case your patch is ok, or it's not and you're just going to cause all
> sort of interesting random problems including possible memory
> corruption.
Of course the driver may be buggy, but it asks for the correct mask.
This particular device is not capable of handling 32-bit DMA. The driver detects
the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32
until 5.1. As Christoph said, it should always be possible to use fewer bits
than the maximum.
Similar devices that are new enough to use b43 rather than b43legacy work with
new kernels; however, they have and use 32-bit DMA.
Larry
^ permalink raw reply
* [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: Michael Büsch @ 2019-06-10 18:49 UTC (permalink / raw)
To: Kalle Valo; +Cc: H Buus, Larry Finger, linux-wireless
In-Reply-To: <946c86bf-7e90-a981-b9fc-757adb98adfa@hbuus.com>
[-- Attachment #1: Type: text/plain, Size: 840 bytes --]
The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
The warning serves no purpose. So let's just remove it.
Reported-by: H Buus <ubuntu@hbuus.com>
Signed-off-by: Michael Büsch <m@bues.ch>
---
diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
index e809dae4c470..66a76fd83248 100644
--- a/drivers/ssb/driver_gpio.c
+++ b/drivers/ssb/driver_gpio.c
@@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
return ssb_gpio_chipco_init(bus);
else if (ssb_extif_available(&bus->extif))
return ssb_gpio_extif_init(bus);
- else
- WARN_ON(1);
-
return -1;
}
@@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
ssb_extif_available(&bus->extif)) {
gpiochip_remove(&bus->gpio);
return 0;
- } else {
- WARN_ON(1);
}
-
return -1;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: Larry Finger @ 2019-06-10 19:16 UTC (permalink / raw)
To: Michael Büsch, Kalle Valo; +Cc: H Buus, linux-wireless
In-Reply-To: <20190610204927.2de21c9a@wiggum>
On 6/10/19 1:49 PM, Michael Büsch wrote:
> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
> The warning serves no purpose. So let's just remove it.
>
> Reported-by: H Buus <ubuntu@hbuus.com>
> Signed-off-by: Michael Büsch <m@bues.ch>
>
> ---
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
Larry
>
> diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
> index e809dae4c470..66a76fd83248 100644
> --- a/drivers/ssb/driver_gpio.c
> +++ b/drivers/ssb/driver_gpio.c
> @@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
> return ssb_gpio_chipco_init(bus);
> else if (ssb_extif_available(&bus->extif))
> return ssb_gpio_extif_init(bus);
> - else
> - WARN_ON(1);
> -
> return -1;
> }
>
> @@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
> ssb_extif_available(&bus->extif)) {
> gpiochip_remove(&bus->gpio);
> return 0;
> - } else {
> - WARN_ON(1);
> }
> -
> return -1;
> }
>
^ permalink raw reply
* [PATCH] cfg80211: fix memory leak of wiphy device name
From: Eric Biggers @ 2019-06-10 20:02 UTC (permalink / raw)
To: linux-wireless, Johannes Berg
Cc: gregkh, linux-kernel, rafael, syzkaller-bugs
In-Reply-To: <00000000000026f98d058a0944ed@google.com>
From: Eric Biggers <ebiggers@google.com>
In wiphy_new_nm(), if an error occurs after dev_set_name() and
device_initialize() have already been called, it's necessary to call
put_device() (via wiphy_free()) to avoid a memory leak.
Reported-by: syzbot+7fddca22578bc67c3fe4@syzkaller.appspotmail.com
Fixes: 1f87f7d3a3b4 ("cfg80211: add rfkill support")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
net/wireless/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 037816163e70d..458f5e0906875 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -514,7 +514,7 @@ struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
&rdev->rfkill_ops, rdev);
if (!rdev->rfkill) {
- kfree(rdev);
+ wiphy_free(&rdev->wiphy);
return NULL;
}
--
2.22.0.rc2.383.gf4fbbf30c2-goog
^ permalink raw reply related
* Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: H Buus @ 2019-06-11 4:56 UTC (permalink / raw)
To: Larry Finger, Michael Büsch, Kalle Valo; +Cc: linux-wireless
In-Reply-To: <de857b70-fbc2-9c29-b31e-d544a33c8ced@lwfinger.net>
On 6/10/2019 3:16 PM, Larry Finger wrote:
> On 6/10/19 1:49 PM, Michael Büsch wrote:
>> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
>> The warning serves no purpose. So let's just remove it.
>>
>> Reported-by: H Buus <ubuntu@hbuus.com>
>> Signed-off-by: Michael Büsch <m@bues.ch>
>>
>> ---
>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
>
> Larry
Works for me. Thanks!
>>
>> diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
>> index e809dae4c470..66a76fd83248 100644
>> --- a/drivers/ssb/driver_gpio.c
>> +++ b/drivers/ssb/driver_gpio.c
>> @@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
>> return ssb_gpio_chipco_init(bus);
>> else if (ssb_extif_available(&bus->extif))
>> return ssb_gpio_extif_init(bus);
>> - else
>> - WARN_ON(1);
>> -
>> return -1;
>> }
>> @@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
>> ssb_extif_available(&bus->extif)) {
>> gpiochip_remove(&bus->gpio);
>> return 0;
>> - } else {
>> - WARN_ON(1);
>> }
>> -
>> return -1;
>> }
>>
>
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-11 5:56 UTC (permalink / raw)
To: Larry Finger, Aaro Koskinen, Christoph Hellwig,
Christian Zigotzky, Michael Ellerman
Cc: linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <3ed1ccfe-d7ca-11b9-17b3-303d1ae1bb0f@lwfinger.net>
On Mon, 2019-06-10 at 13:44 -0500, Larry Finger wrote:
> On 6/7/19 11:21 PM, Benjamin Herrenschmidt wrote:
> >
> > > Please try the attached patch. I'm not really pleased with it and I will
> > > continue to determine why the fallback to a 30-bit mask fails, but at least this
> > > one works for me.
> >
> > Your patch only makes sense if the device is indeed capable of
> > addressing 31-bits.
> >
> > So either the driver is buggy and asks for a too small mask in which
> > case your patch is ok, or it's not and you're just going to cause all
> > sort of interesting random problems including possible memory
> > corruption.
>
> Of course the driver may be buggy, but it asks for the correct mask.
>
> This particular device is not capable of handling 32-bit DMA. The driver detects
> the 32-bit failure and falls back to 30 bits. It works on x86, and did on PPC32
> until 5.1. As Christoph said, it should always be possible to use fewer bits
> than the maximum.
No, I don't think it *worked* on ppc32 before Christoph patch. I think
it "mostly sort-of worked" :-)
The reason I'm saying that is if your system has more than 1GB of RAM,
then you'll have chunks of memory that the device simply cannot
address.
Before Christoph patches, we had no ZONE_DMA or ZONE_DMA32 covering the
30-bit limited space, so any memory allocation could in theory land
above 30-bits, causing all sort of horrible things to happen with that
driver.
The reason I think it sort-of-mostly-worked is that to get more than
1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
buffers aren't allocated in Highmem.... so you got lucky.
That said, there is such as thing as no-copy send on network, so I
wouldn't be surprised if some things would still have failed, just not
frequent enough for you to notice.
> Similar devices that are new enough to use b43 rather than b43legacy work with
> new kernels; however, they have and use 32-bit DMA.
Cheres,
Ben.
^ permalink raw reply
* Re: ath10k QCA9377 firmware crashes and fails to recover
From: Daniel Drake @ 2019-06-11 6:01 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k, linux-wireless, Endless Linux Upstreaming Team
In-Reply-To: <87h89lei7e.fsf@kamboji.qca.qualcomm.com>
Hi Kalle,
On Thu, May 23, 2019 at 3:36 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Daniel Drake <drake@endlessm.com> writes:
>
> > We are experiencing failures with QCA9377 wifi, using Linux 4.18 and
> > Linux 5.0 with the latest firmware version:
> >
> > ath10k_pci 0000:02:00.0: firmware crashed! (guid
> > 54a4649a-1240-4459-9442-9d498c49de79)
> > ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
> > 0x003821ff sub 1a3b:2b31
> > ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
> > ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00002-QCATFSWPZ-5
> > api 5 features ignore-otp crc32 c3e0d04f
>
> Is this a regression? For example, have you tried older firmware
> versions?
Sorry for the delayed response, as we were testing old versions.
It doesn't seem to be a regression, at least we tested:
Linux 5.0 / latest firmware API 6
ath10k_pci 0000:02:00.0: firmware crashed! (guid
697a3b62-bf3a-4953-bf3d-058eb3b828ff)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.2.1-00021-QCARMSWP-1 api
6 features wowlan,ignore-otp crc32 42e41877
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 8aedfa4a
ath10k_pci 0000:02:00.0: htt-ver 3.56 wmi-op 4 htt-op 3 cal otp
max-sta 32 raw 0 hwcrypto 1
Linux 4.18 / latest firmware API 5
ath10k_pci 0000:02:00.0: firmware crashed! (guid
54a4649a-1240-4459-9442-9d498c49de79)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00002-QCATFSWPZ-5
api 5 features ignore-otp crc32 c3e0d04f
Linux 4.15 / older firmware
ath10k_pci 0000:02:00.0: firmware crashed! (guid
7e1505fa-49e1-4fab-a7c5-a2352f1a47f6)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00267-1 api 5
features ignore-otp crc32 79cea2c7
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 8aedfa4a
ath10k_pci 0000:02:00.0: htt-ver 3.1 wmi-op 4 htt-op 3 cal otp max-sta
32 raw 0 hwcrypto 1
Linux 4.13 / same older firmware
ath10k_pci 0000:02:00.0: firmware crashed! (uuid
701e7d5e-b405-408c-ae27-7de285c38c8f)
ath10k_pci 0000:02:00.0: qca9377 hw1.1 target 0x05020001 chip_id
0x003821ff sub 1a3b:2b31
ath10k_pci 0000:02:00.0: kconfig debug 0 debugfs 1 tracing 1 dfs 0 testmode 0
ath10k_pci 0000:02:00.0: firmware ver WLAN.TF.1.0-00267-1 api 5
features ignore-otp crc32 79cea2c7
ath10k_pci 0000:02:00.0: board_file api 2 bmi_id N/A crc32 8aedfa4a
ath10k_pci 0000:02:00.0: htt-ver 3.1 wmi-op 4 htt-op 3 cal otp max-sta
32 raw 0 hwcrypto 1
Any further suggestions?
Thanks
Daniel
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-11 6:05 UTC (permalink / raw)
To: Larry Finger
Cc: Christoph Hellwig, Aaro Koskinen, Christian Zigotzky,
Michael Ellerman, linux-kernel, linux-wireless, linuxppc-dev
In-Reply-To: <153c13f5-a829-1eab-a3c5-fecfb84127ff@lwfinger.net>
On Mon, Jun 10, 2019 at 11:09:47AM -0500, Larry Finger wrote:
>>> return -EIO;
>>>
>>> For b43legacy, dev->dma_mask is 0xc265684800000000.
>>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x3fffffff, and
>>> the routine returns -EIO.
>>>
>>> For b43, dev->dma_mask is 0xc265684800000001,
>>> dma_supported(dev, mask) is 0xc08b000000000000, mask is 0x77777777, and
>>> the routine returns 0.
>>
>> I don't fully understand what values the above map to. Can you send
>> me your actual debugging patch as well?
>
> I do not understand why the if statement returns true as neither of the
> values is zero. After seeing the x86 output shown below, I also do not
> understand all the trailing zeros.
>
> My entire patch is attached. That output came from this section:
What might be confusing in your output is that dev->dma_mask is a pointer,
and we are setting it in dma_set_mask. That is before we only check
if the pointer is set, and later we override it. Of course this doesn't
actually explain the failure. But what is even more strange to me
is that you get a return value from dma_supported() that isn't 0 or 1,
as that function is supposed to return a boolean, and I really can't see
how mask >= __phys_to_dma(dev, min_mask), would return anything but 0
or 1. Does the output change if you use the correct printk specifiers?
i.e. with a debug patch like this:
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 2c2772e9702a..9e5b30b12b10 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -378,6 +378,7 @@ EXPORT_SYMBOL(dma_direct_map_resource);
int dma_direct_supported(struct device *dev, u64 mask)
{
u64 min_mask;
+ bool ret;
if (IS_ENABLED(CONFIG_ZONE_DMA))
min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
@@ -391,7 +392,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
* use __phys_to_dma() here so that the SME encryption mask isn't
* part of the check.
*/
- return mask >= __phys_to_dma(dev, min_mask);
+ ret = (mask >= __phys_to_dma(dev, min_mask));
+ if (!ret)
+ dev_info(dev,
+ "%s: failed (mask = 0x%llx, min_mask = 0x%llx/0x%llx, dma bits = %d\n",
+ __func__, mask, min_mask, __phys_to_dma(dev, min_mask), ARCH_ZONE_DMA_BITS);
+ return ret;
}
size_t dma_direct_max_mapping_size(struct device *dev)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index f7afdadb6770..6c57ccdee2ae 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -317,8 +317,14 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
int dma_set_mask(struct device *dev, u64 mask)
{
- if (!dev->dma_mask || !dma_supported(dev, mask))
+ if (!dev->dma_mask) {
+ dev_info(dev, "no DMA mask set!\n");
return -EIO;
+ }
+ if (!dma_supported(dev, mask)) {
+ printk("DMA not supported\n");
+ return -EIO;
+ }
arch_dma_set_mask(dev, mask);
dma_check_mask(dev, mask);
^ permalink raw reply related
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-11 6:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Larry Finger, Aaro Koskinen, Christoph Hellwig,
Christian Zigotzky, Michael Ellerman, linuxppc-dev,
linux-wireless, linux-kernel
In-Reply-To: <c91ccbddd6a58dbee5705f10ed1d98fb44bd8f8d.camel@kernel.crashing.org>
On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt wrote:
> The reason I think it sort-of-mostly-worked is that to get more than
> 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> buffers aren't allocated in Highmem.... so you got lucky.
>
> That said, there is such as thing as no-copy send on network, so I
> wouldn't be surprised if some things would still have failed, just not
> frequent enough for you to notice.
Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
will bounce buffer highmem pages for the driver under all circumstances.
^ permalink raw reply
* [PATCH 0/2] take into account external PA when configuring tx power
From: Lorenzo Bianconi @ 2019-06-11 6:38 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo
Refer to proper eeprom fields configuring tx power when TSSI calibration is
disabled. Moreover initialize per-channel tx power
Lorenzo Bianconi (2):
mt76: mt7615: init per-channel target power
mt76: mt7615: take into account extPA when configuring tx power
.../wireless/mediatek/mt76/mt7615/eeprom.c | 12 ++++-
.../wireless/mediatek/mt76/mt7615/eeprom.h | 17 +++++++
.../net/wireless/mediatek/mt76/mt7615/init.c | 46 +++++++++++++++++++
.../net/wireless/mediatek/mt76/mt7615/mcu.c | 10 ++--
.../wireless/mediatek/mt76/mt7615/mt7615.h | 3 +-
5 files changed, 82 insertions(+), 6 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH 1/2] mt76: mt7615: init per-channel target power
From: Lorenzo Bianconi @ 2019-06-11 6:38 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo
In-Reply-To: <cover.1560234876.git.lorenzo@kernel.org>
Set per-channel target power as the minimum between the regulatory
tx power and the value configured in the eeprom
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
.../net/wireless/mediatek/mt76/mt7615/init.c | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
index 693e597a3230..3f826e4f1cd6 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
@@ -165,6 +165,46 @@ static int mt7615_init_debugfs(struct mt7615_dev *dev)
return 0;
}
+static void
+mt7615_init_txpower(struct mt7615_dev *dev,
+ struct ieee80211_supported_band *sband)
+{
+ int i, n_chains = hweight8(dev->mt76.antenna_mask);
+ u8 *eep = (u8 *)dev->mt76.eeprom.data;
+
+ for (i = 0; i < sband->n_channels; i++) {
+ struct ieee80211_channel *chan = &sband->channels[i];
+ u8 target_power = 0;
+ int j;
+
+ for (j = 0; j < n_chains; j++) {
+ int index;
+
+ index = mt7615_eeprom_get_power_index(chan, j);
+ target_power = max(target_power, eep[index]);
+ }
+
+ target_power = DIV_ROUND_UP(target_power, 2);
+ switch (n_chains) {
+ case 4:
+ target_power += 6;
+ break;
+ case 3:
+ target_power += 4;
+ break;
+ case 2:
+ target_power += 3;
+ break;
+ default:
+ break;
+ }
+
+ chan->max_power = min_t(int, chan->max_reg_power,
+ target_power);
+ chan->orig_mpwr = target_power;
+ }
+}
+
int mt7615_register_device(struct mt7615_dev *dev)
{
struct ieee80211_hw *hw = mt76_hw(dev);
@@ -212,6 +252,9 @@ int mt7615_register_device(struct mt7615_dev *dev)
if (ret)
return ret;
+ mt7615_init_txpower(dev, &dev->mt76.sband_2g.sband);
+ mt7615_init_txpower(dev, &dev->mt76.sband_5g.sband);
+
hw->max_tx_fragments = MT_TXP_MAX_BUF_NUM;
return mt7615_init_debugfs(dev);
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] mt76: mt7615: take into account extPA when configuring tx power
From: Lorenzo Bianconi @ 2019-06-11 6:38 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo
In-Reply-To: <cover.1560234876.git.lorenzo@kernel.org>
When TSSI calibration is disabled (which it means the device has been
equipped with an external power amplifier) we need to refer to
different eeprom fields in order to properly configure tx power
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
please note I have not been able to test this patch since I have no devices
with external PA
---
.../net/wireless/mediatek/mt76/mt7615/eeprom.c | 12 +++++++++++-
.../net/wireless/mediatek/mt76/mt7615/eeprom.h | 17 +++++++++++++++++
.../net/wireless/mediatek/mt76/mt7615/init.c | 9 ++++++---
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 10 ++++++----
.../net/wireless/mediatek/mt76/mt7615/mt7615.h | 3 ++-
5 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c b/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c
index 023c8bbc767d..dc94f52e6e8b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.c
@@ -110,7 +110,8 @@ static void mt7615_eeprom_parse_hw_cap(struct mt7615_dev *dev)
}
}
-int mt7615_eeprom_get_power_index(struct ieee80211_channel *chan,
+int mt7615_eeprom_get_power_index(struct mt7615_dev *dev,
+ struct ieee80211_channel *chan,
u8 chain_idx)
{
int index;
@@ -118,6 +119,15 @@ int mt7615_eeprom_get_power_index(struct ieee80211_channel *chan,
if (chain_idx > 3)
return -EINVAL;
+ /* TSSI disabled */
+ if (mt7615_ext_pa_enabled(dev, chan->band)) {
+ if (chan->band == NL80211_BAND_2GHZ)
+ return MT_EE_EXT_PA_2G_TARGET_POWER;
+ else
+ return MT_EE_EXT_PA_5G_TARGET_POWER;
+ }
+
+ /* TSSI enabled */
if (chan->band == NL80211_BAND_2GHZ) {
index = MT_EE_TX0_2G_TARGET_POWER + chain_idx * 6;
} else {
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.h b/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.h
index 3c9086b67f51..f4a4280768d2 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/eeprom.h
@@ -11,16 +11,22 @@ enum mt7615_eeprom_field {
MT_EE_VERSION = 0x002,
MT_EE_MAC_ADDR = 0x004,
MT_EE_NIC_CONF_0 = 0x034,
+ MT_EE_NIC_CONF_1 = 0x036,
MT_EE_WIFI_CONF = 0x03e,
MT_EE_TX0_2G_TARGET_POWER = 0x058,
MT_EE_TX0_5G_G0_TARGET_POWER = 0x070,
MT_EE_TX1_5G_G0_TARGET_POWER = 0x098,
+ MT_EE_EXT_PA_2G_TARGET_POWER = 0x0f2,
+ MT_EE_EXT_PA_5G_TARGET_POWER = 0x0f3,
MT_EE_TX2_5G_G0_TARGET_POWER = 0x142,
MT_EE_TX3_5G_G0_TARGET_POWER = 0x16a,
__MT_EE_MAX = 0x3bf
};
+#define MT_EE_NIC_CONF_TSSI_2G BIT(5)
+#define MT_EE_NIC_CONF_TSSI_5G BIT(6)
+
#define MT_EE_NIC_WIFI_CONF_BAND_SEL GENMASK(5, 4)
enum mt7615_eeprom_band {
MT_EE_DUAL_BAND,
@@ -59,4 +65,15 @@ mt7615_get_channel_group(int channel)
return MT_CH_5G_UNII_3;
}
+static inline bool
+mt7615_ext_pa_enabled(struct mt7615_dev *dev, enum nl80211_band band)
+{
+ u8 *eep = dev->mt76.eeprom.data;
+
+ if (band == NL80211_BAND_5GHZ)
+ return !(eep[MT_EE_NIC_CONF_1 + 1] & MT_EE_NIC_CONF_TSSI_5G);
+ else
+ return !(eep[MT_EE_NIC_CONF_1 + 1] & MT_EE_NIC_CONF_TSSI_2G);
+}
+
#endif
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/init.c b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
index 3f826e4f1cd6..859de2454ec6 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/init.c
@@ -9,6 +9,7 @@
#include <linux/etherdevice.h>
#include "mt7615.h"
#include "mac.h"
+#include "eeprom.h"
static void mt7615_phy_init(struct mt7615_dev *dev)
{
@@ -169,18 +170,20 @@ static void
mt7615_init_txpower(struct mt7615_dev *dev,
struct ieee80211_supported_band *sband)
{
- int i, n_chains = hweight8(dev->mt76.antenna_mask);
+ int i, n_chains = hweight8(dev->mt76.antenna_mask), target_chains;
u8 *eep = (u8 *)dev->mt76.eeprom.data;
+ enum nl80211_band band = sband->band;
+ target_chains = mt7615_ext_pa_enabled(dev, band) ? 1 : n_chains;
for (i = 0; i < sband->n_channels; i++) {
struct ieee80211_channel *chan = &sband->channels[i];
u8 target_power = 0;
int j;
- for (j = 0; j < n_chains; j++) {
+ for (j = 0; j < target_chains; j++) {
int index;
- index = mt7615_eeprom_get_power_index(chan, j);
+ index = mt7615_eeprom_get_power_index(dev, chan, j);
target_power = max(target_power, eep[index]);
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 79baa455034c..f3dd76f88ff1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -1149,9 +1149,10 @@ int mt7615_mcu_set_tx_power(struct mt7615_dev *dev)
{
int i, ret, n_chains = hweight8(dev->mt76.antenna_mask);
struct cfg80211_chan_def *chandef = &dev->mt76.chandef;
+ int freq = chandef->center_freq1, len, target_chains;
u8 *req, *data, *eep = (u8 *)dev->mt76.eeprom.data;
+ enum nl80211_band band = chandef->chan->band;
struct ieee80211_hw *hw = mt76_hw(dev);
- int freq = chandef->center_freq1, len;
struct {
u8 center_chan;
u8 dbdc_idx;
@@ -1159,7 +1160,7 @@ int mt7615_mcu_set_tx_power(struct mt7615_dev *dev)
u8 rsv;
} __packed req_hdr = {
.center_chan = ieee80211_frequency_to_channel(freq),
- .band = chandef->chan->band,
+ .band = band,
};
s8 tx_power;
@@ -1190,10 +1191,11 @@ int mt7615_mcu_set_tx_power(struct mt7615_dev *dev)
tx_power = max_t(s8, tx_power, 0);
dev->mt76.txpower_cur = tx_power;
- for (i = 0; i < n_chains; i++) {
+ target_chains = mt7615_ext_pa_enabled(dev, band) ? 1 : n_chains;
+ for (i = 0; i < target_chains; i++) {
int index = -MT_EE_NIC_CONF_0;
- ret = mt7615_eeprom_get_power_index(chandef->chan, i);
+ ret = mt7615_eeprom_get_power_index(dev, chandef->chan, i);
if (ret < 0)
goto out;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h b/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
index 7c08d3b93a2a..f02ffcffe637 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mt7615.h
@@ -105,7 +105,8 @@ u32 mt7615_reg_map(struct mt7615_dev *dev, u32 addr);
int mt7615_register_device(struct mt7615_dev *dev);
void mt7615_unregister_device(struct mt7615_dev *dev);
int mt7615_eeprom_init(struct mt7615_dev *dev);
-int mt7615_eeprom_get_power_index(struct ieee80211_channel *chan,
+int mt7615_eeprom_get_power_index(struct mt7615_dev *dev,
+ struct ieee80211_channel *chan,
u8 chain_idx);
int mt7615_dma_init(struct mt7615_dev *dev);
void mt7615_dma_cleanup(struct mt7615_dev *dev);
--
2.21.0
^ permalink raw reply related
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-11 6:58 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Larry Finger, Aaro Koskinen, Christian Zigotzky, Michael Ellerman,
linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <20190611060816.GA20158@lst.de>
On Tue, 2019-06-11 at 08:08 +0200, Christoph Hellwig wrote:
> On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt
> wrote:
> > The reason I think it sort-of-mostly-worked is that to get more
> > than
> > 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> > buffers aren't allocated in Highmem.... so you got lucky.
> >
> > That said, there is such as thing as no-copy send on network, so I
> > wouldn't be surprised if some things would still have failed, just
> > not
> > frequent enough for you to notice.
>
> Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
> will bounce buffer highmem pages for the driver under all
> circumstances.
... which b43legacy doesn't set to the best of my knowledge ...
Which makes me wonder how come it didn't work even with your patches ?
AFAIK, we have less than 1GB of lowmem unless the config has been
tweaked....
Cheers,
Ben.
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Benjamin Herrenschmidt @ 2019-06-11 6:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Larry Finger, Aaro Koskinen, Christian Zigotzky, Michael Ellerman,
linuxppc-dev, linux-wireless, linux-kernel
In-Reply-To: <fdfc817d1dcdc83f5bc45f0ab12cbce0c61e6702.camel@kernel.crashing.org>
On Tue, 2019-06-11 at 16:58 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2019-06-11 at 08:08 +0200, Christoph Hellwig wrote:
> > On Tue, Jun 11, 2019 at 03:56:33PM +1000, Benjamin Herrenschmidt
> > wrote:
> > > The reason I think it sort-of-mostly-worked is that to get more
> > > than
> > > 1GB of RAM, those machines use CONFIG_HIGHMEM. And *most* network
> > > buffers aren't allocated in Highmem.... so you got lucky.
> > >
> > > That said, there is such as thing as no-copy send on network, so I
> > > wouldn't be surprised if some things would still have failed, just
> > > not
> > > frequent enough for you to notice.
> >
> > Unless NETIF_F_HIGHDMA is set on a netdev, the core networkign code
> > will bounce buffer highmem pages for the driver under all
> > circumstances.
>
> ... which b43legacy doesn't set to the best of my knowledge ...
>
> Which makes me wonder how come it didn't work even with your patches ?
> AFAIK, we have less than 1GB of lowmem unless the config has been
> tweaked....
Ah stupid me ... it's dma_set_mask that failed, since it has no idea
that the calling driver is limited to lowmem.
That's also why the "wrong" patch worked.
So yes, a ZONE_DMA at 30-bits will work, though it's somewhat overkill.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v3 3/5] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
From: Adrian Hunter @ 2019-06-11 7:17 UTC (permalink / raw)
To: Doug Anderson
Cc: Ulf Hansson, Kalle Valo, Arend van Spriel,
brcm80211-dev-list.pdl@broadcom.com,
linux-rockchip@lists.infradead.org, Double Lo,
briannorris@chromium.org, linux-wireless@vger.kernel.org,
Naveen Gupta, Madhan Mohan R, mka@chromium.org, Wright Feng,
Chi-Hsien Lin, netdev@vger.kernel.org,
brcm80211-dev-list@cypress.com, Franky Lin,
linux-kernel@vger.kernel.org, Hante Meuleman, YueHaibing,
David S. Miller
In-Reply-To: <CAD=FV=U8eo78Ee9xjhGXJMv=8YF9o89KLX024GH3iBRnRjCRvQ@mail.gmail.com>
On 10/06/19 7:50 PM, Doug Anderson wrote:
> Hi,
>
> On Mon, Jun 10, 2019 at 1:56 AM Hunter, Adrian <adrian.hunter@intel.com> wrote:
>>
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>>> @@ -16,6 +16,7 @@
>>> #include <linux/mmc/sdio_ids.h>
>>> #include <linux/mmc/sdio_func.h>
>>> #include <linux/mmc/card.h>
>>> +#include <linux/mmc/core.h>
>>
>> SDIO function drivers should not really include linux/mmc/core.h
>> (Also don't know why linux/mmc/card.h is included)
>
> OK, so I guess you're requesting an extra level of "sdio_" wrappers
> for all the functions I need to call. I don't think the wrappers buy
> us a ton other than to abstract things a little bit and make it look
> prettier. :-) ...but certainly I can code that up if that's what
> everyone wants.
I guess it is really up to Ulf.
>
> Just to make sure, I looked in "drivers/net/wireless/" and I do see
> quite a few instances of "mmc_" functions being used. That doesn't
> mean all these instances are correct but it does appear to be
> commonplace. Selected examples:
>
> drivers/net/wireless/ath/ath10k/sdio.c:
> ret = mmc_hw_reset(ar_sdio->func->card->host);
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c:
> mmc_set_data_timeout(md, func->card);
> mmc_wait_for_req(func->card->host, mr);
>
> drivers/net/wireless/marvell/mwifiex/sdio.c:
> mmc_hw_reset(func->card->host);
>
> drivers/net/wireless/rsi/rsi_91x_sdio.c:
> err = mmc_wait_for_cmd(host, &cmd, 3);
>
>
> ...anyway, I'll give it a few days and if nobody else chimes in then
> I'll assume you indeed want "sdio_" wrappers for things and I'll post
> a v4. If patch #1 happens to land in the meantime then I won't
> object. ;-)
>
>
> -Doug
>
^ permalink raw reply
* Re: [PATCH 1/2] mt76: mt7615: init per-channel target power
From: Sven Eckelmann @ 2019-06-11 7:28 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: nbd, lorenzo.bianconi, linux-wireless, ryder.lee, royluo
In-Reply-To: <ade300b855949dcbe0a278e363415bd56b2e1299.1560234877.git.lorenzo@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]
On Tuesday, 11 June 2019 08:38:52 CEST Lorenzo Bianconi wrote:
> + switch (n_chains) {
> + case 4:
> + target_power += 6;
> + break;
> + case 3:
> + target_power += 4;
> + break;
> + case 2:
> + target_power += 3;
> + break;
> + default:
> + break;
> + }
Any reason why you use different value for 3 chains than ath9k? Following
values are used in ath9k:
* 1 chain: 0 dB
* 2 chains: 3 dB (max combined gain ~3.010299956639812 dB)
* 3 chains: 5 dB (max combined gain ~4.771212547196624 dB)
* 4 chains: not supported (max combined gain 6.020599913279624 dB)
Here are the definitions from ath9k (values are saved in .5 dB steps)
drivers/net/wireless/ath/ath9k/eeprom.h:#define POWER_CORRECTION_FOR_TWO_CHAIN 6 /* 10*log10(2)*2 */
drivers/net/wireless/ath/ath9k/eeprom.h:#define POWER_CORRECTION_FOR_THREE_CHAIN 10 /* 10*log10(3)*2 */
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-11 7:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Christoph Hellwig, Larry Finger, Aaro Koskinen,
Christian Zigotzky, Michael Ellerman, linuxppc-dev,
linux-wireless, linux-kernel
In-Reply-To: <fdfc817d1dcdc83f5bc45f0ab12cbce0c61e6702.camel@kernel.crashing.org>
On Tue, Jun 11, 2019 at 04:58:12PM +1000, Benjamin Herrenschmidt wrote:
> ... which b43legacy doesn't set to the best of my knowledge ...
>
> Which makes me wonder how come it didn't work even with your patches ?
> AFAIK, we have less than 1GB of lowmem unless the config has been
> tweaked....
It needs to bounce to somewhere. And the dma-direct code is pretty
strict to require a zone it can do allocations from when setting the
dma mask. As was the old ppc64 code, but not the ppc32 code that
allowed setting any DMA mask. And something about the more strict
validation seem to trip up now.
^ permalink raw reply
* Re: [BISECTED REGRESSION] b43legacy broken on G4 PowerBook
From: Christoph Hellwig @ 2019-06-11 7:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Christoph Hellwig, Larry Finger, Aaro Koskinen,
Christian Zigotzky, Michael Ellerman, linuxppc-dev,
linux-wireless, linux-kernel
In-Reply-To: <b30ced162fa96d0ca63b8b9629d6fe9bc5c78746.camel@kernel.crashing.org>
On Tue, Jun 11, 2019 at 04:59:54PM +1000, Benjamin Herrenschmidt wrote:
> Ah stupid me ... it's dma_set_mask that failed, since it has no idea
> that the calling driver is limited to lowmem.
>
> That's also why the "wrong" patch worked.
>
> So yes, a ZONE_DMA at 30-bits will work, though it's somewhat overkill.
Well, according to Larry it doesn't actually work, which is odd.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox