X86 platform drivers
 help / color / mirror / Atom feed
* [PATCH 0841/1285] Replace numeric parameter like 0444 with macro
@ 2016-08-02 11:51 Baole Ni
  2016-08-11 21:21 ` Peter Feuerer
  0 siblings, 1 reply; 3+ messages in thread
From: Baole Ni @ 2016-08-02 11:51 UTC (permalink / raw)
  To: peter, dvhart, david.vrabel, jgross, bhelgaas, m.chehab, pawel,
	m.szyprowski, kyungmin.park, k.kozlowski
  Cc: platform-driver-x86, linux-kernel, chuansheng.liu, baolex.ni

I find that the developers often just specified the numeric value
when calling a macro which is defined with a parameter for access permission.
As we know, these numeric value for access permission have had the corresponding macro,
and that using macro can improve the robustness and readability of the code,
thus, I suggest replacing the numeric parameter with the macro.

Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
Signed-off-by: Baole Ni <baolex.ni@intel.com>
---
 drivers/platform/x86/acerhdf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 460fa67..a4706dd 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -96,13 +96,13 @@ static struct platform_device *acerhdf_dev;
 
 module_param(kernelmode, uint, 0);
 MODULE_PARM_DESC(kernelmode, "Kernel mode fan control on / off");
-module_param(interval, uint, 0600);
+module_param(interval, uint, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(interval, "Polling interval of temperature check");
-module_param(fanon, uint, 0600);
+module_param(fanon, uint, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(fanon, "Turn the fan on above this temperature");
-module_param(fanoff, uint, 0600);
+module_param(fanoff, uint, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(fanoff, "Turn the fan off below this temperature");
-module_param(verbose, uint, 0600);
+module_param(verbose, uint, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(verbose, "Enable verbose dmesg output");
 module_param_string(force_bios, force_bios, 16, 0);
 MODULE_PARM_DESC(force_bios, "Force BIOS version and omit BIOS check");
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 0841/1285] Replace numeric parameter like 0444 with macro
  2016-08-02 11:51 [PATCH 0841/1285] Replace numeric parameter like 0444 with macro Baole Ni
@ 2016-08-11 21:21 ` Peter Feuerer
  2016-08-19 20:02   ` Darren Hart
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Feuerer @ 2016-08-11 21:21 UTC (permalink / raw)
  To: Baole Ni, dvhart, david.vrabel, jgross, bhelgaas, m.chehab, pawel,
	m.szyprowski, kyungmin.park, k.kozlowski
  Cc: platform-driver-x86, linux-kernel, chuansheng.liu

2. August 2016 14:29 Uhr, "Baole Ni" <baolex.ni@intel.com> schrieb:
> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
> 
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> Signed-off-by: Baole Ni <baolex.ni@intel.com>

Acked-by: Peter Feuerer <peter@piie.net>

> ---
> drivers/platform/x86/acerhdf.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 460fa67..a4706dd 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -96,13 +96,13 @@ static struct platform_device *acerhdf_dev;
> 
> module_param(kernelmode, uint, 0);
> MODULE_PARM_DESC(kernelmode, "Kernel mode fan control on / off");
> -module_param(interval, uint, 0600);
> +module_param(interval, uint, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(interval, "Polling interval of temperature check");
> -module_param(fanon, uint, 0600);
> +module_param(fanon, uint, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(fanon, "Turn the fan on above this temperature");
> -module_param(fanoff, uint, 0600);
> +module_param(fanoff, uint, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(fanoff, "Turn the fan off below this temperature");
> -module_param(verbose, uint, 0600);
> +module_param(verbose, uint, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(verbose, "Enable verbose dmesg output");
> module_param_string(force_bios, force_bios, 16, 0);
> MODULE_PARM_DESC(force_bios, "Force BIOS version and omit BIOS check");
> -- 
> 2.9.2

-- 
thanks,
--peter;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 0841/1285] Replace numeric parameter like 0444 with macro
  2016-08-11 21:21 ` Peter Feuerer
@ 2016-08-19 20:02   ` Darren Hart
  0 siblings, 0 replies; 3+ messages in thread
From: Darren Hart @ 2016-08-19 20:02 UTC (permalink / raw)
  To: Peter Feuerer
  Cc: Baole Ni, david.vrabel, jgross, bhelgaas, m.chehab, pawel,
	m.szyprowski, kyungmin.park, k.kozlowski, platform-driver-x86,
	linux-kernel, chuansheng.liu

On Thu, Aug 11, 2016 at 09:21:48PM +0000, Peter Feuerer wrote:
> 2. August 2016 14:29 Uhr, "Baole Ni" <baolex.ni@intel.com> schrieb:
> > I find that the developers often just specified the numeric value
> > when calling a macro which is defined with a parameter for access permission.
> > As we know, these numeric value for access permission have had the corresponding macro,
> > and that using macro can improve the robustness and readability of the code,
> > thus, I suggest replacing the numeric parameter with the macro.
> > 
> > Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> > Signed-off-by: Baole Ni <baolex.ni@intel.com>
> 
> Acked-by: Peter Feuerer <peter@piie.net>

Due to objections elsewhere in the platform drivers subsystem, I have rejected
these patches as a group, leaving it to the authors to make the argument for
inclusion if they disagree with the decision.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-08-19 20:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-02 11:51 [PATCH 0841/1285] Replace numeric parameter like 0444 with macro Baole Ni
2016-08-11 21:21 ` Peter Feuerer
2016-08-19 20:02   ` Darren Hart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox