convert airo wireless driver to module_param
Subsystems (2)
PATCH 1/1 convert airo wireless driver to module_param
Replaces MODULE_PARM with module_param calls in airo wireless driver.
net/wireless/airo.c renamed from net/wireless/airo.c.orig +14 -14
@@ -246,36 +246,36 @@ MODULE_DESCRIPTION("Support for Cisco/Ai for PCMCIA when used with airo_cs.");MODULE_LICENSE ("Dual BSD/GPL" );MODULE_SUPPORTED_DEVICE ("Aironet 4500, 4800 and Cisco 340/350" ); -MODULE_PARM (io,"1-4i" ); -MODULE_PARM (irq,"1-4i" ); -MODULE_PARM (basic_rate,"i" ); -MODULE_PARM (rates,"1-8i" ); -MODULE_PARM (ssids,"1-3s" ); -MODULE_PARM (auto_wep,"i" ); +module_param_array (io, int,NULL ,0444 ); +module_param_array (irq, int,NULL ,0444 ); +module_param (basic_rate, int,0644 ); +module_param_array (rates, int,NULL ,0644 ); +module_param_array (ssids, int,NULL ,0644 ); +module_param (auto_wep, int,0644 );MODULE_PARM_DESC (auto_wep,"If non-zero, the driver will keep looping through \ the authentication options until an association is made. The value of \ auto_wep is number of the wep keys to check. A value of 2 will try using \ the key at index 0 and index 1." ); -MODULE_PARM (aux_bap,"i" ); +module_param (aux_bap, int,0644 );MODULE_PARM_DESC (aux_bap,"If non-zero, the driver will switch into a mode \ than seems to work better for older cards with some older buses. Before \ switching it checks that the switch is needed." ); -MODULE_PARM (maxencrypt,"i" ); +module_param (maxencrypt, int,0644 );MODULE_PARM_DESC (maxencrypt,"The maximum speed that the card can do \ encryption. Units are in 512kbs. Zero (default) means there is no limit. \ Older cards used to be limited to 2mbs (4)." ); -MODULE_PARM (adhoc,"i" ); +module_param (adhoc, int,0644 );MODULE_PARM_DESC (adhoc,"If non-zero, the card will start in adhoc mode." ); -MODULE_PARM (probe,"i" ); +module_param (probe, int,0644 );MODULE_PARM_DESC (probe,"If zero, the driver won't start the card." ); -MODULE_PARM (proc_uid,"i" ); +module_param (proc_uid, int,0644 );MODULE_PARM_DESC (proc_uid,"The uid that the /proc files will belong to." ); -MODULE_PARM (proc_gid,"i" ); +module_param (proc_gid, int,0644 );MODULE_PARM_DESC (proc_gid,"The gid that the /proc files will belong to." ); -MODULE_PARM (airo_perm,"i" ); +module_param (airo_perm, int,0644 );MODULE_PARM_DESC (airo_perm,"The permission bits of /proc/[driver/]aironet." ); -MODULE_PARM (proc_perm,"i" ); +module_param (proc_perm, int,0644 );MODULE_PARM_DESC (proc_perm,"The permission bits of the files in /proc" );/* This is a kind of sloppy hack to get this information to OUT4500 and
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 9 lines · Stefan Sperling
Replaces
MODULE_PARMwithmodule_paramcalls in airo wireless driver.net/wireless/airo.c renamed from net/wireless/airo.c.orig +0
-1@@ -246,36 +246,36 @@ MODULE_DESCRIPTION("Support for Cisco/Ai for PCMCIA when used with airo_cs.");MODULE_LICENSE ("Dual BSD/GPL" );MODULE_SUPPORTED_DEVICE ("Aironet 4500, 4800 and Cisco 340/350" ); -MODULE_PARM (io,"1-4i" );
net/wireless/airo.c MODULE_DESCRIPTION("Support for Cisco/Ai · line 253
Quoted 10 lines
net/wireless/airo.c renamed from net/wireless/airo.c.orig +5
-5-MODULE_PARM (irq,"1-4i" ); -MODULE_PARM (basic_rate,"i" ); -MODULE_PARM (rates,"1-8i" ); -MODULE_PARM (ssids,"1-3s" ); -MODULE_PARM (auto_wep,"i" ); +module_param_array (io, int,NULL ,0444 ); +module_param_array (irq, int,NULL ,0444 ); +module_param (basic_rate, int,0644 ); +module_param_array (rates, int,NULL ,0644 ); +module_param_array (ssids, int,NULL ,0644 );
Why make these writable, even by root?
Why make them visible?
Rusty's first set of module_param() conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.
Many of these shouldn't be exposed in sysfs IMO.
Quoted 34 lines
net/wireless/airo.c renamed from net/wireless/airo.c.orig +9
-8+module_param (auto_wep, int,0644 );MODULE_PARM_DESC (auto_wep,"If non-zero, the driver will keep looping through \ the authentication options until an association is made. The value of \ auto_wep is number of the wep keys to check. A value of 2 will try using \ the key at index 0 and index 1." ); -MODULE_PARM (aux_bap,"i" ); +module_param (aux_bap, int,0644 );MODULE_PARM_DESC (aux_bap,"If non-zero, the driver will switch into a mode \ than seems to work better for older cards with some older buses. Before \ switching it checks that the switch is needed." ); -MODULE_PARM (maxencrypt,"i" ); +module_param (maxencrypt, int,0644 );MODULE_PARM_DESC (maxencrypt,"The maximum speed that the card can do \ encryption. Units are in 512kbs. Zero (default) means there is no limit. \ Older cards used to be limited to 2mbs (4)." ); -MODULE_PARM (adhoc,"i" ); +module_param (adhoc, int,0644 );MODULE_PARM_DESC (adhoc,"If non-zero, the card will start in adhoc mode." ); -MODULE_PARM (probe,"i" ); +module_param (probe, int,0644 );MODULE_PARM_DESC (probe,"If zero, the driver won't start the card." ); -MODULE_PARM (proc_uid,"i" ); +module_param (proc_uid, int,0644 );MODULE_PARM_DESC (proc_uid,"The uid that the /proc files will belong to." ); -MODULE_PARM (proc_gid,"i" ); +module_param (proc_gid, int,0644 );MODULE_PARM_DESC (proc_gid,"The gid that the /proc files will belong to." ); -MODULE_PARM (airo_perm,"i" ); +module_param (airo_perm, int,0644 );MODULE_PARM_DESC (airo_perm,"The permission bits of /proc/[driver/]aironet." ); -MODULE_PARM (proc_perm,"i" ); +module_param (proc_perm, int,0644 );MODULE_PARM_DESC (proc_perm,"The permission bits of the files in /proc" );
Signature · 5 lines
~Randy _______________________________________________ Kernel-janitors mailing list [email protected] http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 9 lines · Randy.Dunlap
Why make these writable, even by root?
Why make them visible?Rusty's first set of
module_param()conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.Many of these shouldn't be exposed in sysfs IMO.
OK, I wasn't too sure about permissions anyway.
I thought the general idea was to allow autoconfiguration through sysfs, and expose a driver's internal state to diagnostic tools in user space. So I only 0444'd parameters that should obviously not be changed at runtime, such as IO ports and IRQs.
I'll try again and make everything invisible except debug/verbosity flags. Since you mentioned bitmasks, is it safe to make irq bitmasks writable as well? I made them read-only.
thanks
stefan
Signature · 2 lines
Give me a fish and I will eat today. Teach me to fish and I will eat forever.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 6 lines · Randy.Dunlap
Stefan Sperling wrote:
Rusty's first set of
module_param()conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.
Not in this one:
http://marc.theaimsgroup.com/?l=linux-kernel&m=109826168201622&w=2
This is the reference link from the janitors TODO file. Everything 0444, except a debug flag, which is 0644.
Can someone please clarify the "official guidelines" for module parameter permissions in sysfs? Shall I use 0 or 0444 for parameters that should not be modified, i.e. everything except debug and verbosity flags?
thanks
stefan
Signature · 2 lines
Give me a fish and I will eat today. Teach me to fish and I will eat forever.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 12 lines · Stefan Sperling
Quoted 8 lines · Randy.Dunlap
Stefan Sperling wrote:
Rusty's first set of
module_param()conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.
Quoted 3 lines
Not in this one:
http://marc.theaimsgroup.com/?l=linux-kernel&m9826168201622&w=2
True. That's why we post patches and have review (one reason).
Quoted 7 lines
This is the reference link from the janitors TODO file. Everything 0444, except a debug flag, which is 0644.
Can someone please clarify the "official guidelines" for module parameter permissions in sysfs? Shall I use 0 or 0444 for parameters that should not be modified, i.e. everything except debug and verbosity flags?
Compare with Rusty's (other) conversion patch (with wrap):
Greg KH just told me (paraphrased):
if it can be modified on the fly, put it into sysfs (permissions non-zero). If it cannot be modified, make permissions = 0.
Signature · 5 lines
~Randy _______________________________________________ Kernel-janitors mailing list [email protected] http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 10 lines · Stefan Sperling
Quoted 6 lines · Randy.Dunlap
Stefan Sperling wrote:
Rusty's first set of
module_param()conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.Not in this one:
Quoted 7 lines
http://marc.theaimsgroup.com/?l=linux-kernel&m=109826168201622&w=2
This is the reference link from the janitors TODO file. Everything 0444, except a debug flag, which is 0644.
Can someone please clarify the "official guidelines" for module parameter permissions in sysfs?
"When it makes sense to have it exposed to userspace"
Yeah, it's vague, sorry, but it all depends.
For things that can be changed on the fly, expose it.
For things that don't really matter, and no one will ever look them up, don't. I think the irq stuff is in the "don't" category, as almost no one messes with them anymore.
Does this help?
thanks,
greg k-h
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 19 lines · Stefan Sperling
Quoted 9 lines · Randy.Dunlap
Why make these writable, even by root?
Why make them visible?Rusty's first set of
module_param()conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.Many of these shouldn't be exposed in sysfs IMO.
OK, I wasn't too sure about permissions anyway.
I thought the general idea was to allow autoconfiguration through sysfs, and expose a driver's internal state to diagnostic tools in user space. So I only 0444'd parameters that should obviously not be changed at runtime, such as IO ports and IRQs.
Quoted 3 lines
I'll try again and make everything invisible except debug/verbosity flags. Since you mentioned bitmasks, is it safe to make irq bitmasks writable as well? I made them read-only.
I mentioned bitmasks because some debug variables aren't boolean, they are bit flags, or sometimes they are an integer (debug level).
No, I don't think that irq bitmasks should be writeable. Having any system reource data writeable (like IRQ) is asking for trouble IMO.
Signature · 5 lines
~Randy _______________________________________________ Kernel-janitors mailing list [email protected] http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 7 lines · Randy.Dunlap
Quoted 4 lines
I'll try again and make everything invisible except debug/verbosity flags. Since you mentioned bitmasks, is it safe to make irq bitmasks writable as well?
I made them read-only.I mentioned bitmasks because some debug variables aren't boolean, they are bit flags, or sometimes they are an integer (debug level).
Oops, I misread that one, sorry.
I only found IRQ bitmasks among the parameters, thus I mixed that up...
Quoted 3 lines
No, I don't think that irq bitmasks should be writeable. Having any system reource data writeable (like IRQ) is asking for trouble IMO.
no way! ;-)
thanks
stefan
Signature · 2 lines
Give me a fish and I will eat today. Teach me to fish and I will eat forever.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 5 lines · Greg KH
Quoted 2 lines
Can someone please clarify the "official guidelines" for module parameter permissions in sysfs?
"When it makes sense to have it exposed to userspace"
Quoted 8 lines
Yeah, it's vague, sorry, but it all depends.
For things that can be changed on the fly, expose it.
For things that don't really matter, and no one will ever look them up, don't. I think the irq stuff is in the "don't" category, as almost no one messes with them anymore.Does this help?
It does.
Quoted 3 lines
thanks,
greg k-h
thanks
stefan
Signature · 2 lines
Give me a fish and I will eat today. Teach me to fish and I will eat forever.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors
Quoted 15 lines · Randy.Dunlap
Quoted 9 lines
Quoted 4 lines
Rusty's first set of
module_param()conversion patches, which I mostly followed, use permission (last parameter) of 0 (not visible in sysfs) except for fields that can be modified. Often this is a "debug" or "verbose" flag or bitmask.Not in this one:
http://marc.theaimsgroup.com/?l=linux-kernel&m=109826168201622&w=2
True. That's why we post patches and have review (one reason).
Quoted 3 lines
This is the reference link from the janitors TODO file. Everything 0444, except a debug flag, which is 0644.
Quoted 8 lines
Quoted 4 lines
Can someone please clarify the "official guidelines" for module parameter permissions in sysfs? Shall I use 0 or 0444 for parameters that should not be modified, i.e. everything except debug and verbosity flags?
Compare with Rusty's (other) conversion patch (with wrap):
Yikes! He's quick.
He's already converted some of the drivers I have been converting. Last time I looked he hadn't touched the wireless drivers yet...
Well, at least he did not convert all of them yet... ;-) I'll post the missing ones soon so.
stefan
Signature · 2 lines
Give me a fish and I will eat today. Teach me to fish and I will eat forever.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors