[PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

This set of patches introduces a driver for the RTL8192SE PCI devices.

The patches are as follows:

01/12: Introduce file def.h
02/12: Introduce dynamic management routines dm.c and dm.h
03/12: Introduce firmware routines fw.c and fw.h
04/12: Introduce hardware control routines hw.c and hw.h
05/12: Introduce LED routines led.c and led.h
06/12: Introduce PHY routines phy.c and phy.h
07/12: Introduce register definition header
08/12: Introduce radio control routines
09/12: Introduce main routine (sw.c and sw.h)
10/12: Introduce tables
11/12: Introduce TX/RX routines
12/12: Modify Kconfig and Makefiles to build rtl8192se

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

John,

This material is for 2.6.40.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Borislav Petkov]

On Tue, May 03, 2011 at 09:47:35AM -0500, Larry Finger wrote:
> This set of patches introduces a driver for the RTL8192SE PCI devices.
> 
> The patches are as follows:
> 
> 01/12: Introduce file def.h
> 02/12: Introduce dynamic management routines dm.c and dm.h
> 03/12: Introduce firmware routines fw.c and fw.h
> 04/12: Introduce hardware control routines hw.c and hw.h
> 05/12: Introduce LED routines led.c and led.h
> 06/12: Introduce PHY routines phy.c and phy.h
> 07/12: Introduce register definition header
> 08/12: Introduce radio control routines
> 09/12: Introduce main routine (sw.c and sw.h)
> 10/12: Introduce tables
> 11/12: Introduce TX/RX routines
> 12/12: Modify Kconfig and Makefiles to build rtl8192se

I think I have such a device in my Thinkpad:

03:00.0 Network controller [0280]: Realtek Semiconductor Co., Ltd. RTL8191SEvB Wireless LAN Controller [10ec:8172] (rev 10)

I could give them a run soon. Do you have them in a tree somewhere I
could pull?

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/03/2011 10:01 AM, Borislav Petkov wrote:
> I think I have such a device in my Thinkpad:
>
> 03:00.0 Network controller [0280]: Realtek Semiconductor Co., Ltd. RTL8191SEvB Wireless LAN Controller [10ec:8172] (rev 10)
>
> I could give them a run soon. Do you have them in a tree somewhere I
> could pull?

Your device is claimed by the new driver.

You can get the code from

git://git.kernel.org/pub/scm/linux/kernel/git/lwfinger/rtl8192se.git

This tree is based on Linville's wireless-testing tree with these 12 patches 
applied.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Borislav Petkov]

On Tue, May 03, 2011 at 03:03:40PM -0500, Larry Finger wrote:
> On 05/03/2011 10:01 AM, Borislav Petkov wrote:
> >I think I have such a device in my Thinkpad:
> >
> >03:00.0 Network controller [0280]: Realtek Semiconductor Co., Ltd. RTL8191SEvB Wireless LAN Controller [10ec:8172] (rev 10)
> >
> >I could give them a run soon. Do you have them in a tree somewhere I
> >could pull?
> 
> Your device is claimed by the new driver.
> 
> You can get the code from
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/lwfinger/rtl8192se.git
> 
> This tree is based on Linville's wireless-testing tree with these 12
> patches applied.

Ok, pulled, merged with mainline, built and currently using it to read
mail over imap! I wish everything else in Linux worked that way :).

Good job, thanks!

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/04/2011 12:01 PM, Borislav Petkov wrote:
> On Tue, May 03, 2011 at 03:03:40PM -0500, Larry Finger wrote:
>> On 05/03/2011 10:01 AM, Borislav Petkov wrote:
>>> I think I have such a device in my Thinkpad:
>>>
>>> 03:00.0 Network controller [0280]: Realtek Semiconductor Co., Ltd. RTL8191SEvB Wireless LAN Controller [10ec:8172] (rev 10)
>>>
>>> I could give them a run soon. Do you have them in a tree somewhere I
>>> could pull?
>>
>> Your device is claimed by the new driver.
>>
>> You can get the code from
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/lwfinger/rtl8192se.git
>>
>> This tree is based on Linville's wireless-testing tree with these 12
>> patches applied.
>
> Ok, pulled, merged with mainline, built and currently using it to read
> mail over imap! I wish everything else in Linux worked that way :).
>
> Good job, thanks!

You are very welcome. I'm glad to hear that it works for you. That driver missed 
the 2.6.39 merge window due to an ASPM problem that crashed my computer without 
any possibility of seeing what caused the blowup. I was somewhat afraid that 
others would get similar results even though Realsil did not see the problem.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Borislav Petkov]

On Wed, May 04, 2011 at 12:20:37PM -0500, Larry Finger wrote:
> You are very welcome. I'm glad to hear that it works for you. That
> driver missed the 2.6.39 merge window due to an ASPM problem that
> crashed my computer without any possibility of seeing what caused
> the blowup. I was somewhat afraid that others would get similar
> results even though Realsil did not see the problem.

Right, so I saw the oops below while playing with the driver earlier
but I attribute it to the fact that radio was switched off in the BIOS
(rfkill show said "Hard blocked: yes" for the wireless device). After
changing that I'm running smoothly so far. I'll keep you posted if I see
anything while using it in the next weeks.

Thanks.

--
May  4 18:27:20 gere avahi-daemon[976]: Withdrawing workstation service for wlan0.
May  4 18:27:20 gere kernel: [ 1109.575431] rtl8192se 0000:03:00.0: PCI INT A disabled
May  4 18:27:27 gere kernel: [ 1116.452554] rtl8192se 0000:03:00.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
May  4 18:27:27 gere kernel: [ 1116.458855] rtl8192se 0000:03:00.0: setting latency timer to 64
May  4 18:27:27 gere kernel: [ 1116.486730] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
May  4 18:27:27 gere kernel: [ 1116.487028] IP: [<ffffffffa02fd9aa>] rtl_reg_notifier+0x3a/0xe0 [rtlwifi]
May  4 18:27:27 gere kernel: [ 1116.487028] PGD 13d3f2067 PUD 13b92f067 PMD 0 
May  4 18:27:27 gere kernel: [ 1116.487028] Oops: 0000 [#1] PREEMPT SMP 
May  4 18:27:27 gere kernel: [ 1116.487028] last sysfs file: /sys/devices/pci0000:00/0000:00:07.0/0000:03:00.0/ieee80211/phy3/uevent
May  4 18:27:27 gere kernel: [ 1116.487028] CPU 1 
May  4 18:27:27 gere kernel: [ 1116.487028] Modules linked in: rtl8192se(+) nls_utf8 nls_cp437 arc4 ipv6 fuse vfat fat loop btusb bluetooth usbhid snd_hda_codec_hdmi snd_hda_codec_conexant snd_hda_intel option usb_wwan usb_storage snd_hda_codec snd_hwdep snd_pcm usbserial thinkpad_acpi snd_seq snd_timer ohci_hcd snd_seq_device snd pcspkr ehci_hcd rtlwifi soundcore thermal nvram snd_page_alloc usbcore edac_core video processor evdev mac80211 ac thermal_sys cfg80211 battery rfkill k8temp button i2c_piix4 [last unloaded: rtl8192se]
May  4 18:27:27 gere kernel: [ 1116.523056] 
May  4 18:27:27 gere kernel: [ 1116.523056] Pid: 2387, comm: modprobe Not tainted 2.6.39-rc5-wl+ #15 LENOVO 01972NG/INVALID
May  4 18:27:27 gere kernel: [ 1116.523056] RIP: 0010:[<ffffffffa02fd9aa>]  [<ffffffffa02fd9aa>] rtl_reg_notifier+0x3a/0xe0 [rtlwifi]
May  4 18:27:27 gere kernel: [ 1116.523056] RSP: 0018:ffff88013d1a3b18  EFLAGS: 00010246
May  4 18:27:27 gere kernel: [ 1116.523056] RAX: ffff88013d211be0 RBX: ffff88013d210180 RCX: 00000000000001a0
May  4 18:27:27 gere kernel: [ 1116.523056] RDX: 000000000000000d RSI: 0000000000000000 RDI: 0000000000000000
May  4 18:27:27 gere kernel: [ 1116.523056] RBP: ffff88013d1a3b28 R08: 00000000000009bc R09: 0000000000000994
May  4 18:27:27 gere kernel: [ 1116.523056] R10: 000000000000000e R11: ffffffffa03077f4 R12: 0000000000000000
May  4 18:27:27 gere kernel: [ 1116.523056] R13: ffff88013d210180 R14: 0000000000000000 R15: 0000000000000001
May  4 18:27:27 gere kernel: [ 1116.523056] FS:  00007f6219dbb700(0000) GS:ffff88013fd00000(0000) knlGS:0000000000000000
May  4 18:27:27 gere kernel: [ 1116.523056] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
May  4 18:27:27 gere kernel: [ 1116.523056] CR2: 0000000000000004 CR3: 000000013d3e7000 CR4: 00000000000006e0
May  4 18:27:27 gere kernel: [ 1116.523056] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
May  4 18:27:27 gere kernel: [ 1116.523056] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
May  4 18:27:27 gere kernel: [ 1116.523056] Process modprobe (pid: 2387, threadinfo ffff88013d1a2000, task ffff88013b945eb0)
May  4 18:27:27 gere kernel: [ 1116.523056] Stack:
May  4 18:27:27 gere kernel: [ 1116.523056]  0000000000000000 0000000000000031 ffff88013d1a3b98 ffffffffa003d134
May  4 18:27:27 gere kernel: [ 1116.523056]  ffffffffa00584c0 0000000000000000 0000000000000002 ffffffff00000000
May  4 18:27:27 gere kernel: [ 1116.523056]  ffffea0004521d50 000000009552559e ffff88013e4010d0 ffff88013d210180
May  4 18:27:27 gere kernel: [ 1116.523056] Call Trace:
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa003d134>] wiphy_update_regulatory+0x324/0x510 [cfg80211]
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa003a0ac>] wiphy_register+0x1cc/0x340 [cfg80211]
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff811052cf>] ? __kmalloc+0x11f/0x1d0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa009c31d>] ieee80211_register_hw+0x18d/0x530 [mac80211]
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa0303a1b>] rtl_pci_probe+0x1773/0x19ef [rtlwifi]
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff811e3e2f>] local_pci_probe+0x5f/0xd0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff811e44f8>] pci_device_probe+0x88/0xb0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff8135419a>] ? driver_sysfs_add+0x7a/0xb0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff81354496>] driver_probe_device+0x96/0x1b0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff81354653>] __driver_attach+0xa3/0xb0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff813545b0>] ? driver_probe_device+0x1b0/0x1b0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff8135347e>] bus_for_each_dev+0x5e/0x90
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff8135411e>] driver_attach+0x1e/0x20
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff81353c95>] bus_add_driver+0xc5/0x270
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa0097000>] ? 0xffffffffa0096fff
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff81354c26>] driver_register+0x76/0x140
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa0097000>] ? 0xffffffffa0096fff
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff811e4796>] __pci_register_driver+0x56/0xd0
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffffa0097023>] rtl92se_module_init+0x23/0x59 [rtl8192se]
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff810002d4>] do_one_initcall+0x44/0x180
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff8108242a>] sys_init_module+0xba/0x200
May  4 18:27:27 gere kernel: [ 1116.523056]  [<ffffffff8147536b>] system_call_fastpath+0x16/0x1b
May  4 18:27:27 gere kernel: [ 1116.523056] Code: 24 08 66 66 66 66 90 48 89 fb 49 89 f4 e8 7f d5 db ff 48 8b 40 38 f6 80 8b 27 00 00 08 75 49 48 8b bb 90 00 00 00 e8 96 fc ff ff 
May  4 18:27:27 gere kernel: <41>[ 1116.523056]  83 7c 24 04 03 74 0e 31 c0 48 8b 1c 24 4c 8b 64 24 08 c9 c3 
May  4 18:27:27 gere kernel: [ 1116.523056] RIP  [<ffffffffa02fd9aa>] rtl_reg_notifier+0x3a/0xe0 [rtlwifi]
May  4 18:27:27 gere kernel: [ 1116.523056]  RSP <ffff88013d1a3b18>
May  4 18:27:27 gere kernel: [ 1116.523056] CR2: 0000000000000004
May  4 18:27:27 gere kernel: [ 1116.524232] ---[ end trace 9da2e30d01853e47 ]---

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/04/2011 01:06 PM, Borislav Petkov wrote:
> On Wed, May 04, 2011 at 12:20:37PM -0500, Larry Finger wrote:
>> You are very welcome. I'm glad to hear that it works for you. That
>> driver missed the 2.6.39 merge window due to an ASPM problem that
>> crashed my computer without any possibility of seeing what caused
>> the blowup. I was somewhat afraid that others would get similar
>> results even though Realsil did not see the problem.
>
> Right, so I saw the oops below while playing with the driver earlier
> but I attribute it to the fact that radio was switched off in the BIOS
> (rfkill show said "Hard blocked: yes" for the wireless device). After
> changing that I'm running smoothly so far. I'll keep you posted if I see
> anything while using it in the next weeks.
>
> Thanks.

Thanks for the dump. I don't think the oops was due to the wireless being 
switched off. It looks as if you do not have the CRDA package installed, thus 
the regulatory domain was not setup correctly. That is a condition I realize 
that I have not tested. As soon as I have a fix, I'll let you know. BTW, if my 
analysis is correct, this bug has been around since 2.6.37 when rtl8192ce was 
merged.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Walter Goldens]

> This set of patches introduces a driver for the RTL8192SE PCI devices.

Any estimates on the *SU driver? :)

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/03/2011 10:59 AM, Walter Goldens wrote:
>
>> This set of patches introduces a driver for the RTL8192SE PCI devices.
>
> Any estimates on the *SU driver? :)

Now that rtl8192se is submitted, I will start work on the driver for the 
RTL8192DE devices. After that will be the RTL8191SU. It has lower priority 
because r8712u from staging handles those devices. It is not based on mac80211, 
but it works.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Gábor Stefanik]

On Tue, May 3, 2011 at 6:45 PM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> On 05/03/2011 10:59 AM, Walter Goldens wrote:
>>
>>> This set of patches introduces a driver for the RTL8192SE PCI devices.
>>
>> Any estimates on the *SU driver? :)
>
> Now that rtl8192se is submitted, I will start work on the driver for the
> RTL8192DE devices. After that will be the RTL8191SU. It has lower priority
> because r8712u from staging handles those devices. It is not based on
> mac80211, but it works.

Again, may I remind you that the preferred behavior in the Linux world
is release-early, release-often? :-)

>
> 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
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/03/2011 03:19 PM, Gábor Stefanik wrote:
> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<Larry.Finger@lwfinger.net>  wrote:
>> On 05/03/2011 10:59 AM, Walter Goldens wrote:
>>>
>>>> This set of patches introduces a driver for the RTL8192SE PCI devices.
>>>
>>> Any estimates on the *SU driver? :)
>>
>> Now that rtl8192se is submitted, I will start work on the driver for the
>> RTL8192DE devices. After that will be the RTL8191SU. It has lower priority
>> because r8712u from staging handles those devices. It is not based on
>> mac80211, but it works.
>
> Again, may I remind you that the preferred behavior in the Linux world
> is release-early, release-often? :-)

That I know, but "code first" still applies. If the comparison between rtl8192ce 
and rtl8192cu has any validity for the rtl8192s varieties, roughly half of this 
driver will be shared. In addition, we do have all the USB plumbing in place, 
but we are still talking about a few thousand lines of code. At last contact, 
Realtek has no interest in this project, thus I will be on my own.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Gábor Stefanik]

2011/5/3 Larry Finger <Larry.Finger@lwfinger.net>:
> On 05/03/2011 03:19 PM, Gábor Stefanik wrote:
>>
>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<Larry.Finger@lwfinger.net>
>>  wrote:
>>>
>>> On 05/03/2011 10:59 AM, Walter Goldens wrote:
>>>>
>>>>> This set of patches introduces a driver for the RTL8192SE PCI devices.
>>>>
>>>> Any estimates on the *SU driver? :)
>>>
>>> Now that rtl8192se is submitted, I will start work on the driver for the
>>> RTL8192DE devices. After that will be the RTL8191SU. It has lower
>>> priority
>>> because r8712u from staging handles those devices. It is not based on
>>> mac80211, but it works.
>>
>> Again, may I remind you that the preferred behavior in the Linux world
>> is release-early, release-often? :-)
>
> That I know, but "code first" still applies. If the comparison between
> rtl8192ce and rtl8192cu has any validity for the rtl8192s varieties, roughly
> half of this driver will be shared. In addition, we do have all the USB
> plumbing in place, but we are still talking about a few thousand lines of
> code. At last contact, Realtek has no interest in this project, thus I will
> be on my own.
>
> Larry

I do not mean immediately submitting everything for inclusion -
however, a publicly accessible Git tree would be nice. A non-rtlwifi,
badly coded but functional and GPL-compatible driver is still better
than having to use ndiswrapper/load a binary blob.

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/03/2011 03:38 PM, Gábor Stefanik wrote:
> 2011/5/3 Larry Finger<Larry.Finger@lwfinger.net>:
>> On 05/03/2011 03:19 PM, Gábor Stefanik wrote:
>>>
>>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<Larry.Finger@lwfinger.net>
>>>   wrote:
>>>>
>>>> On 05/03/2011 10:59 AM, Walter Goldens wrote:
>>>>>
>>>>>> This set of patches introduces a driver for the RTL8192SE PCI devices.
>>>>>
>>>>> Any estimates on the *SU driver? :)
>>>>
>>>> Now that rtl8192se is submitted, I will start work on the driver for the
>>>> RTL8192DE devices. After that will be the RTL8191SU. It has lower
>>>> priority
>>>> because r8712u from staging handles those devices. It is not based on
>>>> mac80211, but it works.
>>>
>>> Again, may I remind you that the preferred behavior in the Linux world
>>> is release-early, release-often? :-)
>>
>> That I know, but "code first" still applies. If the comparison between
>> rtl8192ce and rtl8192cu has any validity for the rtl8192s varieties, roughly
>> half of this driver will be shared. In addition, we do have all the USB
>> plumbing in place, but we are still talking about a few thousand lines of
>> code. At last contact, Realtek has no interest in this project, thus I will
>> be on my own.
>>
>> Larry
>
> I do not mean immediately submitting everything for inclusion -
> however, a publicly accessible Git tree would be nice. A non-rtlwifi,
> badly coded but functional and GPL-compatible driver is still better
> than having to use ndiswrapper/load a binary blob.

We are in better shape than having to use ndiswrapper or any non-GPL code. The 
r8712u driver in staging is for the RTL8192SU, and it works perfectly well. It 
just doesn't use mac80211, which is why it needs to be replaced.

Larry

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Gábor Stefanik]

2011/5/3 Larry Finger <Larry.Finger@lwfinger.net>:
> On 05/03/2011 03:38 PM, Gábor Stefanik wrote:
>>
>> 2011/5/3 Larry Finger<Larry.Finger@lwfinger.net>:
>>>
>>> On 05/03/2011 03:19 PM, Gábor Stefanik wrote:
>>>>
>>>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<Larry.Finger@lwfinger.net>
>>>>  wrote:
>>>>>
>>>>> On 05/03/2011 10:59 AM, Walter Goldens wrote:
>>>>>>
>>>>>>> This set of patches introduces a driver for the RTL8192SE PCI
>>>>>>> devices.
>>>>>>
>>>>>> Any estimates on the *SU driver? :)
>>>>>
>>>>> Now that rtl8192se is submitted, I will start work on the driver for
>>>>> the
>>>>> RTL8192DE devices. After that will be the RTL8191SU. It has lower
>>>>> priority
>>>>> because r8712u from staging handles those devices. It is not based on
>>>>> mac80211, but it works.
>>>>
>>>> Again, may I remind you that the preferred behavior in the Linux world
>>>> is release-early, release-often? :-)
>>>
>>> That I know, but "code first" still applies. If the comparison between
>>> rtl8192ce and rtl8192cu has any validity for the rtl8192s varieties,
>>> roughly
>>> half of this driver will be shared. In addition, we do have all the USB
>>> plumbing in place, but we are still talking about a few thousand lines of
>>> code. At last contact, Realtek has no interest in this project, thus I
>>> will
>>> be on my own.
>>>
>>> Larry
>>
>> I do not mean immediately submitting everything for inclusion -
>> however, a publicly accessible Git tree would be nice. A non-rtlwifi,
>> badly coded but functional and GPL-compatible driver is still better
>> than having to use ndiswrapper/load a binary blob.
>
> We are in better shape than having to use ndiswrapper or any non-GPL code.
> The r8712u driver in staging is for the RTL8192SU, and it works perfectly
> well.

What about RTL8192DE?

> It just doesn't use mac80211, which is why it needs to be replaced.
>
> Larry
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH 00/12] rtlwifi: rtl8192se: Merge new driver

[Larry Finger]

On 05/03/2011 04:09 PM, Gábor Stefanik wrote:
> 2011/5/3 Larry Finger<Larry.Finger@lwfinger.net>:
>> On 05/03/2011 03:38 PM, Gábor Stefanik wrote:
>>>
>>> 2011/5/3 Larry Finger<Larry.Finger@lwfinger.net>:
>>>>
>>>> On 05/03/2011 03:19 PM, Gábor Stefanik wrote:
>>>>>
>>>>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<Larry.Finger@lwfinger.net>
>>>>>   wrote:
>>>>>>
>>>>>> On 05/03/2011 10:59 AM, Walter Goldens wrote:
>>>>>>>
>>>>>>>> This set of patches introduces a driver for the RTL8192SE PCI
>>>>>>>> devices.
>>>>>>>
>>>>>>> Any estimates on the *SU driver? :)
>>>>>>
>>>>>> Now that rtl8192se is submitted, I will start work on the driver for
>>>>>> the
>>>>>> RTL8192DE devices. After that will be the RTL8191SU. It has lower
>>>>>> priority
>>>>>> because r8712u from staging handles those devices. It is not based on
>>>>>> mac80211, but it works.
>>>>>
>>>>> Again, may I remind you that the preferred behavior in the Linux world
>>>>> is release-early, release-often? :-)
>>>>
>>>> That I know, but "code first" still applies. If the comparison between
>>>> rtl8192ce and rtl8192cu has any validity for the rtl8192s varieties,
>>>> roughly
>>>> half of this driver will be shared. In addition, we do have all the USB
>>>> plumbing in place, but we are still talking about a few thousand lines of
>>>> code. At last contact, Realtek has no interest in this project, thus I
>>>> will
>>>> be on my own.
>>>>
>>>> Larry
>>>
>>> I do not mean immediately submitting everything for inclusion -
>>> however, a publicly accessible Git tree would be nice. A non-rtlwifi,
>>> badly coded but functional and GPL-compatible driver is still better
>>> than having to use ndiswrapper/load a binary blob.
>>
>> We are in better shape than having to use ndiswrapper or any non-GPL code.
>> The r8712u driver in staging is for the RTL8192SU, and it works perfectly
>> well.
>
> What about RTL8192DE?

I'm working on that one today. If all goes well, it might make 2.6.40, but that 
may be a little tight on time. Publish early and often does apply here.

Larry

r8712u driver for the rtl8192su chip.

[Ali Bahar]

Hi Larry,

this is more of a quick FYI, regarding Staging's r8712u driver.

On Tue, May 03, 2011 at 03:56:19PM -0500, Larry Finger wrote:
> On 05/03/2011 03:38 PM, Gábor Stefanik wrote:
> >2011/5/3 Larry Finger<Larry.Finger@lwfinger.net>:
> >>On 05/03/2011 03:19 PM, Gábor Stefanik wrote:

> code. The r8712u driver in staging is for the RTL8192SU, and it
> works perfectly well. It just doesn't use mac80211, which is why it
> needs to be replaced.
> 
> Larry

I ran iwconfig on this device in order to put it into Monitor mode, or
even to change its ssid. Both failed because the Set ioctls were not
permitted.
Again, this is more of an FYI, since I don't really use this device.

regards,
ali
PS I did not CC the thread's original list of recipients, because this
tangents somewhat from its main topic.

PPS for the benefit of googlers: the device is the ASUS WL-167G V3
wireless USB adapter.


Additional Info:
~~~~~~~~~~~~~~~
root@hashbang Tue Jun 21 18:24:39 ~$ ifconfig wlan2
wlan2     Link encap:Ethernet  HWaddr 48:5b:39:eb:d2:7c  
          BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:0 errors:0 dropped:0 overruns:0 frame:0
          TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

root@hashbang Tue Jun 21 18:24:57 ~$ iwconfig wlan2
wlan2     unassociated  Nickname:"rtl_wifi"
          Mode:Auto  Access Point: Not-Associated   Sensitivity:0/0  
          Retry:off   RTS thr:off   Fragment thr:off
          Encryption key:off
          Power Management:off
          Link Quality:0  Signal level:0  Noise level:0
          Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
          Tx excessive retries:0  Invalid misc:0   Missed beacon:0

root@hashbang Tue Jun 21 18:25:02 ~$ ethtool -i wlan2
driver: r8712u
version: 
firmware-version: 
bus-info: 1-1:1.0
root@hashbang Tue Jun 21 18:25:09 ~$ lsusb|grep :1791
Bus 001 Device 002: ID 0b05:1791 ASUSTek Computer, Inc. 
root@hashbang Tue Jun 21 18:28:48 ~$ iwconfig wlan2 mode Monitor
Error for wireless request "Set Mode" (8B06) :
    SET failed on device wlan2 ; Invalid argument.
root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
Error for wireless request "Set ESSID" (8B1A) :
    SET failed on device wlan2 ; Operation not permitted.
root@hashbang Tue Jun 21 18:30:47 ~$ uname -r
3.0.0-rc2-wl_m_Sat12jun2011
root@hashbang Tue Jun 21 18:36:57 ~$ 



root@hashbang Tue Jun 21 20:17:28 ~$ iwconfig wlan2
wlan2     unassociated  Nickname:"rtl_wifi"
          Mode:Auto  Access Point: Not-Associated   Sensitivity:0/0  
          Retry:off   RTS thr:off   Fragment thr:off
          Encryption key:off
          Power Management:off
          Link Quality:0  Signal level:0  Noise level:0
          Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
          Tx excessive retries:0  Invalid misc:0   Missed beacon:0

root@hashbang Tue Jun 21 20:18:26 ~$ iwconfig wlan2 mode Managed
root@hashbang Tue Jun 21 20:18:37 ~$ iwconfig wlan2 essid "h55m"
channel 11
Error for wireless request "Set ESSID" (8B1A) :
    SET failed on device wlan2 ; Operation not permitted.
root@hashbang Tue Jun 21 20:19:00 ~$ iwconfig wlan2
wlan2     unassociated  Nickname:"rtl_wifi"
          Mode:Managed Access Point: Not-Associated Sensitivity:0/0  
          Retry:off   RTS thr:off   Fragment thr:off
          Encryption key:off
          Power Management:off
          Link Quality:0  Signal level:0  Noise level:0
          Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
          Tx excessive retries:0  Invalid misc:0   Missed beacon:0

root@hashbang Tue Jun 21 20:19:10 ~$ 

Re: r8712u driver for the rtl8192su chip.

[Larry Finger]

On 06/21/2011 07:49 AM, Ali Bahar wrote:
> Hi Larry,
>
> this is more of a quick FYI, regarding Staging's r8712u driver.
>
> I ran iwconfig on this device in order to put it into Monitor mode, or
> even to change its ssid. Both failed because the Set ioctls were not
> permitted.
> Again, this is more of an FYI, since I don't really use this device.
>
> PPS for the benefit of googlers: the device is the ASUS WL-167G V3
> wireless USB adapter.
>
> root@hashbang Tue Jun 21 18:25:09 ~$ lsusb|grep :1791
> Bus 001 Device 002: ID 0b05:1791 ASUSTek Computer, Inc.
> root@hashbang Tue Jun 21 18:28:48 ~$ iwconfig wlan2 mode Monitor
> Error for wireless request "Set Mode" (8B06) :
>      SET failed on device wlan2 ; Invalid argument.
> root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
> Error for wireless request "Set ESSID" (8B1A) :
>      SET failed on device wlan2 ; Operation not permitted.
> root@hashbang Tue Jun 21 18:30:47 ~$ uname -r
> 3.0.0-rc2-wl_m_Sat12jun2011
> root@hashbang Tue Jun 21 18:36:57 ~$

Setting monitor mode is not implemented; however, setting the essid is 
implemented. Your command failed because you ran the command as an unprivileged 
user. Notice the differences in the error messages.

Larry

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

On Tue, Jun 21, 2011 at 05:08:04PM -0500, Larry Finger wrote:
> On 06/21/2011 07:49 AM, Ali Bahar wrote:

> >this is more of a quick FYI, regarding Staging's r8712u driver.

> >root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
> >Error for wireless request "Set ESSID" (8B1A) :
> >     SET failed on device wlan2 ; Operation not permitted.
> >root@hashbang Tue Jun 21 18:30:47 ~$ uname -r

> is implemented. Your command failed because you ran the command as
> an unprivileged user. Notice the differences in the error messages.

How is root unprivileged? The shell-prompt shows the user.

Right after the failure, I ran the exact same steps, in the same
shell, on a different card of the same box. It worked, as it always
has.

thanks,
ali

Re: r8712u driver for the rtl8192su chip.

[Julian Calaby]

Ali,

