Re: rtl8723bu: low signal, fails to associate

2018-08-29 Thread Jes Sorensen
On 08/23/2018 09:36 PM, James Cameron wrote:
> G'day Carlo, Mylene, and James,
> 
> Thanks for your earlier reports about RTL8723bu.  Have you any more
> recent experiences you might share?
> 
> I'm evaluating a sample laptop which worked fine with Windows 10, but
> not very well with Ubuntu 18.04, and kernel v4.15 or kernel v4.18.4.
> 
> The laptop is by Hena, model NT16-PRO-C-E, with a wireless device on
> internal USB (0x0bda:0xb720) which loads rtl8xxxu, identifying as
> RTL8723BU.
> 
> http://dev.laptop.org/~quozl/z/1ft0qv.txt (dmesg)
> 
> Symptoms are low RSSI on scan, very short range, and often a failure
> to associate over a distance of two metres in a radio quiet location.
> 
> Symptoms began after first power off, which suggests that device
> registers programmed by the previous operating system Windows 10 had
> not been reset by reboot into the Ubuntu 18.04 installer.  The device
> worked fine in Ubuntu 18.04 before the first power off.
> 
> Jes, let me know if there is anything I can do to help.

It's been a while since I had time to look at the 8723bu support, and
rtl8xxxu doesn't have BT coexist support. I notice that your laptop does
load the bluetooth module for 8723bu which I believe fiddles with the
antenna configuration and is likely to take control of the antennas. I
suspect this is why you see low signal quality on the WiFi side.

If you blacklist the BT module, does it work better?

Jes



Re: [PATCH] rtl8xxxu: Fix trailing semicolon

2018-01-21 Thread Jes Sorensen
On 01/17/2018 05:56 AM, Luis de Bethencourt wrote:
> The trailing semicolon is an empty statement that does no operation.
> Removing it since it doesn't do anything.
> 
> Signed-off-by: Luis de Bethencourt <lui...@kernel.org>
> ---
> 
> Hi,
> 
> After fixing the same thing in drivers/staging/rtl8723bs/, Joe Perches
> suggested I fix it treewide [0].
> 
> Best regards 
> Luis
> 
> 
> [0] 
> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115410.html
> [1] 
> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115390.html
> 
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Jes Sorensen <jes.soren...@gmail.com>


> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 95e3993d8a33..8828baf26e7b 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1172,7 +1172,7 @@ struct rtl8723bu_c2h {
>  
>   u8 basic_rate:1;
>   u8 bt_has_reset:1;
> - u8 dummy4_1:1;;
> + u8 dummy4_1:1;
>   u8 ignore_wlan:1;
>   u8 auto_report:1;
>   u8 dummy4_2:3;
> 



Re: RTL8723bu: poor signal and connection troubles

2018-01-21 Thread Jes Sorensen
On 01/11/2018 05:30 AM, Barry Day wrote:
> On Wed, Jan 10, 2018 at 12:15:51PM +, Carlo Caione wrote:
>> Hi,
>> this is a follow up email to [0] since the problem was never fully
>> investigated / solved and I keep seeing the same problem also on my
>> hardware.
>>
>> Also in my case the hardware is a rtl8723bt transceiver
>> (0x0bda:0xb720), this time shipped on the internal USB bus in a cheap
>> laptop branded Zyrex Sky 232.
>>
>> As already reported by Mylene the problem is that using this
>> transceiver and the latest Linus master you can barely see any WiFi
>> network around and also when a WiFi network is actually seen, the
>> connection is impossible since the signal strength is too low to have
>> a reliable connection. Of course during my tests BT is off and no BT
>> driver is probed at all.
>>
>> Using the downstream driver at [1] everything works correctly.
>>
>> I tried to debug a bit the issue, in particular comparing functions
>> and registers related to the antenna setup (.power_on, .enable_rf,
>> .phy_init_antenna_selection, .phy_iq_calibrate hooks) but everything
>> seems pretty much the same on the two drivers (even though slightly
>> differences do exist).
>>
>> Any idea / suggestion on how to debug this problem? I guess it's worth
>> to start looking at this since several platforms are being affected
>> now.
>>
>> Cheers,
>>
>> [0] https://www.spinics.net/lists/netdev/msg468028.html
>> [1] https://github.com/lwfinger/rtl8723bu
>>
>> -- 
>> Carlo Caione  |  +44.7384.69.16.04  |  Endless
> 
> I've found the same. The signal strength using the original driver
> from Realtek is significantly higher than when using rtl8xxxu.
> I also was not able to find anything wrong in the rtl8xxxu source code
> that would be causing the difference, which leads to the thought it is
> missing something that exists in the original driver.

One issue with rtl8723bu is that it needs to coexist with the bluetooth
driver. When I wrote the rtl8723bu support that was no BT support for
the chip in the kernel and it worked fine, at least for me. However if
you load the BT driver for the dongle which someone pushed into the
kernel since then, it is likely to hijack the antennas causing the weak
signal you describe.

Jes


Re: Slow connection with rtl8xxxu and 8192eu chipset

2017-11-28 Thread Jes Sorensen
On 11/28/2017 11:49 AM, Nikolay Borisov wrote:
> 
> 
> On 28.11.2017 18:35, Jes Sorensen wrote:
>> On 11/28/2017 11:26 AM, Nikolay Borisov wrote:
>>>
>>>
>>> On 28.11.2017 18:16, Jes Sorensen wrote:
>>>> On 11/14/2017 05:39 AM, Nikolay Borisov wrote:
>>>>>
>>>>>
>>>>> I just tested with verbatim 4.14 and even though the wireless works, 
>>>>> iwconfig reports something strange: 
>>>>>
>>>>> iwconfig wifi1
>>>>> wifi1 no wireless extensions.
>>>>>
>>>>> However, my device works as expected (albeit still slow): 
>>>>>
>>>>> wifi1 Link encap:Ethernet  HWaddr 18:d6:c7:0d:47:3c  
>>>>>   inet addr:10.20.1.175  Bcast:10.20.1.255  Mask:255.255.255.0
>>>>>   inet6 addr: fe80::281d:5f27:eb1b:8ded/64 Scope:Link
>>>>>   UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>>>>>   RX packets:38903 errors:0 dropped:0 overruns:0 frame:0
>>>>>   TX packets:24689 errors:0 dropped:0 overruns:0 carrier:0
>>>>>   collisions:0 txqueuelen:1000 
>>>>>   RX bytes:51524413 (51.5 MB)  TX bytes:5503039 (5.5 MB)
>>>>
>>>> iwconfig has been deprecated for a decade, but maybe you could elaborate
>>>> on what you feel is strange in this output?
>>>
>>> Well there are 2 things:
>>>
>>> 1. The low bitrate - which I've confirmed doing actual data test, ie,
>>> not just the number but the fact that the speeds are low.
>>>
>>> 2. The worse signal level/link quality.
>>
>> I don't see either of those in the iwconfig output
>>
>> The low TX bitrate is a known issue. Somehow the firmware rate handling
>> doesn't seem to work (or it works differently than I assumed) similar to
>> for the 8188eu. It's something I need to figure out when I have some time.
>>
>> As for link quality is this something you measured or read out of 'iw' ?
> 
> Well there is:
> 
> Link Quality=26/70  Signal level=-84 dBm < - with rtl8xxxu driver
> 
> Link Quality=81/100  Signal level=100/100  Noise level=0/100 <-- with
> vendor's driver. Another vector was the icon of network manager had only
> 2 bars with the in-tree driver and had full bars with out (yeah, this is
> a bit subjective metric but it's the best i've got).

OK, the link reporting from rtl8xxxu isn't complete, so I wouldn't put
too much into this.

Cheers,
Jes



Re: Slow connection with rtl8xxxu and 8192eu chipset

2017-11-28 Thread Jes Sorensen
On 11/28/2017 11:26 AM, Nikolay Borisov wrote:
> 
> 
> On 28.11.2017 18:16, Jes Sorensen wrote:
>> On 11/14/2017 05:39 AM, Nikolay Borisov wrote:
>>>
>>>
>>> On 14.11.2017 11:26, Nikolay Borisov wrote:
>>>> (Please CC as I'm not subscribed)
>>>>
>>>> Hello,
>>>>
>>>> I have the tp-link tl-wn822N usb wifi dongle. lsbusb reports it as
>>>>
>>>> Bus 001 Device 003: ID 2357:0108
>>>>
>>>> Unfortunately with the in-kernel rtl8xxxu driver I don't get very good
>>>> results:
>>>>
>>>> wifi1 IEEE 802.11  ESSID:"HOME"
>>>>   Mode:Managed  Frequency:2.462 GHz  Access Point:
>>>> 30:B5:C2:75:A4:CD
>>>>   Bit Rate=1 Mb/s   Tx-Power=20 dBm
>>>>   Retry short limit:7   RTS thr=2347 B   Fragment thr:off
>>>>   Power Management:off
>>>>   Link Quality=26/70  Signal level=-84 dBm
>>>>   Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
>>>>   Tx excessive retries:0  Invalid misc:165   Missed beacon:0
>>>>
>>>>
>>>> At the same time if I use an out of tree driver acquired from github:
>>>> https://github.com/Mange/rtl8192eu-linux-driver I get the following:
>>>>
>>>> wifi1 IEEE 802.11bgn  ESSID:"HOME"  Nickname:"<WIFI@REALTEK>"
>>>>   Mode:Managed  Frequency:2.462 GHz  Access Point:
>>>> 30:B5:C2:75:A4:CD
>>>>   Bit Rate:144.4 Mb/s   Sensitivity:0/0
>>>>   Retry:off   RTS thr:off   Fragment thr:off
>>>>   Power Management:off
>>>>   Link Quality=81/100  Signal level=100/100  Noise level=0/100
>>>>   Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
>>>>   Tx excessive retries:0  Invalid misc:0   Missed beacon:0
>>>>
>>>> Clearly this is a software problem of the in-kernel driver. I'm using
>>>> v4.10.17 with commit c14239f23adb ("rtl8xxxu: Add another 8192eu device
>>>> to the USB list") so that my device is recognised. Latest commit for
>>>> that driver in my kernel is: c59f13bbead4 ("rtl8xxxu: Work around issue
>>>> with 8192eu and 8723bu devices not reconnecting").
>>>>
>>>> Any ideas what I can do to further debug this, I'd really like to use
>>>> the in-kernel driver ?
>>>
>>> I just tested with verbatim 4.14 and even though the wireless works, 
>>> iwconfig reports something strange: 
>>>
>>> iwconfig wifi1
>>> wifi1 no wireless extensions.
>>>
>>> However, my device works as expected (albeit still slow): 
>>>
>>> wifi1 Link encap:Ethernet  HWaddr 18:d6:c7:0d:47:3c  
>>>   inet addr:10.20.1.175  Bcast:10.20.1.255  Mask:255.255.255.0
>>>   inet6 addr: fe80::281d:5f27:eb1b:8ded/64 Scope:Link
>>>   UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>>>   RX packets:38903 errors:0 dropped:0 overruns:0 frame:0
>>>   TX packets:24689 errors:0 dropped:0 overruns:0 carrier:0
>>>   collisions:0 txqueuelen:1000 
>>>   RX bytes:51524413 (51.5 MB)  TX bytes:5503039 (5.5 MB)
>>
>> iwconfig has been deprecated for a decade, but maybe you could elaborate
>> on what you feel is strange in this output?
> 
> Well there are 2 things:
> 
> 1. The low bitrate - which I've confirmed doing actual data test, ie,
> not just the number but the fact that the speeds are low.
> 
> 2. The worse signal level/link quality.

I don't see either of those in the iwconfig output

The low TX bitrate is a known issue. Somehow the firmware rate handling
doesn't seem to work (or it works differently than I assumed) similar to
for the 8188eu. It's something I need to figure out when I have some time.

As for link quality is this something you measured or read out of 'iw' ?

Thanks,
Jes


Re: Slow connection with rtl8xxxu and 8192eu chipset

2017-11-28 Thread Jes Sorensen
On 11/14/2017 05:39 AM, Nikolay Borisov wrote:
> 
> 
> On 14.11.2017 11:26, Nikolay Borisov wrote:
>> (Please CC as I'm not subscribed)
>>
>> Hello,
>>
>> I have the tp-link tl-wn822N usb wifi dongle. lsbusb reports it as
>>
>> Bus 001 Device 003: ID 2357:0108
>>
>> Unfortunately with the in-kernel rtl8xxxu driver I don't get very good
>> results:
>>
>> wifi1 IEEE 802.11  ESSID:"HOME"
>>   Mode:Managed  Frequency:2.462 GHz  Access Point:
>> 30:B5:C2:75:A4:CD
>>   Bit Rate=1 Mb/s   Tx-Power=20 dBm
>>   Retry short limit:7   RTS thr=2347 B   Fragment thr:off
>>   Power Management:off
>>   Link Quality=26/70  Signal level=-84 dBm
>>   Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
>>   Tx excessive retries:0  Invalid misc:165   Missed beacon:0
>>
>>
>> At the same time if I use an out of tree driver acquired from github:
>> https://github.com/Mange/rtl8192eu-linux-driver I get the following:
>>
>> wifi1 IEEE 802.11bgn  ESSID:"HOME"  Nickname:""
>>   Mode:Managed  Frequency:2.462 GHz  Access Point:
>> 30:B5:C2:75:A4:CD
>>   Bit Rate:144.4 Mb/s   Sensitivity:0/0
>>   Retry:off   RTS thr:off   Fragment thr:off
>>   Power Management:off
>>   Link Quality=81/100  Signal level=100/100  Noise level=0/100
>>   Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
>>   Tx excessive retries:0  Invalid misc:0   Missed beacon:0
>>
>> Clearly this is a software problem of the in-kernel driver. I'm using
>> v4.10.17 with commit c14239f23adb ("rtl8xxxu: Add another 8192eu device
>> to the USB list") so that my device is recognised. Latest commit for
>> that driver in my kernel is: c59f13bbead4 ("rtl8xxxu: Work around issue
>> with 8192eu and 8723bu devices not reconnecting").
>>
>> Any ideas what I can do to further debug this, I'd really like to use
>> the in-kernel driver ?
> 
> I just tested with verbatim 4.14 and even though the wireless works, 
> iwconfig reports something strange: 
> 
> iwconfig wifi1
> wifi1 no wireless extensions.
> 
> However, my device works as expected (albeit still slow): 
> 
> wifi1 Link encap:Ethernet  HWaddr 18:d6:c7:0d:47:3c  
>   inet addr:10.20.1.175  Bcast:10.20.1.255  Mask:255.255.255.0
>   inet6 addr: fe80::281d:5f27:eb1b:8ded/64 Scope:Link
>   UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>   RX packets:38903 errors:0 dropped:0 overruns:0 frame:0
>   TX packets:24689 errors:0 dropped:0 overruns:0 carrier:0
>   collisions:0 txqueuelen:1000 
>   RX bytes:51524413 (51.5 MB)  TX bytes:5503039 (5.5 MB)

iwconfig has been deprecated for a decade, but maybe you could elaborate
on what you feel is strange in this output?

The 8192eu driver has some issues, it looks like firmware rate support
isn't working and it therefore transmits everything at low rates. I have
to find some time to look into this.

Jes



Re: Wifi RTL8723bu driver test: failed to scan

2017-11-28 Thread Jes Sorensen
On 11/22/2017 04:51 AM, Mylene JOSSERAND wrote:
> Hello Jes Sorensen,
> 
> I am currently testing a LM811 Wifi/BT USB dongle [1] on a Sinlinx
> SinA33 Allwinner SoC board [2]. I saw that I should use the realtek
> driver RTL8723BU for this USB dongle.
> 
> Currently, I am only testing the Wifi and the mainline driver (kernel
> 4.14-rc7) does not seem to work. At least, the scanning does not output
> anything.
> 
> I tested the driver recommended by LM Technologies [3] and it works
> fine (scan, connect and ping are ok). Before investigating on the
> differences between these two drivers, do you have any idea about this
> issue?
> 
> Here are the commands and output I got with mainline's driver:

I have not looked at the driver these LM Technologies people are
referring to, but I am guessing it's the vendor code.

8723bu is a little dicey because it has BT in the chip and if you enable
that the two drivers need to interact, which rtl8xxxu currently doesn't
know about. Check your dmesg output to make sure you don't have some BT
thing loaded hijacking the chip.

Jes


Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs

2017-10-11 Thread Jes Sorensen

On 10/11/2017 04:41 AM, Kalle Valo wrote:

Jes Sorensen <jes.soren...@gmail.com> writes:


On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.


While this isn't harmful, to me this looks like pointless patch churn
for zero gain and it's just ugly.


In general I find it useful to mark fall through cases. And it's just a
comment with two words, so they cannot hurt your eyes that much.


I don't see them being harmful in the code, but I don't see them of much 
use either. If it happened as part of natural code development, fine. My 
objection is to people running around doing this systematically causing 
patch churn for little to zero gain.


Jes


Re: [PATCH] rtl8xxxu: mark expected switch fall-throughs

2017-10-10 Thread Jes Sorensen

On 10/10/2017 03:30 PM, Gustavo A. R. Silva wrote:

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.


While this isn't harmful, to me this looks like pointless patch churn 
for zero gain and it's just ugly.


Jes



Cc: Jes Sorensen <jes.soren...@gmail.com>
Cc: Kalle Valo <kv...@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
---
  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 7806a4d..e66be05 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1153,6 +1153,7 @@ void rtl8xxxu_gen1_config_channel(struct ieee80211_hw *hw)
switch (hw->conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
ht = false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
opmode |= BW_OPMODE_20MHZ;
rtl8xxxu_write8(priv, REG_BW_OPMODE, opmode);
@@ -1280,6 +1281,7 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw)
switch (hw->conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
ht = false;
+   /* fall through */
case NL80211_CHAN_WIDTH_20:
rf_mode_bw |= WMAC_TRXPTCL_CTL_BW_20;
subchannel = 0;
@@ -1748,9 +1750,11 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv 
*priv)
case 3:
priv->ep_tx_low_queue = 1;
priv->ep_tx_count++;
+   /* fall through */
case 2:
priv->ep_tx_normal_queue = 1;
priv->ep_tx_count++;
+   /* fall through */
case 1:
priv->ep_tx_high_queue = 1;
priv->ep_tx_count++;
@@ -5691,6 +5695,7 @@ static int rtl8xxxu_set_key(struct ieee80211_hw *hw, enum 
set_key_cmd cmd,
break;
case WLAN_CIPHER_SUITE_TKIP:
key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
+   /* fall through */
default:
return -EOPNOTSUPP;
}





Re: rtlwifi handling of sequence numbers with aggregation

2017-08-29 Thread Jes Sorensen

On 08/29/2017 10:18 AM, Larry Finger wrote:

On 08/25/2017 09:15 PM, Larry Finger wrote:
Based on my prejudices, I would suspect the USB driver before that of 
the PCI version. I have BCc'd my contact at Realtek to see what he has 
to say on the issue.


My contact at Realtek replies that the PCI code is correct. He further 
states that "Since mac80211 uses ieee80211_tx_next_seq() to store next 
sequence number in sta->tid_seq[tid] and fill the sequence number in 
AMPDU parameters, I think driver doesn't need to maintain the seq_number 
anymore. So, let's remove 'u16 seq_number' from 'struct rtl_tid_data', 
please refer to attachment." I have not had a chance to test his patch, 
but I trust his analysis.


I don't see any attachment, but I'm happy with a correct solution if 
you'll work with them on getting that pushed in.


Cheers,
Jes



rtlwifi handling of sequence numbers with aggregation

2017-08-25 Thread Jes Sorensen
Hi,

Looking at some bits in rtlwifi I came across a discrepancy between the
PCI and USB code.

Consider the following code:

In rtl_pci_tx():
if (ieee80211_is_data_qos(fc)) {
tid = rtl_get_tid(skb);
if (sta) {
sta_entry = (struct rtl_sta_info *)sta->drv_priv;
seq_number = (le16_to_cpu(hdr->seq_ctrl) &
  IEEE80211_SCTL_SEQ) >> 4;
seq_number += 1;

if (!ieee80211_has_morefrags(hdr->frame_control))
sta_entry->tids[tid].seq_number = seq_number;
}

In _rtl_usb_tx_preprocess():
if (ieee80211_is_data_qos(fc)) {
qc = ieee80211_get_qos_ctl(hdr);
tid = qc[0] & IEEE80211_QOS_CTL_TID_MASK;
seq_number = (le16_to_cpu(hdr->seq_ctrl) &
 IEEE80211_SCTL_SEQ) >> 4;
seq_number += 1;
seq_number <<= 4;
}
[snip]
if (!ieee80211_has_morefrags(hdr->frame_control)) {
if (qc)
mac->tids[tid].seq_number = seq_number;
}

The seq_number is picked up from ieee80211_ops->ampdu_action()
which calls into rtl_tx_agg_start():
tid_data = _entry->tids[tid];

RT_TRACE(rtlpriv, COMP_SEND, DBG_DMESG,
 "on ra = %pM tid = %d seq:%d\n", sta->addr, tid,
 tid_data->seq_number);

*ssn = tid_data->seq_number;

My question here is why does the USB code shift seq_number << 4 while
the PCI code doesn't? I assume one of these is wrong, but which one?

Jes


Re: [PATCH] rt2x00: improve calling conventions for register accessors

2017-05-16 Thread Jes Sorensen

On 05/16/2017 10:19 AM, Arnd Bergmann wrote:

On Tue, May 16, 2017 at 3:44 PM, Jes Sorensen <jes.soren...@gmail.com> wrote:

On 05/16/2017 07:55 AM, Stanislaw Gruszka wrote:


On Mon, May 15, 2017 at 10:39:51AM -0400, David Miller wrote:


Passing return values by reference is and always has been a really
poor way to achieve what these functions are doing.

And frankly, whilst the tool could see what's going on here better, we
should be making code easier rather than more difficult to audit.

I am therefore very much in favor of Arnd's change.

This isn't even a situation where there are multiple return values,
such as needing to signal an error and return an unsigned value at the
same time.

These functions return _one_ value, and therefore they should be
returned as a true return value.



In rt2x00 driver we use poor convention in other kind of registers
accessors like bbp, mac, eeprom. I dislike to changing only rfcsr
accessors and leaving others in the old way. And changing all accessors
would be massive and error prone change, which I'm not prefer either.



That's why you do it in multiple smaller patches rather than one ugly giant
patch.


I did  the first step using a search in vim using

s:\(rt2800_rfcsr_read(.*,.*\), &\(.*\));:\2 = \1);:

but had to introduce a conversion function

static void rt2800_rfcsr_readreg(struct rt2x00_dev *rt2x00dev,
 const unsigned int word, u8 *value)
{
*value = rt2800_rfcsr_read(rt2x00dev, word);
}

to keep the correct types in place for struct rt2x00debug. I now
did all the other ones too, and removed that helper again. The
result in much nicer, but I basically ended up having to do
the same regex search for all of these at once:

static void rt2400pci_bbp_read(struct rt2x00_dev *rt2x00dev,
static void rt2500pci_bbp_read(struct rt2x00_dev *rt2x00dev,
static void rt2500usb_register_read(struct rt2x00_dev *rt2x00dev,
static void rt2500usb_register_read_lock(struct rt2x00_dev *rt2x00dev,
static void rt2500usb_bbp_read(struct rt2x00_dev *rt2x00dev,
static void _rt2500usb_register_read(struct rt2x00_dev *rt2x00dev,
static void rt2800_bbp_read(struct rt2x00_dev *rt2x00dev,
static void rt2800_eeprom_read(struct rt2x00_dev *rt2x00dev,
static void rt2800_rfcsr_readreg(struct rt2x00_dev *rt2x00dev,
static void rt2800_bbp_dcoc_read(struct rt2x00_dev *rt2x00dev,
void (*register_read)(struct rt2x00_dev *rt2x00dev,
void (*register_read_lock)(struct rt2x00_dev *rt2x00dev,
static inline void rt2800_register_read(struct rt2x00_dev *rt2x00dev,
static inline void rt2800_register_read_lock(struct rt2x00_dev *rt2x00dev,
static inline void rt2x00_rf_read(struct rt2x00_dev *rt2x00dev,
static inline void rt2x00_eeprom_read(struct rt2x00_dev *rt2x00dev,
void (*read)(struct rt2x00_dev *rt2x00dev, \
static inline void rt2x00mmio_register_read(struct rt2x00_dev *rt2x00dev,
static inline void _rt2x00_desc_read(__le32 *desc, const u8 word, __le32 *value)
static inline void rt2x00_desc_read(__le32 *desc, const u8 word, u32 *value)
static inline void rt2x00usb_register_read(struct rt2x00_dev *rt2x00dev,
static inline void rt2x00usb_register_read_lock(struct rt2x00_dev *rt2x00dev,
static void rt61pci_bbp_read(struct rt2x00_dev *rt2x00dev,
static void rt73usb_bbp_read(struct rt2x00_dev *rt2x00dev,

and that ended up as a 300KB patch [1]. Splitting it up is clearly possibly,
but I fear that would be more error-prone as we then need to add
those helpers for the other debug stuff as well, and remove it again
afterwards.


True - if the automatic conversion works without automatic intervention, 
I am less worried about it. Personally I would still focus on converting 
one function at a time to reduce the impact of each patch.


Cheers,
Jes


Re: [PATCH] rt2x00: improve calling conventions for register accessors

2017-05-16 Thread Jes Sorensen

On 05/16/2017 07:55 AM, Stanislaw Gruszka wrote:

On Mon, May 15, 2017 at 10:39:51AM -0400, David Miller wrote:

Passing return values by reference is and always has been a really
poor way to achieve what these functions are doing.

And frankly, whilst the tool could see what's going on here better, we
should be making code easier rather than more difficult to audit.

I am therefore very much in favor of Arnd's change.

This isn't even a situation where there are multiple return values,
such as needing to signal an error and return an unsigned value at the
same time.

These functions return _one_ value, and therefore they should be
returned as a true return value.


In rt2x00 driver we use poor convention in other kind of registers
accessors like bbp, mac, eeprom. I dislike to changing only rfcsr
accessors and leaving others in the old way. And changing all accessors
would be massive and error prone change, which I'm not prefer either.


That's why you do it in multiple smaller patches rather than one ugly 
giant patch.


The rt2x00 current register accessor functions makes the Realtek vendor 
driver equivalent ones look pretty, which is saying something.


Jes



Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

2017-04-06 Thread Jes Sorensen

On 04/06/2017 02:32 PM, Larry Finger wrote:

On 04/06/2017 04:49 AM, Hans de Goede wrote:

Hi,

On 06-04-17 11:04, Bastien Nocera wrote:

On Thu, 2017-04-06 at 08:55 +0200, Hans de Goede wrote:





Good thank you. So what is the plan with the github version ?

Note that my submission contains a few small fixes on top of
the github version, for which I intended to submit a pull-req
but I've not gotten around to that yet, I've done so now:

https://github.com/hadess/rtl8723bs/pull/125

But do we want to keep maintaining the github version (for a while
at least) I wonder, as that does mean double work?


My plan is to remove everything and point to the upstream tree as soon
as the support has landed in Linus' tree.

While there is some use in keeping it around for older kernels, we keep
getting asked to support even older kernels than reasonable, or have
users complaining about non-working machines once they use the driver,
which simply uncovers kernel bugs that were fixed upstream.

I don't think it's a good use of our time to carry on supporting this
out-of-tree. I'll however make sure to try and document the migration
in a way that's helpful to users.


Ok, that works for me.


I agree. I have enough to do.


You could simply break the build, and have it spit out a warning saying 
the git repo is kept there to track the old source.


It might be useful to have a repository of the original code to go look at.

Cheers,
Jes




Re: [PATCH] staging: Add rtl8723bs sdio wifi driver

2017-03-30 Thread Jes Sorensen

On 03/30/2017 03:15 AM, Greg Kroah-Hartman wrote:

On Wed, Mar 29, 2017 at 08:20:19PM -0500, Larry Finger wrote:

On 03/29/2017 12:47 PM, Hans de Goede wrote:

The rtl8723bs is found on quite a few systems used by Linux users,
such as on Atom systems (Intel Computestick and various other
Atom based devices) and on many (budget) ARM boards such as
the CHIP.

The plan moving forward with this is for the new clean,
written from scratch, rtl8xxxu driver to eventually gain
support for sdio devices. But there is no clear timeline
for that, so lets add this driver included in staging for now.

Cc: Bastien Nocera <had...@hadess.net>
Cc: Larry Finger <larry.fin...@lwfinger.net>
Cc: Jes Sorensen <jes.soren...@gmail.com>
Signed-off-by: Hans de Goede <hdego...@redhat.com>


Hans,

I am still building the driver on my CherryTrail tablet, but I did see some
warnings and errors listed by Smatch as follows:


[snip]

The indent problems can probably be ignored, but many of the others are
serious enough to be addressed now.

Once this driver is merged into staging, the Outreachy folks will be really 
busy.

Thanks for posting this driver,


So, no objection from you?  Great, I'll go queue it up later today...

thanks,

greg k-h



As much as I hate adding more Realtek vendor code to the kernel, I do 
agree this is the right thing to do in the interim. Not sure when I'll 
get to SDIO support in rtl8xxxu, but I will process patches, hint hint :)


Jes



Re: TL-WN823N

2017-02-21 Thread Jes Sorensen

On 02/16/2017 10:20 AM, madmaxxx wrote:

Hi,

I have a problem with rtl8xxxu driver, which seems to not work with a
dongle tp-link TL-WN823N model. I just filled out a 4.9.10 kernel and the
device is not fully recognized .. I can only scan network and not link.

Thank you


There is nothing in this output that looks unusual, you need to provide 
more data.


Jes



lsusb:
Bus 002 Device 002: ID 2357:0109

dmesg:
[gio feb 16 16:08:33 2017] usb 2-1: Vendor: Realtek
[gio feb 16 16:08:33 2017] usb 2-1: Product: \x03802.11n NI
[gio feb 16 16:08:33 2017] usb 2-1: Serial:
[gio feb 16 16:08:33 2017] usb 2-1: rtl8192eu_parse_efuse: dumping efuse
(0x200 bytes):
[gio feb 16 16:08:33 2017] usb 2-1: 00: 29 81 00 7c 01 40 03 00
[gio feb 16 16:08:33 2017] usb 2-1: 08: 40 74 04 50 14 00 00 00
[gio feb 16 16:08:33 2017] usb 2-1: 10: 28 28 28 29 29 29 2a 2a
[gio feb 16 16:08:33 2017] usb 2-1: 18: 2b 2b 2b f2 ef ef ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 20: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 28: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 30: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 38: ff ff 28 28 28 28 28 28
[gio feb 16 16:08:33 2017] usb 2-1: 40: 2a 2a 29 2a 2a f2 ef ef
[gio feb 16 16:08:33 2017] usb 2-1: 48: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 50: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 58: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 60: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 68: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 70: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 78: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 80: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 88: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 90: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 98: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: a0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: a8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: b0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: b8: a1 22 21 00 00 00 ff ff
[gio feb 16 16:08:33 2017] usb 2-1: c0: ff 01 00 10 00 00 00 ff
[gio feb 16 16:08:33 2017] usb 2-1: c8: 00 00 ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: d0: 57 23 09 01 e7 47 02 98
[gio feb 16 16:08:33 2017] usb 2-1: d8: de d0 1e a4 d7 0a 03 52
[gio feb 16 16:08:33 2017] usb 2-1: e0: 65 61 6c 74 65 6b 20 0e
[gio feb 16 16:08:33 2017] usb 2-1: e8: 03 38 30 32 2e 31 31 6e
[gio feb 16 16:08:33 2017] usb 2-1: f0: 20 4e 49 43 20 00 00 ff
[gio feb 16 16:08:33 2017] usb 2-1: f8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 100: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 108: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 110: ff ff ff ff ff ff ff 0d
[gio feb 16 16:08:33 2017] usb 2-1: 118: 03 00 05 00 30 00 00 00
[gio feb 16 16:08:33 2017] usb 2-1: 120: 00 93 ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 128: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 130: f6 a8 98 2d 03 92 98 00
[gio feb 16 16:08:33 2017] usb 2-1: 138: fc 8c 00 11 9b 44 02 0a
[gio feb 16 16:08:33 2017] usb 2-1: 140: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 148: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 150: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 158: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 160: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 168: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 170: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 178: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 180: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 188: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 190: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 198: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1a0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1a8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1b0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1b8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1c0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1c8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1d0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1d8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1e0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1e8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1f0: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: 1f8: ff ff ff ff ff ff ff ff
[gio feb 16 16:08:33 2017] usb 2-1: RTL8192EU rev B (SMIC) 2T2R, TX
queues 3, WiFi=1, BT=0, GPS=0, HI PA=0
[gio feb 16 16:08:33 2017] usb 2-1: RTL8192EU MAC: 98:de:d0:1e:a4:d7
[gio feb 16 16:08:33 2017] usb 

Re: R92SU - Realtek RTL8192SU/RTL8191SU/RTL8188SU USB Wireless Network Adapters

2017-02-14 Thread Jes Sorensen

On 02/14/2017 01:28 PM, poma wrote:

Hello fellows!

https://github.com/chunkeey/rtl8192su/tree/master/r92su
r92su seems like the simplest and most effective replacement for r8712u.

https://bugzilla.redhat.com/show_bug.cgi?id=1421383
Christian, I can not add you there, so I'm asking here,
is there any particular reason why r92su is not already accepted in mainline?

Is there an intention to "Realtek RTL8192SU/RTL8191SU/RTL8188SU USB Wireless Network 
Adapters" join "RTL8XXXu USB mac80211 Wireless LAN Driver" support?


I see no reason why SU support cannot be added to rtl8xxxu, however 
given the age of these chips and the amount of time I have to work on 
the code, it is pretty far from the top of my priority list.


If someone else is going to work on it, I am happy to work with that 
person to integrate the code.


Jes




Re: [PATCH] rtl8xxxu: Add (0x2357 0x0107) to the tested devices list.

2017-02-13 Thread Jes Sorensen

On 02/13/2017 04:18 AM, Kalle Valo wrote:

Aaryn Coutanche  writes:


From: Aaryn Coutanche 

---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 +++
 1 file changed, 3 insertions(+)


Signed-off-by is missing, please read Documentation/SubmittingPatches.
Also write a short commit log and document the device you used to test
this etc.



Aaryn,

I already mentioned the Signed-off-by part to you - please make sure to 
always include that.


In general a commit message should have a headline and then a body 
message with a little more detail.


Cheers,
Jes



[PATCH 1/5] rtl8xxxu: Mark 8192eu device 0x0bda:0x818b as tested

2017-01-17 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Device reported as working fine, so tell the driver not to warn about
it being untested.

Reported-by: Aex Aey <aex...@gmail.com>
Signed-off-by: Jes Sorensen <jes.soren...@gmail.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 3a86675..99cfd2b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6000,6 +6000,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
case 0x8176:
case 0x8178:
case 0x817f:
+   case 0x818b:
untested = 0;
break;
}
-- 
2.7.4



[PATCH 3/5] rtl8xxxu: Add USB ID for D-Link DWA-131 rev E1 (rtl8192eu)

2017-01-17 Thread Jes . Sorensen
From: Axel Köllhofer <axelkoellho...@web.de>

This was tested by David Patiño.

Reported-by: David Patiño <davidpatin...@gmail.com>
Signed-off-by: Axel Köllhofer <axelkoellho...@web.de>
Signed-off-by: Jes Sorensen <jes.soren...@gmail.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 016ea24..a8386f4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6200,6 +6200,9 @@ static struct usb_device_id dev_table[] = {
 /* TP-Link TL-WN822N v4 */
 {USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0108, 0xff, 0xff, 0xff),
.driver_info = (unsigned long)_fops},
+/* D-Link DWA-131 rev E1, tested by David Patiño */
+{USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x3319, 0xff, 0xff, 0xff),
+   .driver_info = (unsigned long)_fops},
 /* Tested by Myckel Habets */
 {USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0109, 0xff, 0xff, 0xff),
.driver_info = (unsigned long)_fops},
-- 
2.7.4



[PATCH 5/5] rtl8xxxu: Update author/maintainer contact info

2017-01-17 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Update copyright year and email address.

Signed-off-by: Jes Sorensen <jes.soren...@gmail.com>
---
 MAINTAINERS| 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   | 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c | 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c | 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 4 ++--
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h  | 2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c8df0e1..0b5c80e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10584,7 +10584,7 @@ F:  drivers/net/wireless/realtek/rtlwifi/
 F: drivers/net/wireless/realtek/rtlwifi/rtl8192ce/
 
 RTL8XXXU WIRELESS DRIVER (rtl8xxxu)
-M: Jes Sorensen <jes.soren...@redhat.com>
+M: Jes Sorensen <jes.soren...@gmail.com>
 L: linux-wireless@vger.kernel.org
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git 
rtl8xxxu-devel
 S: Maintained
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index df551b2..95e3993 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014 - 2016 Jes Sorensen <jes.soren...@redhat.com>
+ * Copyright (c) 2014 - 2017 Jes Sorensen <jes.soren...@gmail.com>
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of version 2 of the GNU General Public License as
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c
index f9e2050..a41a296 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c
@@ -1,7 +1,7 @@
 /*
  * RTL8XXXU mac80211 USB driver - 8188c/8188r/8192c specific subdriver
  *
- * Copyright (c) 2014 - 2016 Jes Sorensen <jes.soren...@redhat.com>
+ * Copyright (c) 2014 - 2017 Jes Sorensen <jes.soren...@gmail.com>
  *
  * Portions, notably calibration code:
  * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved.
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index a1178c5..80fee69 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1,7 +1,7 @@
 /*
  * RTL8XXXU mac80211 USB driver - 8192e specific subdriver
  *
- * Copyright (c) 2014 - 2016 Jes Sorensen <jes.soren...@redhat.com>
+ * Copyright (c) 2014 - 2017 Jes Sorensen <jes.soren...@gmail.com>
  *
  * Portions, notably calibration code:
  * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved.
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c
index aef3730..1746311 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c
@@ -1,7 +1,7 @@
 /*
  * RTL8XXXU mac80211 USB driver - 8723a specific subdriver
  *
- * Copyright (c) 2014 - 2016 Jes Sorensen <jes.soren...@redhat.com>
+ * Copyright (c) 2014 - 2017 Jes Sorensen <jes.soren...@gmail.com>
  *
  * Portions, notably calibration code:
  * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved.
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index 02b8ddd..c4b86a8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1,7 +1,7 @@
 /*
  * RTL8XXXU mac80211 USB driver - 8723b specific subdriver
  *
- * Copyright (c) 2014 - 2016 Jes Sorensen <jes.soren...@redhat.com>
+ * Copyright (c) 2014 - 2017 Jes Sorensen <jes.soren...@gmail.com>
  *
  * Portions, notably calibration code:
  * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved.
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 3585a04..e544dd1 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1,7 +1,7 @@
 /*
  * RTL8XXXU mac80211 USB driver
  *
- * Copyright (c) 2014 - 2016 Jes Sorensen <jes.soren...@redhat.com>
+ * Copyright (c) 2014 - 2017 Jes Sorensen <jes.soren...@gmail.com>
  *
  * Portions, notably calibration code:
  * Copyright(c) 2007 - 2011 Realtek Corporation. All rights reserved.
@@ -48,7 +48,7 @@ static bool rtl8xxxu_dma_aggregation;
 static int rtl8xx

[PATCH 2/5] rtl8xxxu: Add another 8192eu device to the USB list

2017-01-17 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

TP-Link TL-WN822N v4 (2357:0108)

Reported-by: Gregory Auzanneau <li...@reolight.net>
Signed-off-by: Jes Sorensen <jes.soren...@gmail.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 99cfd2b..016ea24 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6197,6 +6197,9 @@ static struct usb_device_id dev_table[] = {
.driver_info = (unsigned long)_fops},
 {USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x818b, 0xff, 0xff, 
0xff),
.driver_info = (unsigned long)_fops},
+/* TP-Link TL-WN822N v4 */
+{USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0108, 0xff, 0xff, 0xff),
+   .driver_info = (unsigned long)_fops},
 /* Tested by Myckel Habets */
 {USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0109, 0xff, 0xff, 0xff),
.driver_info = (unsigned long)_fops},
-- 
2.7.4



[PATCH 4/5] rtl8xxxu: Add additional USB IDs for rtl8192eu devices

2017-01-17 Thread Jes . Sorensen
From: Axel Köllhofer <axelkoellho...@web.de>

These IDs originate from the vendor driver

Signed-off-by: Axel Köllhofer <axelkoellho...@web.de>
Signed-off-by: Jes Sorensen <jes.soren...@gmail.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a8386f4..3585a04 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6354,6 +6354,13 @@ static struct usb_device_id dev_table[] = {
.driver_info = (unsigned long)_fops},
 {USB_DEVICE_AND_INTERFACE_INFO(0x7392, 0x7822, 0xff, 0xff, 0xff),
.driver_info = (unsigned long)_fops},
+/* found in rtl8192eu vendor driver */
+{USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0107, 0xff, 0xff, 0xff),
+   .driver_info = (unsigned long)_fops},
+{USB_DEVICE_AND_INTERFACE_INFO(0x2019, 0xab33, 0xff, 0xff, 0xff),
+   .driver_info = (unsigned long)_fops},
+{USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x818c, 0xff, 0xff, 
0xff),
+   .driver_info = (unsigned long)_fops},
 #endif
 { }
 };
-- 
2.7.4



[PATCH 0/5] Update USB IDs and email adress

2017-01-17 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@gmail.com>

Hi,

This patchset adds some additional USB IDs for rtl8192eu devices, as
well as updates the email address and the copyright.

I've been preoccupied with some other stuff the last little while, so
I am a little out of touch with whether or not the patch window is
open at the moment. Please yell at me if you want me to resubmit these later.

Cheers,
Jes


Axel Köllhofer (2):
  rtl8xxxu: Add USB ID for D-Link DWA-131 rev E1 (rtl8192eu)
  rtl8xxxu: Add additional USB IDs for rtl8192eu devices

Jes Sorensen (3):
  rtl8xxxu: Mark 8192eu device 0x0bda:0x818b as tested
  rtl8xxxu: Add another 8192eu device to the USB list
  rtl8xxxu: Update author/maintainer contact info

 MAINTAINERS|  2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192c.c |  2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c |  2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723a.c |  2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c |  2 +-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 18 --
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h  |  2 +-
 8 files changed, 23 insertions(+), 9 deletions(-)

-- 
2.7.4



Re: Support of rtl8723bs

2017-01-16 Thread Jes Sorensen
Hanno Zulla  writes:
> Hi,
>
> has there been any progress on the rtl8723bs / sdio issue?
>
> There were no updates to this mailing list or the usual git repositories
> since this was last discussed, unless I missed the proper place to look
> for this.
>
> I'm sorry that I can't be more helpful with actual development, but am
> eager to help with testing this device.
>
> Thank you,
> Hanno

Hanno,

The 8723bs project is currently in idle mode, I haven't had time to
spend time on it for the last couple of months.

Cheers,
Jes


Re: [PATCH] net: wireless: realtek: constify rate_control_ops structures

2016-12-06 Thread Jes Sorensen
Larry Finger  writes:
> On 12/02/2016 03:50 AM, Bhumika Goyal wrote:
>> The structures rate_control_ops are only passed as an argument to the
>> functions ieee80211_rate_control_{register/unregister}. This argument is
>> of type const, so rate_control_ops having this property can also be
>> declared as const.
>> Done using Coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct rate_control_ops i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> @@
>> ieee80211_rate_control_register(@p)
>>
>> @ok2@
>> identifier r1.i;
>> position p;
>> @@
>> ieee80211_rate_control_unregister(@p)
>>
>> @bad@
>> position p!={r1.p,ok1.p,ok2.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> static
>> +const
>> struct rate_control_ops i={...};
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct rate_control_ops i;
>>
>> File size before:
>>text data bss dec hex filename
>>1991  104   02095 82f wireless/realtek/rtlwifi/rc.o
>>
>> File size after:
>>text data bss dec hex filename
>>20950   02095 wireless/realtek/rtlwifi/rc.o
>>
[snip]
> The content of your patch is OK; however, your subject is not. By
> convention, "net: wireless: realtek:" is assumed. We do, however,
> include "rtlwifi:" to indicate which part of
> drivers/net/wireless/realtek/ is referenced.

In addition, the first part of the description is useful and the file
size information is reasonable too, but ~20 lines of coccinelle scripts
in the commit message is rather pointless.

Jes


Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting

2016-11-30 Thread Jes Sorensen
Kalle Valo <kv...@codeaurora.org> writes:
> Jes Sorensen <jes.soren...@redhat.com> writes:
>
>> Luca Coelho <l...@coelho.fi> writes:
>>> On Wed, 2016-11-30 at 10:46 +0200, Kalle Valo wrote:
>>>> jes.soren...@redhat.com writes:
>>>> 
>>>> > From: Jes Sorensen <jes.soren...@redhat.com>
>>>> > 
>>>> > Hi,
>>>> > 
>>>> > This patch works around the issue with 8192eu and 8723bu devices not
>>>> > being able to reconnect. It is still not clear why this fails, and
>>>> > looking at rtlwifi it does issue the command for 8192ee and 8723be
>>>> > devices.
>>>> > 
>>>> > Until we have a better idea of what is going on, I have commented out
>>>> > the offending code. I prefer not deleting it until we have a better
>>>> > understanding of why this is causing issues.
>>>> > 
>>>> > Credits to Barry Day for tracking down and isolating the issue.
>>>> > 
>>>> > Kalle, not sure if this can make it to 4.9, but if it's possible it
>>>> > would be ideal since this is causing real issues in the field.
>>>> 
>>>> For 4.9 this is too late, sorry. I'll queue this to 4.10.
>>>
>>> You should then CC stable so it gets into 4.9.x when it reaches 4.10.
>>
>> Yeah good idea - Kalle do you want me to repost it with the CC line or
>> are you happy to add it?
>
> I can add it. To which stable releases should this go to?

Lets target 4.8 and 4.9 - I just verified it applies against 4.8.

Thanks,
Jes


Re: [PATCH 0/1] rtl8xxxu: Workaround for 8192eu/8723bu not reconnecting

2016-11-30 Thread Jes Sorensen
Luca Coelho <l...@coelho.fi> writes:
> On Wed, 2016-11-30 at 10:46 +0200, Kalle Valo wrote:
>> jes.soren...@redhat.com writes:
>> 
>> > From: Jes Sorensen <jes.soren...@redhat.com>
>> > 
>> > Hi,
>> > 
>> > This patch works around the issue with 8192eu and 8723bu devices not
>> > being able to reconnect. It is still not clear why this fails, and
>> > looking at rtlwifi it does issue the command for 8192ee and 8723be
>> > devices.
>> > 
>> > Until we have a better idea of what is going on, I have commented out
>> > the offending code. I prefer not deleting it until we have a better
>> > understanding of why this is causing issues.
>> > 
>> > Credits to Barry Day for tracking down and isolating the issue.
>> > 
>> > Kalle, not sure if this can make it to 4.9, but if it's possible it
>> > would be ideal since this is causing real issues in the field.
>> 
>> For 4.9 this is too late, sorry. I'll queue this to 4.10.
>
> You should then CC stable so it gets into 4.9.x when it reaches 4.10.

Yeah good idea - Kalle do you want me to repost it with the CC line or
are you happy to add it?

Cheers,
Jes


[PATCH 1/1] rtl8xxxu: Work around issue with 8192eu and 8723bu devices not reconnecting

2016-11-29 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The H2C MEDIA_STATUS_RPT command for some reason causes 8192eu and
8723bu devices not being able to reconnect.

Reported-by: Barry Day <brise...@gmail.com>
Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a9137ab..3a86675 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4372,6 +4372,13 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv 
*priv,
 void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
  u8 macid, bool connect)
 {
+#ifdef RTL8XXXU_GEN2_REPORT_CONNECT
+   /*
+* Barry Day reports this causes issues with 8192eu and 8723bu
+* devices reconnecting. The reason for this is unclear, but
+* until it is better understood, leave the code in place but
+* disabled, so it is not lost.
+*/
struct h2c_cmd h2c;
 
memset(, 0, sizeof(struct h2c_cmd));
@@ -4383,6 +4390,7 @@ void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv 
*priv,
h2c.media_status_rpt.parm &= ~BIT(0);
 
rtl8xxxu_gen2_h2c_cmd(priv, , sizeof(h2c.media_status_rpt));
+#endif
 }
 
 void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
-- 
2.7.4



Re: [PATCH] rtl8xxxu: Fix fail to reconnect to AP

2016-11-28 Thread Jes Sorensen
Barry Day  writes:
> Removed the report_connect functions as the h2c commands used are
> not required for a soft-mac driver and they prevent reconnecting to an
> AP for a couple of the chipsets.
>
> Signed-off-by: Barry Day 
> ---
>  rtl8xxxu.h   |  6 --
>  rtl8xxxu_8192c.c |  1 -
>  rtl8xxxu_8192e.c |  1 -
>  rtl8xxxu_8723a.c |  1 -
>  rtl8xxxu_8723b.c |  1 -
>  rtl8xxxu_core.c  | 36 
>  6 files changed, 46 deletions(-)

Hi Barry,

Does removing the h2c command on the 8723bu and 8192eu make the
reconnect process work properly?

Do you have any documentation justifying why this isn't needed on gen1
parts? I am rather cautious of just removing them, but we may remove the
call for gen2, at least until we understand better how that firmware
command really works.

Cheers,
Jes

> diff --git a/rtl8xxxu.h b/rtl8xxxu.h
> index df551b2..3b1f62d 100644
> --- a/rtl8xxxu.h
> +++ b/rtl8xxxu.h
> @@ -1335,8 +1335,6 @@ struct rtl8xxxu_fileops {
> bool ht40);
>   void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
> u32 ramask, int sgi);
> - void (*report_connect) (struct rtl8xxxu_priv *priv,
> - u8 macid, bool connect);
>   void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
>struct ieee80211_tx_info *tx_info,
>struct rtl8xxxu_txdesc32 *tx_desc, bool sgi,
> @@ -1422,10 +1420,6 @@ 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);
> -void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> -   u8 macid, bool connect);
> -void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> -   u8 macid, bool connect);
>  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv);
>  void rtl8xxxu_gen1_enable_rf(struct rtl8xxxu_priv *priv);
>  void rtl8xxxu_gen1_disable_rf(struct rtl8xxxu_priv *priv);
> diff --git a/rtl8xxxu_8192c.c b/rtl8xxxu_8192c.c
> index f9e2050..d5c37e0 100644
> --- a/rtl8xxxu_8192c.c
> +++ b/rtl8xxxu_8192c.c
> @@ -566,7 +566,6 @@ struct rtl8xxxu_fileops rtl8192cu_fops = {
>   .usb_quirks = rtl8xxxu_gen1_usb_quirks,
>   .set_tx_power = rtl8xxxu_gen1_set_tx_power,
>   .update_rate_mask = rtl8xxxu_update_rate_mask,
> - .report_connect = rtl8xxxu_gen1_report_connect,
>   .fill_txdesc = rtl8xxxu_fill_txdesc_v1,
>   .writeN_block_size = 128,
>   .rx_agg_buf_size = 16000,
> diff --git a/rtl8xxxu_8192e.c b/rtl8xxxu_8192e.c
> index a1178c5..401aac4 100644
> --- a/rtl8xxxu_8192e.c
> +++ b/rtl8xxxu_8192e.c
> @@ -1648,7 +1648,6 @@ struct rtl8xxxu_fileops rtl8192eu_fops = {
>   .usb_quirks = rtl8xxxu_gen2_usb_quirks,
>   .set_tx_power = rtl8192e_set_tx_power,
>   .update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> - .report_connect = rtl8xxxu_gen2_report_connect,
>   .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
>   .writeN_block_size = 128,
>   .tx_desc_size = sizeof(struct rtl8xxxu_txdesc40),
> diff --git a/rtl8xxxu_8723a.c b/rtl8xxxu_8723a.c
> index aef3730..9c13db5 100644
> --- a/rtl8xxxu_8723a.c
> +++ b/rtl8xxxu_8723a.c
> @@ -383,7 +383,6 @@ struct rtl8xxxu_fileops rtl8723au_fops = {
>   .usb_quirks = rtl8xxxu_gen1_usb_quirks,
>   .set_tx_power = rtl8xxxu_gen1_set_tx_power,
>   .update_rate_mask = rtl8xxxu_update_rate_mask,
> - .report_connect = rtl8xxxu_gen1_report_connect,
>   .fill_txdesc = rtl8xxxu_fill_txdesc_v1,
>   .writeN_block_size = 1024,
>   .rx_agg_buf_size = 16000,
> diff --git a/rtl8xxxu_8723b.c b/rtl8xxxu_8723b.c
> index 02b8ddd..30dc66e 100644
> --- a/rtl8xxxu_8723b.c
> +++ b/rtl8xxxu_8723b.c
> @@ -1665,7 +1665,6 @@ 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,
> - .report_connect = rtl8xxxu_gen2_report_connect,
>   .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
>   .writeN_block_size = 1024,
>   .tx_desc_size = sizeof(struct rtl8xxxu_txdesc40),
> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
> index a9137ab..03e88d2 100644
> --- a/rtl8xxxu_core.c
> +++ b/rtl8xxxu_core.c
> @@ -4352,39 +4352,6 @@ void rtl8xxxu_gen2_update_rate_mask(struct 
> rtl8xxxu_priv *priv,
>   rtl8xxxu_gen2_h2c_cmd(priv, , sizeof(h2c.b_macid_cfg));
>  }
>  
> -void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> -   u8 macid, bool connect)
> -{
> - struct h2c_cmd h2c;
> -
> - memset(, 0, sizeof(struct h2c_cmd));
> -
> - h2c.joinbss.cmd = H2C_JOIN_BSS_REPORT;
> -
> - if (connect)
> - h2c.joinbss.data = 

Re: [PATCH 0/7] rtl8xxxu: Pending patches

2016-11-28 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Mon, Nov 21, 2016 at 11:59:47AM -0500, Jes Sorensen wrote:
>> Barry Day <brise...@gmail.com> writes:
>> > Testing is simple. Connect to an AP in the usual
>> > manner..disconnect..reconnect.  The 8192eu will fail to reconnect and
>> > John Heenan reported the 8723bu also fails to reconnect. Even though
>> > he was directly stopping and restarting wpa_supplicant, it's the same
>> > thing to the driver - connect..disconnect..reconnect.
>> 
>> Thanks for the details - I'll have a look shortly.
>> 
>> > With using a usb_device pointer, each message starts with the usb bus
>> > address.  Plug it into a different port and that address could
>> > change. By using a pointer to the device associated with the wiphy
>> > each message will begin with the driver name. Just makes it easier for
>> > the average user to report what's in the log because he can just grep
>> > for "rtl8xxxu".
>> 
>> I see - that would be problematic for me as I quite often have 3-4 of
>> these things plugged in at the same time. Not knowing which port spits
>> out the message would make it a lot harder to track. In fact my primary
>> devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the
>> motherboard, so the moment I plug in any other dongle I'll have two.
>> 
>> The alternative would be to add a prefer to the individual messages.
>> 
>> Cheers,
>> Jes
>
> I should have mentioned it also places the usb address after the driver name.

If you're willing to cook up a patch, I'll have a look at it - I want to
see how it behaves though before deciding whether to approve it.

Cheers,
Jes


Re: [PATCH] rtl8xxxu: fix tx rate debug output

2016-11-28 Thread Jes Sorensen
Arnd Bergmann  writes:
> We accidentally print the rate before we know it for txdesc_v2:

Hi Arnd,

Thanks for the patch - Barry Day already posted a patch for this which
Kalle has applied to the wireless tree.

Cheers,
Jes


>
> wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function 
> 'rtl8xxxu_fill_txdesc_v2':
> wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4848:3: error: 'rate' may be used 
> uninitialized in this function [-Werror=maybe-uninitialized]
>
> txdesc_v1 got it right, so let's do it the same way here.
>
> Fixes: b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have 
> access to retry count")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e57b8ae..a9137abc3ad9 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4844,16 +4844,16 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, 
> struct ieee80211_hdr *hdr,
>  
>   tx_desc40 = (struct rtl8xxxu_txdesc40 *)tx_desc32;
>  
> - if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
> - dev_info(dev, "%s: TX rate: %d, pkt size %d\n",
> -  __func__, rate, cpu_to_le16(tx_desc40->pkt_size));
> -
>   if (rate_flags & IEEE80211_TX_RC_MCS &&
>   !ieee80211_is_mgmt(hdr->frame_control))
>   rate = tx_info->control.rates[0].idx + DESC_RATE_MCS0;
>   else
>   rate = tx_rate->hw_value;
>  
> + if (rtl8xxxu_debug & RTL8XXXU_DEBUG_TX)
> + dev_info(dev, "%s: TX rate: %d, pkt size %d\n",
> +  __func__, rate, cpu_to_le16(tx_desc40->pkt_size));
> +
>   seq_number = IEEE80211_SEQ_TO_SN(le16_to_cpu(hdr->seq_ctrl));
>  
>   tx_desc40->txdw4 = cpu_to_le32(rate);


Re: linux-next: build warning after merge of the wireless-drivers-next tree

2016-11-28 Thread Jes Sorensen
Kalle Valo <kv...@codeaurora.org> writes:
> Barry Day <brise...@gmail.com> writes:
>
>> On Mon, Nov 28, 2016 at 09:25:30AM +0200, Kalle Valo wrote:
>>> Stephen Rothwell <s...@canb.auug.org.au> writes:
>>> 
>>> > Hi all,
>>> >
>>> > After merging the wireless-drivers-next tree, today's linux-next build
>>> > (x86_64 allmodconfig) produced this warning:
>>> >
>>> > In file included from include/linux/usb/ch9.h:35:0,
>>> >  from include/linux/usb.h:5,
>>> >  from 
>>> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:32:
>>> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function 
>>> > 'rtl8xxxu_fill_txdesc_v2':
>>> > include/linux/device.h:1214:36: warning: 'rate' may be used uninitialized 
>>> > in this function [-Wmaybe-uninitialized]
>>> >  #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
>>> > ^
>>> > drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:4841:6: note: 
>>> > 'rate' was declared here
>>> >   u32 rate;
>>> >   ^
>>> >
>>> > Introduced by commit
>>> >
>>> >   b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order to have 
>>> > access to retry count")
>>> >
>>> > This is a correct diagnosis.
>>> 
>>> Thanks for the report. Jes, can you send a patch to fix this? (Unless
>>> someone else beats to it.)
>>> 
>>> -- 
>>> Kalle Valo
>>
>> I posted a patch on the 26th that fixes this
>
> Thanks, I see it:
>
> https://patchwork.kernel.org/patch/9448225/
>
> The commit log doesn't mention anything about the compilation warning,
> I'll add that. Also a Fixes line is nice to have.

I'm happy with this fix

Acked-by: Jes Sorensen <jes.soren...@redhat.com>


Re: rtl8xxxu: tx rate reported before set

2016-11-28 Thread Jes Sorensen
Kalle Valo  writes:
> Barry Day  wrote:
>> Move the dev_info call that attempts to show the rate used before it is set.
>> 
>> Signed-off-by: Barry Day 
>
> Jes, I would like to take this directly to fix the compiler warning. Also I'll
> change the commit log to:

Kalle,

I'm totally fine with this, please go ahead.

Barry thanks for fixing this.

Cheers,
Jes

> rtl8xxxu: tx rate reported before set
>
> Move the dev_info call that attempts to show the rate used before it is set.
> Fixes a compiler warning:
>
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c: In function
> 'rtl8xxxu_fill_txdesc_v2':
> include/linux/device.h:1214:36: warning: 'rate' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>  #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
>
> Fixes: b4c3d9cfb607 ("rtl8xxxu: Pass tx_info to fill_txdesc in order
> to have access to retry count")
> Reported-by: Stephen Rothwell 
> Signed-off-by: Barry Day 


Re: [PATCH] rtl8xxxu: Fix failure to reconnect to AP

2016-11-22 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Mon, Nov 21, 2016 at 11:57:22AM -0500, Jes Sorensen wrote:
>> Jes Sorensen <jes.soren...@redhat.com> writes:
>> >>  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> >> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, 
>> >> struct ieee80211_vif *vif,
>> >>   sgi = 1;
>> >>   rcu_read_unlock();
>> >>  
>> >> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x);
>> >> +
>> >>   priv->fops->update_rate_mask(priv, ramask, sgi);
>> >>  
>> >>   rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>> 
>> I believe this change only matters because you disable RXFLTMAP2
>> above. If we really have to write to RXFLTMAP2 to make this work, I
>> suspect we need to keep some sort of state information.
>> 
>> I would also be curious if RXFLTMAP2 gets reset somehow by the firmware,
>> and we do not account for that.
>> 
>> Cheers,
>> Jes
>
> I'll redo the patch without touching REG_RXFLTMAP2. I don't think it's needed
> to fix the fail to reconnect issue.
>
> I haven't had a proper look at the 8723 chips yet but the vendor drivers for
> the others don't do a h2c cmd for disconnect but I'll test leaving it in to 
> see
> if it makes any difference. A 8723bu arrived in the mail today so now I can
> test it too and I discovered yesterday I have a 8723au but it's in a cheap
> Android tablet.

Let me know what you find out - if the h2c command causes the failure
that would be very bizarre but certainly interesting to learn.

Cheers,
Jes


Re: [PATCH 0/7] rtl8xxxu: Pending patches

2016-11-21 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Sat, Nov 19, 2016 at 06:53:42PM -0500, Jes Sorensen wrote:
>> Barry Day <brise...@gmail.com> writes:
>> > On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
>> >> Barry Day <brise...@gmail.com> writes:
>> >> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, jes.soren...@redhat.com wrote:
>> >> >> From: Jes Sorensen <jes.soren...@redhat.com>
>> >> >> 
>> >> >> Kalle,
>> >> >> 
>> >> >> Please find attached a number of patches for the rtl8xxxu
>> >> >> driver.
>> >> >> 
>> >> >> The issues reported with wpa_supplicant on 8723bu still needs further
>> >> >> investigation.
>> >> >> 
>> >> >
>> >> > The patch I posted that you want tested more will also fix the
>> >> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
>> >> > what it should be. I feel fixing that first will be beneficial for
>> >> > fixing any other issues.
>> >> 
>> >> Interesting, I was thinking that might be the case. I do want to dig
>> >> into this further to understand it better. If we use your solution I
>> >> will want to make sure we cover both gen1 and gen2 parts.
>> >> 
>> >> > The recent merge has made my local branch of rtl8xxxu-devel 14
>> >> > commits ahead.
>> >> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
>> >> 
>> >> In general you need to use 'git pull --rebase' on my tree. I rebase it
>> >> to stay in sync with Kalle's tree.
>> >> 
>> >> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
>> >> bits. If it's the patch you posted earlier I can dig it out and play
>> >> with it - I am still catching up though, so please be patient.
>> >
>> > yes it's an 8192eu.
>> 
>> Gotcha - how do you do your testing to reproduce the problem btw? Most
>> of my testing is using the 8723au as a primary device and the next
>> device as a follow-on, so I may not see as long a run with the device
>> active as you see.
>> 
>
> Testing is simple. Connect to an AP in the usual
> manner..disconnect..reconnect.  The 8192eu will fail to reconnect and
> John Heenan reported the 8723bu also fails to reconnect. Even though
> he was directly stopping and restarting wpa_supplicant, it's the same
> thing to the driver - connect..disconnect..reconnect.

Thanks for the details - I'll have a look shortly.

> With using a usb_device pointer, each message starts with the usb bus
> address.  Plug it into a different port and that address could
> change. By using a pointer to the device associated with the wiphy
> each message will begin with the driver name. Just makes it easier for
> the average user to report what's in the log because he can just grep
> for "rtl8xxxu".

I see - that would be problematic for me as I quite often have 3-4 of
these things plugged in at the same time. Not knowing which port spits
out the message would make it a lot harder to track. In fact my primary
devel box for this (Lenovo Yoga 13) has an rtl8723au soldered on the
motherboard, so the moment I plug in any other dongle I'll have two.

The alternative would be to add a prefer to the individual messages.

Cheers,
Jes


Re: [PATCH] rtl8xxxu: Fix failure to reconnect to AP

2016-11-21 Thread Jes Sorensen
Jes Sorensen <jes.soren...@redhat.com> writes:
> Barry Day <brise...@gmail.com> writes:
>> The rtl8192e and rtl8723 fail to reconnect to an AP after being
>> disconnected. Ths patch fixes that without affecting the rtl8192cu.
>> I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
>> After going through the orginal realtek code for the rtl8723, I am
>> confident the patch is applicable to both.
>>
>> Signed-off-by: Barry Day <brise...@gmail.com>
>> ---
>>  rtl8xxxu_core.c | 18 ++
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> Hi Barry,
>
> Thank you for the patch. There are a couple of items which I am not
> 100% sure about the order of.
>
>> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
>> index 04141e5..6ac10d2 100644
>> --- a/rtl8xxxu_core.c
>> +++ b/rtl8xxxu_core.c
>> @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct 
>> rtl8xxxu_priv *priv,
>>  void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
>>u8 macid, bool connect)
>>  {
>> +u8 val8;
>>  struct h2c_cmd h2c;
>>  
>>  memset(, 0, sizeof(struct h2c_cmd));
>>  
>>  h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT;
>> -if (connect)
>> +if (connect) {
>>  h2c.media_status_rpt.parm |= BIT(0);
>> -else
>> -h2c.media_status_rpt.parm &= ~BIT(0);
>> +rtl8xxxu_gen2_h2c_cmd(priv, ,
>> +sizeof(h2c.media_status_rpt));
>> +} else {
>> +val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>> +val8 &= ~BEACON_FUNCTION_ENABLE;
>> +
>> +rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> +rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
>> +rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
>> +}
>>  
>> -rtl8xxxu_gen2_h2c_cmd(priv, , sizeof(h2c.media_status_rpt));
>>  }

Barry,

So looking at this again, I am pretty sure you will break monitor mode
with this. By setting REG_RXFLTMAP2 to 0x you stop reception of all
data frames.

The other thing here is that you change removes the part notifying the
firmware that we disconnected since you now only send the
media_start_rpt command on connect, but not on disconnect.

I am curious if the problem goes away if you simply add the BEACON_CTRL
and REG_DUAL_TSF_RST parts?

>
> This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
> on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
>
>>  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, 
>> struct ieee80211_vif *vif,
>>  sgi = 1;
>>  rcu_read_unlock();
>>  
>> +rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x);
>> +
>>  priv->fops->update_rate_mask(priv, ramask, sgi);
>>  
>>  rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);

I believe this change only matters because you disable RXFLTMAP2
above. If we really have to write to RXFLTMAP2 to make this work, I
suspect we need to keep some sort of state information.

I would also be curious if RXFLTMAP2 gets reset somehow by the firmware,
and we do not account for that.

Cheers,
Jes



Re: [PATCH 0/7] rtl8xxxu: Pending patches

2016-11-19 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Fri, Nov 18, 2016 at 09:00:10PM -0500, Jes Sorensen wrote:
>> Barry Day <brise...@gmail.com> writes:
>> > On Fri, Nov 18, 2016 at 04:44:21PM -0500, jes.soren...@redhat.com wrote:
>> >> From: Jes Sorensen <jes.soren...@redhat.com>
>> >> 
>> >> Kalle,
>> >> 
>> >> Please find attached a number of patches for the rtl8xxxu
>> >> driver.
>> >> 
>> >> The issues reported with wpa_supplicant on 8723bu still needs further
>> >> investigation.
>> >> 
>> >
>> > The patch I posted that you want tested more will also fix the
>> > wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
>> > what it should be. I feel fixing that first will be beneficial for
>> > fixing any other issues.
>> 
>> Interesting, I was thinking that might be the case. I do want to dig
>> into this further to understand it better. If we use your solution I
>> will want to make sure we cover both gen1 and gen2 parts.
>> 
>> > The recent merge has made my local branch of rtl8xxxu-devel 14
>> > commits ahead.
>> > Do I need to do a reset and submit a new patch for the DWA-131 dongle?
>> 
>> In general you need to use 'git pull --rebase' on my tree. I rebase it
>> to stay in sync with Kalle's tree.
>> 
>> The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
>> bits. If it's the patch you posted earlier I can dig it out and play
>> with it - I am still catching up though, so please be patient.
>
> yes it's an 8192eu.

Gotcha - how do you do your testing to reproduce the problem btw? Most
of my testing is using the 8723au as a primary device and the next
device as a follow-on, so I may not see as long a run with the device
active as you see.

> Would you accept a patch that adds a struct device pointer to rtl8xxxu_priv
> and used as the device pointer in the logging functions? Then all the messages
> will start with the driver name making them easier to find.

How do you mean? Right now I have a struct usb_device pointer and
dereference that for ->dev to use with dev_info() messages etc. Do you
want to see more than that?

Cheers,
Jes


Re: [PATCH 0/7] rtl8xxxu: Pending patches

2016-11-18 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Fri, Nov 18, 2016 at 04:44:21PM -0500, jes.soren...@redhat.com wrote:
>> From: Jes Sorensen <jes.soren...@redhat.com>
>> 
>> Kalle,
>> 
>> Please find attached a number of patches for the rtl8xxxu
>> driver.
>> 
>> The issues reported with wpa_supplicant on 8723bu still needs further
>> investigation.
>> 
>
> The patch I posted that you want tested more will also fix the
> wpa_supplicant issue.  Currently I'm looking at why the tx rate is not
> what it should be. I feel fixing that first will be beneficial for
> fixing any other issues.

Interesting, I was thinking that might be the case. I do want to dig
into this further to understand it better. If we use your solution I
will want to make sure we cover both gen1 and gen2 parts.

> The recent merge has made my local branch of rtl8xxxu-devel 14 commits ahead.
> Do I need to do a reset and submit a new patch for the DWA-131 dongle?

In general you need to use 'git pull --rebase' on my tree. I rebase it
to stay in sync with Kalle's tree.

The DWA-131 is the 8192eu? Sorry a bit behind and my mind is losing
bits. If it's the patch you posted earlier I can dig it out and play
with it - I am still catching up though, so please be patient.

Cheers,
Jes




[PATCH 0/7] rtl8xxxu: Pending patches

2016-11-18 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Kalle,

Please find attached a number of patches for the rtl8xxxu
driver.

The issues reported with wpa_supplicant on 8723bu still needs further
investigation.

Note the memory leak issue has only been seen with 8188eu devices so
far, but it's serious and needs to be plugged.

These are what I currently have in my pending queue. Apologies if I
already posted some of these, trying to get back in sync after
Plumbers.

Cheers,
Jes


Jes Sorensen (6):
  rtl8xxxu: Fix memory leak in handling rxdesc16 packets
  rtl8xxxu: Fix big-endian problem reporting mactime
  rtl8xxxu: Fix rtl8723bu driver reload issue
  rtl8xxxu: Fix rtl8192eu driver reload issue
  rtl8xxxu: Obtain RTS rates from mac80211
  rtl8xxxu: Pass tx_info to fill_txdesc in order to have access to retry
count

Wei Yongjun (1):
  rtl8xxxu: Fix non static symbol warning

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  31 +++---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c |  10 +-
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c |   4 +
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 114 ++---
 4 files changed, 104 insertions(+), 55 deletions(-)

-- 
2.7.4



[PATCH 3/7] rtl8xxxu: Fix rtl8723bu driver reload issue

2016-11-18 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The generic disable_rf() function clears bits 22 and 23 in
REG_RX_WAIT_CCA, however we did not re-enable them again in
rtl8723b_enable_rf()

This resolves the problem for me with 8723bu devices not working again
after reloading the driver.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index 6c086b5..02b8ddd 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1498,6 +1498,10 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv 
*priv)
u32 val32;
u8 val8;
 
+   val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+   val32 |= (BIT(22) | BIT(23));
+   rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
/*
 * No indication anywhere as to what 0x0790 does. The 2 antenna
 * vendor code preserves bits 6-7 here.
-- 
2.7.4



[PATCH 7/7] rtl8xxxu: Fix non static symbol warning

2016-11-18 Thread Jes . Sorensen
From: Wei Yongjun <weiyongj...@huawei.com>

Fixes the following sparse warning:

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c:1559:6: warning:
 symbol 'rtl8192eu_power_off' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongj...@huawei.com>
Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index a793fed..a1178c5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1556,7 +1556,7 @@ static int rtl8192eu_power_on(struct rtl8xxxu_priv *priv)
return ret;
 }
 
-void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
+static void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
 {
u8 val8;
u16 val16;
-- 
2.7.4



[PATCH 5/7] rtl8xxxu: Obtain RTS rates from mac80211

2016-11-18 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Use the mac80211 provided rate for RTS rather than the hard coded
24Mbps as suggested by the vendor drivers.

Reported-by: Andrea Merello <andrea.mere...@gmail.com>
Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  6 +--
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 46 ++
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 08d587a..bc3c990 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1340,7 +1340,7 @@ struct rtl8xxxu_fileops {
void (*fill_txdesc) (struct ieee80211_hdr *hdr,
 struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
 u16 rate_flag, bool sgi, bool short_preamble,
-bool ampdu_enable);
+bool ampdu_enable, u32 rts_rate);
int writeN_block_size;
int rx_agg_buf_size;
char tx_desc_size;
@@ -1437,11 +1437,11 @@ bool rtl8xxxu_gen2_simularity_compare(struct 
rtl8xxxu_priv *priv,
 void rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
 struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
 u16 rate_flag, bool sgi, bool short_preamble,
-bool ampdu_enable);
+bool ampdu_enable, u32 rts_rate);
 void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
 struct rtl8xxxu_txdesc32 *tx_desc32, u32 rate,
 u16 rate_flag, bool sgi, bool short_preamble,
-bool ampdu_enable);
+bool ampdu_enable, u32 rts_rate);
 
 extern struct rtl8xxxu_fileops rtl8192cu_fops;
 extern struct rtl8xxxu_fileops rtl8192eu_fops;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a5e6ec2..a4ac557 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4762,7 +4762,7 @@ void
 rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
struct rtl8xxxu_txdesc32 *tx_desc, u32 rate,
u16 rate_flag, bool sgi, bool short_preamble,
-   bool ampdu_enable)
+   bool ampdu_enable, u32 rts_rate)
 {
u16 seq_number;
 
@@ -4796,15 +4796,16 @@ rtl8xxxu_fill_txdesc_v1(struct ieee80211_hdr *hdr,
if (sgi)
tx_desc->txdw5 |= cpu_to_le32(TXDESC32_SHORT_GI);
 
+   /*
+* rts_rate is zero if RTS/CTS or CTS to SELF are not enabled
+*/
+   tx_desc->txdw4 |= cpu_to_le32(rts_rate << TXDESC32_RTS_RATE_SHIFT);
if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS) {
-   /*
-* Use RTS rate 24M - does the mac80211 tell
-* us which to use?
-*/
-   tx_desc->txdw4 |= cpu_to_le32(DESC_RATE_24M <<
- TXDESC32_RTS_RATE_SHIFT);
tx_desc->txdw4 |= cpu_to_le32(TXDESC32_RTS_CTS_ENABLE);
tx_desc->txdw4 |= cpu_to_le32(TXDESC32_HW_RTS_ENABLE);
+   } else if (rate_flag & IEEE80211_TX_RC_USE_CTS_PROTECT) {
+   tx_desc->txdw4 |= cpu_to_le32(TXDESC32_CTS_SELF_ENABLE);
+   tx_desc->txdw4 |= cpu_to_le32(TXDESC32_HW_RTS_ENABLE);
}
 }
 
@@ -4816,7 +4817,7 @@ void
 rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
struct rtl8xxxu_txdesc32 *tx_desc32, u32 rate,
u16 rate_flag, bool sgi, bool short_preamble,
-   bool ampdu_enable)
+   bool ampdu_enable, u32 rts_rate)
 {
struct rtl8xxxu_txdesc40 *tx_desc40;
u16 seq_number;
@@ -4849,15 +4850,19 @@ rtl8xxxu_fill_txdesc_v2(struct ieee80211_hdr *hdr,
if (short_preamble)
tx_desc40->txdw5 |= cpu_to_le32(TXDESC40_SHORT_PREAMBLE);
 
+   tx_desc40->txdw4 |= cpu_to_le32(rts_rate << TXDESC40_RTS_RATE_SHIFT);
+   /*
+* rts_rate is zero if RTS/CTS or CTS to SELF are not enabled
+*/
if (rate_flag & IEEE80211_TX_RC_USE_RTS_CTS) {
-   /*
-* Use RTS rate 24M - does the mac80211 tell
-* us which to use?
-*/
-   tx_desc40->txdw4 |= cpu_to_le32(DESC_RATE_24M <<
-   TXDESC40_RTS_RATE_SHIFT);
tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_RTS_CTS_ENABLE);
tx_desc40->txdw3 |= cpu_to_le32(TXDESC40_HW_RTS_ENABLE);
+   } else if (rate_flag & IEEE80211_TX

[PATCH 4/7] rtl8xxxu: Fix rtl8192eu driver reload issue

2016-11-18 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The 8192eu suffered from two issues when reloading the driver.

The same problems as with the 8723bu where REG_RX_WAIT_CCA bits 22 and
23 didn't get set in rtl8192e_enable_rf().

In addition it also seems prone to issues when setting REG_RF_CTRL to
0 intead of just disabling the RF_ENABLE bit. Similar to what was
causing issues with the 8188eu.

With this patch I can successfully reload the driver and reassociate
to an APi with an 8192eu dongle.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index df54d27..a793fed 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1461,7 +1461,9 @@ static int rtl8192eu_active_to_emu(struct rtl8xxxu_priv 
*priv)
int count, ret = 0;
 
/* Turn off RF */
-   rtl8xxxu_write8(priv, REG_RF_CTRL, 0);
+   val8 = rtl8xxxu_read8(priv, REG_RF_CTRL);
+   val8 &= ~RF_ENABLE;
+   rtl8xxxu_write8(priv, REG_RF_CTRL, val8);
 
/* Switch DPDT_SEL_P output from register 0x65[2] */
val8 = rtl8xxxu_read8(priv, REG_LEDCFG2);
@@ -1593,6 +1595,10 @@ static void rtl8192e_enable_rf(struct rtl8xxxu_priv 
*priv)
u32 val32;
u8 val8;
 
+   val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+   val32 |= (BIT(22) | BIT(23));
+   rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
val8 = rtl8xxxu_read8(priv, REG_GPIO_MUXCFG);
val8 |= BIT(5);
rtl8xxxu_write8(priv, REG_GPIO_MUXCFG, val8);
-- 
2.7.4



[PATCH 1/7] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

2016-11-18 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

A device running without RX package aggregation could return more data
in the USB packet than the actual network packet. In this case the
could would clone the skb but then determine that that there was no
packet to handle and exit without freeing the cloned skb first.

This has so far only been observed with 8188eu devices, but could
affect others.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b2d7f6e..a96ff17 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5197,7 +5197,12 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, 
struct sk_buff *skb)
pkt_offset = roundup(pkt_len + drvinfo_sz + desc_shift +
 sizeof(struct rtl8xxxu_rxdesc16), 128);
 
-   if (pkt_cnt > 1)
+   /*
+* Only clone the skb if there's enough data at the end to
+* at least cover the rx descriptor
+*/
+   if (pkt_cnt > 1 &&
+   urb_len > (pkt_offset + sizeof(struct rtl8xxxu_rxdesc16)))
next_skb = skb_clone(skb, GFP_ATOMIC);
 
rx_status = IEEE80211_SKB_RXCB(skb);
-- 
2.7.4



[PATCH 2/7] rtl8xxxu: Fix big-endian problem reporting mactime

2016-11-18 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The full RX descriptor is converted so converting tsfl again would
return it to it's original endian value.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 4 ++--
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 10166289..08d587a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -238,7 +238,7 @@ struct rtl8xxxu_rxdesc16 {
u32 pattern1match:1;
u32 pattern0match:1;
 #endif
-   __le32 tsfl;
+   u32 tsfl;
 #if 0
u32 bassn:12;
u32 bavld:1;
@@ -368,7 +368,7 @@ struct rtl8xxxu_rxdesc24 {
u32 ldcp:1;
u32 splcp:1;
 #endif
-   __le32 tsfl;
+   u32 tsfl;
 };
 
 struct rtl8xxxu_txdesc32 {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a96ff17..a5e6ec2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5220,7 +5220,7 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, 
struct sk_buff *skb)
rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
   rx_desc->rxmcs);
 
-   rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+   rx_status->mactime = rx_desc->tsfl;
rx_status->flag |= RX_FLAG_MACTIME_START;
 
if (!rx_desc->swdec)
@@ -5290,7 +5290,7 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, 
struct sk_buff *skb)
rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
   rx_desc->rxmcs);
 
-   rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+   rx_status->mactime = rx_desc->tsfl;
rx_status->flag |= RX_FLAG_MACTIME_START;
 
if (!rx_desc->swdec)
-- 
2.7.4



Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC

2016-11-16 Thread Jes Sorensen
John Heenan  writes:
> Barry Day has submitted real world reports for the 8192eu and 8192cu.
> This needs to be acknowledged. I have submitted real world reports for
> the 8723bu.

Lets get this a little more clear - first of all, I have asked you to
investigate which part resolves the problem. Rather than 'I randomly
moved something around and it happens to work for me'.

> When it comes down to it, it looks like the kernel code changes are
> really going to be very trivial to fix this problem and we need to
> take the focus off dramatic outbursts over style issues to a strategy
> for getting usable results from real world testing.
>
> Addressing style issues in a dramatic manner to me looks like a mean
> sport for maintainers who line up to easy target first time
> contributors. This mean attitude comes from the top with a well known
> comment about "publicly making fun of people". The polite comments
> over style from Joe Perches and Rafał Miłecki are welcomed.

Once bad code is in place, it is way harder to get rid of it again. It
is *normal* for maintainers to ask contributors to do things
correctly. In addition you have been asked repeatedly by multiple people
to respect coding style, but every patch you posted violated it again in
a different way, instead of spending the little time it would take for
you to get it right.

> An effective strategy would be to insert some printk statements to
> trace what init steps vendor derived drivers do each time
> wpa_supplicant is called and ask real world testers to report their
> results. This is a lot more productive and less error prone than
> laboriously pouring over vendor source code. Alternative drivers that
> use vendor code from Realtek is enormously complicated and a huge pain
> to make sense of.
>
> Joe Sorensen's driver code is far easier to make sense of and it is a
> shame Realtek don't come to the party. Joe Sorensens's code take takes
> advantage of the excellent work of kernel contributors to the mac80211
> driver.

Now you are pissing on my name - do you really want to be taken
seriously here?

> Previous comments I made about enable_rf, rtl8xxxu_start,
> rtl8xxxu_init_device etc should be clarified. I will leave it for the
> moment as it currently serves no direct useful purpose.

I have made it very clear I want this issue resolved, but I want it
done right.

Jes


Re: Avery's notes from LPC2016 wireless track (Santa Fe)

2016-11-15 Thread Jes Sorensen
Avery Pennarun  writes:
> On Thu, Nov 3, 2016 at 5:55 PM, Barry Day  wrote:
>> Thanks for that. Can I take this as meaning there won't be any videos?
>> I would like to have seen Jes Sorensen's talk on rtl8xxxu
>
> As far as I know, no talks at this LPC were recorded.

They weren't but my slides should be available from the Plumbers site.

https://www.linuxplumbersconf.org/2016/ocw/proposals/4089

Cheers,
Jes


Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC

2016-11-15 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Mon, Oct 31, 2016 at 06:41:30PM -0400, Jes Sorensen wrote:
>> Barry Day <brise...@gmail.com> writes:
>> > On Mon, Oct 31, 2016 at 05:25:12PM -0400, Jes Sorensen wrote:
>> >> As mentioned previously, if this is to be changed here, it has to be
>> >> matched in the _stop section too. It also has to be investigated exactly
>> >> why this matters for 8723bu. It is possible this matters for other
>> >> devices, but we need to find out exactly what causes this not moving a
>> >> major block of init code around like this.
>> >
>> > I've tested this on the 8192e and 8192c. The same problems occurs with the
>> > 8192e but not the 8192c. Stopping and restarting wpa_supplicant had
>> > no affect on the 8192c ability to connect to an AP.
>> 
>> Which version of the driver? I did push in some changes for 8192eu
>> recently that fixed the issue with 8192eu driver reloading, and I would
>> be interested in knowing if this didn't fix the problem for you?
>> 
>> Second, could we please keep this on the linux-wireless list where it
>> belongs. Everybody else doesn't necessarily want to receive a copy via
>> netdev and linux-kernel
>
> The tests were done with the latest version of rtl8xxxu-devel. I haven't
> noticed any occurence of the previous issue with the 8192eu.

OK, I am back from my trip, but still burried alive catching up on
things. This is very much on the list of things I want to investigate.

Jes


Re: [PATCH] rtl8xxxu: Fix failure to reconnect to AP

2016-11-14 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Mon, Nov 14, 2016 at 08:24:45AM -0500, Jes Sorensen wrote:
>> Barry Day <brise...@gmail.com> writes:
>> > The rtl8192e and rtl8723 fail to reconnect to an AP after being
>> > disconnected. Ths patch fixes that without affecting the rtl8192cu.
>> > I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
>> > After going through the orginal realtek code for the rtl8723, I am
>> > confident the patch is applicable to both.
>> >
>> > Signed-off-by: Barry Day <brise...@gmail.com>
>> > ---
>> >  rtl8xxxu_core.c | 18 ++
>> >  1 file changed, 14 insertions(+), 4 deletions(-)
>> 
>> Hi Barry,
>> 
>> Thank you for the patch. There are a couple of items which I am not
>> 100% sure about the order of.
>> 
>> > diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
>> > index 04141e5..6ac10d2 100644
>> > --- a/rtl8xxxu_core.c
>> > +++ b/rtl8xxxu_core.c
>> > @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct 
>> > rtl8xxxu_priv *priv,
>> >  void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
>> >  u8 macid, bool connect)
>> >  {
>> > +  u8 val8;
>> >struct h2c_cmd h2c;
>> >  
>> >memset(, 0, sizeof(struct h2c_cmd));
>> >  
>> >h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT;
>> > -  if (connect)
>> > +  if (connect) {
>> >h2c.media_status_rpt.parm |= BIT(0);
>> > -  else
>> > -  h2c.media_status_rpt.parm &= ~BIT(0);
>> > +  rtl8xxxu_gen2_h2c_cmd(priv, ,
>> > +  sizeof(h2c.media_status_rpt));
>> > +  } else {
>> > +  val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>> > +  val8 &= ~BEACON_FUNCTION_ENABLE;
>> > +
>> > +  rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> > +  rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
>> > +  rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
>> > +  }
>> >  
>> > -  rtl8xxxu_gen2_h2c_cmd(priv, , sizeof(h2c.media_status_rpt));
>> >  }
>> 
>> This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
>> on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
>
> Even in monitor mode the interface has to brought up to use it which invokes
> rtl8xxxu_start which sets it back to accepting frames.
>
>
>> >  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> > @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, 
>> > struct ieee80211_vif *vif,
>> >sgi = 1;
>> >rcu_read_unlock();
>> >  
>> > +  rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x);
>> > +
>> >priv->fops->update_rate_mask(priv, ramask, sgi);
>> >  
>> >rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>> 
>> Here you enable RXFLTMAP2 for all devices - this doesn't balance with
>> the above.
>
> The original realtek code I have for the 8192cu does the disconnect the
> same way as in this patch. I tested the 8192cu using the patched
> gen2_report connect and it works. That would make things consistent across
> all chipsets.

My concern is that you only set FLATMAP and BEACON_CTRL for gen2
devices, which has zero impact on gen1 devices, such as the 8192cu. If
need to do this for gen2 (8723bu/8192eu) I assume we need to do it for
gen1 as well.

I'd like to see us do a little more investigating on this first.

Jes


Re: [PATCH] rtl8xxxu: Fix failure to reconnect to AP

2016-11-14 Thread Jes Sorensen
Barry Day  writes:
> The rtl8192e and rtl8723 fail to reconnect to an AP after being
> disconnected. Ths patch fixes that without affecting the rtl8192cu.
> I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
> After going through the orginal realtek code for the rtl8723, I am
> confident the patch is applicable to both.
>
> Signed-off-by: Barry Day 
> ---
>  rtl8xxxu_core.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)

Hi Barry,

Thank you for the patch. There are a couple of items which I am not
100% sure about the order of.

> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
> index 04141e5..6ac10d2 100644
> --- a/rtl8xxxu_core.c
> +++ b/rtl8xxxu_core.c
> @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct 
> rtl8xxxu_priv *priv,
>  void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> u8 macid, bool connect)
>  {
> + u8 val8;
>   struct h2c_cmd h2c;
>  
>   memset(, 0, sizeof(struct h2c_cmd));
>  
>   h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT;
> - if (connect)
> + if (connect) {
>   h2c.media_status_rpt.parm |= BIT(0);
> - else
> - h2c.media_status_rpt.parm &= ~BIT(0);
> + rtl8xxxu_gen2_h2c_cmd(priv, ,
> + sizeof(h2c.media_status_rpt));
> + } else {
> + val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> + val8 &= ~BEACON_FUNCTION_ENABLE;
> +
> + rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
> + }
>  
> - rtl8xxxu_gen2_h2c_cmd(priv, , sizeof(h2c.media_status_rpt));
>  }

This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
on purpose for monitor mode, but you now disable it for 8192eu/8723bu.

>  void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, 
> struct ieee80211_vif *vif,
>   sgi = 1;
>   rcu_read_unlock();
>  
> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x);
> +
>   priv->fops->update_rate_mask(priv, ramask, sgi);
>  
>   rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);

Here you enable RXFLTMAP2 for all devices - this doesn't balance with
the above.

I am not against the aproach, but I think we need to investigate a
little further what is going on.

Cheers,
Jes


Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower

2016-11-10 Thread Jes Sorensen
Dave Jones <s.dave.jo...@gmail.com> writes:
> On Fri, Nov 04, 2016 at 09:56:00AM -0400, Jes Sorensen wrote:
>>
>> Joe Perches <j...@perches.com> writes:
>> > On Sun, 2016-10-30 at 19:02 -0400, Jes Sorensen wrote:
>> >> Code is 80 characters wide, and comments are /* */ never the ugly C++
>> >> crap.
>> >
>> > You might look at the recent Linus Torvalds authored commit
>> > 5e467652ffef (?printk: re-organize log_output() to be more legible")
>> > which does both of those: c99 // comments and > 80 columns.
>> >
>> > Absolutes are for zealots.
>>
>> What Linus does in his code, is totally up to him. What I pull into the
>> driver that *I* maintain, is up to me. It is perfectly normal to expect
>> submitters to respect the coding style of the piece of code they are
>> trying to edit.
>
> Bullshit.  It's perfectly normal to respect Linux coding style described in
> Documentation/CodingStyle.  Now let's back to the topic, could you
> apply John's patch or you just wanna improve your driver is 100% bug free?

First of all, I call for proper CodingStyle to be applied to my driver,
and I expect someone posting a patch to respect the codingstyle of the
driver in question. It is simple respect for the code. If you consider
that BS - that is on you!

Second I am NOT applying that patch as I have stated repeatedly because
I am not convinced it is safe to do so and it changes the code flow for
one type of chip and not the rest. In addition it uses a broken approach
to doing chip specific changes.

In short, the patch is broken!

Jes


Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower

2016-11-04 Thread Jes Sorensen
Joe Perches <j...@perches.com> writes:
> On Sun, 2016-10-30 at 19:02 -0400, Jes Sorensen wrote:
>> Code is 80 characters wide, and comments are /* */ never the ugly C++
>> crap.
>
> You might look at the recent Linus Torvalds authored commit
> 5e467652ffef (?printk: re-organize log_output() to be more legible")
> which does both of those: c99 // comments and > 80 columns.
>
> Absolutes are for zealots.

What Linus does in his code, is totally up to him. What I pull into the
driver that *I* maintain, is up to me. It is perfectly normal to expect
submitters to respect the coding style of the piece of code they are
trying to edit.

Jes


Re: [PATCH v2] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC

2016-11-03 Thread Jes Sorensen
John Heenan  writes:
> The rtl8723bu wireless IC shows evidence of a more agressive approach to
> power saving, powering down its RF side when there is no wireless
> interfacing but leaving USB interfacing intact. This makes the wireless
> IC more suitable for use in devices which need to keep their power use
> as low as practical, such as tablets and Surface Pro type devices.
>
> In effect this means that a full initialisation must be performed
> whenever a wireless interface is brought up. It also means that
> interpretations of power status from general wireless registers should
> not be relied on to influence an init sequence.
>
> The patch works by forcing a fuller initialisation and forcing it to
> occur more often in code paths (such as occurs during a low level
> authentication that initiates wireless interfacing).
>
> The initialisation sequence is now more consistent with code based
> directly on vendor code. For example while the vendor derived code
> interprets a register as indcating a particular powered state, it does
> not use this information to influence its init sequence.
>
> Only devices that use the rtl8723bu driver are affected by this patch.
>
> With this patch wpa_supplicant reliably and consistently connects with
> an AP. Before a workaround such as executing rmmod and modprobe before
> each call to wpa_supplicant worked with some distributions.
>
> Signed-off-by: John Heenan 
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>

I am at Linux Plumbers this week, so my response time is slow. Next week
I am on PTO, so I will not respond.

First of all, why do you keep CC'ing multiple mailing lists that do not
matter? This discussion belongs on linux-wireless not on netdev or lkml.
CC'ing Kalle directly is not going to get him to apply this broken patch
for you.

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e5..ab2f2ef 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3900,7 +3900,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>* Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>* initialized. First MAC returns 0xea, second MAC returns 0x00
>*/
> - if (val8 == 0xea)
> + if (val8 == 0xea || priv->fops == _fops)
>   macpower = false;
>   else
>   macpower = true;

Why oh why do you insist on not using the *standard* way of coping with
this? 'priv-rtl_chip' is used everywhere else, but you just have to do
something awful like this?

> @@ -5779,6 +5779,12 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>  
>   ret = 0;
>  
> + if(priv->fops == _fops) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
> + goto error_out;
> + }
> +
>   init_usb_anchor(>rx_anchor);
>   init_usb_anchor(>tx_anchor);
>   init_usb_anchor(>int_anchor);

Read Documentation/CodingStyle - as others already pointed you at.

> @@ -6080,9 +6086,11 @@ static int rtl8xxxu_probe(struct usb_interface 
> *interface,
>   goto exit;
>   }
>  
> - ret = rtl8xxxu_init_device(hw);
> - if (ret)
> - goto exit;
> + if(priv->fops != _fops) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
> + goto exit;
> + }
>  
>   hw->wiphy->max_scan_ssids = 1;
>   hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;

Again coding style violation.

Second, I am NOT going to accept any patches that fundamentally changes
the init sequence of the code for just one device.

I already told you I want to find out *why* this matters, and what part
of rtl8xxxu_init_device() is the culprit. I want to understand the
actual problem, not just blindly move stuff around.

Jes


Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC

2016-10-31 Thread Jes Sorensen
Barry Day <brise...@gmail.com> writes:
> On Mon, Oct 31, 2016 at 05:25:12PM -0400, Jes Sorensen wrote:
>> As mentioned previously, if this is to be changed here, it has to be
>> matched in the _stop section too. It also has to be investigated exactly
>> why this matters for 8723bu. It is possible this matters for other
>> devices, but we need to find out exactly what causes this not moving a
>> major block of init code around like this.
>
> I've tested this on the 8192e and 8192c. The same problems occurs with the
> 8192e but not the 8192c. Stopping and restarting wpa_supplicant had
> no affect on the 8192c ability to connect to an AP.

Which version of the driver? I did push in some changes for 8192eu
recently that fixed the issue with 8192eu driver reloading, and I would
be interested in knowing if this didn't fix the problem for you?

Second, could we please keep this on the linux-wireless list where it
belongs. Everybody else doesn't necessarily want to receive a copy via
netdev and linux-kernel

Jes


Re: [PATCH] rtl8xxxu: Fix for agressive power saving by rtl8723bu wireless IC

2016-10-31 Thread Jes Sorensen
John Heenan  writes:
> The rtl8723bu wireless IC shows evidence of a more agressive approach to
> power saving, powering down its RF side when there is no wireless
> interfacing but leaving USB interfacing intact. This makes the wireless
> IC more suitable for use in devices which need to keep their power use
> as low as practical, such as tablets and Surface Pro type devices.
>
> In effect this means that a full initialisation must be performed
> whenever a wireless interface is brought up. It also means that
> interpretations of power status from general wireless registers should
> not be relied on to influence an init sequence.
>
> The patch works by forcing a fuller initialisation and forcing it to
> occur more often in code paths (such as occurs during a low level
> authentication that initiates wireless interfacing).
>
> The initialisation sequence is now more consistent with code based
> directly on vendor code. For example while the vendor derived code
> interprets a register as indcating a particular powered state, it does
> not use this information to influence its init sequence.
>
> The rtl8723bu device has a unique USB VID and PID. This is taken
> advantage of for the patch to ensure only rtl8723bu devices are affected
> by this patch.
>
> With this patch wpa_supplicant reliably and consistently connects with
> an AP. Before a workaround such as executing rmmod and modprobe before
> each call to wpa_supplicant worked with some distributions.
>
> Signed-off-by: John Heenan 
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 24 
> ++
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e5..f36e674 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -79,6 +79,8 @@ MODULE_PARM_DESC(dma_agg_pages, "Set DMA aggregation pages 
> (range 1-127, 0 to di
>  #define RTL8XXXU_TX_URB_LOW_WATER25
>  #define RTL8XXXU_TX_URB_HIGH_WATER   32
>  
> +#define USB_PRODUCT_ID_RTL8723BU 0xb720
> +

Absolutely not! You have no guarantee that this is the only id used for
8723bu devices, and adding a #define for each is not going to happen.

>  static int rtl8xxxu_submit_rx_urb(struct rtl8xxxu_priv *priv,
> struct rtl8xxxu_rx_urb *rx_urb);
>  
> @@ -3892,6 +3894,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>   u8 val8;
>   u16 val16;
>   u32 val32;
> +  struct usb_device_descriptor *udesc = >udev->descriptor;

Indentaiton
  
>   /* Check if MAC is already powered on */
>   val8 = rtl8xxxu_read8(priv, REG_CR);
> @@ -3900,7 +3903,9 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>* Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>* initialized. First MAC returns 0xea, second MAC returns 0x00
>*/
> - if (val8 == 0xea)
> + if (val8 == 0xea
> + || (udesc->idVendor == USB_VENDOR_ID_REALTEK
> + &&  udesc->idProduct == USB_PRODUCT_ID_RTL8723BU))
>   macpower = false;
>   else
>   macpower = true;

Please respect proper kernel coding style!

> @@ -5776,9 +5781,17 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>   struct rtl8xxxu_tx_urb *tx_urb;
>   unsigned long flags;
>   int ret, i;
> + struct usb_device_descriptor *udesc = >udev->descriptor;
>  
>   ret = 0;
>  
> + if(udesc->idVendor == USB_VENDOR_ID_REALTEK
> + && udesc->idProduct == USB_PRODUCT_ID_RTL8723BU) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
> + goto error_out;
> + }
> +

As mentioned previously, if this is to be changed here, it has to be
matched in the _stop section too. It also has to be investigated exactly
why this matters for 8723bu. It is possible this matters for other
devices, but we need to find out exactly what causes this not moving a
major block of init code around like this.

>   init_usb_anchor(>rx_anchor);
>   init_usb_anchor(>tx_anchor);
>   init_usb_anchor(>int_anchor);
> @@ -6080,9 +6093,12 @@ static int rtl8xxxu_probe(struct usb_interface 
> *interface,
>   goto exit;
>   }
>  
> - ret = rtl8xxxu_init_device(hw);
> - if (ret)
> + if(!(id->idVendor == USB_VENDOR_ID_REALTEK
> + && id->idProduct == USB_PRODUCT_ID_RTL8723BU)) {
> + ret = rtl8xxxu_init_device(hw);
> + if (ret)
>   goto exit;
> + }

Again, this coding style abuse will never go into this driver,

Jes


Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower

2016-10-30 Thread Jes Sorensen
John Heenan  writes:
> Thanks for your reply.
>
> The code was tested on a Cube i9 which has an internal rtl8723bu.
>
> No other devices were tested.
>
> I am happy to accept in an ideal context hard coding macpower is
> undesirable, the comment is undesirable and it is wrong to assume the
> issue is not unique to the rtl8723bu.
>
> Your reply is idealistic. What can I do now?  I should of course have
> factored out other untested devices in my patches. The apparent
> concern you have with process over outcome is a useful lesson.
>
> We are not in an ideal situation. The comment is of course relevant
> and useful to starting a process to fixing a real bug I do not have
> sufficient information to refine any further for and others do. In the
> circumstances nothing really more can be expected.

Well you should start by reporting the issue and either providing a
patch that only affects 8723bu, or work on a generic solution. I
appreciate patches, but I do not appreciate patches that will make
something work for one person and break for everyone else - I spent a
lot of time making sure the driver works across the different devices.

The comment violates all Linux standards - first rule when modifying
code is to respect the style of the code you are dealing with.

Code is 80 characters wide, and comments are /* */ never the ugly C++
crap.

> My patch cover letter, [PATCH 0/2] provides evidence of a mess with
> regard to determining macpower for the rtl8723bu and what is
> subsequently required. This is important.
>
> The kernel driver code is very poorly documented and there is not a
> single source reference to device documentation. For example macpower
> is noting more than a setting that is true or false according to
> whether a read of a particular register return 0xef or not. Such value
> was never obtained so a full init sequence was never performed.

The kernel driver is documented with the information I have - there is
NO device documentation because Realtek refuses to provide any. I have
written the driver based on what I have retried by reading the vendor
drivers. If you can provide better documentation, I certainly would love
to get it.

> It would be helpful if you could provide a link to device references.
> As it is, how am I supposed to revise the patch without relevant
> information?

Look at the USB device table, it shows you which devices are supported.

> My patch code works with the Cube i9, as is, despite a lack of
> adequate information. Before it did not. That is a powerful statement

The driver works with a lot of different devices in itself that is a
powerful statement!

Yes I want to see it work with as many devices as possible, but just
moving things around without balancing it and not explaining why is not
a fix. If we move more of the init sequence to _start() you also have to
move matching pieces to _stop().

Jes


Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower

2016-10-30 Thread Jes Sorensen
John Heenan  writes:
> Code tests show data returned by rtl8xxxu_read8(priv, REG_CR), used to set
> macpower, is never 0xea. It is only ever 0x01 (first time after modprobe)
> using wpa_supplicant and 0x00 thereafter using wpa_supplicant. These results
> occurs with 'Fix for authentication failure' [PATCH 1/2] in place.
>
> Whatever was returned, code tests always showed that at least
> rtl8xxxu_init_queue_reserved_page(priv);
> is always required. Not called if macpower set to true.
>
> Please see cover letter, [PATCH 0/2], for more information from tests.
>
> For rtl8xxxu-devel branch of 
> git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
>
> Signed-off-by: John Heenan 
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index f25b4df..aae05f3 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3904,6 +3904,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>   macpower = false;
>   else
>   macpower = true;
> + macpower = false; // Code testing shows macpower must always be set to 
> false to avoid failure
>  
>   ret = fops->power_on(priv);
>   if (ret < 0) {

Sorry but this patch is neither serious nor acceptable. First of all,
hardcoding macpower like this right after an if statement is plain
wrong, second your comments violate all kernel rules.

Second, you argue this was tested using code test - on which device? Did
you test it on all rtl8xxxu based devices or just rtl8723bu?

NACK

Jes


Re: [PATCH] rtl8xxxu: Add D-Link DWA-131 rev E1

2016-10-26 Thread Jes Sorensen
Barry Day  writes:
> This is a rtl8192eu dongle and has been tested
>
> Signed-off-by: Barry Day 
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks, I'll add it to my queue, unless Kalle grabs it first. Glad to
know it's working for you!

Jes

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e5..d426836 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6009,7 +6009,7 @@ static int rtl8xxxu_probe(struct usb_interface 
> *interface,
>   untested = 0;
>   break;
>   case 0x2001:
> - if (id->idProduct == 0x3308)
> + if (id->idProduct == 0x3308 || id->idProduct == 0x3319)
>   untested = 0;
>   break;
>   case 0x2357:
> @@ -6188,6 +6188,8 @@ static struct usb_device_id dev_table[] = {
>   .driver_info = (unsigned long)_fops},
>  {USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x818b, 0xff, 0xff, 
> 0xff),
>   .driver_info = (unsigned long)_fops},
> +{USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x3319, 0xff, 0xff, 0xff),
> + .driver_info = (unsigned long)_fops},
>  /* Tested by Myckel Habets */
>  {USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0109, 0xff, 0xff, 0xff),
>   .driver_info = (unsigned long)_fops},


Re: [PATCH 1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

2016-10-03 Thread Jes Sorensen
Kalle Valo <kv...@codeaurora.org> writes:
> jes.soren...@redhat.com writes:
>
>> From: Jes Sorensen <jes.soren...@redhat.com>
>>
>> A device running without RX package aggregation could return more data
>> in the USB packet than the actual network packet. In this case the
>> could would clone the skb but then determine that that there was no
>> packet to handle and exit without freeing the cloned skb first.
>
> s/case the/case we/? I can edit that before applying the patch.

Sounds good - thanks!

Jes


Re: [PATCH 2/2] rtl8xxxu: Fix big-endian problem reporting mactime

2016-10-03 Thread Jes Sorensen
Kalle Valo <kv...@codeaurora.org> writes:
> jes.soren...@redhat.com writes:
>
>> From: Jes Sorensen <jes.soren...@redhat.com>
>>
>> The full RX descriptor is converted so converting tsfl again would
>> return it to it's original endian value.
>>
>> Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
>
> I'll add:
>
> Cc: sta...@vger.kernel.org # 4.8+

Appreciate it!

Jes


Re: RTL8192EU on rtl8xxxu driver breaks every few minutes

2016-10-03 Thread Jes Sorensen
"Franc[e]sco"  writes:
> Thanks for the patch, just tested the rtl8xxxu-devel branch and it seems
> to have the same exact issue with the same dmesg output with the
> addition of "rtl8192eu_active_to_emu: Disabling MAC timed out" when I
> disconnect the dongle.
>
> I checked out and compiled the whole kernel from the branch because the
> 4.7.4 sources seemed to be missing the rtl8192eu_active_to_emu function.

There are other patches related to the 8192eu in the wireless tree which
fix some issues with the dongle if you reload the driver. Alternatively
pull my git tree and build that.

Jes


Re: [PATCH] realtek: rtl8xxxu: Use const init arrays

2016-10-01 Thread Jes Sorensen
Joe Perches <j...@perches.com> writes:
> On Sat, 2016-10-01 at 16:32 -0400, Jes Sorensen wrote:
>> Your output shows it moving to the text segment - if it's in a different
>> segment, eg. rodata, you should use output demonstrating that to justify
>> the change.
>
> For size, rodata _is_ text

Well then maybe use something which provides accurate data.

Jes


Re: [PATCH] realtek: rtl8xxxu: Use const init arrays

2016-10-01 Thread Jes Sorensen
Joe Perches <j...@perches.com> writes:
> On Sat, 2016-10-01 at 14:53 -0400, Jes Sorensen wrote:
>> Joe Perches <j...@perches.com> writes:
>> > Make the init arrays const to reduce data.
>> > $ size drivers/net/wireless/realtek/rtl8xxxu/built-in.o*
>> > (allyesconfig: x86-32)
>> >    text   data bss dec hex filename
>> >   80107 13651 58 93816 16e78
>> > drivers/net/wireless/realtek/rtl8xxxu/built-in.o.new
>> >   65303 28435 58 93796 16e64
>> > drivers/net/wireless/realtek/rtl8xxxu/built-in.o.old
>> In total you grow the kernel by 20 bytes. You reduce the data segment
>> substantially while growing the text segment instead.
>
> No, not really.   The alignment boundaries move a bit for
> this particular compilation.  It could go the other way for
> a different compiler version or set of CONFIG options.
>
> What's important is multiple pages of .data move to .rodata.

Your output shows it moving to the text segment - if it's in a different
segment, eg. rodata, you should use output demonstrating that to justify
the change.

Jes


Re: [PATCH] realtek: rtl8xxxu: Use const init arrays

2016-10-01 Thread Jes Sorensen
Joe Perches  writes:
> Make the init arrays const to reduce data.
>
> $ size drivers/net/wireless/realtek/rtl8xxxu/built-in.o* (allyesconfig: 
> x86-32)
>text  data bss dec hex filename
>   80107 13651  58   93816   16e78 
> drivers/net/wireless/realtek/rtl8xxxu/built-in.o.new
>   65303 28435  58   93796   16e64 
> drivers/net/wireless/realtek/rtl8xxxu/built-in.o.old
>
> Signed-off-by: Joe Perches 

In total you grow the kernel by 20 bytes. You reduce the data segment
substantially while growing the text segment instead.

If any architecture replicates the text segment onto individual numa
nodes, this would actually be a real loss rather than a win. Some archs
used to do this, not sure if they are doing it anymore.

I am not against this patch, but I am not sure it's really a win either.

Jes


Re: [PATCH 1/2] rtl8xxxu: Fix rtl8723bu driver reload issue

2016-10-01 Thread Jes Sorensen
Greg KH <g...@kroah.com> writes:
> On Fri, Sep 30, 2016 at 07:35:17PM -0400, jes.soren...@redhat.com wrote:
>> From: Jes Sorensen <jes.soren...@redhat.com>
>> 
>> The generic disable_rf() function clears bits 22 and 23 in
>> REG_RX_WAIT_CCA, however we did not re-enable them again in
>> rtl8723b_enable_rf()
>> 
>> This resolves the problem for me with 8723bu devices not working again
>> after reloading the driver.
>> 
>> Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
>> ---
>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 
>>  1 file changed, 4 insertions(+)
>
> 
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
>
> 

DOH Kalle told me to CC stable I took it for granted that you meant CC
in the email sense. Guess I get to blame my self for looking like a
fool in public.

I'll send you an email directly with the SHA once it hits Linus' tree
instead.

Jes


Re: RTL8192EU on rtl8xxxu driver breaks every few minutes

2016-09-30 Thread Jes Sorensen
"Franc[e]sco" <lolisamu...@waifu.club> writes:
> Oops, forgot to forward the previous reply to the mailing list.
>
> I have attached the output of iw link
>
> the AP is an asus dsl-n55u router in 2.4 GHz mode, using WPA2-Personal
> with AES encryption. It's also running a second 5GHz wireless network
> which has a different SSID.
>
> Also, this seems to be some kind of power saving kicking in, as the
> dongle keeps working as long as I keep doing things over ssh.

I just pushed a patch into rtl8xxxu-devel which may resolve this issue.
There were problems with the 8192eu not handling driver reload very
well. It is possible the network scripts you run would trigger the shut
down and restart that caused this problem.

I would be interested in knowing if this patch resolves the problem for
you.

Jes

PS: Please fix your mail client - adding unlisted-recipients to the Cc
line and cutting out the person you are responding to is really
annoying.

>From 93064d0ae3e9d97c03a3aabd71e6048e1ac82f46 Mon Sep 17 00:00:00 2001
From: Jes Sorensen <jes.soren...@redhat.com>
Date: Fri, 30 Sep 2016 19:18:34 -0400
Subject: [PATCH 2/2] rtl8xxxu: Fix rtl8192eu driver reload issue

The 8192eu suffered from two issues when reloading the driver.

The same problems as with the 8723bu where REG_RX_WAIT_CCA bits 22 and
23 didn't get set in rtl8192e_enable_rf().

In addition it also seems prone to issues when setting REG_RF_CTRL to
0 intead of just disabling the RF_ENABLE bit. Similar to what was
causing issues with the 8188eu.

With this patch I can successfully reload the driver and reassociate
to an APi with an 8192eu dongle.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index df54d27..a793fed 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1461,7 +1461,9 @@ static int rtl8192eu_active_to_emu(struct rtl8xxxu_priv *priv)
 	int count, ret = 0;
 
 	/* Turn off RF */
-	rtl8xxxu_write8(priv, REG_RF_CTRL, 0);
+	val8 = rtl8xxxu_read8(priv, REG_RF_CTRL);
+	val8 &= ~RF_ENABLE;
+	rtl8xxxu_write8(priv, REG_RF_CTRL, val8);
 
 	/* Switch DPDT_SEL_P output from register 0x65[2] */
 	val8 = rtl8xxxu_read8(priv, REG_LEDCFG2);
@@ -1593,6 +1595,10 @@ static void rtl8192e_enable_rf(struct rtl8xxxu_priv *priv)
 	u32 val32;
 	u8 val8;
 
+	val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+	val32 |= (BIT(22) | BIT(23));
+	rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
 	val8 = rtl8xxxu_read8(priv, REG_GPIO_MUXCFG);
 	val8 |= BIT(5);
 	rtl8xxxu_write8(priv, REG_GPIO_MUXCFG, val8);
-- 
2.7.4



[PATCH 1/2] rtl8xxxu: Fix rtl8723bu driver reload issue

2016-09-30 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The generic disable_rf() function clears bits 22 and 23 in
REG_RX_WAIT_CCA, however we did not re-enable them again in
rtl8723b_enable_rf()

This resolves the problem for me with 8723bu devices not working again
after reloading the driver.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index 6c086b5..02b8ddd 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1498,6 +1498,10 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv 
*priv)
u32 val32;
u8 val8;
 
+   val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+   val32 |= (BIT(22) | BIT(23));
+   rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
/*
 * No indication anywhere as to what 0x0790 does. The 2 antenna
 * vendor code preserves bits 6-7 here.
-- 
2.7.4



[PATCH 2/2] rtl8xxxu: Fix rtl8192eu driver reload issue

2016-09-30 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The 8192eu suffered from two issues when reloading the driver.

The same problems as with the 8723bu where REG_RX_WAIT_CCA bits 22 and
23 didn't get set in rtl8192e_enable_rf().

In addition it also seems prone to issues when setting REG_RF_CTRL to
0 intead of just disabling the RF_ENABLE bit. Similar to what was
causing issues with the 8188eu.

With this patch I can successfully reload the driver and reassociate
to an APi with an 8192eu dongle.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index df54d27..a793fed 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1461,7 +1461,9 @@ static int rtl8192eu_active_to_emu(struct rtl8xxxu_priv 
*priv)
int count, ret = 0;
 
/* Turn off RF */
-   rtl8xxxu_write8(priv, REG_RF_CTRL, 0);
+   val8 = rtl8xxxu_read8(priv, REG_RF_CTRL);
+   val8 &= ~RF_ENABLE;
+   rtl8xxxu_write8(priv, REG_RF_CTRL, val8);
 
/* Switch DPDT_SEL_P output from register 0x65[2] */
val8 = rtl8xxxu_read8(priv, REG_LEDCFG2);
@@ -1593,6 +1595,10 @@ static void rtl8192e_enable_rf(struct rtl8xxxu_priv 
*priv)
u32 val32;
u8 val8;
 
+   val32 = rtl8xxxu_read32(priv, REG_RX_WAIT_CCA);
+   val32 |= (BIT(22) | BIT(23));
+   rtl8xxxu_write32(priv, REG_RX_WAIT_CCA, val32);
+
val8 = rtl8xxxu_read8(priv, REG_GPIO_MUXCFG);
val8 |= BIT(5);
rtl8xxxu_write8(priv, REG_GPIO_MUXCFG, val8);
-- 
2.7.4



[PATCH 0/2] rtl8xxxu: Fix driver reload issues with 8723bu and 8192eu

2016-09-30 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Hi,

These two patches fix issues where rtl8723bu and rtl8192eu dongles
would stop receving data if one reloaded the driver module, requiring
it to be physically removed and reinserted.

Both issues are applicable to 4.9. The first patch for 8723bu is
applicable to stable 4.7 and 4.8. The second patch will only be
applicable to stable 4.8, once patches currently in flight are merged
by the network maintainers.

Cheers,
Jes


Jes Sorensen (2):
  rtl8xxxu: Fix rtl8723bu driver reload issue
  rtl8xxxu: Fix rtl8192eu driver reload issue

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 +++-
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.7.4



[PATCH 1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

2016-09-29 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

A device running without RX package aggregation could return more data
in the USB packet than the actual network packet. In this case the
could would clone the skb but then determine that that there was no
packet to handle and exit without freeing the cloned skb first.

This has so far only been observed with 8188eu devices, but could
affect others.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b2d7f6e..a96ff17 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5197,7 +5197,12 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, 
struct sk_buff *skb)
pkt_offset = roundup(pkt_len + drvinfo_sz + desc_shift +
 sizeof(struct rtl8xxxu_rxdesc16), 128);
 
-   if (pkt_cnt > 1)
+   /*
+* Only clone the skb if there's enough data at the end to
+* at least cover the rx descriptor
+*/
+   if (pkt_cnt > 1 &&
+   urb_len > (pkt_offset + sizeof(struct rtl8xxxu_rxdesc16)))
next_skb = skb_clone(skb, GFP_ATOMIC);
 
rx_status = IEEE80211_SKB_RXCB(skb);
-- 
2.7.4



[PATCH 2/2] rtl8xxxu: Fix big-endian problem reporting mactime

2016-09-29 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

The full RX descriptor is converted so converting tsfl again would
return it to it's original endian value.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 4 ++--
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 10166289..08d587a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -238,7 +238,7 @@ struct rtl8xxxu_rxdesc16 {
u32 pattern1match:1;
u32 pattern0match:1;
 #endif
-   __le32 tsfl;
+   u32 tsfl;
 #if 0
u32 bassn:12;
u32 bavld:1;
@@ -368,7 +368,7 @@ struct rtl8xxxu_rxdesc24 {
u32 ldcp:1;
u32 splcp:1;
 #endif
-   __le32 tsfl;
+   u32 tsfl;
 };
 
 struct rtl8xxxu_txdesc32 {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a96ff17..a5e6ec2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5220,7 +5220,7 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, 
struct sk_buff *skb)
rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
   rx_desc->rxmcs);
 
-   rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+   rx_status->mactime = rx_desc->tsfl;
rx_status->flag |= RX_FLAG_MACTIME_START;
 
if (!rx_desc->swdec)
@@ -5290,7 +5290,7 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, 
struct sk_buff *skb)
rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
   rx_desc->rxmcs);
 
-   rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+   rx_status->mactime = rx_desc->tsfl;
rx_status->flag |= RX_FLAG_MACTIME_START;
 
if (!rx_desc->swdec)
-- 
2.7.4



Re: [PATCHv3] wireless: check A-MSDU inner frame source address on AP interfaces

2016-09-28 Thread Jes Sorensen
Johannes Berg <johan...@sipsolutions.net> writes:
> On Wed, 2016-09-28 at 11:39 -0400, Jes Sorensen wrote:
>
>> > No Jes, you're wrong this time - this is changing internal API so
>> > it does have to touch all users thereof.
>> 
>> Even in this case, change the individual components in individual
>> patches and post them as a set.
>
> No, still wrong - it has to be committed as a single patch so it
> doesn't break bisect.
>
>> Changes to staging needs to go in via staging, and rtl8723au is gone
>> from the staging tree.
>> 
>
> I've previously taken API change patches that touch staging, if people
> feel so inclined, and I don't think Greg will mind. I'm going to keep
> doing that unless Dave tells me he won't pull from me when I do it :)

I'll still argue this could be handled better through gradual migration
rather than one large patch that touches too many places, but if you are
willing to take it, I am not going to fight you over it :)

Jes


Re: [PATCHv3] wireless: check A-MSDU inner frame source address on AP interfaces

2016-09-28 Thread Jes Sorensen
Johannes Berg <johan...@sipsolutions.net> writes:
> On Wed, 2016-09-28 at 11:19 -0400, Jes Sorensen wrote:
>> 
>> I understand the intentions of this patch are all good, but you need
>> to not post patches that include both staging and mainline drivers at
>> the same time. In general make it a patchset and do one patch per
>> driver.
>> 
>> Ideally split up changes to generic code into their own patches too.
>
> No Jes, you're wrong this time - this is changing internal API so it
> does have to touch all users thereof.

Even in this case, change the individual components in individual
patches and post them as a set.

>> Last drivers/staging/rtl8723au is gone - so your patch is going to
>> fail to apply anyway.
>
> It's there in my tree, for now, so I guess I'll see if it's still there
> when I take this in :)

Changes to staging needs to go in via staging, and rtl8723au is gone
from the staging tree.

Cheers,
Jes


Re: [PATCHv3] wireless: check A-MSDU inner frame source address on AP interfaces

2016-09-28 Thread Jes Sorensen
Michael Braun  writes:
> When using WPA security, the station and thus the required key is
> identified by its mac address when packets are received. So a
> station usually cannot spoof its source mac address.
>
> But when a station sends an A-MSDU frame, port control and crypto
> is done using the outer mac address, while the packets delivered
> and forwarded use the inner mac address.
>
> IEEE 802.11-2012 mandates that the outer source mac address should
> match the inner source address (section 8.3.2.2). For the
> destination mac address, matching is not required (section 10.23.15).
>
> Signed-off-by: Michael Braun 
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/d3.c|  3 ++-
>  .../net/wireless/marvell/mwifiex/11n_rxreorder.c   | 18 +++---
>  drivers/staging/rtl8723au/core/rtw_recv.c  |  2 +-
>  include/net/cfg80211.h |  9 +++
>  net/mac80211/rx.c  | 11 +++--
>  net/wireless/util.c| 28 
> +-
>  6 files changed, 38 insertions(+), 33 deletions(-)

I understand the intentions of this patch are all good, but you need to
not post patches that include both staging and mainline drivers at the
same time. In general make it a patchset and do one patch per driver.

Ideally split up changes to generic code into their own patches too.

Last drivers/staging/rtl8723au is gone - so your patch is going to fail
to apply anyway.

Jes


Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages

2016-09-24 Thread Jes Sorensen
Joe Perches  writes:
> On Sat, 2016-09-24 at 14:06 -0500, Larry Finger wrote:
>> On 09/24/2016 12:32 PM, Joe Perches wrote:
> []
>> o Reindent all the switch/case blocks to a more normal
>>   kernel style (git diff -w would show no changes here)
>> That sounds like busy work to me, but if you want to do it, go ahead.
>
> It's really just to make the comparison case block reductions
> easier to verify for later steps done
>
>> > o cast, spacing and parenthesis reductions
>> >   Lots of odd and somewhat unique styles in various
>> >   drivers, looks like too many individual authors without
>> >   a style guide / code enforcer using slightly different
>> >   personalized code.  Glancing at the code, it looks to be
>> >   similar logic, just written in different styles.
>> Same comment.
>
> Same rationale
>
>> > o Logic changes like
>> >   from:
>> > if (foo) func(..., bar, ...); else func(..., baz, ...);
>> >   to:
>> > func(..., foo ? bar : baz, ...);
>> >   to make the case statement code blocks more consistent
>> >   and emit somewhat smaller object code.
>> I find if .. else constructs much easier to read than the cond ?  :  
>> form. I would reject any such patches.
>
>  I think object code reduction generally a good thing
> but then again, I'm not a maintainer here.

I missed this part, but I am with Larry here - 'foo ? bar : boo' are
just obfuscating the code and far less clear than if or switch
statements.

Jes


Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages

2016-09-24 Thread Jes Sorensen
Larry Finger  writes:
> On 09/24/2016 12:32 PM, Joe Perches wrote:
>> Is there any value in that or is Jes' work going to make
>> doing any or all of this unnecessary and futile?
>
> That is not yet determined. The only driver that is to be replaced at
> this point is rtl8192cu. Jes only has USB I/O for his driver. We are
> looking at adding SDIO, and once that is done, PCI should be possible.

If someone else wants to address PCI then it could happen quite soon,
but at the current schedule I don't see PCI happen in my driver for at
least a year, probably more.

If you can reduce the size of rtlwifi in the mean time that probably
isn't going to upset a lot of people.

Jes


[PATCH 4/4] rtl8xxxu: Stop log spam from each successful interrupt

2016-09-20 Thread Jes . Sorensen
From: Larry Finger <larry.fin...@lwfinger.net>

As soon as debugging is turned on, the logs are filled with messages
reporting the interrupt status. As this quantity is usually zero, this
output is not needed. In fact, there will be a report if the status is
not zero, thus the debug line in question could probably be deleted.
Rather than taking that action, I have changed it to only be printed
when the newly added RTL8XXXU_DEBUG_INTERRUPT bit is set in the debug
mask.

