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 <[email protected]>
---
John,
This material is for 2.6.40.
Larry
On 05/03/2011 04:09 PM, G?bor Stefanik wrote:
> 2011/5/3 Larry Finger<[email protected]>:
>> On 05/03/2011 03:38 PM, G?bor Stefanik wrote:
>>>
>>> 2011/5/3 Larry Finger<[email protected]>:
>>>>
>>>> On 05/03/2011 03:19 PM, G?bor Stefanik wrote:
>>>>>
>>>>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<[email protected]>
>>>>> 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
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
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
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
On Tue, May 3, 2011 at 6:45 PM, Larry Finger <[email protected]> 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
> This set of patches introduces a driver for the RTL8192SE PCI devices.
Any estimates on the *SU driver? :)
2011/5/3 Larry Finger <[email protected]>:
> On 05/03/2011 03:38 PM, G?bor Stefanik wrote:
>>
>> 2011/5/3 Larry Finger<[email protected]>:
>>>
>>> On 05/03/2011 03:19 PM, G?bor Stefanik wrote:
>>>>
>>>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<[email protected]>
>>>> ?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. :-)
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
On 05/03/2011 03:38 PM, G?bor Stefanik wrote:
> 2011/5/3 Larry Finger<[email protected]>:
>> On 05/03/2011 03:19 PM, G?bor Stefanik wrote:
>>>
>>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<[email protected]>
>>> 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
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
On 05/03/2011 03:19 PM, G?bor Stefanik wrote:
> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<[email protected]> 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
2011/5/3 Larry Finger <[email protected]>:
> On 05/03/2011 03:19 PM, G?bor Stefanik wrote:
>>
>> On Tue, May 3, 2011 at 6:45 PM, Larry Finger<[email protected]>
>> ?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. :-)
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
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
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
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<[email protected]>:
> >>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 ~$
Ali,
On Wed, Jun 22, 2011 at 11:00, Ali Bahar <[email protected]> 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: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
On Wed, Jun 22, 2011 at 4:03 AM, Julian Calaby <[email protected]> wrote:
> Ali,
>
> On Wed, Jun 22, 2011 at 11:00, Ali Bahar <[email protected]> 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: [email protected]
> 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
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
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
> >>>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
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
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
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
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
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
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
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<[email protected]>
> >Cc: Larry Finger<[email protected]>
> >---
> > 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
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
Signed-off-by: Ali Bahar <[email protected]>
Cc: Larry Finger <[email protected]>
---
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
On Sat, Jul 16, 2011 at 02:44:45PM +0300, Kalle Valo wrote:
> Ali Bahar <[email protected]> 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
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 <[email protected]>
Cc: Larry Finger <[email protected]>
---
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
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<[email protected]>
> Cc: Larry Finger<[email protected]>
> ---
> 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
Ali Bahar <[email protected]> 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
Ali Bahar <[email protected]> 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
Hi Kalle,
On Sat, Jul 16, 2011 at 12:23:42PM +0300, Kalle Valo wrote:
> Ali Bahar <[email protected]> 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
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))
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
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