On Wed, Jun 22, 2011 at 11:00, Ali Bahar <ali@internetdog.org> wrote:
> On Tue, Jun 21, 2011 at 05:08:04PM -0500, Larry Finger wrote:
>> On 06/21/2011 07:49 AM, Ali Bahar wrote:
>
>> >this is more of a quick FYI, regarding Staging's r8712u driver.
>
>> >root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
>> >Error for wireless request "Set ESSID" (8B1A) :
>> >     SET failed on device wlan2 ; Operation not permitted.
>> >root@hashbang Tue Jun 21 18:30:47 ~$ uname -r
>
> How is root unprivileged? The shell-prompt shows the user.

In general, root prompts have a '#' at the end of the prompt, this one
has a '$' which is indicative of a non-privileged user.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: r8712u driver for the rtl8192su chip.

[Gábor Stefanik]

On Wed, Jun 22, 2011 at 4:03 AM, Julian Calaby <julian.calaby@gmail.com> wrote:
> Ali,
>
> On Wed, Jun 22, 2011 at 11:00, Ali Bahar <ali@internetdog.org> wrote:
>> On Tue, Jun 21, 2011 at 05:08:04PM -0500, Larry Finger wrote:
>>> On 06/21/2011 07:49 AM, Ali Bahar wrote:
>>
>>> >this is more of a quick FYI, regarding Staging's r8712u driver.
>>
>>> >root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
>>> >Error for wireless request "Set ESSID" (8B1A) :
>>> >     SET failed on device wlan2 ; Operation not permitted.
>>> >root@hashbang Tue Jun 21 18:30:47 ~$ uname -r
>>
>> How is root unprivileged? The shell-prompt shows the user.
>
> In general, root prompts have a '#' at the end of the prompt, this one
> has a '$' which is indicative of a non-privileged user.

I'm pretty sure that is not true for Bash. Perhaps for csh or Dash.

>
> Thanks,
>
> --
> Julian Calaby
>
> Email: julian.calaby@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
> .Plan: http://sites.google.com/site/juliancalaby/
> --
> 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
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

Hi Gábor,


> > In general, root prompts have a '#' at the end of the prompt, this one
> > has a '$' which is indicative of a non-privileged user.
> 
> I'm pretty sure that is not true for Bash. Perhaps for csh or Dash.

Of course, $PS1 sets this prompt to anything you want, but we're
getting sidetracked. :-)

thanks,
ali

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

Hi Julian,

On Wed, Jun 22, 2011 at 12:03:26PM +1000, Julian Calaby wrote:

> >> >root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
> >> >Error for wireless request "Set ESSID" (8B1A) :
> >> >     SET failed on device wlan2 ; Operation not permitted.
> >> >root@hashbang Tue Jun 21 18:30:47 ~$ uname -r
> >
> > How is root unprivileged? The shell-prompt shows the user.
> 
> In general, root prompts have a '#' at the end of the prompt, this one
> has a '$' which is indicative of a non-privileged user.

This box sets it differently. I understand how it could be missed in
my post.

Thanks,
ali

Re: r8712u driver for the rtl8192su chip.

[Larry Finger]

On 06/21/2011 08:00 PM, Ali Bahar wrote:
> On Tue, Jun 21, 2011 at 05:08:04PM -0500, Larry Finger wrote:
>> On 06/21/2011 07:49 AM, Ali Bahar wrote:
>
>>> this is more of a quick FYI, regarding Staging's r8712u driver.
>
>>> root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
>>> Error for wireless request "Set ESSID" (8B1A) :
>>>      SET failed on device wlan2 ; Operation not permitted.
>>> root@hashbang Tue Jun 21 18:30:47 ~$ uname -r
>
>> is implemented. Your command failed because you ran the command as
>> an unprivileged user. Notice the differences in the error messages.
>
> How is root unprivileged? The shell-prompt shows the user.
>
> Right after the failure, I ran the exact same steps, in the same
> shell, on a different card of the same box. It worked, as it always
> has.

I have no idea what you are doing wrong, but I get the following:

finger@larrylap:~/wireless-testing-new> sudo iwconfig wlan9 essid junk
finger@larrylap:~/wireless-testing-new>

The above is with r8712u running on wireless-testing 3.0-rc4.

Larry

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

> >>>this is more of a quick FYI, regarding Staging's r8712u driver.
> >
> >>>root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
> >>>Error for wireless request "Set ESSID" (8B1A) :
> >>>     SET failed on device wlan2 ; Operation not permitted.

> I have no idea what you are doing wrong, but I get the following:

Again, the test procedure was identical, and deliberately so.

This was all just FYI; I was just providing some data. Thank you for
your reply, and for your ongoing work.

 
> finger@larrylap:~/wireless-testing-new> sudo iwconfig wlan9 essid junk
> finger@larrylap:~/wireless-testing-new>
> 
> The above is with r8712u running on wireless-testing 3.0-rc4.

I'd tried 3.0.0-rc3-wl_m_Sun19jun2011 (ie wireless rc3, built on
Sunday) as well. Same wext and OS, on a different box.

Since you are rewriting this driver, I won't try to dig into this on
my end.

thanks,
ali

Re: r8712u driver for the rtl8192su chip.

[Larry Finger]

On 06/21/2011 09:56 PM, Ali Bahar wrote:
>
>>>>> this is more of a quick FYI, regarding Staging's r8712u driver.
>>>
>>>>> root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
>>>>> Error for wireless request "Set ESSID" (8B1A) :
>>>>>      SET failed on device wlan2 ; Operation not permitted.
>
>> I have no idea what you are doing wrong, but I get the following:
>
> Again, the test procedure was identical, and deliberately so.
>
> This was all just FYI; I was just providing some data. Thank you for
> your reply, and for your ongoing work.
>
>
>> finger@larrylap:~/wireless-testing-new>  sudo iwconfig wlan9 essid junk
>> finger@larrylap:~/wireless-testing-new>
>>
>> The above is with r8712u running on wireless-testing 3.0-rc4.
>
> I'd tried 3.0.0-rc3-wl_m_Sun19jun2011 (ie wireless rc3, built on
> Sunday) as well. Same wext and OS, on a different box.
>
> Since you are rewriting this driver, I won't try to dig into this on
> my end.

I am not rewriting this driver in the immediate future, thus you need to find 
out what is wrong.

As I said earlier, your error certainly looks like a privilege problem. Adding 
to what I posted earlier, I get

finger@larrylap:~> iwconfig wlan9 essid test
Error for wireless request "Set ESSID" (8B1A) :
     SET failed on device wlan9 ; Operation not permitted.
finger@larrylap:~> sudo iwconfig wlan9 essid test
finger@larrylap:~>

As you can see, my unprivileged version gets exactly the error message that you 
reported and the one as root works.

Larry

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

On Tue, Jun 21, 2011 at 10:09:28PM -0500, Larry Finger wrote:
> On 06/21/2011 09:56 PM, Ali Bahar wrote:

> >Since you are rewriting this driver, I won't try to dig into this on
> >my end.
> 
> I am not rewriting this driver in the immediate future, thus you
> need to find out what is wrong.

OK.

 
> As I said earlier, your error certainly looks like a privilege
> problem. Adding to what I posted earlier, I get
> 
> finger@larrylap:~> iwconfig wlan9 essid test
> Error for wireless request "Set ESSID" (8B1A) :
>     SET failed on device wlan9 ; Operation not permitted.
> finger@larrylap:~> sudo iwconfig wlan9 essid test
> finger@larrylap:~>
> 
> As you can see, my unprivileged version gets exactly the error
> message that you reported and the one as root works.

I know. 

thanks,
ali

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

Hi Larry,


On Tue, Jun 21, 2011 at 10:09:28PM -0500, Larry Finger wrote:
> On 06/21/2011 09:56 PM, Ali Bahar wrote:
> >
> >>>>>this is more of a quick FYI, regarding Staging's r8712u driver.
> >>>
> >>>>>root@hashbang Tue Jun 21 18:30:06 ~$ iwconfig wlan2 essid "h55m"
> >>>>>Error for wireless request "Set ESSID" (8B1A) :
> >>>>>     SET failed on device wlan2 ; Operation not permitted.

> I am not rewriting this driver in the immediate future, thus you
> need to find out what is wrong.
 
The cause is that the driver's handler for the Set expects that the
interface is Up. In my case, it was not, and so it returns a -1. This
pops back up the call-chain until it gets misinterpreted as an EPERM. 

I'll use this Set as an example, though the same pattern is seen
elsewhere.
Upon entry into r8711_wx_set_scan(), this check is done:

   1090         if (padapter->bup == false)
   1091                 return -1;


The adapter structure has no comments as to what bup is (nor for
bDriverStopped). Current usage suggests that bup indicates that the
interface is Up (and bDriverStopped indicates that the driver has been
cleanly stopped)


The Fix:
~~~~~~~
1. the -1 return-values must be changed, or translated, so as to not
   conflict with EPERM/errno.
2. The 802.11 cfg SETs ought not check bup, or the definition of bup
   has to be made distinct from the interface's Up status.

I tried to dig into the history of this code, but found none before
2010. The source which came on the product's cdrom (ASUS WL-167G V3)
is very old, and too different. Realtek's website provides recent
code, but is significantly newer than that which you seem to have
based your code on. And, unfortunately, it contains the same usage of
bup!

So, if the above analysis is correct, should the patch be based on the
Staging code?

Additional Info:
~~~~~~~~~~~~~~~
The question did arise as to whether the driver's expectation (of the
interface being Up) is legitimate. But this was dismissed because:
- I did not expect it to be so. There really is no reason why it'd be
  obligatory!
- Other drivers handle Sets of their 802.11 configuration while the
  interface is Down;
- the frequent case is for such Sets to take place before an IP number
  is obtained via DHCP! So the interface will not be Up;
- the 802.11 Standard ought not take a stance with respect to this.
  Indeed, to the extent that I dug into this, it does not seem to
  have. 

regards,
ali

Re: r8712u driver for the rtl8192su chip.

