diff mbox

[1/1] New driver: rtl8xxxu (mac80211)

Message ID 55E4EE6F.2010008@lwfinger.net (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show

Commit Message

Larry Finger Sept. 1, 2015, 12:16 a.m. UTC
On 08/31/2015 06:43 PM, Jes Sorensen wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>> On 08/30/2015 09:39 PM, Jes Sorensen wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>> On 08/30/2015 01:41 PM, Jes Sorensen wrote:
>>>>>>> +{USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x817e, 0xff,
>>>>>> 0xff, 0xff),
>>>>>>> +	.driver_info = (unsigned long)&rtl8192cu_fops},
>>>>>>
>>>>>> I have tested a device with USB ID 0x0bda (USB_VENDOR_ID_REALTEK):0x817e.
>>>>>
>>>>> Were you happy with the results, or did it cause problems? Ie. did you
>>>>> try on x86 or on PPC32?
>>>>
>>>> As covered in the other mail, it works very well at G rates.
>>>
>>> I see - I am a little confused, it works well on G rates but not on N
>>> rates, or was this due to the AP used?
>>
>> The AP is AC1200. It supports full N rates on other devices.
>
> You only manage to make it work on G rates against this AP? Urgh, I
> don't like that at all.
>
> I thought you reported 30+Mbps performances though?, thats more than you
> get on G.

OK, I get a bit better than the top G rate of 27 Mbps, but the connection 
reports a 54 Mbps raw rate. My Intel 7260 gets RX of 95 and TX of 53 Mbps from 
the same location relative to the AP. For the Intel, the listed raw rate is 300 
Mbps.

>>>>> I don't think any of this is showstopper material for inclusion right
>>>>> now, albeit I do want to address them.
>>>>
>>>> The scheduling while atomic problems do need to be fixed, and I am
>>>> still working on the failure to get a wifi device on PPC.
>>>
>>> It's already fixed :)
>>
>> A full debug=0x3fff showed me one problem on the PPC. I will be
>> testing a patch later today.
>
> Interesting, let me know what you find.

Thanks for the fixes. I'm still getting the sleeping while atomic BUG splat from 
rtl8723a_h2c_cmd+0x48. Today I know that is not a lock dependency issue. I have 
one in VirtualBox that fires and disables further checking, but that driver is 
not loading now.

I have made some progress on the big-endian issues. When the firmware file is 
read from disk, the first byte read must be the first byte written to the 
device. In the slow write N routine, four bytes are picked and written using the 
write32 routine, which does a cpu_to_32() byte swap. As a result, the order of 
bytes to the device is 03, 02, 01, 00! Rather than byte swap twice, I am calling 
the regular writeN routine with each hunk. After a bit of thinking about the 
best blocksize to use, I settled on 64. That reduces the number of full-size 
hunks that are written without unduly increasing the worst-case number of writes 
needed for the remainder.

On PPC, I now get a device registered; however, the scan data are garbled. That 
shouldn't be too difficult to debug, but just in case, I have attached the patch 
for rtl8xxxu_slow_writeN().

Larry

Comments

Jes Sorensen Sept. 1, 2015, 4:54 a.m. UTC | #1
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 08/31/2015 06:43 PM, Jes Sorensen wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>> On 08/30/2015 09:39 PM, Jes Sorensen wrote:
>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>> On 08/30/2015 01:41 PM, Jes Sorensen wrote:
>>>>>>>> +{USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x817e, 0xff,
>>>>>>> 0xff, 0xff),
>>>>>>>> +	.driver_info = (unsigned long)&rtl8192cu_fops},
>>>>>>>
>>>>>>> I have tested a device with USB ID 0x0bda (USB_VENDOR_ID_REALTEK):0x817e.
>>>>>>
>>>>>> Were you happy with the results, or did it cause problems? Ie. did you
>>>>>> try on x86 or on PPC32?
>>>>>
>>>>> As covered in the other mail, it works very well at G rates.
>>>>
>>>> I see - I am a little confused, it works well on G rates but not on N
>>>> rates, or was this due to the AP used?
>>>
>>> The AP is AC1200. It supports full N rates on other devices.
>>
>> You only manage to make it work on G rates against this AP? Urgh, I
>> don't like that at all.
>>
>> I thought you reported 30+Mbps performances though?, thats more than you
>> get on G.
>
> OK, I get a bit better than the top G rate of 27 Mbps, but the
> connection reports a 54 Mbps raw rate. My Intel 7260 gets RX of 95 and
> TX of 53 Mbps from the same location relative to the AP. For the
> Intel, the listed raw rate is 300 Mbps.

It's possible the connection reporting I am doing is wrong. I haven't
figured out how to report the actual transmission speed, which is what
the mac80211 stack wants. Some of the other drivers fake it or repor RX
speeds, but that isn't what one is meant to report.

>>> A full debug=0x3fff showed me one problem on the PPC. I will be
>>> testing a patch later today.
>>
>> Interesting, let me know what you find.
>
> Thanks for the fixes. I'm still getting the sleeping while atomic BUG
> splat from rtl8723a_h2c_cmd+0x48. Today I know that is not a lock
> dependency issue. I have one in VirtualBox that fires and disables
> further checking, but that driver is not loading now.

This is bothersome, I'll have to dig futher into this.

> I have made some progress on the big-endian issues. When the firmware
> file is read from disk, the first byte read must be the first byte
> written to the device. In the slow write N routine, four bytes are
> picked and written using the write32 routine, which does a cpu_to_32()
> byte swap. As a result, the order of bytes to the device is 03, 02,
> 01, 00! Rather than byte swap twice, I am calling the regular writeN
> routine with each hunk. After a bit of thinking about the best
> blocksize to use, I settled on 64. That reduces the number of
> full-size hunks that are written without unduly increasing the
> worst-case number of writes needed for the remainder.
>
> On PPC, I now get a device registered; however, the scan data are
> garbled. That shouldn't be too difficult to debug, but just in case, I
> have attached the patch for rtl8xxxu_slow_writeN().

As I mentioned in a direct email - calling writeN from slow_writeN is
not a solution. I found some devices fail if you try to use the fast
writeN routine. Maybe rtl8xxxu_raw_write32() - without the byteswap
would be a better approach, or simply putting the relevant code into the
slow_writeN function.

I am flying back to New York tomorrow, and I'll have some time on the
plane, so I'll try to have a look at that.

Cheers,
Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Larry Finger Sept. 1, 2015, 5:17 a.m. UTC | #2
On 08/31/2015 11:54 PM, Jes Sorensen wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>> On 08/31/2015 06:43 PM, Jes Sorensen wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>> On 08/30/2015 09:39 PM, Jes Sorensen wrote:
>>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>>> On 08/30/2015 01:41 PM, Jes Sorensen wrote:
>>>>>>>>> +{USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x817e, 0xff,
>>>>>>>> 0xff, 0xff),
>>>>>>>>> +	.driver_info = (unsigned long)&rtl8192cu_fops},
>>>>>>>>
>>>>>>>> I have tested a device with USB ID 0x0bda (USB_VENDOR_ID_REALTEK):0x817e.
>>>>>>>
>>>>>>> Were you happy with the results, or did it cause problems? Ie. did you
>>>>>>> try on x86 or on PPC32?
>>>>>>
>>>>>> As covered in the other mail, it works very well at G rates.
>>>>>
>>>>> I see - I am a little confused, it works well on G rates but not on N
>>>>> rates, or was this due to the AP used?
>>>>
>>>> The AP is AC1200. It supports full N rates on other devices.
>>>
>>> You only manage to make it work on G rates against this AP? Urgh, I
>>> don't like that at all.
>>>
>>> I thought you reported 30+Mbps performances though?, thats more than you
>>> get on G.
>>
>> OK, I get a bit better than the top G rate of 27 Mbps, but the
>> connection reports a 54 Mbps raw rate. My Intel 7260 gets RX of 95 and
>> TX of 53 Mbps from the same location relative to the AP. For the
>> Intel, the listed raw rate is 300 Mbps.
>
> It's possible the connection reporting I am doing is wrong. I haven't
> figured out how to report the actual transmission speed, which is what
> the mac80211 stack wants. Some of the other drivers fake it or repor RX
> speeds, but that isn't what one is meant to report.
>
>>>> A full debug=0x3fff showed me one problem on the PPC. I will be
>>>> testing a patch later today.
>>>
>>> Interesting, let me know what you find.
>>
>> Thanks for the fixes. I'm still getting the sleeping while atomic BUG
>> splat from rtl8723a_h2c_cmd+0x48. Today I know that is not a lock
>> dependency issue. I have one in VirtualBox that fires and disables
>> further checking, but that driver is not loading now.
>
> This is bothersome, I'll have to dig futher into this.
>
>> I have made some progress on the big-endian issues. When the firmware
>> file is read from disk, the first byte read must be the first byte
>> written to the device. In the slow write N routine, four bytes are
>> picked and written using the write32 routine, which does a cpu_to_32()
>> byte swap. As a result, the order of bytes to the device is 03, 02,
>> 01, 00! Rather than byte swap twice, I am calling the regular writeN
>> routine with each hunk. After a bit of thinking about the best
>> blocksize to use, I settled on 64. That reduces the number of
>> full-size hunks that are written without unduly increasing the
>> worst-case number of writes needed for the remainder.
>>
>> On PPC, I now get a device registered; however, the scan data are
>> garbled. That shouldn't be too difficult to debug, but just in case, I
>> have attached the patch for rtl8xxxu_slow_writeN().
>
> As I mentioned in a direct email - calling writeN from slow_writeN is
> not a solution. I found some devices fail if you try to use the fast
> writeN routine. Maybe rtl8xxxu_raw_write32() - without the byteswap
> would be a better approach, or simply putting the relevant code into the
> slow_writeN function.
>
> I am flying back to New York tomorrow, and I'll have some time on the
> plane, so I'll try to have a look at that.

I also found that trying to write all N bytes at once failed, but writing 64 at 
a time is OK. I will add the USB calls to slow_writeN.

Larry


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jes Sorensen Sept. 1, 2015, 5:26 a.m. UTC | #3
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 08/31/2015 11:54 PM, Jes Sorensen wrote:
>> As I mentioned in a direct email - calling writeN from slow_writeN is
>> not a solution. I found some devices fail if you try to use the fast
>> writeN routine. Maybe rtl8xxxu_raw_write32() - without the byteswap
>> would be a better approach, or simply putting the relevant code into the
>> slow_writeN function.
>>
>> I am flying back to New York tomorrow, and I'll have some time on the
>> plane, so I'll try to have a look at that.
>
> I also found that trying to write all N bytes at once failed, but
> writing 64 at a time is OK. I will add the USB calls to slow_writeN.

I see, it's possible we need to add it as a parameter to writeN or add
the size to the fops struct for the different device types. In fact the
latter might be a cleaner approach, especially if it works for 8192/8188
devices with 64 byte writes. Getting rid of slow_writeN altogether would
be nice.

Jes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/rtl8xxxu.c b/drivers/net/wireless/rtl8xxxu.c
index 8773884..80602b8 100644
--- a/drivers/net/wireless/rtl8xxxu.c
+++ b/drivers/net/wireless/rtl8xxxu.c
@@ -1099,17 +1099,18 @@  rtl8xxxu_writeN(struct rtl8xxxu_priv *priv, u16 addr, u8 *buf, u16 len)
 static int
 rtl8xxxu_slow_writeN(struct rtl8xxxu_priv *priv, u16 addr, u8 *buf, u16 len)
 {
-	u32 blocksize = sizeof(u32);
-	u32 *buf4 = (u32 *)buf;
+	u32 blocksize = 64;
 	int i, offset, count, remainder;
 
+	/* This routine does not do any btye swapping on big-endian machines.
+	 * It should be used for writing firmware, and nothing else */
 	count = len / blocksize;
 	remainder = len % blocksize;
 
 	for (i = 0; i < count; i++) {
 		offset = i * blocksize;
-		rtl8xxxu_write32(priv, (REG_FW_START_ADDRESS + offset),
-				 buf4[i]);
+		rtl8xxxu_writeN(priv, (REG_FW_START_ADDRESS + offset),
+				(buf + offset), blocksize);
 	}
 
 	if (remainder) {