Signed-off-by: Larry Finger <larry.fin...@lwfinger.net>
Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 1 +
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index a10a57c..10166289 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -29,6 +29,7 @@
 #define RTL8XXXU_DEBUG_H2C 0x800
 #define RTL8XXXU_DEBUG_ACTION  0x1000
 #define RTL8XXXU_DEBUG_EFUSE   0x2000
+#define RTL8XXXU_DEBUG_INTERRUPT   0x4000
 
 #define RTW_USB_CONTROL_MSG_TIMEOUT500
 #define RTL8XXXU_MAX_REG_POLL  500
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 71145eb..b2d7f6e 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5375,7 +5375,8 @@ static void rtl8xxxu_int_complete(struct urb *urb)
struct device *dev = >udev->dev;
int ret;
 
-   dev_dbg(dev, "%s: status %i\n", __func__, urb->status);
+   if (rtl8xxxu_debug & RTL8XXXU_DEBUG_INTERRUPT)
+   dev_dbg(dev, "%s: status %i\n", __func__, urb->status);
if (urb->status == 0) {
usb_anchor_urb(urb, >int_anchor);
ret = usb_submit_urb(urb, GFP_ATOMIC);
-- 
2.7.4



[PATCH 1/4] rtl8xxxu: Fix off by one error calculating pubq

2016-09-20 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

This was detected tracing the 8188eu driver, but doesn't seem to make
any difference when using it.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index ca92022..98fcd7b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3869,7 +3869,7 @@ static void rtl8xxxu_init_queue_reserved_page(struct 
rtl8xxxu_priv *priv)
val32 = (nq << RQPN_NPQ_SHIFT) | (eq << RQPN_EPQ_SHIFT);
rtl8xxxu_write32(priv, REG_RQPN_NPQ, val32);
 
-   pubq = fops->total_page_num - hq - lq - nq;
+   pubq = fops->total_page_num - hq - lq - nq - 1;
 
val32 = RQPN_LOAD;
val32 |= (hq << RQPN_HI_PQ_SHIFT);
-- 
2.7.4



[PATCH 2/4] rtl8xxxu: Clean up llt_init() API

2016-09-20 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Remove last_tx_page argument from the llt_init() function. The
rtl8xxxu_fileops structure contains the correct TX_TOTAL_PAGE_NUM
value for the device, and rtl8xxxu_auto_llt_table() doesn't need to
know the value in the first place.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  | 6 +++---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 ++---
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 1f54b89..a10a57c 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1318,7 +1318,7 @@ struct rtl8xxxu_fileops {
int (*power_on) (struct rtl8xxxu_priv *priv);
void (*power_off) (struct rtl8xxxu_priv *priv);
void (*reset_8051) (struct rtl8xxxu_priv *priv);
-   int (*llt_init) (struct rtl8xxxu_priv *priv, u8 last_tx_page);
+   int (*llt_init) (struct rtl8xxxu_priv *priv);
void (*init_phy_bb) (struct rtl8xxxu_priv *priv);
int (*init_phy_rf) (struct rtl8xxxu_priv *priv);
void (*phy_init_antenna_selection) (struct rtl8xxxu_priv *priv);
@@ -1400,14 +1400,14 @@ int rtl8xxxu_load_firmware(struct rtl8xxxu_priv *priv, 
char *fw_name);
 void rtl8xxxu_firmware_self_reset(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_power_off(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_reset_8051(struct rtl8xxxu_priv *priv);
-int rtl8xxxu_auto_llt_table(struct rtl8xxxu_priv *priv, u8 last_tx_page);
+int rtl8xxxu_auto_llt_table(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_gen2_prepare_calibrate(struct rtl8xxxu_priv *priv, u8 start);
 int rtl8xxxu_flush_fifo(struct rtl8xxxu_priv *priv);
 int rtl8xxxu_gen2_h2c_cmd(struct rtl8xxxu_priv *priv,
  struct h2c_cmd *h2c, int len);
 int rtl8xxxu_active_to_lps(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_disabled_to_emu(struct rtl8xxxu_priv *priv);
-int rtl8xxxu_init_llt_table(struct rtl8xxxu_priv *priv, u8 last_tx_page);
+int rtl8xxxu_init_llt_table(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_gen1_phy_iq_calibrate(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_gen1_init_phy_bb(struct rtl8xxxu_priv *priv);
 void rtl8xxxu_gen1_set_tx_power(struct rtl8xxxu_priv *priv,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 98fcd7b..c628b90 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -2472,10 +2472,13 @@ static int rtl8xxxu_llt_write(struct rtl8xxxu_priv 
*priv, u8 address, u8 data)
return ret;
 }
 
-int rtl8xxxu_init_llt_table(struct rtl8xxxu_priv *priv, u8 last_tx_page)
+int rtl8xxxu_init_llt_table(struct rtl8xxxu_priv *priv)
 {
int ret;
int i;
+   u8 last_tx_page;
+
+   last_tx_page = priv->fops->total_page_num;
 
for (i = 0; i < last_tx_page; i++) {
ret = rtl8xxxu_llt_write(priv, i, i + 1);
@@ -2503,7 +2506,7 @@ exit:
return ret;
 }
 
-int rtl8xxxu_auto_llt_table(struct rtl8xxxu_priv *priv, u8 last_tx_page)
+int rtl8xxxu_auto_llt_table(struct rtl8xxxu_priv *priv)
 {
u32 val32;
int ret = 0;
@@ -3988,7 +3991,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 
dev_dbg(dev, "%s: macpower %i\n", __func__, macpower);
if (!macpower) {
-   ret = priv->fops->llt_init(priv, TX_TOTAL_PAGE_NUM);
+   ret = priv->fops->llt_init(priv);
if (ret) {
dev_warn(dev, "%s: LLT table init failed\n", __func__);
goto exit;
-- 
2.7.4



[PATCH 0/4] rtl8xxxu - four patches for 4.9

2016-09-20 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Hi,

This is a small set of patches, all but one are preparatory for the
8188eu support I am working on. The last is Larry's patch to reduce
debug messages.

It would be great to get in for 4.9.

Cheers,
Jes


Jes Sorensen (3):
  rtl8xxxu: Fix off by one error calculating pubq
  rtl8xxxu: Clean up llt_init() API
  rtl8xxxu: Use a struct rtl8xxxu_fileops * in rtl8xxxu_init_device()

Larry Finger (1):
  rtl8xxxu: Stop log spam from each successful interrupt

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h   |  7 ++--
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 49 --
 2 files changed, 31 insertions(+), 25 deletions(-)

-- 
2.7.4



[PATCH 3/4] rtl8xxxu: Use a struct rtl8xxxu_fileops * in rtl8xxxu_init_device()

2016-09-20 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

This saves some 217, or about, derefences of priv->fops.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 37 +++---
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c628b90..71145eb 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3886,6 +3886,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 {
struct rtl8xxxu_priv *priv = hw->priv;
struct device *dev = >udev->dev;
+   struct rtl8xxxu_fileops *fops = priv->fops;
bool macpower;
int ret;
u8 val8;
@@ -3904,7 +3905,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
else
macpower = true;
 
-   ret = priv->fops->power_on(priv);
+   ret = fops->power_on(priv);
if (ret < 0) {
dev_warn(dev, "%s: Failed power on\n", __func__);
goto exit;
@@ -3921,7 +3922,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
/*
 * Set RX page boundary
 */
-   rtl8xxxu_write16(priv, REG_TRXFF_BNDY + 2, priv->fops->trxff_boundary);
+   rtl8xxxu_write16(priv, REG_TRXFF_BNDY + 2, fops->trxff_boundary);
 
ret = rtl8xxxu_download_firmware(priv);
dev_dbg(dev, "%s: download_firmware %i\n", __func__, ret);
@@ -3932,8 +3933,8 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
if (ret)
goto exit;
 
-   if (priv->fops->phy_init_antenna_selection)
-   priv->fops->phy_init_antenna_selection(priv);
+   if (fops->phy_init_antenna_selection)
+   fops->phy_init_antenna_selection(priv);
 
ret = rtl8xxxu_init_mac(priv);
 
@@ -3946,7 +3947,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
if (ret)
goto exit;
 
-   ret = priv->fops->init_phy_rf(priv);
+   ret = fops->init_phy_rf(priv);
if (ret)
goto exit;
 
@@ -3971,7 +3972,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
/*
 * Set TX buffer boundary
 */
-   val8 = priv->fops->total_page_num + 1;
+   val8 = fops->total_page_num + 1;
 
rtl8xxxu_write8(priv, REG_TXPKTBUF_BCNQ_BDNY, val8);
rtl8xxxu_write8(priv, REG_TXPKTBUF_MGQ_BDNY, val8);
@@ -3984,14 +3985,14 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
 * The vendor drivers set PBP for all devices, except 8192e.
 * There is no explanation for this in any of the sources.
 */
-   val8 = (priv->fops->pbp_rx << PBP_PAGE_SIZE_RX_SHIFT) |
-   (priv->fops->pbp_tx << PBP_PAGE_SIZE_TX_SHIFT);
+   val8 = (fops->pbp_rx << PBP_PAGE_SIZE_RX_SHIFT) |
+   (fops->pbp_tx << PBP_PAGE_SIZE_TX_SHIFT);
if (priv->rtl_chip != RTL8192E)
rtl8xxxu_write8(priv, REG_PBP, val8);
 
dev_dbg(dev, "%s: macpower %i\n", __func__, macpower);
if (!macpower) {
-   ret = priv->fops->llt_init(priv);
+   ret = fops->llt_init(priv);
if (ret) {
dev_warn(dev, "%s: LLT table init failed\n", __func__);
goto exit;
@@ -4000,12 +4001,12 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
/*
 * Chip specific quirks
 */
-   priv->fops->usb_quirks(priv);
+   fops->usb_quirks(priv);
 
/*
 * Enable TX report and TX report timer for 8723bu/8188eu/...
 */
-   if (priv->fops->has_tx_report) {
+   if (fops->has_tx_report) {
val8 = rtl8xxxu_read8(priv, REG_TX_REPORT_CTRL);
val8 |= TX_REPORT_CTRL_TIMER_ENABLE;
rtl8xxxu_write8(priv, REG_TX_REPORT_CTRL, val8);
@@ -4140,8 +4141,8 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
rtl8xxxu_write8(priv, REG_RSV_CTRL, val8);
}
 
-   if (priv->fops->init_aggregation)
-   priv->fops->init_aggregation(priv);
+   if (fops->init_aggregation)
+   fops->init_aggregation(priv);
 
/*
 * Enable CCK and OFDM block
@@ -4158,7 +4159,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
/*
 * Start out with default power levels for channel 6, 20MHz
 */
-   priv->fops->set_tx_power(priv, 1, false);
+   

Re: RTL8192EU on rtl8xxxu driver breaks every few minutes

2016-09-20 Thread Jes Sorensen
"Franc[e]sco"  writes:
> Hi, I happen to own a RTL8192EU WiFi dongle which is marked as untested
> by the rtl8xxxu driver.
>
> I'm on a linux from scratch system using kernel 4.7.3 and wpa_supplicant
> 2.5.
>
> The dongle appears to connect and work fine, but after around 10 minutes
> it deauthenticates and enters and endless loop of authentication
> timeout. This continues even if I bring the interface down and back up
> again. The only way to restore the connection is to physically remove
> and re-plug the usb dongle.
>
> I have attached a dmesg log of connecting and entering the auth loop.

Please provide details about the AP you are trying to connect to and iw
link output.

Jes


Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

2016-09-18 Thread Jes Sorensen
Kalle Valo <kv...@codeaurora.org> writes:
> Jes Sorensen <jes.soren...@redhat.com> writes:
>
>> Joe Perches <j...@perches.com> writes:
>>> I think it'd be nicer to use dev_dbg for all these cases
>>> and as well use some new macro that includes the test
>>>
>>> Something like:
>>>
>>> #define rtl8xxxu_dbg(type, fmt, ...)\
>>> do {\
>>> if (rtl8xxxu_debug & (type))\
>>> dev_dbg(dev, fmt, ##__VA_ARGS__);   \
>>> } while (0)
>>
>> Yuck yuck yuck, no thanks!
>>
>> Any attempt of adding that kinda grossness to the driver will get a
>> NACK.
>
> Huh, how is that ugly? To me it's the opposite, original code is ugly
> and Joes' proposal makes sense. Lots of wireless drivers have something
> similar.

Sorry it's a classic case of obfuscating the code for zero gain. If
someone else likes this kinda wrapper in their code, by all means go
ahead. In my book it's just bad coding taste.

Jes


Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

2016-09-18 Thread Jes Sorensen
Larry Finger <larry.fin...@lwfinger.net> writes:
> On 09/17/2016 03:59 PM, Jes Sorensen wrote:
>> Larry Finger <larry.fin...@lwfinger.net> writes:
>>> As soon as debugging is turned on, the logs are filled with messages
>>> reporting the interrupt status. As this quantity is usually zero, this
>>> output is not needed. In fact, there will be a report if the status is
>>> not zero, thus the debug line in question could probably be deleted.
>>> Rather than taking that action, I have changed it to only be printed
>>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>>
>> Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
>> that.
>>
>> Which device do you see this with?
>
> OK. I will change the flag.
>
> I found this with a TP-Link TL-MN8200ND, which has some variant of the
> RTL8188CU chip. It transmits, but I see no evidence that the receiver
> is functioning at all. The same is true for driver rtl8192cu. Only the
> driver from Realtek's web site actually works.

I assume you mean TL-WN8200ND? That device is 'interesting' in the least
positive sense of the word. It seems abandoned by the manufacturer
too. I have one of them but never managed to get it working, not with
any driver under Linux nor Windows.

TP-Link shipped a driver disc with it, but you cannot install that in
any recent version of Windows because the OS ships with it's own driver
for the 8192cu/8188cu series and the device uses the common USB ID. I
have been meaning to see if I could find a box with Vista on it to
install their driver and run a USB trace on it.

> One other problem that I have found is that the debug option on module
> load seems to be ignored. So far, I've had to hard wire the
> flags. Once I find the reason, I'll send a patch for that as well.

That is odd - I use it regularly and haven't had problems with it.

Cheers,
Jes


Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

2016-09-17 Thread Jes Sorensen
Larry Finger  writes:
> As soon as debugging is turned on, the logs are filled with messages
> reporting the interrupt status. As this quantity is usually zero, this
> output is not needed. In fact, there will be a report if the status is
> not zero, thus the debug line in question could probably be deleted.
> Rather than taking that action, I have changed it to only be printed
> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.

Wrong flag, please add a RTL8XXXU_DEBUG_INTERRUPT flag instead and use
that.

Which device do you see this with?

Thanks,
Jes


Re: [PATCH] rtl8xxxu: Stop log spam from each successful interrupt

2016-09-17 Thread Jes Sorensen
Joe Perches  writes:
> On Sat, 2016-09-17 at 12:09 -0500, Larry Finger wrote:
>> As soon as debugging is turned on, the logs are filled with messages
>> reporting the interrupt status. As this quantity is usually zero, this
>> output is not needed. In fact, there will be a report if the status is
>> not zero, thus the debug line in question could probably be deleted.
>> Rather than taking that action, I have changed it to only be printed
>> when the RTL8XXXU_DEBUG_USB bit is set in the debug mask.
>
> There are many uses of
>   if (rtl8xxxu_debug & ) {
>   dev_info(dev, ...)
>
> Emitting debugging information at KERN_INFO is odd.

Not at all, it's a pain to enable it in debug fs post loading the
driver, especially if you need the output immediately during driver
init. That is why the flags are there.

> I think it'd be nicer to use dev_dbg for all these cases
> and as well use some new macro that includes the test
>
> Something like:
>
> #define rtl8xxxu_dbg(type, fmt, ...)  \
> do {  \
>   if (rtl8xxxu_debug & (type))\
>   dev_dbg(dev, fmt, ##__VA_ARGS__);   \
> } while (0)

Yuck yuck yuck, no thanks!

Any attempt of adding that kinda grossness to the driver will get a
NACK.

There is a reason the debug flag is there - it allows you to enable
specific items on driver load. If you enable that flag, expect to see
stuff in your log.

Jes


[PATCH 0/1] Fix problem with rtl8192eu firmware reload

2016-09-13 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Hi,

This one goes on top of my previous patches, albeit it probably
applies out of order.

It resolves the issue where firmware wouldn't load correctly if
reloading the driver module.

Cheers,
Jes


Jes Sorensen (1):
  rtl8xxxu: Implment 8192e specific power down sequence

 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 144 -
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h  |   1 +
 2 files changed, 144 insertions(+), 1 deletion(-)

-- 
2.7.4



[PATCH 1/1] rtl8xxxu: Implement 8192e specific power down sequence

2016-09-13 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

This powers down the 8192e correctly, or at least to the point where
the firmware will load again, when reloading the driver module.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 144 -
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h  |   1 +
 2 files changed, 144 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 841522e..df54d27 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1396,6 +1396,114 @@ exit:
return ret;
 }
 
+static int rtl8192eu_active_to_lps(struct rtl8xxxu_priv *priv)
+{
+   struct device *dev = >udev->dev;
+   u8 val8;
+   u16 val16;
+   u32 val32;
+   int retry, retval;
+
+   rtl8xxxu_write8(priv, REG_TXPAUSE, 0xff);
+
+   retry = 100;
+   retval = -EBUSY;
+   /*
+* Poll 32 bit wide 0x05f8 for 0x to ensure no TX is pending.
+*/
+   do {
+   val32 = rtl8xxxu_read32(priv, REG_SCH_TX_CMD);
+   if (!val32) {
+   retval = 0;
+   break;
+   }
+   } while (retry--);
+
+   if (!retry) {
+   dev_warn(dev, "Failed to flush TX queue\n");
+   retval = -EBUSY;
+   goto out;
+   }
+
+   /* Disable CCK and OFDM, clock gated */
+   val8 = rtl8xxxu_read8(priv, REG_SYS_FUNC);
+   val8 &= ~SYS_FUNC_BBRSTB;
+   rtl8xxxu_write8(priv, REG_SYS_FUNC, val8);
+
+   udelay(2);
+
+   /* Reset whole BB */
+   val8 = rtl8xxxu_read8(priv, REG_SYS_FUNC);
+   val8 &= ~SYS_FUNC_BB_GLB_RSTN;
+   rtl8xxxu_write8(priv, REG_SYS_FUNC, val8);
+
+   /* Reset MAC TRX */
+   val16 = rtl8xxxu_read16(priv, REG_CR);
+   val16 &= 0xff00;
+   val16 |= (CR_HCI_TXDMA_ENABLE | CR_HCI_RXDMA_ENABLE);
+   rtl8xxxu_write16(priv, REG_CR, val16);
+
+   val16 = rtl8xxxu_read16(priv, REG_CR);
+   val16 &= ~CR_SECURITY_ENABLE;
+   rtl8xxxu_write16(priv, REG_CR, val16);
+
+   val8 = rtl8xxxu_read8(priv, REG_DUAL_TSF_RST);
+   val8 |= DUAL_TSF_TX_OK;
+   rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, val8);
+
+out:
+   return retval;
+}
+
+static int rtl8192eu_active_to_emu(struct rtl8xxxu_priv *priv)
+{
+   u8 val8;
+   int count, ret = 0;
+
+   /* Turn off RF */
+   rtl8xxxu_write8(priv, REG_RF_CTRL, 0);
+
+   /* Switch DPDT_SEL_P output from register 0x65[2] */
+   val8 = rtl8xxxu_read8(priv, REG_LEDCFG2);
+   val8 &= ~LEDCFG2_DPDT_SELECT;
+   rtl8xxxu_write8(priv, REG_LEDCFG2, val8);
+
+   /* 0x0005[1] = 1 turn off MAC by HW state machine*/
+   val8 = rtl8xxxu_read8(priv, REG_APS_FSMCO + 1);
+   val8 |= BIT(1);
+   rtl8xxxu_write8(priv, REG_APS_FSMCO + 1, val8);
+
+   for (count = RTL8XXXU_MAX_REG_POLL; count; count--) {
+   val8 = rtl8xxxu_read8(priv, REG_APS_FSMCO + 1);
+   if ((val8 & BIT(1)) == 0)
+   break;
+   udelay(10);
+   }
+
+   if (!count) {
+   dev_warn(>udev->dev, "%s: Disabling MAC timed out\n",
+__func__);
+   ret = -EBUSY;
+   goto exit;
+   }
+
+exit:
+   return ret;
+}
+
+static int rtl8192eu_emu_to_disabled(struct rtl8xxxu_priv *priv)
+{
+   u8 val8;
+
+   /* 0x04[12:11] = 01 enable WL suspend */
+   val8 = rtl8xxxu_read8(priv, REG_APS_FSMCO + 1);
+   val8 &= ~(BIT(3) | BIT(4));
+   val8 |= BIT(3);
+   rtl8xxxu_write8(priv, REG_APS_FSMCO + 1, val8);
+
+   return 0;
+}
+
 static int rtl8192eu_power_on(struct rtl8xxxu_priv *priv)
 {
u16 val16;
@@ -1446,6 +1554,40 @@ exit:
return ret;
 }
 
+void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
+{
+   u8 val8;
+   u16 val16;
+
+   rtl8xxxu_flush_fifo(priv);
+
+   val8 = rtl8xxxu_read8(priv, REG_TX_REPORT_CTRL);
+   val8 &= ~TX_REPORT_CTRL_TIMER_ENABLE;
+   rtl8xxxu_write8(priv, REG_TX_REPORT_CTRL, val8);
+
+   /* Turn off RF */
+   rtl8xxxu_write8(priv, REG_RF_CTRL, 0x00);
+
+   rtl8192eu_active_to_lps(priv);
+
+   /* Reset Firmware if running in RAM */
+   if (rtl8xxxu_read8(priv, REG_MCU_FW_DL) & MCU_FW_RAM_SEL)
+   rtl8xxxu_firmware_self_reset(priv);
+
+   /* Reset MCU */
+   val16 = rtl8xxxu_read16(priv, REG_SYS_FUNC);
+   val16 &= ~SYS_FUNC_CPU_ENABLE;
+   rtl8xxxu_write16(priv, REG_SYS_FUNC, val16);
+
+   /* Reset MCU ready status */
+   rtl8xxxu_write8(priv, REG_MCU_FW_DL, 0x00);
+
+   rtl8xxxu_reset_8051(priv);
+
+   rtl8192eu_active_to_emu(priv);
+   rtl8192eu_emu_to_disabled(pr

Re: [PATCH] staging: squash lines for simple wrapper functions

2016-09-12 Thread Jes Sorensen
Masahiro Yamada  writes:
> Remove unneeded variables and assignments.
>
> While we are here, clean up the following as well:
>   - refactor rtl8723a_get_bcn_valid() a bit
>   - remove unneeded casts in sii164Get{Vendor,Device}ID()
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  drivers/staging/android/ion/ion.c|  6 +-
>  .../staging/lustre/lustre/obdclass/linux/linux-module.c  |  5 +
>  drivers/staging/lustre/lustre/ptlrpc/client.c|  5 +
>  drivers/staging/lustre/lustre/ptlrpc/events.c|  5 +
>  drivers/staging/rtl8723au/core/rtw_wlan_util.c   |  7 +--
>  drivers/staging/rtl8723au/hal/hal_com.c  |  6 +-
>  drivers/staging/sm750fb/ddk750_sii164.c  | 16 
> 
>  7 files changed, 10 insertions(+), 40 deletions(-)

1) Do not submit one giant patch modifying multiple drivers in one go
2) drivers/staging/rtl8723au is gone - had you followed 1), you wouldn't
   necessarily have had to respin this patch.
3) Consider if your changes, even if technically correct, actually
   improve the code (see below)

Jes

> diff --git a/drivers/staging/rtl8723au/core/rtw_wlan_util.c 
> b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
> index 694cf17..7ab47f0 100644
> --- a/drivers/staging/rtl8723au/core/rtw_wlan_util.c
> +++ b/drivers/staging/rtl8723au/core/rtw_wlan_util.c
> @@ -1202,12 +1202,7 @@ unsigned int update_supported_rate23a(unsigned char 
> *ptn, unsigned int ptn_sz)
>  
>  unsigned int update_MSC_rate23a(struct ieee80211_ht_cap *pHT_caps)
>  {
> - unsigned int mask;
> -
> - mask = pHT_caps->mcs.rx_mask[0] << 12 |
> - pHT_caps->mcs.rx_mask[1] << 20;
> -
> - return mask;
> + return pHT_caps->mcs.rx_mask[0] << 12 | pHT_caps->mcs.rx_mask[1] << 20;
>  }

The use of the mask variable didn't cause any harm, and it was certainly
a lot nicer to read the way it was.

>  int support_short_GI23a(struct rtw_adapter *padapter,
> diff --git a/drivers/staging/rtl8723au/hal/hal_com.c 
> b/drivers/staging/rtl8723au/hal/hal_com.c
> index 9d7b11b..dfbeebe 100644
> --- a/drivers/staging/rtl8723au/hal/hal_com.c
> +++ b/drivers/staging/rtl8723au/hal/hal_com.c
> @@ -708,11 +708,7 @@ void rtl8723a_bcn_valid(struct rtw_adapter *padapter)
>  
>  bool rtl8723a_get_bcn_valid(struct rtw_adapter *padapter)
>  {
> - bool retval;
> -
> - retval = (rtl8723au_read8(padapter, REG_TDECTRL + 2) & BIT(0)) ? true : 
> false;
> -
> - return retval;
> + return !!(rtl8723au_read8(padapter, REG_TDECTRL + 2) & BIT(0));
>  }

One word: Yuck!

Talk about obfuscating the code for the sake of obfuscation!

>  void rtl8723a_set_beacon_interval(struct rtw_adapter *padapter, u16 interval)
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
> b/drivers/staging/sm750fb/ddk750_sii164.c
> index 67f36e7..28818e1 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -36,12 +36,8 @@ static char *gDviCtrlChipName = "Silicon Image SiI 164";
>   */
>  unsigned short sii164GetVendorID(void)
>  {
> - unsigned short vendorID;
> -
> - vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_VENDOR_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_VENDOR_ID_LOW);
> -
> - return vendorID;
> + return (i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_HIGH) << 8) |
> + i2cReadReg(SII164_I2C_ADDRESS, SII164_VENDOR_ID_LOW);
>  }
>  
>  /*
> @@ -53,12 +49,8 @@ unsigned short sii164GetVendorID(void)
>   */
>  unsigned short sii164GetDeviceID(void)
>  {
> - unsigned short deviceID;
> -
> - deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_DEVICE_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_DEVICE_ID_LOW);
> -
> - return deviceID;
> + return (i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_HIGH) << 8) |
> + i2cReadReg(SII164_I2C_ADDRESS, SII164_DEVICE_ID_LOW);
>  }

Getting rid of the casts would be nice, merging them into a giant
multi-line return line certainly isn't an improvement in my book. That
said, I will leave that to the maintainer of that driver to decide what
is preferred.

Jes


Re: Support of rtl8723bs

2016-09-12 Thread Jes Sorensen
Bastien Nocera  writes:
> On Mon, 2016-09-12 at 17:48 +0200, Hanno Zulla wrote:
>> Hi,
>> 
>> > Longer term I think it makes sense to add SDIO support to rtl8xxxu.
>> > The
>> > differences between the USB version and the SDIO version are rather
>> > small.
>> 
>> 
>> This is beyond my expertise, sadly.
>> 
>> Is there a good blueprint / example of a previous case where a USB
>> driver had SDIO support added that one might learn from?
>
> I don't know on top of my head, sorry. Best look in the kernel sources
> directly.

The issues I am aware of is to start out changing the register access
macros into function pointers and stick them all into the fileops
structure. Provide a set of SDIO ones to match the USB ones. Then you
will need some code to detect the device, as that part will obviously be
different from the USB device detection.

I know there are a few issues to look out for in the hw register init
files to look out for too, that does some special casing for SDIO, but
that should be limited. I am happy to point someone in the right
direction on that when they get to that.

Cheers,
Jes


Re: Support of rtl8723bs

2016-09-12 Thread Jes Sorensen
Bastien Nocera  writes:
> On Mon, 2016-09-12 at 11:20 +0200, Hanno Zulla wrote:
>> Hi Jes,
>> hello linux-wireless,
>> it as a guinea pig to test on my hardware. The newer vendor driver
>> crashed on Ubuntu' 4.4 sources and doesn't compile yet on Ubuntu 4.8-
>> rc
>> sources.
>> https://github.com/anthonywong/rtl8723bs/issues/1
>
> I really don't care about the vendor driver. There's no way to track
> changes other than through code dumps.
>
> Longer term, it seems likely that improving Jes' driver is the way to
> go. At least there's 170k less lines of code to read in the rtl8723bs
> driver to get things going.

Longer term I think it makes sense to add SDIO support to rtl8xxxu. The
differences between the USB version and the SDIO version are rather
small.

That said, I have quite a lot of things on my TODO list so it is going
to take some time until I can look at this. It may make sense to pull
a cleaned up version of the vendor driver into staging until then if
there is a desperate need for it, but otherwise any help would be
appreciated :)

Jes


[PATCH - FYI - do not apply] staging: Remove rtl8723au driver

2016-09-10 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Hi,

I sent Greg the full version of this patch, removing the old rtl8723au
driver. I didn't want to spam the list with a 2M+ patch so this is the
summary version. 

Cheers,
Jes

This driver is superseded by rtl8xxxu and has been marked as scheduled
for deletion since 4.6

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 MAINTAINERS| 7 -
 drivers/staging/Kconfig| 2 -
 drivers/staging/Makefile   | 1 -
 drivers/staging/rtl8723au/Kconfig  |33 -
 drivers/staging/rtl8723au/Makefile |53 -
 drivers/staging/rtl8723au/TODO |16 -
 drivers/staging/rtl8723au/core/rtw_ap.c|  1738 ---
 drivers/staging/rtl8723au/core/rtw_cmd.c   |  1470 ---
 drivers/staging/rtl8723au/core/rtw_efuse.c |   538 -
 drivers/staging/rtl8723au/core/rtw_ieee80211.c |   855 --
 drivers/staging/rtl8723au/core/rtw_mlme.c  |  2314 
 drivers/staging/rtl8723au/core/rtw_mlme_ext.c  |  6187 --
 drivers/staging/rtl8723au/core/rtw_pwrctrl.c   |   607 -
 drivers/staging/rtl8723au/core/rtw_recv.c  |  2204 
 drivers/staging/rtl8723au/core/rtw_security.c  |  1630 ---
 drivers/staging/rtl8723au/core/rtw_sreset.c|   214 -
 drivers/staging/rtl8723au/core/rtw_sta_mgt.c   |   439 -
 drivers/staging/rtl8723au/core/rtw_wlan_util.c |  1537 ---
 drivers/staging/rtl8723au/core/rtw_xmit.c  |  2337 
 drivers/staging/rtl8723au/hal/Hal8723PwrSeq.c  |80 -
 drivers/staging/rtl8723au/hal/Hal8723UHWImg_CE.c   |   136 -
 .../staging/rtl8723au/hal/HalDMOutSrc8723A_CE.c|  1097 --
 drivers/staging/rtl8723au/hal/HalHWImg8723A_BB.c   |   565 -
 drivers/staging/rtl8723au/hal/HalHWImg8723A_MAC.c  |   187 -
 drivers/staging/rtl8723au/hal/HalHWImg8723A_RF.c   |   259 -
 drivers/staging/rtl8723au/hal/HalPwrSeqCmd.c   |   156 -
 drivers/staging/rtl8723au/hal/hal_com.c|   853 --
 drivers/staging/rtl8723au/hal/hal_intf.c   |42 -
 drivers/staging/rtl8723au/hal/odm.c|  1732 ---
 drivers/staging/rtl8723au/hal/odm_HWConfig.c   |   396 -
 drivers/staging/rtl8723au/hal/odm_RegConfig8723A.c |88 -
 drivers/staging/rtl8723au/hal/odm_debug.c  |39 -
 drivers/staging/rtl8723au/hal/odm_interface.c  |49 -
 .../staging/rtl8723au/hal/rtl8723a_bt-coexist.c| 11265 ---
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c   |   755 --
 drivers/staging/rtl8723au/hal/rtl8723a_dm.c|   194 -
 drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c  |  2076 
 drivers/staging/rtl8723au/hal/rtl8723a_phycfg.c|   961 --
 drivers/staging/rtl8723au/hal/rtl8723a_rf6052.c|   503 -
 drivers/staging/rtl8723au/hal/rtl8723a_rxdesc.c|69 -
 drivers/staging/rtl8723au/hal/rtl8723a_sreset.c|55 -
 drivers/staging/rtl8723au/hal/rtl8723au_recv.c |   267 -
 drivers/staging/rtl8723au/hal/rtl8723au_xmit.c |   520 -
 drivers/staging/rtl8723au/hal/usb_halinit.c|  1269 ---
 drivers/staging/rtl8723au/hal/usb_ops_linux.c  |   690 --
 drivers/staging/rtl8723au/include/Hal8723APhyCfg.h |   162 -
 drivers/staging/rtl8723au/include/Hal8723APhyReg.h |  1078 --
 drivers/staging/rtl8723au/include/Hal8723PwrSeq.h  |   126 -
 .../staging/rtl8723au/include/Hal8723UHWImg_CE.h   |29 -
 .../staging/rtl8723au/include/HalDMOutSrc8723A.h   |64 -
 .../staging/rtl8723au/include/HalHWImg8723A_BB.h   |38 -
 .../staging/rtl8723au/include/HalHWImg8723A_FW.h   |28 -
 .../staging/rtl8723au/include/HalHWImg8723A_MAC.h  |26 -
 .../staging/rtl8723au/include/HalHWImg8723A_RF.h   |25 -
 drivers/staging/rtl8723au/include/HalPwrSeqCmd.h   |   130 -
 drivers/staging/rtl8723au/include/HalVerDef.h  |   114 -
 drivers/staging/rtl8723au/include/drv_types.h  |   274 -
 drivers/staging/rtl8723au/include/hal_com.h|   182 -
 drivers/staging/rtl8723au/include/hal_intf.h   |   115 -
 drivers/staging/rtl8723au/include/ieee80211.h  |   341 -
 drivers/staging/rtl8723au/include/ioctl_cfg80211.h |66 -
 drivers/staging/rtl8723au/include/mlme_osdep.h |24 -
 drivers/staging/rtl8723au/include/odm.h|   860 --
 drivers/staging/rtl8723au/include/odm_HWConfig.h   |   153 -
 .../staging/rtl8723au/include/odm_RegConfig8723A.h |27 -
 .../staging/rtl8723au/include/odm_RegDefine11N.h   |   165 -
 drivers/staging/rtl8723au/include/odm_debug.h  |   117 -
 drivers/staging/rtl8723au/include/odm_interface.h  |62 -
 drivers/staging/rtl8723au/include/odm_precomp.h|49 -
 drivers/staging/rtl8723au/include/odm_reg.h|   111 -
 drivers/staging/rtl8723au/include/osdep_intf.h |45 -
 drivers/staging/rtl8723au/include/osdep_service.h  |87 -
 drivers/staging/rtl8723au/include/recv_osdep.h |36 -
 .../rtl8723au/include

[PATCH 0/2] Fix spelling and reset device on module unload

2016-09-09 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

Hi,

Two fixes for the rtl8xxxu driver.

I am working on a much larger set which adds 8188eu support, and while
it's close to there, I am still chasing one issue where it doesn't
come back if reloading the driver module.

In the interim, these two should be good to apply.

Cheers,
Jes


Colin Ian King (1):
  rtl8xxxu: fix spelling mistake "firmare" -> "firmware"

Jes Sorensen (1):
  rtl8xxxu: Reset device on module unload if still attached

 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.7.4



[PATCH 2/2] rtl8xxxu: fix spelling mistake "firmare" -> "firmware"

2016-09-09 Thread Jes . Sorensen
From: Colin Ian King <colin.k...@canonical.com>

Trivial fix to spelling mistakes in dev_dbg message.

Signed-off-by: Colin Ian King <colin.k...@canonical.com>
Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index d2611a4..ca92022 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3921,11 +3921,11 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
rtl8xxxu_write16(priv, REG_TRXFF_BNDY + 2, priv->fops->trxff_boundary);
 
ret = rtl8xxxu_download_firmware(priv);
-   dev_dbg(dev, "%s: download_fiwmare %i\n", __func__, ret);
+   dev_dbg(dev, "%s: download_firmware %i\n", __func__, ret);
if (ret)
goto exit;
ret = rtl8xxxu_start_firmware(priv);
-   dev_dbg(dev, "%s: start_fiwmare %i\n", __func__, ret);
+   dev_dbg(dev, "%s: start_firmware %i\n", __func__, ret);
if (ret)
goto exit;
 
-- 
2.7.4



[PATCH 1/2] rtl8xxxu: Reset device on module unload if still attached

2016-09-09 Thread Jes . Sorensen
From: Jes Sorensen <jes.soren...@redhat.com>

If the USB dongle is still attached, reset it on module unload to
avoid scans failing when reloading the driver.

Signed-off-by: Jes Sorensen <jes.soren...@redhat.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c362083..d2611a4 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -6129,6 +6129,11 @@ static void rtl8xxxu_disconnect(struct usb_interface 
*interface)
mutex_destroy(>usb_buf_mutex);
mutex_destroy(>h2c_mutex);
 
+   if (priv->udev->state != USB_STATE_NOTATTACHED) {
+   dev_info(>udev->dev,
+"Device still attached, trying to reset\n");
+   usb_reset_device(priv->udev);
+   }
usb_put_dev(priv->udev);
ieee80211_free_hw(hw);
 }
-- 
2.7.4



Re: rtl8xxxu: fix spelling mistake "firmare" -> "firmware"

2016-09-08 Thread Jes Sorensen
Kalle Valo  writes:
> Colin Ian King  wrote:
>> From: Colin Ian King 
>> 
>> Trivial fix to spelling mistakes in dev_dbg message.
>> 
>> Signed-off-by: Colin Ian King 
>> Reviewed-by: Julian Calaby 
>
> I assume Jes will take this.

It's on my list, sorry been swamped in LPC stuff and other work the last
couple of days.

Cheers,
Jes


Re: [PATCH 0/5] Some cleanup patches for drivers/staging/rtl8723au/core/rtw_mlme.c

2016-09-06 Thread Jes Sorensen
Joe Perches <j...@perches.com> writes:
> On Tue, 2016-09-06 at 12:00 -0400, Jes Sorensen wrote:
>
>> Nothing wrong with these patches, however I intend to post a patch to
>> remove this driver soon, so it's kind of a waste of your time to spend
>> too many cycles on it.
>
> It might be useful to mark any drivers you're
> planning on removing as "Obsolete" in MAINTAINERS.
>
> checkpatch will then emit a "don't waste your time"
> message if anyone scans an obsolete driver/file.

I see, wasn't aware of that. I did add a printk to it for 4.6 notifying
users that the driver was going to go away.

Jes


Re: [PATCH 0/5] Some cleanup patches for drivers/staging/rtl8723au/core/rtw_mlme.c

2016-09-06 Thread Jes Sorensen
Matthias Beyer  writes:
> This patchset fixes some errors and warnings reported by checkpatch.pl.
>
> Matthias Beyer (5):
>   drivers: staging: rtl8723au: core: Fix checkpatch.pl errors
>   drivers: staging: rtl8723au: core: simplify if-break-else
>   drivers: staging: rtl8723au: core: Refactor pointless branching
>   drivers: staging: rtl8723au: core: Fix "space prohibited" warning
>   drivers: staging: rtl8723au: core: Fix indentation
>
>  drivers/staging/rtl8723au/core/rtw_mlme.c | 72 
> ++-
>  1 file changed, 33 insertions(+), 39 deletions(-)

Nothing wrong with these patches, however I intend to post a patch to
remove this driver soon, so it's kind of a waste of your time to spend
too many cycles on it.

Best regards,
Jes


Re: fix:rtl8xxxu_core: mark symbols static where possible

2016-09-03 Thread Jes Sorensen
Kalle Valo  writes:
> Baoyou Xie  wrote:
>> We get 1 warning about global functions without a declaration
>> in the rtl8xxxu rtl8xxxu_core.c when building with W=1:
>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:898:1:
>> warning: no previous prototype for 'rtl8xxxu_gen1_h2c_cmd'
>> [-Wmissing-prototypes]
>> 
>> In fact, this function is only used in the file in which it is declared
>> and don't need a declaration, but can be made static.
>> so this patch marks it 'static'.
>> 
>> Signed-off-by: Baoyou Xie 
>
> The title should be "rtl8xxxu: ". See:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject_name
>
> Also I assume Jes will take this.

Yes to both accounts!

Thanks,
Jes


  1   2   3   4   5   6   7   >