[Larry Finger]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On 06/27/2011 02:23 AM, Ali Bahar wrote:
> Hi Larry,
>
>
> The cause is that the driver's handler for the Set expects that the
> interface is Up. In my case, it was not, and so it returns a -1. This
> pops back up the call-chain until it gets misinterpreted as an EPERM.
>
> I'll use this Set as an example, though the same pattern is seen
> elsewhere.
> Upon entry into r8711_wx_set_scan(), this check is done:
>
>     1090         if (padapter->bup == false)
>     1091                 return -1;
>
>
> The adapter structure has no comments as to what bup is (nor for
> bDriverStopped). Current usage suggests that bup indicates that the
> interface is Up (and bDriverStopped indicates that the driver has been
> cleanly stopped)

The meaning of "bup" is that it is a boolean indicating whether the interface is 
up or not, just as you surmised.

The attached patch has been floating around for a while. Does it help?

Larry


[-- Attachment #2: r8712u_set_essid --]
[-- Type: text/plain, Size: 845 bytes --]

Index: linux-2.6/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
===================================================================
--- linux-2.6.orig/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ linux-2.6/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -990,8 +990,6 @@ static int r8711_wx_set_wap(struct net_d
 	struct wlan_network *pnetwork = NULL;
 	enum NDIS_802_11_AUTHENTICATION_MODE	authmode;
 
-	if (padapter->bup == false)
-		return -1;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
 		return -1;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING) == true)
@@ -1182,8 +1180,6 @@ static int r8711_wx_set_essid(struct net
 	struct list_head *phead;
 	u32 len;
 
-	if (padapter->bup == false)
-		return -1;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
 		return -1;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

Hi Larry,

On Sat, Jul 02, 2011 at 12:09:08PM -0500, Larry Finger wrote:
> On 06/27/2011 02:23 AM, Ali Bahar wrote:

> >The cause is that the driver's handler for the Set expects that the
> >interface is Up. In my case, it was not, and so it returns a -1. This
> >pops back up the call-chain until it gets misinterpreted as an EPERM.

> The attached patch has been floating around for a while. Does it help?

Of course, it will, but the general problem with the -1 return-values
(and any other Up assumptions) will remain.

As I'd indicated earlier, I have no interest in, or use for, this
device. I dug into it because there seemed to be a need within this
ML (and I was already looking for an 802.11 project anyway.)

If that is the case, I can implement the changes, merge recent Realtek
changes, and determine how much nl80211 interface is needed.
On the other hand, if changes are on-hold, then I'll focus elsewhere.

regards,
ali


> Index: linux-2.6/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> ===================================================================
> --- linux-2.6.orig/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ linux-2.6/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -990,8 +990,6 @@ static int r8711_wx_set_wap(struct net_d
>  	struct wlan_network *pnetwork = NULL;
>  	enum NDIS_802_11_AUTHENTICATION_MODE	authmode;
>  
> -	if (padapter->bup == false)
> -		return -1;
>  	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
>  		return -1;
>  	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING) == true)
> @@ -1182,8 +1180,6 @@ static int r8711_wx_set_essid(struct net
>  	struct list_head *phead;
>  	u32 len;
>  
> -	if (padapter->bup == false)
> -		return -1;
>  	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
>  		return -1;
>  	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))

Re: r8712u driver for the rtl8192su chip.

[Larry Finger]

On 07/02/2011 08:06 PM, Ali Bahar wrote:
> Hi Larry,
>
> On Sat, Jul 02, 2011 at 12:09:08PM -0500, Larry Finger wrote:
>> On 06/27/2011 02:23 AM, Ali Bahar wrote:
>
>>> The cause is that the driver's handler for the Set expects that the
>>> interface is Up. In my case, it was not, and so it returns a -1. This
>>> pops back up the call-chain until it gets misinterpreted as an EPERM.
>
>> The attached patch has been floating around for a while. Does it help?
>
> Of course, it will, but the general problem with the -1 return-values
> (and any other Up assumptions) will remain.
>
> As I'd indicated earlier, I have no interest in, or use for, this
> device. I dug into it because there seemed to be a need within this
> ML (and I was already looking for an 802.11 project anyway.)
>
> If that is the case, I can implement the changes, merge recent Realtek
> changes, and determine how much nl80211 interface is needed.
> On the other hand, if changes are on-hold, then I'll focus elsewhere.
>
> regards,
> ali
>
>
>> Index: linux-2.6/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> +++ linux-2.6/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
>> @@ -990,8 +990,6 @@ static int r8711_wx_set_wap(struct net_d
>>   	struct wlan_network *pnetwork = NULL;
>>   	enum NDIS_802_11_AUTHENTICATION_MODE	authmode;
>>
>> -	if (padapter->bup == false)
>> -		return -1;
>>   	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
>>   		return -1;
>>   	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING) == true)
>> @@ -1182,8 +1180,6 @@ static int r8711_wx_set_essid(struct net
>>   	struct list_head *phead;
>>   	u32 len;
>>
>> -	if (padapter->bup == false)
>> -		return -1;
>>   	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
>>   		return -1;
>>   	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))

If you want to work on it, I will review and test the patches. It will be a long 
time until I get to it.

Larry

Re: r8712u driver for the rtl8192su chip.

[Ali Bahar]

On Sat, Jul 02, 2011 at 08:24:01PM -0500, Larry Finger wrote:
> On 07/02/2011 08:06 PM, Ali Bahar wrote:

> >device. I dug into it because there seemed to be a need within this
> >ML (and I was already looking for an 802.11 project anyway.)

> >>  	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
> >>  		return -1;
> >>  	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
> 
> If you want to work on it, I will review and test the patches. It

OK.

> will be a long time until I get to it.

I understand.

later,
ali

[PATCH 0/2] staging: r8712u: Most return-values changed from -1 to

[Ali Bahar]

Correcting most of the -1 return-values that this driver's ioctl handlers
return. They have been changed to the closest corresponding errno macro.


Unfortunately, very few of these could be tested! Often, wext does not care
about the specifics of the failure; it just checks for a negative
return-value. So these bugs do not propagate to the user.

Without instrumenting the code (of the driver, or of wext), this left
very few testables! eg the following (note the ampersand)
  iwlist wlan2 scan & iwconfig wlan2 ap any
was the only way of triggering one of the failures. 
Prior to the tests, I spent a lot of time on verifying the call chains.

The git-log of this file includes a few other such corrections to the
return values. So this patch should fit-in well.

Ali Bahar (2):
  staging: r8712u: Most return-values changed from -1 to proper errno
    macros.
  staging: r8712u: checkpatch errors: trailing whitespace on 2 lines.

 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  121 +++++++++++++++++++++----
 1 files changed, 105 insertions(+), 16 deletions(-)

-- 
1.7.6

[PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Ali Bahar]

The ioctl handlers were frequently returning -1 upon failure. Most of
these have now been changed to proper errno macros.
The few remaining ones have been left untouched because either the
handler is not called (and so cannot be tested), or the function never
fails (and so cannot be system-tested), or requires new code to
distinguish its failures.

Signed-off-by: Ali Bahar <ali@internetDog.org>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  121 +++++++++++++++++++++----
 1 files changed, 105 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 40e6b5c..bb97d8b 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -526,7 +526,7 @@ static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
 		memcpy(buf, pie , ielen);
 		pos = buf;
 		if (ielen < RSN_HEADER_LEN) {
-			ret  = -1;
+			ret  = -EINVAL;
 			goto exit;
 		}
 		if (r8712_parse_wpa_ie(buf, ielen, &group_cipher,
@@ -689,6 +689,11 @@ static const long frequency_list[] = {
 	5825
 };
 
+/*
+ * This function intends to handle the Set Freq command.
+ * Currently, the request comes via the Wireless Extensions' SIOCSIWFREQ ioctl.
+ *
+ */
 static int r8711_wx_set_freq(struct net_device *dev,
 			     struct iw_request_info *info,
 			     union iwreq_data *wrqu, char *extra)
@@ -723,6 +728,11 @@ static int r8711_wx_set_freq(struct net_device *dev,
 	return rc;
 }
 
+/*
+ * This function intends to handle the Get Freq command.
+ * Currently, the request comes via the Wireless Extensions' SIOCGIWFREQ ioctl.
+ *
+ */
 static int r8711_wx_get_freq(struct net_device *dev,
 			     struct iw_request_info *info,
 			     union iwreq_data *wrqu, char *extra)
@@ -736,11 +746,21 @@ static int r8711_wx_get_freq(struct net_device *dev,
 			       pcur_bss->Configuration.DSConfig-1] * 100000;
 		wrqu->freq.e = 1;
 		wrqu->freq.i = pcur_bss->Configuration.DSConfig;
-	} else
-		return -1;
+	} else {
+		return -ENOLINK;
+	}
 	return 0;
 }
 
+/*
+ * This function intends to handle the Set Mode command.
+ * Currently, the request comes via the Wireless Extensions' SIOCSIWMODE ioctl.
+ *
+ * Note: Currently (July 2011) the call to
+ * r8712_set_802_11_infrastructure_mode() always returns true. Since we have no
+ * specific failure reason, we return EPERM in lieu of general failure.
+ *
+ */
 static int r8711_wx_set_mode(struct net_device *dev,
 			     struct iw_request_info *a,
 			     union iwreq_data *wrqu, char *b)
@@ -769,10 +789,15 @@ static int r8711_wx_set_mode(struct net_device *dev,
 	else
 		r8712_setopmode_cmd(padapter, Ndis802_11AutoUnknown);
 	if (!r8712_set_802_11_infrastructure_mode(padapter, networkType))
-		return -1;
+		return -EPERM; /* Unknown failure. */
 	return 0;
 }
 
+/*
+ * This function intends to handle the Get Mode command.
+ * Currently, the request comes via the Wireless Extensions' SIOCGIWMODE ioctl.
+ *
+ */
 static int r8711_wx_get_mode(struct net_device *dev, struct iw_request_info *a,
 			     union iwreq_data *wrqu, char *b)
 {
@@ -975,6 +1000,16 @@ static int r871x_wx_set_priv(struct net_device *dev,
  * s2. set_802_11_authentication_mode()
  * s3. set_802_11_encryption_mode()
  * s4. set_802_11_bssid()
+ *
+ * This function intends to handle the Set AP command, which specifies the
+ * MAC# of a preferred Access Point.
+ * Currently, the request comes via Wireless Extensions' SIOCSIWAP ioctl.
+ *
+ * For this operation to succeed, there is no need for the interface to be Up.
+ *
+ * Note: Currently (July 2011) r8712_set_802_11_bssid() may fail for one of
+ * several reasons. For now, we let -1 be returned as a catch-all failure
+ * indication, until that function's implementation is updated. 
  */
 static int r8711_wx_set_wap(struct net_device *dev,
 			 struct iw_request_info *info,
@@ -995,7 +1030,7 @@ static int r8711_wx_set_wap(struct net_device *dev,
 	if (padapter->bup == false)
 		return -1;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
-		return -1;
+		return -EBUSY;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING) == true)
 		return ret;
 	if (temp->sa_family != ARPHRD_ETHER)
@@ -1014,14 +1049,14 @@ static int r8711_wx_set_wap(struct net_device *dev,
 		if (!memcmp(dst_bssid, temp->sa_data, ETH_ALEN)) {
 			if (r8712_set_802_11_infrastructure_mode(padapter,
 			    pnetwork->network.InfrastructureMode) == false)
-				ret = -1;
+				ret = -EPERM; /* Unknown failure. */
 			break;
 		}
 	}
 	spin_unlock_irqrestore(&queue->lock, irqL);
 	if (!ret) {
 		if (!r8712_set_802_11_authentication_mode(padapter, authmode))
-			ret = -1;
+			ret = -ENOMEM;
 		else {
 			if (!r8712_set_802_11_bssid(padapter, temp->sa_data))
 				ret = -1;
@@ -1074,6 +1109,19 @@ static int r871x_wx_set_mlme(struct net_device *dev,
 	return ret;
 }
 
+/**
+ *
+ * This function intends to handle the Set Scan command.
+ * Currently, the request comes via Wireless Extensions' SIOCSIWSCAN ioctl.
+ *
+ * For this operation to succeed, the interface is brought Up beforehand.
+ *
+ * Note: It is not clear what value we ought to return when hw_init_completed
+ * is false. If the firmware could not be found/loaded, which errno should we
+ * return?
+ * The same question arises for bDriverStopped being true.
+ *
+ */ 
 static int r8711_wx_set_scan(struct net_device *dev,
 			struct iw_request_info *a,
 			union iwreq_data *wrqu, char *extra)
@@ -1088,7 +1136,7 @@ static int r8711_wx_set_scan(struct net_device *dev,
 		return -1;
 	}
 	if (padapter->bup == false)
-		return -1;
+		return -ENETDOWN;
 	if (padapter->hw_init_completed == false)
 		return -1;
 	if ((check_fwstate(pmlmepriv, _FW_UNDER_SURVEY|_FW_UNDER_LINKING)) ||
@@ -1122,6 +1170,13 @@ static int r8711_wx_set_scan(struct net_device *dev,
 	return 0;
 }
 
+/**
+ *
+ * This function intends to handle the Get Scan command.
+ * Currently, the request comes via Wireless Extensions' SIOCGIWSCAN ioctl.
+ *
+ *
+ */
 static int r8711_wx_get_scan(struct net_device *dev,
 				struct iw_request_info *a,
 				union iwreq_data *wrqu, char *extra)
@@ -1169,6 +1224,16 @@ static int r8711_wx_get_scan(struct net_device *dev,
  * s2. set_802_11_authenticaion_mode()
  * s3. set_802_11_encryption_mode()
  * s4. set_802_11_ssid()
+ *
+ * This function intends to handle the Set ESSID command.
+ * Currently, the request comes via the Wireless Extensions' SIOCSIWESSID ioctl.
+ *
+ * For this operation to succeed, there is no need for the interface to be Up.
+ *
+ * Note: Currently (July 2011) the call to
+ * r8712_set_802_11_infrastructure_mode() always returns true. Since we have no
+ * specific failure reason, we return EPERM in lieu of general failure.
+ *
  */
 static int r8711_wx_set_essid(struct net_device *dev,
 				struct iw_request_info *a,
@@ -1187,7 +1252,7 @@ static int r8711_wx_set_essid(struct net_device *dev,
 	if (padapter->bup == false)
 		return -1;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
-		return -1;
+		return -EBUSY;
 	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
 		return 0;
 	if (wrqu->essid.length > IW_ESSID_MAX_SIZE)
@@ -1215,7 +1280,7 @@ static int r8711_wx_set_essid(struct net_device *dev,
 				if (!r8712_set_802_11_infrastructure_mode(
 				     padapter,
 				     pnetwork->network.InfrastructureMode))
-					return -1;
+					return -EPERM; /* Unknown failure. */
 				break;
 			}
 		}
@@ -1225,6 +1290,12 @@ static int r8711_wx_set_essid(struct net_device *dev,
 	return -EINPROGRESS;
 }
 
+/**
+ *
+ * This function intends to handle the Get ESSID command.
+ * Currently, the request comes via Wireless Extensions' SIOCGIWESSID ioctl.
+ *
+ */
 static int r8711_wx_get_essid(struct net_device *dev,
 				struct iw_request_info *a,
 				union iwreq_data *wrqu, char *extra)
@@ -1240,10 +1311,16 @@ static int r8711_wx_get_essid(struct net_device *dev,
 		memcpy(extra, pcur_bss->Ssid.Ssid, len);
 		wrqu->essid.flags = 1;
 	} else
-		ret = -1;
+		ret = -ENOLINK;
 	return ret;
 }
 
+/**
+ *
+ * This function intends to handle the Set Rate command.
+ * Currently, the request comes via the Wireless Extensions' SIOCSIWRATE ioctl.
+ *
+ */
 static int r8711_wx_set_rate(struct net_device *dev,
 				struct iw_request_info *a,
 				union iwreq_data *wrqu, char *extra)
@@ -1312,10 +1389,16 @@ set_rate:
 			datarates[i] = 0xff;
 	}
 	if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
-		ret = -1;
+		ret = -ENOMEM;
 	return ret;
 }
 
+/**
+ *
+ * This function intends to handle the Get Rate command.
+ * Currently, the request comes via the Wireless Extensions' SIOCGIWRATE ioctl.
+ *
+ */
 static int r8711_wx_get_rate(struct net_device *dev,
 			     struct iw_request_info *info,
 			     union iwreq_data *wrqu, char *extra)
@@ -1371,10 +1454,16 @@ static int r8711_wx_get_rate(struct net_device *dev,
 			wrqu->bitrate.value = max_rate * 500000;
 		}
 	} else
-		return -1;
+		return -ENOLINK;
 	return 0;
 }
 
+/**
+ *
+ * This function intends to handle the Get RTS Threshold command.
+ * Currently, the request comes via the Wireless Extensions' SIOCGIWRTS ioctl.
+ *
+ */
 static int r8711_wx_get_rts(struct net_device *dev,
 				struct iw_request_info *info,
 				union iwreq_data *wrqu, char *extra)
@@ -1701,7 +1790,7 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
 	param_len = sizeof(struct ieee_param) + pext->key_len;
 	param = (struct ieee_param *)_malloc(param_len);
 	if (param == NULL)
-		return -1;
+		return -ENOMEM;
 	memset(param, 0, param_len);
 	param->cmd = IEEE_CMD_SET_ENCRYPTION;
 	memset(param->sta_addr, 0xff, ETH_ALEN);
@@ -1719,7 +1808,7 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
 		alg_name = "CCMP";
 		break;
 	default:
-		return -1;
+		return -EINVAL;
 	}
 	strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
 	if (pext->ext_flags & IW_ENCODE_EXT_GROUP_KEY)
@@ -1785,7 +1874,7 @@ static int dummy(struct net_device *dev,
 		struct iw_request_info *a,
 		union iwreq_data *wrqu, char *b)
 {
-	return -1;
+	return -ENOSYS;
 }
 
 static int r8711_drvext_hdl(struct net_device *dev,
-- 
1.7.6

Re: [PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Larry Finger]

On 07/15/2011 12:11 PM, Ali Bahar wrote:
> The ioctl handlers were frequently returning -1 upon failure. Most of
> these have now been changed to proper errno macros.
> The few remaining ones have been left untouched because either the
> handler is not called (and so cannot be tested), or the function never
> fails (and so cannot be system-tested), or requires new code to
> distinguish its failures.
>
> Signed-off-by: Ali Bahar<ali@internetDog.org>
> Cc: Larry Finger<Larry.Finger@lwfinger.net>
> ---
>   drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  121 +++++++++++++++++++++----
>   1 files changed, 105 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 40e6b5c..bb97d8b 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -526,7 +526,7 @@ static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
>   		memcpy(buf, pie , ielen);
>   		pos = buf;
>   		if (ielen<  RSN_HEADER_LEN) {
> -			ret  = -1;
> +			ret  = -EINVAL;
>   			goto exit;
>   		}
>   		if (r8712_parse_wpa_ie(buf, ielen,&group_cipher,
> @@ -689,6 +689,11 @@ static const long frequency_list[] = {
>   	5825
>   };
>
> +/*
> + * This function intends to handle the Set Freq command.
> + * Currently, the request comes via the Wireless Extensions' SIOCSIWFREQ ioctl.
> + *
> + */

This comment is not needed. The function name is descriptive and it is clear 
that it is part of WEXT.

>   static int r8711_wx_set_freq(struct net_device *dev,
>   			     struct iw_request_info *info,
>   			     union iwreq_data *wrqu, char *extra)
> @@ -723,6 +728,11 @@ static int r8711_wx_set_freq(struct net_device *dev,
>   	return rc;
>   }
>
> +/*
> + * This function intends to handle the Get Freq command.
> + * Currently, the request comes via the Wireless Extensions' SIOCGIWFREQ ioctl.
> + *
> + */

Ditto.

>   static int r8711_wx_get_freq(struct net_device *dev,
>   			     struct iw_request_info *info,
>   			     union iwreq_data *wrqu, char *extra)
> @@ -736,11 +746,21 @@ static int r8711_wx_get_freq(struct net_device *dev,
>   			       pcur_bss->Configuration.DSConfig-1] * 100000;
>   		wrqu->freq.e = 1;
>   		wrqu->freq.i = pcur_bss->Configuration.DSConfig;
> -	} else
> -		return -1;
> +	} else {
> +		return -ENOLINK;
> +	}
>   	return 0;
>   }
>
> +/*
> + * This function intends to handle the Set Mode command.
> + * Currently, the request comes via the Wireless Extensions' SIOCSIWMODE ioctl.
> + *

The part above is not needed.

> + * Note: Currently (July 2011) the call to
> + * r8712_set_802_11_infrastructure_mode() always returns true. Since we have no
> + * specific failure reason, we return EPERM in lieu of general failure.
> + *
> + */
>   static int r8711_wx_set_mode(struct net_device *dev,
>   			     struct iw_request_info *a,
>   			     union iwreq_data *wrqu, char *b)
> @@ -769,10 +789,15 @@ static int r8711_wx_set_mode(struct net_device *dev,
>   	else
>   		r8712_setopmode_cmd(padapter, Ndis802_11AutoUnknown);
>   	if (!r8712_set_802_11_infrastructure_mode(padapter, networkType))
> -		return -1;
> +		return -EPERM; /* Unknown failure. */
>   	return 0;

As r8712_set_802_11_infrastructure_mode() always returns true. it would be 
better to rewrite it as a void function and completely do away with the test here.

>   }
>
> +/*
> + * This function intends to handle the Get Mode command.
> + * Currently, the request comes via the Wireless Extensions' SIOCGIWMODE ioctl.
> + *
> + */

Another senseless comment.

>   static int r8711_wx_get_mode(struct net_device *dev, struct iw_request_info *a,
>   			     union iwreq_data *wrqu, char *b)
>   {
> @@ -975,6 +1000,16 @@ static int r871x_wx_set_priv(struct net_device *dev,
>    * s2. set_802_11_authentication_mode()
>    * s3. set_802_11_encryption_mode()
>    * s4. set_802_11_bssid()
> + *
> + * This function intends to handle the Set AP command, which specifies the
> + * MAC# of a preferred Access Point.
> + * Currently, the request comes via Wireless Extensions' SIOCSIWAP ioctl.
> + *
> + * For this operation to succeed, there is no need for the interface to be Up.
> + *
> + * Note: Currently (July 2011) r8712_set_802_11_bssid() may fail for one of
> + * several reasons. For now, we let -1 be returned as a catch-all failure
> + * indication, until that function's implementation is updated.
>    */

Ditto.

>   static int r8711_wx_set_wap(struct net_device *dev,
>   			 struct iw_request_info *info,
> @@ -995,7 +1030,7 @@ static int r8711_wx_set_wap(struct net_device *dev,
>   	if (padapter->bup == false)
>   		return -1;
>   	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true)
> -		return -1;
> +		return -EBUSY;
>   	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING) == true)
>   		return ret;
>   	if (temp->sa_family != ARPHRD_ETHER)
> @@ -1014,14 +1049,14 @@ static int r8711_wx_set_wap(struct net_device *dev,
>   		if (!memcmp(dst_bssid, temp->sa_data, ETH_ALEN)) {
>   			if (r8712_set_802_11_infrastructure_mode(padapter,
>   			    pnetwork->network.InfrastructureMode) == false)
> -				ret = -1;
> +				ret = -EPERM; /* Unknown failure. */
>   			break;
>   		}
>   	}
>   	spin_unlock_irqrestore(&queue->lock, irqL);
>   	if (!ret) {
>   		if (!r8712_set_802_11_authentication_mode(padapter, authmode))
> -			ret = -1;
> +			ret = -ENOMEM;
>   		else {
>   			if (!r8712_set_802_11_bssid(padapter, temp->sa_data))
>   				ret = -1;
> @@ -1074,6 +1109,19 @@ static int r871x_wx_set_mlme(struct net_device *dev,
>   	return ret;
>   }
>
> +/**
> + *
> + * This function intends to handle the Set Scan command.
> + * Currently, the request comes via Wireless Extensions' SIOCSIWSCAN ioctl.
> + *
> + * For this operation to succeed, the interface is brought Up beforehand.
> + *
> + * Note: It is not clear what value we ought to return when hw_init_completed
> + * is false. If the firmware could not be found/loaded, which errno should we
> + * return?
> + * The same question arises for bDriverStopped being true.
> + *
> + */

The above comment is too long.

>   static int r8711_wx_set_scan(struct net_device *dev,
>   			struct iw_request_info *a,
>   			union iwreq_data *wrqu, char *extra)
> @@ -1088,7 +1136,7 @@ static int r8711_wx_set_scan(struct net_device *dev,
>   		return -1;
>   	}
>   	if (padapter->bup == false)
> -		return -1;
> +		return -ENETDOWN;
>   	if (padapter->hw_init_completed == false)
>   		return -1;
>   	if ((check_fwstate(pmlmepriv, _FW_UNDER_SURVEY|_FW_UNDER_LINKING)) ||
> @@ -1122,6 +1170,13 @@ static int r8711_wx_set_scan(struct net_device *dev,
>   	return 0;
>   }
>
> +/**
> + *
> + * This function intends to handle the Get Scan command.
> + * Currently, the request comes via Wireless Extensions' SIOCGIWSCAN ioctl.
> + *
> + *
> + */

Comment not needed.

>   static int r8711_wx_get_scan(struct net_device *dev,
>   				struct iw_request_info *a,
>   				union iwreq_data *wrqu, char *extra)
> @@ -1169,6 +1224,16 @@ static int r8711_wx_get_scan(struct net_device *dev,
>    * s2. set_802_11_authenticaion_mode()
>    * s3. set_802_11_encryption_mode()
>    * s4. set_802_11_ssid()
> + *
> + * This function intends to handle the Set ESSID command.
> + * Currently, the request comes via the Wireless Extensions' SIOCSIWESSID ioctl.
> + *
> + * For this operation to succeed, there is no need for the interface to be Up.
> + *
> + * Note: Currently (July 2011) the call to
> + * r8712_set_802_11_infrastructure_mode() always returns true. Since we have no
> + * specific failure reason, we return EPERM in lieu of general failure.
> + *
>    */

Why is the business about r8712_set_802_11_infrastructure_mode() repeated?

>   static int r8711_wx_set_essid(struct net_device *dev,
>   				struct iw_request_info *a,
> @@ -1187,7 +1252,7 @@ static int r8711_wx_set_essid(struct net_device *dev,
>   	if (padapter->bup == false)
>   		return -1;
>   	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
> -		return -1;
> +		return -EBUSY;
>   	if (check_fwstate(pmlmepriv, _FW_UNDER_LINKING))
>   		return 0;
>   	if (wrqu->essid.length>  IW_ESSID_MAX_SIZE)
> @@ -1215,7 +1280,7 @@ static int r8711_wx_set_essid(struct net_device *dev,
>   				if (!r8712_set_802_11_infrastructure_mode(
>   				     padapter,
>   				     pnetwork->network.InfrastructureMode))
> -					return -1;
> +					return -EPERM; /* Unknown failure. */
>   				break;
>   			}
>   		}
> @@ -1225,6 +1290,12 @@ static int r8711_wx_set_essid(struct net_device *dev,
>   	return -EINPROGRESS;
>   }
>
> +/**
> + *
> + * This function intends to handle the Get ESSID command.
> + * Currently, the request comes via Wireless Extensions' SIOCGIWESSID ioctl.
> + *
> + */

Not needed.

>   static int r8711_wx_get_essid(struct net_device *dev,
>   				struct iw_request_info *a,
>   				union iwreq_data *wrqu, char *extra)
> @@ -1240,10 +1311,16 @@ static int r8711_wx_get_essid(struct net_device *dev,
>   		memcpy(extra, pcur_bss->Ssid.Ssid, len);
>   		wrqu->essid.flags = 1;
>   	} else
> -		ret = -1;
> +		ret = -ENOLINK;
>   	return ret;
>   }
>
> +/**
> + *
> + * This function intends to handle the Set Rate command.
> + * Currently, the request comes via the Wireless Extensions' SIOCSIWRATE ioctl.
> + *
> + */

Ditto.

>   static int r8711_wx_set_rate(struct net_device *dev,
>   				struct iw_request_info *a,
>   				union iwreq_data *wrqu, char *extra)
> @@ -1312,10 +1389,16 @@ set_rate:
>   			datarates[i] = 0xff;
>   	}
>   	if (r8712_setdatarate_cmd(padapter, datarates) != _SUCCESS)
> -		ret = -1;
> +		ret = -ENOMEM;
>   	return ret;
>   }
>
> +/**
> + *
> + * This function intends to handle the Get Rate command.
> + * Currently, the request comes via the Wireless Extensions' SIOCGIWRATE ioctl.
> + *
> + */

Ditto.

>   static int r8711_wx_get_rate(struct net_device *dev,
>   			     struct iw_request_info *info,
>   			     union iwreq_data *wrqu, char *extra)
> @@ -1371,10 +1454,16 @@ static int r8711_wx_get_rate(struct net_device *dev,
>   			wrqu->bitrate.value = max_rate * 500000;
>   		}
>   	} else
> -		return -1;
> +		return -ENOLINK;
>   	return 0;
>   }
>
> +/**
> + *
> + * This function intends to handle the Get RTS Threshold command.
> + * Currently, the request comes via the Wireless Extensions' SIOCGIWRTS ioctl.
> + *
> + */

Yet again.

>   static int r8711_wx_get_rts(struct net_device *dev,
>   				struct iw_request_info *info,
>   				union iwreq_data *wrqu, char *extra)
> @@ -1701,7 +1790,7 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
>   	param_len = sizeof(struct ieee_param) + pext->key_len;
>   	param = (struct ieee_param *)_malloc(param_len);
>   	if (param == NULL)
> -		return -1;
> +		return -ENOMEM;
>   	memset(param, 0, param_len);
>   	param->cmd = IEEE_CMD_SET_ENCRYPTION;
>   	memset(param->sta_addr, 0xff, ETH_ALEN);
> @@ -1719,7 +1808,7 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
>   		alg_name = "CCMP";
>   		break;
>   	default:
> -		return -1;
> +		return -EINVAL;
>   	}
>   	strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
>   	if (pext->ext_flags&  IW_ENCODE_EXT_GROUP_KEY)
> @@ -1785,7 +1874,7 @@ static int dummy(struct net_device *dev,
>   		struct iw_request_info *a,
>   		union iwreq_data *wrqu, char *b)
>   {
> -	return -1;
> +	return -ENOSYS;
>   }
>
>   static int r8711_drvext_hdl(struct net_device *dev,

Comments in the Linux kernel are relatively rare. Only things that are not 
immediately obvious should receive special annotation.

Larry

Re: [PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Ali Bahar]

On Fri, Jul 15, 2011 at 04:00:10PM -0500, Larry Finger wrote:
> On 07/15/2011 12:11 PM, Ali Bahar wrote:
> >The ioctl handlers were frequently returning -1 upon failure. Most of
> >these have now been changed to proper errno macros.
> >The few remaining ones have been left untouched because either the
> >handler is not called (and so cannot be tested), or the function never
> >fails (and so cannot be system-tested), or requires new code to
> >distinguish its failures.
> >
> >Signed-off-by: Ali Bahar<ali@internetDog.org>
> >Cc: Larry Finger<Larry.Finger@lwfinger.net>
> >---
> >  drivers/staging/rtl8712/rtl871x_ioctl_linux.c |  121 +++++++++++++++++++++----
> >  1 files changed, 105 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >index 40e6b5c..bb97d8b 100644
> >--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >@@ -526,7 +526,7 @@ static int r871x_set_wpa_ie(struct _adapter *padapter, char *pie,
> >  		memcpy(buf, pie , ielen);
> >  		pos = buf;
> >  		if (ielen<  RSN_HEADER_LEN) {
> >-			ret  = -1;
> >+			ret  = -EINVAL;
> >  			goto exit;
> >  		}
> >  		if (r8712_parse_wpa_ie(buf, ielen,&group_cipher,
> >@@ -689,6 +689,11 @@ static const long frequency_list[] = {
> >  	5825
> >  };
> >
> >+/*
> >+ * This function intends to handle the Set Freq command.
> >+ * Currently, the request comes via the Wireless Extensions' SIOCSIWFREQ ioctl.
> >+ *
> >+ */
> 
> This comment is not needed. The function name is descriptive and it
> is clear that it is part of WEXT.

OK.
See below.



> >  static int r8711_wx_set_mode(struct net_device *dev,
> >  			     struct iw_request_info *a,
> >  			     union iwreq_data *wrqu, char *b)
> >@@ -769,10 +789,15 @@ static int r8711_wx_set_mode(struct net_device *dev,
> >  	else
> >  		r8712_setopmode_cmd(padapter, Ndis802_11AutoUnknown);
> >  	if (!r8712_set_802_11_infrastructure_mode(padapter, networkType))
> >-		return -1;
> >+		return -EPERM; /* Unknown failure. */
> >  	return 0;
> 
> As r8712_set_802_11_infrastructure_mode() always returns true. it
> would be better to rewrite it as a void function and completely do
> away with the test here.

Agreed. I was going to wait till after the/any merge with Realtek's
latest. Instead, I'll do it now.
Similarly with the other such checks.




> Comments in the Linux kernel are relatively rare. Only things that
> are not immediately obvious should receive special annotation.

I'll undo all the comments.

Please don't interpret the following as the start of a debate on
commenting Linux; it's not my place to do so. I'm just explaining, for
public record, why the above was done.

I am aware of the current state of the Linux code's documentation.
Habitually, I comment the big-picture as I go along. Industry practice
wrt comments differs from Linux's, of course.
The comments were in the function heads, not in the bodies, as per
Documentation/CodingStyle. 
The big-picture of the functions is clear once you are familiar enough
to begin to modify the code. However, and especially for linux, code
is read a thousand times for every time it is written. Most eyeballs
could do with a line or two of explanation.
To me, Documentation/kernel-docs.txt echoed the need for a
big-picture view.
If the goal is to attract more developers (as it is at least Greg KH's
intent), then Linux's code-base has a long way to go. I meant only to
start chipping away at the task.


Anyways. Didn't mean to give you a headache.


> 
> Larry

Thank you for taking the time. Greatly appreciated.

BTW, if there's any particular work that you feel this driver needs,
do let me know. I've planned to look at the latest Reaktek code for
potential merges, and then see if I can interface it with iw
(cfg80211.) I'll also dig to see what it may take to add monitor-mode
to this.
Any such patches will be posted to the Staging ML.

regards,
ali

Re: [PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Kalle Valo]

Ali Bahar <ali@internetdog.org> writes:

> I am aware of the current state of the Linux code's documentation.
> Habitually, I comment the big-picture as I go along. Industry
> practice wrt comments differs from Linux's, of course. The comments
> were in the function heads, not in the bodies, as per
> Documentation/CodingStyle. The big-picture of the functions is clear
> once you are familiar enough to begin to modify the code. However,
> and especially for linux, code is read a thousand times for every
> time it is written. Most eyeballs could do with a line or two of
> explanation. To me, Documentation/kernel-docs.txt echoed the need
> for a big-picture view. 

The problem with comments is that they quickly get out of date
(usually people change the code but forget the comments). It's better
to write code so that everyone understands and, like Larry said, only
the cases which are not obvious need comments.

> If the goal is to attract more developers (as it is at least Greg
> KH's intent), then Linux's code-base has a long way to go. I meant
> only to start chipping away at the task.

There's always a maintenance cost, even on code comments. If we start
documenting every piece of code, we would have a lot less time writing
the actual code. So there needs to be a trade-off and in Linux it has
been that code should be of good quality so that it's easy to read for
everyone but there will be less comments.

And personally I will always choose good code with few comments over
crappy code with full of comments. So IMHO the style decision made in
Linux kernel has been a good one.

Of course I'm not saying that there should not be any comments. It's
very important to document public interfaces, like mac80211.h and
sdio_func.h, as these are used by a lot of people. 

Also having a general overview how a component/driver works would not
be bad at all, it just should just not clutter the code. That could be
in the beginning of the file or maybe in a separate file. But as I
said, they will easily get obsolete.

-- 
Kalle Valo

Re: [PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Ali Bahar]

Hi Kalle,


On Sat, Jul 16, 2011 at 12:23:42PM +0300, Kalle Valo wrote:
> Ali Bahar <ali@internetdog.org> writes:
> 
> > I am aware of the current state of the Linux code's documentation.
> > Habitually, I comment the big-picture as I go along. Industry
> > practice wrt comments differs from Linux's, of course. The comments
> > were in the function heads, not in the bodies, as per
> > Documentation/CodingStyle. The big-picture of the functions is clear
> > once you are familiar enough to begin to modify the code. However,
> > and especially for linux, code is read a thousand times for every
> > time it is written. Most eyeballs could do with a line or two of
> > explanation. To me, Documentation/kernel-docs.txt echoed the need
> > for a big-picture view. 
> 
> The problem with comments is that they quickly get out of date

Oh boy! I did say I don't want to start a debate, but only to explain
my original intentions. Never the less, I thank you for your response.
It suffices to say that we are mostly in agreement. Key is that my
statements were _not_ about in-body documentation.




> the actual code. So there needs to be a trade-off and in Linux it has
> been that code should be of good quality so that it's easy to read for
> everyone but there will be less comments.

My stance is that, if I were to ever argue otherwise, I should first
dig thru lkml archives for past discussions -- which is only one of
the reason why I don't debate this. :-)


> Kalle Valo

thanks,
ali

Re: [PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Kalle Valo]

Ali Bahar <ali@internetdog.org> writes:

>> The problem with comments is that they quickly get out of date
>
> Oh boy! I did say I don't want to start a debate, but only to explain
> my original intentions.

Yeah, I noticed. I just merely wanted to give you reasoning behind the
comment style we have. Sorry if I sounded like I wanted to start a
flamewar, I did not mean to do that :)

-- 
Kalle Valo

Re: [PATCH 1/2] staging: r8712u: Most return-values changed from -1 to proper errno macros.

[Ali Bahar]

On Sat, Jul 16, 2011 at 02:44:45PM +0300, Kalle Valo wrote:
> Ali Bahar <ali@internetdog.org> writes:
> 
> >> The problem with comments is that they quickly get out of date
> >
> > Oh boy! I did say I don't want to start a debate, but only to explain
> > my original intentions.
> 
> Yeah, I noticed. I just merely wanted to give you reasoning behind the

And it was useful. I either did not know it, or had forgotten it.
Thanks.


> comment style we have. Sorry if I sounded like I wanted to start a
> flamewar, I did not mean to do that :)

You didn't sound that way. 
Email is easily misconstrued; I always try to give the benefit of a
doubt.

regards,
ali

[PATCH 2/2] staging: r8712u: checkpatch errors: trailing whitespace on 2 lines.

[Ali Bahar]

Signed-off-by: Ali Bahar <ali@internetDog.org>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index bb97d8b..6e1c85c 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1009,7 +1009,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
  *
  * Note: Currently (July 2011) r8712_set_802_11_bssid() may fail for one of
  * several reasons. For now, we let -1 be returned as a catch-all failure
- * indication, until that function's implementation is updated. 
+ * indication, until that function's implementation is updated.
  */
 static int r8711_wx_set_wap(struct net_device *dev,
 			 struct iw_request_info *info,
@@ -1121,7 +1121,7 @@ static int r871x_wx_set_mlme(struct net_device *dev,
  * return?
  * The same question arises for bDriverStopped being true.
  *
- */ 
+ */
 static int r8711_wx_set_scan(struct net_device *dev,
 			struct iw_request_info *a,
 			union iwreq_data *wrqu, char *extra)
-- 
1.7.6