* [PATCH 0179/1285] Replace numeric parameter like 0444 with macro
@ 2016-08-02 10:47 Baole Ni
2016-08-02 15:30 ` Deucher, Alexander
0 siblings, 1 reply; 2+ messages in thread
From: Baole Ni @ 2016-08-02 10:47 UTC (permalink / raw)
To: alexander.deucher, christian.koenig, airlied, tony.luck, matt,
m.chehab, lho, dougthompson, bp
Cc: dri-devel, linux-kernel, Jammy.Zhou, David1.Zhou, Flora.Cui,
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/gpu/drm/amd/amdgpu/amdgpu_drv.c | 54 ++++++++++++++++-----------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f888c01..f0c9c29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -86,87 +86,87 @@ unsigned amdgpu_pcie_gen_cap = 0;
unsigned amdgpu_pcie_lane_cap = 0;
MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
-module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
+module_param_named(vramlimit, amdgpu_vram_limit, int, S_IRUSR | S_IWUSR);
MODULE_PARM_DESC(gartsize, "Size of PCIE/IGP gart to setup in megabytes (32, 64, etc., -1 = auto)");
-module_param_named(gartsize, amdgpu_gart_size, int, 0600);
+module_param_named(gartsize, amdgpu_gart_size, int, S_IRUSR | S_IWUSR);
MODULE_PARM_DESC(benchmark, "Run benchmark");
-module_param_named(benchmark, amdgpu_benchmarking, int, 0444);
+module_param_named(benchmark, amdgpu_benchmarking, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(test, "Run tests");
-module_param_named(test, amdgpu_testing, int, 0444);
+module_param_named(test, amdgpu_testing, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(audio, "Audio enable (-1 = auto, 0 = disable, 1 = enable)");
-module_param_named(audio, amdgpu_audio, int, 0444);
+module_param_named(audio, amdgpu_audio, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(disp_priority, "Display Priority (0 = auto, 1 = normal, 2 = high)");
-module_param_named(disp_priority, amdgpu_disp_priority, int, 0444);
+module_param_named(disp_priority, amdgpu_disp_priority, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(hw_i2c, "hw i2c engine enable (0 = disable)");
-module_param_named(hw_i2c, amdgpu_hw_i2c, int, 0444);
+module_param_named(hw_i2c, amdgpu_hw_i2c, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(pcie_gen2, "PCIE Gen2 mode (-1 = auto, 0 = disable, 1 = enable)");
-module_param_named(pcie_gen2, amdgpu_pcie_gen2, int, 0444);
+module_param_named(pcie_gen2, amdgpu_pcie_gen2, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(msi, "MSI support (1 = enable, 0 = disable, -1 = auto)");
-module_param_named(msi, amdgpu_msi, int, 0444);
+module_param_named(msi, amdgpu_msi, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default 0 = disable)");
-module_param_named(lockup_timeout, amdgpu_lockup_timeout, int, 0444);
+module_param_named(lockup_timeout, amdgpu_lockup_timeout, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(dpm, "DPM support (1 = enable, 0 = disable, -1 = auto)");
-module_param_named(dpm, amdgpu_dpm, int, 0444);
+module_param_named(dpm, amdgpu_dpm, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(smc_load_fw, "SMC firmware loading(1 = enable, 0 = disable)");
-module_param_named(smc_load_fw, amdgpu_smc_load_fw, int, 0444);
+module_param_named(smc_load_fw, amdgpu_smc_load_fw, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(aspm, "ASPM support (1 = enable, 0 = disable, -1 = auto)");
-module_param_named(aspm, amdgpu_aspm, int, 0444);
+module_param_named(aspm, amdgpu_aspm, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(runpm, "PX runtime pm (1 = force enable, 0 = disable, -1 = PX only default)");
-module_param_named(runpm, amdgpu_runtime_pm, int, 0444);
+module_param_named(runpm, amdgpu_runtime_pm, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(ip_block_mask, "IP Block Mask (all blocks enabled (default))");
-module_param_named(ip_block_mask, amdgpu_ip_block_mask, uint, 0444);
+module_param_named(ip_block_mask, amdgpu_ip_block_mask, uint, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(bapm, "BAPM support (1 = enable, 0 = disable, -1 = auto)");
-module_param_named(bapm, amdgpu_bapm, int, 0444);
+module_param_named(bapm, amdgpu_bapm, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(deep_color, "Deep Color support (1 = enable, 0 = disable (default))");
-module_param_named(deep_color, amdgpu_deep_color, int, 0444);
+module_param_named(deep_color, amdgpu_deep_color, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(vm_size, "VM address space size in gigabytes (default 64GB)");
-module_param_named(vm_size, amdgpu_vm_size, int, 0444);
+module_param_named(vm_size, amdgpu_vm_size, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(vm_block_size, "VM page table size in bits (default depending on vm_size)");
-module_param_named(vm_block_size, amdgpu_vm_block_size, int, 0444);
+module_param_named(vm_block_size, amdgpu_vm_block_size, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(vm_fault_stop, "Stop on VM fault (0 = never (default), 1 = print first, 2 = always)");
-module_param_named(vm_fault_stop, amdgpu_vm_fault_stop, int, 0444);
+module_param_named(vm_fault_stop, amdgpu_vm_fault_stop, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(vm_debug, "Debug VM handling (0 = disabled (default), 1 = enabled)");
-module_param_named(vm_debug, amdgpu_vm_debug, int, 0644);
+module_param_named(vm_debug, amdgpu_vm_debug, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(exp_hw_support, "experimental hw support (1 = enable, 0 = disable (default))");
-module_param_named(exp_hw_support, amdgpu_exp_hw_support, int, 0444);
+module_param_named(exp_hw_support, amdgpu_exp_hw_support, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(sched_jobs, "the max number of jobs supported in the sw queue (default 32)");
-module_param_named(sched_jobs, amdgpu_sched_jobs, int, 0444);
+module_param_named(sched_jobs, amdgpu_sched_jobs, int, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(sched_hw_submission, "the max number of HW submissions (default 2)");
-module_param_named(sched_hw_submission, amdgpu_sched_hw_submission, int, 0444);
+module_param_named(sched_hw_submission, amdgpu_sched_hw_submission, int, S_IRUSR | S_IRGRP | S_IROTH);
#ifdef CONFIG_DRM_AMD_POWERPLAY
MODULE_PARM_DESC(powerplay, "Powerplay component (1 = enable, 0 = disable, -1 = auto (default))");
-module_param_named(powerplay, amdgpu_powerplay, int, 0444);
+module_param_named(powerplay, amdgpu_powerplay, int, S_IRUSR | S_IRGRP | S_IROTH);
#endif
MODULE_PARM_DESC(pcie_gen_cap, "PCIE Gen Caps (0: autodetect (default))");
-module_param_named(pcie_gen_cap, amdgpu_pcie_gen_cap, uint, 0444);
+module_param_named(pcie_gen_cap, amdgpu_pcie_gen_cap, uint, S_IRUSR | S_IRGRP | S_IROTH);
MODULE_PARM_DESC(pcie_lane_cap, "PCIE Lane Caps (0: autodetect (default))");
-module_param_named(pcie_lane_cap, amdgpu_pcie_lane_cap, uint, 0444);
+module_param_named(pcie_lane_cap, amdgpu_pcie_lane_cap, uint, S_IRUSR | S_IRGRP | S_IROTH);
static const struct pci_device_id pciidlist[] = {
#ifdef CONFIG_DRM_AMDGPU_CIK
--
2.9.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH 0179/1285] Replace numeric parameter like 0444 with macro
2016-08-02 10:47 [PATCH 0179/1285] Replace numeric parameter like 0444 with macro Baole Ni
@ 2016-08-02 15:30 ` Deucher, Alexander
0 siblings, 0 replies; 2+ messages in thread
From: Deucher, Alexander @ 2016-08-02 15:30 UTC (permalink / raw)
To: 'Baole Ni',
Koenig, Christian, airlied, tony.luck, matt, m.chehab, lho,
dougthompson, bp
Cc: dri-devel, linux-kernel, Zhou, Jammy, Zhou, David(ChunMing),
Cui, Flora, chuansheng.liu
> -----Original Message-----
> From: Baole Ni [mailto:baolex.ni@intel.com]
> Sent: Tuesday, August 02, 2016 6:47 AM
> To: Deucher, Alexander; Koenig, Christian; airlied@linux.ie;
> tony.luck@intel.com; matt@codeblueprint.co.uk; m.chehab@samsung.com;
> lho@apm.com; dougthompson@xmission.com; bp@alien8.de
> Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; Zhou,
> Jammy; Zhou, David(ChunMing); Cui, Flora; chuansheng.liu@intel.com;
> baolex.ni@intel.com
> Subject: [PATCH 0179/1285] Replace numeric parameter like 0444 with macro
>
> 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/gpu/drm/amd/amdgpu/amdgpu_drv.c | 54 ++++++++++++++++----
> -------------
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index f888c01..f0c9c29 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -86,87 +86,87 @@ unsigned amdgpu_pcie_gen_cap = 0;
> unsigned amdgpu_pcie_lane_cap = 0;
>
> MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in
> megabytes");
> -module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
> +module_param_named(vramlimit, amdgpu_vram_limit, int, S_IRUSR |
> S_IWUSR);
>
FWIW, I find it much easier to read and understand the numeric representation.
Alex
> MODULE_PARM_DESC(gartsize, "Size of PCIE/IGP gart to setup in
> megabytes (32, 64, etc., -1 = auto)");
> -module_param_named(gartsize, amdgpu_gart_size, int, 0600);
> +module_param_named(gartsize, amdgpu_gart_size, int, S_IRUSR |
> S_IWUSR);
>
> MODULE_PARM_DESC(benchmark, "Run benchmark");
> -module_param_named(benchmark, amdgpu_benchmarking, int, 0444);
> +module_param_named(benchmark, amdgpu_benchmarking, int, S_IRUSR
> | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(test, "Run tests");
> -module_param_named(test, amdgpu_testing, int, 0444);
> +module_param_named(test, amdgpu_testing, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(audio, "Audio enable (-1 = auto, 0 = disable, 1 =
> enable)");
> -module_param_named(audio, amdgpu_audio, int, 0444);
> +module_param_named(audio, amdgpu_audio, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(disp_priority, "Display Priority (0 = auto, 1 = normal, 2
> = high)");
> -module_param_named(disp_priority, amdgpu_disp_priority, int, 0444);
> +module_param_named(disp_priority, amdgpu_disp_priority, int, S_IRUSR |
> S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(hw_i2c, "hw i2c engine enable (0 = disable)");
> -module_param_named(hw_i2c, amdgpu_hw_i2c, int, 0444);
> +module_param_named(hw_i2c, amdgpu_hw_i2c, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(pcie_gen2, "PCIE Gen2 mode (-1 = auto, 0 = disable,
> 1 = enable)");
> -module_param_named(pcie_gen2, amdgpu_pcie_gen2, int, 0444);
> +module_param_named(pcie_gen2, amdgpu_pcie_gen2, int, S_IRUSR |
> S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(msi, "MSI support (1 = enable, 0 = disable, -1 =
> auto)");
> -module_param_named(msi, amdgpu_msi, int, 0444);
> +module_param_named(msi, amdgpu_msi, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default
> 0 = disable)");
> -module_param_named(lockup_timeout, amdgpu_lockup_timeout, int,
> 0444);
> +module_param_named(lockup_timeout, amdgpu_lockup_timeout, int,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(dpm, "DPM support (1 = enable, 0 = disable, -1 =
> auto)");
> -module_param_named(dpm, amdgpu_dpm, int, 0444);
> +module_param_named(dpm, amdgpu_dpm, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(smc_load_fw, "SMC firmware loading(1 = enable, 0 =
> disable)");
> -module_param_named(smc_load_fw, amdgpu_smc_load_fw, int, 0444);
> +module_param_named(smc_load_fw, amdgpu_smc_load_fw, int, S_IRUSR
> | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(aspm, "ASPM support (1 = enable, 0 = disable, -1 =
> auto)");
> -module_param_named(aspm, amdgpu_aspm, int, 0444);
> +module_param_named(aspm, amdgpu_aspm, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(runpm, "PX runtime pm (1 = force enable, 0 =
> disable, -1 = PX only default)");
> -module_param_named(runpm, amdgpu_runtime_pm, int, 0444);
> +module_param_named(runpm, amdgpu_runtime_pm, int, S_IRUSR |
> S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(ip_block_mask, "IP Block Mask (all blocks enabled
> (default))");
> -module_param_named(ip_block_mask, amdgpu_ip_block_mask, uint,
> 0444);
> +module_param_named(ip_block_mask, amdgpu_ip_block_mask, uint,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(bapm, "BAPM support (1 = enable, 0 = disable, -1 =
> auto)");
> -module_param_named(bapm, amdgpu_bapm, int, 0444);
> +module_param_named(bapm, amdgpu_bapm, int, S_IRUSR | S_IRGRP |
> S_IROTH);
>
> MODULE_PARM_DESC(deep_color, "Deep Color support (1 = enable, 0 =
> disable (default))");
> -module_param_named(deep_color, amdgpu_deep_color, int, 0444);
> +module_param_named(deep_color, amdgpu_deep_color, int, S_IRUSR |
> S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(vm_size, "VM address space size in gigabytes
> (default 64GB)");
> -module_param_named(vm_size, amdgpu_vm_size, int, 0444);
> +module_param_named(vm_size, amdgpu_vm_size, int, S_IRUSR |
> S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(vm_block_size, "VM page table size in bits (default
> depending on vm_size)");
> -module_param_named(vm_block_size, amdgpu_vm_block_size, int, 0444);
> +module_param_named(vm_block_size, amdgpu_vm_block_size, int,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(vm_fault_stop, "Stop on VM fault (0 = never
> (default), 1 = print first, 2 = always)");
> -module_param_named(vm_fault_stop, amdgpu_vm_fault_stop, int, 0444);
> +module_param_named(vm_fault_stop, amdgpu_vm_fault_stop, int,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(vm_debug, "Debug VM handling (0 = disabled
> (default), 1 = enabled)");
> -module_param_named(vm_debug, amdgpu_vm_debug, int, 0644);
> +module_param_named(vm_debug, amdgpu_vm_debug, int, S_IRUSR |
> S_IWUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(exp_hw_support, "experimental hw support (1 =
> enable, 0 = disable (default))");
> -module_param_named(exp_hw_support, amdgpu_exp_hw_support, int,
> 0444);
> +module_param_named(exp_hw_support, amdgpu_exp_hw_support, int,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(sched_jobs, "the max number of jobs supported in
> the sw queue (default 32)");
> -module_param_named(sched_jobs, amdgpu_sched_jobs, int, 0444);
> +module_param_named(sched_jobs, amdgpu_sched_jobs, int, S_IRUSR |
> S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(sched_hw_submission, "the max number of HW
> submissions (default 2)");
> -module_param_named(sched_hw_submission,
> amdgpu_sched_hw_submission, int, 0444);
> +module_param_named(sched_hw_submission,
> amdgpu_sched_hw_submission, int, S_IRUSR | S_IRGRP | S_IROTH);
>
> #ifdef CONFIG_DRM_AMD_POWERPLAY
> MODULE_PARM_DESC(powerplay, "Powerplay component (1 = enable, 0 =
> disable, -1 = auto (default))");
> -module_param_named(powerplay, amdgpu_powerplay, int, 0444);
> +module_param_named(powerplay, amdgpu_powerplay, int, S_IRUSR |
> S_IRGRP | S_IROTH);
> #endif
>
> MODULE_PARM_DESC(pcie_gen_cap, "PCIE Gen Caps (0: autodetect
> (default))");
> -module_param_named(pcie_gen_cap, amdgpu_pcie_gen_cap, uint, 0444);
> +module_param_named(pcie_gen_cap, amdgpu_pcie_gen_cap, uint,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> MODULE_PARM_DESC(pcie_lane_cap, "PCIE Lane Caps (0: autodetect
> (default))");
> -module_param_named(pcie_lane_cap, amdgpu_pcie_lane_cap, uint,
> 0444);
> +module_param_named(pcie_lane_cap, amdgpu_pcie_lane_cap, uint,
> S_IRUSR | S_IRGRP | S_IROTH);
>
> static const struct pci_device_id pciidlist[] = {
> #ifdef CONFIG_DRM_AMDGPU_CIK
> --
> 2.9.2
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-08-02 18:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-02 10:47 [PATCH 0179/1285] Replace numeric parameter like 0444 with macro Baole Ni
2016-08-02 15:30 ` Deucher, Alexander
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).