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/i915/i915_params.c | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 1779f02..7184e06 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = { .inject_load_failure = 0, };
-module_param_named(modeset, i915.modeset, int, 0400); +module_param_named(modeset, i915.modeset, int, S_IRUSR); MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (0=disable, " "1=on, -1=force vga console preference [default])");
-module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 0600); +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(panel_ignore_lid, "Override lid status (0=autodetect, 1=autodetect disabled [default], " "-1=force lid closed, -2=force lid open)");
-module_param_named_unsafe(semaphores, i915.semaphores, int, 0400); +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR); MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring sync " "(default: -1 (use per-chip defaults))");
-module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400); +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR); MODULE_PARM_DESC(enable_rc6, "Enable power-saving render C-state 6. " "Different stages can be selected via bitmask values " @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6, "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)");
-module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400); +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR); MODULE_PARM_DESC(enable_dc, "Enable power-saving display C-states. " "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
-module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600); +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_fbc, "Enable frame buffer compression for power savings " "(default: -1 (use per-chip default))");
-module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 0400); +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, S_IRUSR); MODULE_PARM_DESC(lvds_channel_mode, "Specify LVDS channel mode " "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
-module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600); +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(lvds_use_ssc, "Use Spread Spectrum Clock with panels [LVDS/eDP] " "(default: auto from VBT)");
-module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 0400); +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, S_IRUSR); MODULE_PARM_DESC(vbt_sdvo_panel_type, "Override/Ignore selection of SDVO panel mode in the VBT " "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
-module_param_named_unsafe(reset, i915.reset, bool, 0600); +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
-module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 0644); +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(enable_hangcheck, "Periodically check GPU activity for detecting hangs. " "WARNING: Disabling this can cause system wide hangs. " "(default: true)");
-module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400); +module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, S_IRUSR); MODULE_PARM_DESC(enable_ppgtt, "Override PPGTT usage. " "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)");
-module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 0400); +module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, S_IRUSR); MODULE_PARM_DESC(enable_execlists, "Override execlists usage. " "(-1=auto [default], 0=disabled, 1=enabled)");
-module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); +module_param_named_unsafe(enable_psr, i915.enable_psr, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_psr, "Enable PSR " "(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) " "Default: -1 (use per-chip default)");
-module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0400); +module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, S_IRUSR); MODULE_PARM_DESC(preliminary_hw_support, "Enable preliminary hardware support.");
-module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, 0400); +module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, S_IRUSR); MODULE_PARM_DESC(disable_power_well, "Disable display power wells when possible " "(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)");
-module_param_named_unsafe(enable_ips, i915.enable_ips, int, 0600); +module_param_named_unsafe(enable_ips, i915.enable_ips, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
-module_param_named(fastboot, i915.fastboot, bool, 0600); +module_param_named(fastboot, i915.fastboot, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time (default: false)");
-module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600); +module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(prefault_disable, "Disable page prefaulting for pread/pwrite/reloc (default:false). " "For developers only.");
-module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, 0600); +module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(load_detect_test, "Force-enable the VGA load detect code for testing (default:false). " "For developers only.");
-module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, 0600); +module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness " "(-1 force normal, 0 machine defaults, 1 force inversion), please " @@ -166,47 +166,47 @@ MODULE_PARM_DESC(invert_brightness, "to dri-devel@lists.freedesktop.org, if your machine needs it. " "It will then be included in an upcoming module version.");
-module_param_named(disable_display, i915.disable_display, bool, 0400); +module_param_named(disable_display, i915.disable_display, bool, S_IRUSR); MODULE_PARM_DESC(disable_display, "Disable display (default: false)");
-module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, 0600); +module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_cmd_parser, "Enable command parsing (1=enabled [default], 0=disabled)");
-module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, 0600); +module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
-module_param_named(mmio_debug, i915.mmio_debug, int, 0600); +module_param_named(mmio_debug, i915.mmio_debug, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(mmio_debug, "Enable the MMIO debug code for the first N failures (default: off). " "This may negatively affect performance.");
-module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, 0600); +module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(verbose_state_checks, "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions.");
-module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0600); +module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(nuclear_pageflip, "Force atomic modeset functionality; asynchronous mode is not yet supported. (default: false).");
/* WA to get away with the default setting in VBT for early platforms.Will be removed */ -module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0400); +module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, S_IRUSR); MODULE_PARM_DESC(edp_vswing, "Ignore/Override vswing pre-emph table selection from VBT " "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))");
-module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400); +module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, S_IRUSR); MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)");
-module_param_named(guc_log_level, i915.guc_log_level, int, 0400); +module_param_named(guc_log_level, i915.guc_log_level, int, S_IRUSR); MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
-module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600); +module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_dp_mst, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); -module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400); +module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, S_IRUSR); MODULE_PARM_DESC(inject_load_failure, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)");
On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
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/i915/i915_params.c | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 1779f02..7184e06 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = { .inject_load_failure = 0, };
-module_param_named(modeset, i915.modeset, int, 0400); +module_param_named(modeset, i915.modeset, int, S_IRUSR);
At least I can't read those macros. Octal is much clearer IMO.
MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (0=disable, " "1=on, -1=force vga console preference [default])");
-module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 0600); +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(panel_ignore_lid, "Override lid status (0=autodetect, 1=autodetect disabled [default], " "-1=force lid closed, -2=force lid open)");
-module_param_named_unsafe(semaphores, i915.semaphores, int, 0400); +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR); MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring sync " "(default: -1 (use per-chip defaults))");
-module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400); +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR); MODULE_PARM_DESC(enable_rc6, "Enable power-saving render C-state 6. " "Different stages can be selected via bitmask values " @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6, "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)");
-module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400); +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR); MODULE_PARM_DESC(enable_dc, "Enable power-saving display C-states. " "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
-module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600); +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_fbc, "Enable frame buffer compression for power savings " "(default: -1 (use per-chip default))");
-module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 0400); +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, S_IRUSR); MODULE_PARM_DESC(lvds_channel_mode, "Specify LVDS channel mode " "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
-module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600); +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(lvds_use_ssc, "Use Spread Spectrum Clock with panels [LVDS/eDP] " "(default: auto from VBT)");
-module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 0400); +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, S_IRUSR); MODULE_PARM_DESC(vbt_sdvo_panel_type, "Override/Ignore selection of SDVO panel mode in the VBT " "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
-module_param_named_unsafe(reset, i915.reset, bool, 0600); +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
-module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 0644); +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(enable_hangcheck, "Periodically check GPU activity for detecting hangs. " "WARNING: Disabling this can cause system wide hangs. " "(default: true)");
-module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400); +module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, S_IRUSR); MODULE_PARM_DESC(enable_ppgtt, "Override PPGTT usage. " "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)");
-module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 0400); +module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, S_IRUSR); MODULE_PARM_DESC(enable_execlists, "Override execlists usage. " "(-1=auto [default], 0=disabled, 1=enabled)");
-module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); +module_param_named_unsafe(enable_psr, i915.enable_psr, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_psr, "Enable PSR " "(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) " "Default: -1 (use per-chip default)");
-module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0400); +module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, S_IRUSR); MODULE_PARM_DESC(preliminary_hw_support, "Enable preliminary hardware support.");
-module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, 0400); +module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, S_IRUSR); MODULE_PARM_DESC(disable_power_well, "Disable display power wells when possible " "(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)");
-module_param_named_unsafe(enable_ips, i915.enable_ips, int, 0600); +module_param_named_unsafe(enable_ips, i915.enable_ips, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
-module_param_named(fastboot, i915.fastboot, bool, 0600); +module_param_named(fastboot, i915.fastboot, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time (default: false)");
-module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600); +module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(prefault_disable, "Disable page prefaulting for pread/pwrite/reloc (default:false). " "For developers only.");
-module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, 0600); +module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(load_detect_test, "Force-enable the VGA load detect code for testing (default:false). " "For developers only.");
-module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, 0600); +module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness " "(-1 force normal, 0 machine defaults, 1 force inversion), please " @@ -166,47 +166,47 @@ MODULE_PARM_DESC(invert_brightness, "to dri-devel@lists.freedesktop.org, if your machine needs it. " "It will then be included in an upcoming module version.");
-module_param_named(disable_display, i915.disable_display, bool, 0400); +module_param_named(disable_display, i915.disable_display, bool, S_IRUSR); MODULE_PARM_DESC(disable_display, "Disable display (default: false)");
-module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, 0600); +module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_cmd_parser, "Enable command parsing (1=enabled [default], 0=disabled)");
-module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, 0600); +module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
-module_param_named(mmio_debug, i915.mmio_debug, int, 0600); +module_param_named(mmio_debug, i915.mmio_debug, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(mmio_debug, "Enable the MMIO debug code for the first N failures (default: off). " "This may negatively affect performance.");
-module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, 0600); +module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(verbose_state_checks, "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions.");
-module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0600); +module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(nuclear_pageflip, "Force atomic modeset functionality; asynchronous mode is not yet supported. (default: false).");
/* WA to get away with the default setting in VBT for early platforms.Will be removed */ -module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0400); +module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, S_IRUSR); MODULE_PARM_DESC(edp_vswing, "Ignore/Override vswing pre-emph table selection from VBT " "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))");
-module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400); +module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, S_IRUSR); MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)");
-module_param_named(guc_log_level, i915.guc_log_level, int, 0400); +module_param_named(guc_log_level, i915.guc_log_level, int, S_IRUSR); MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
-module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600); +module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_dp_mst, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); -module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400); +module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, S_IRUSR); MODULE_PARM_DESC(inject_load_failure, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); -- 2.9.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 02 Aug 2016, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
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/i915/i915_params.c | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 1779f02..7184e06 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = { .inject_load_failure = 0, };
-module_param_named(modeset, i915.modeset, int, 0400); +module_param_named(modeset, i915.modeset, int, S_IRUSR);
At least I can't read those macros. Octal is much clearer IMO.
Besides I think we should consider making most if not all of these parameters group/other readable. Which would then potentially become S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH instead of just 0644.
BR, Jani.
MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (0=disable, " "1=on, -1=force vga console preference [default])");
-module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 0600); +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(panel_ignore_lid, "Override lid status (0=autodetect, 1=autodetect disabled [default], " "-1=force lid closed, -2=force lid open)");
-module_param_named_unsafe(semaphores, i915.semaphores, int, 0400); +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR); MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring sync " "(default: -1 (use per-chip defaults))");
-module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400); +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR); MODULE_PARM_DESC(enable_rc6, "Enable power-saving render C-state 6. " "Different stages can be selected via bitmask values " @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6, "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)");
-module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400); +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR); MODULE_PARM_DESC(enable_dc, "Enable power-saving display C-states. " "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
-module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600); +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_fbc, "Enable frame buffer compression for power savings " "(default: -1 (use per-chip default))");
-module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 0400); +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, S_IRUSR); MODULE_PARM_DESC(lvds_channel_mode, "Specify LVDS channel mode " "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
-module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600); +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(lvds_use_ssc, "Use Spread Spectrum Clock with panels [LVDS/eDP] " "(default: auto from VBT)");
-module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 0400); +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, S_IRUSR); MODULE_PARM_DESC(vbt_sdvo_panel_type, "Override/Ignore selection of SDVO panel mode in the VBT " "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
-module_param_named_unsafe(reset, i915.reset, bool, 0600); +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
-module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 0644); +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(enable_hangcheck, "Periodically check GPU activity for detecting hangs. " "WARNING: Disabling this can cause system wide hangs. " "(default: true)");
-module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400); +module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, S_IRUSR); MODULE_PARM_DESC(enable_ppgtt, "Override PPGTT usage. " "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)");
-module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 0400); +module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, S_IRUSR); MODULE_PARM_DESC(enable_execlists, "Override execlists usage. " "(-1=auto [default], 0=disabled, 1=enabled)");
-module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); +module_param_named_unsafe(enable_psr, i915.enable_psr, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_psr, "Enable PSR " "(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) " "Default: -1 (use per-chip default)");
-module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0400); +module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, S_IRUSR); MODULE_PARM_DESC(preliminary_hw_support, "Enable preliminary hardware support.");
-module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, 0400); +module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, S_IRUSR); MODULE_PARM_DESC(disable_power_well, "Disable display power wells when possible " "(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)");
-module_param_named_unsafe(enable_ips, i915.enable_ips, int, 0600); +module_param_named_unsafe(enable_ips, i915.enable_ips, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
-module_param_named(fastboot, i915.fastboot, bool, 0600); +module_param_named(fastboot, i915.fastboot, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time (default: false)");
-module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600); +module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(prefault_disable, "Disable page prefaulting for pread/pwrite/reloc (default:false). " "For developers only.");
-module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, 0600); +module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(load_detect_test, "Force-enable the VGA load detect code for testing (default:false). " "For developers only.");
-module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, 0600); +module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness " "(-1 force normal, 0 machine defaults, 1 force inversion), please " @@ -166,47 +166,47 @@ MODULE_PARM_DESC(invert_brightness, "to dri-devel@lists.freedesktop.org, if your machine needs it. " "It will then be included in an upcoming module version.");
-module_param_named(disable_display, i915.disable_display, bool, 0400); +module_param_named(disable_display, i915.disable_display, bool, S_IRUSR); MODULE_PARM_DESC(disable_display, "Disable display (default: false)");
-module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, 0600); +module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_cmd_parser, "Enable command parsing (1=enabled [default], 0=disabled)");
-module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, 0600); +module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
-module_param_named(mmio_debug, i915.mmio_debug, int, 0600); +module_param_named(mmio_debug, i915.mmio_debug, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(mmio_debug, "Enable the MMIO debug code for the first N failures (default: off). " "This may negatively affect performance.");
-module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, 0600); +module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(verbose_state_checks, "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions.");
-module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0600); +module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(nuclear_pageflip, "Force atomic modeset functionality; asynchronous mode is not yet supported. (default: false).");
/* WA to get away with the default setting in VBT for early platforms.Will be removed */ -module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0400); +module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, S_IRUSR); MODULE_PARM_DESC(edp_vswing, "Ignore/Override vswing pre-emph table selection from VBT " "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))");
-module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400); +module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, S_IRUSR); MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)");
-module_param_named(guc_log_level, i915.guc_log_level, int, 0400); +module_param_named(guc_log_level, i915.guc_log_level, int, S_IRUSR); MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
-module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600); +module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_dp_mst, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); -module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400); +module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, S_IRUSR); MODULE_PARM_DESC(inject_load_failure, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); -- 2.9.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Aug 02, 2016 at 02:37:37PM +0300, Ville Syrjälä wrote:
On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
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/i915/i915_params.c | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 1779f02..7184e06 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = { .inject_load_failure = 0, };
-module_param_named(modeset, i915.modeset, int, 0400); +module_param_named(modeset, i915.modeset, int, S_IRUSR);
At least I can't read those macros. Octal is much clearer IMO.
Yeah, not really sold on this either. -Daniel
MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (0=disable, " "1=on, -1=force vga console preference [default])");
-module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, 0600); +module_param_named_unsafe(panel_ignore_lid, i915.panel_ignore_lid, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(panel_ignore_lid, "Override lid status (0=autodetect, 1=autodetect disabled [default], " "-1=force lid closed, -2=force lid open)");
-module_param_named_unsafe(semaphores, i915.semaphores, int, 0400); +module_param_named_unsafe(semaphores, i915.semaphores, int, S_IRUSR); MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring sync " "(default: -1 (use per-chip defaults))");
-module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, 0400); +module_param_named_unsafe(enable_rc6, i915.enable_rc6, int, S_IRUSR); MODULE_PARM_DESC(enable_rc6, "Enable power-saving render C-state 6. " "Different stages can be selected via bitmask values " @@ -83,82 +83,82 @@ MODULE_PARM_DESC(enable_rc6, "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " "default: -1 (use per-chip default)");
-module_param_named_unsafe(enable_dc, i915.enable_dc, int, 0400); +module_param_named_unsafe(enable_dc, i915.enable_dc, int, S_IRUSR); MODULE_PARM_DESC(enable_dc, "Enable power-saving display C-states. " "(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
-module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600); +module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_fbc, "Enable frame buffer compression for power savings " "(default: -1 (use per-chip default))");
-module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, 0400); +module_param_named_unsafe(lvds_channel_mode, i915.lvds_channel_mode, int, S_IRUSR); MODULE_PARM_DESC(lvds_channel_mode, "Specify LVDS channel mode " "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)");
-module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, 0600); +module_param_named_unsafe(lvds_use_ssc, i915.panel_use_ssc, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(lvds_use_ssc, "Use Spread Spectrum Clock with panels [LVDS/eDP] " "(default: auto from VBT)");
-module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 0400); +module_param_named_unsafe(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, S_IRUSR); MODULE_PARM_DESC(vbt_sdvo_panel_type, "Override/Ignore selection of SDVO panel mode in the VBT " "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
-module_param_named_unsafe(reset, i915.reset, bool, 0600); +module_param_named_unsafe(reset, i915.reset, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
-module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, 0644); +module_param_named_unsafe(enable_hangcheck, i915.enable_hangcheck, bool, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); MODULE_PARM_DESC(enable_hangcheck, "Periodically check GPU activity for detecting hangs. " "WARNING: Disabling this can cause system wide hangs. " "(default: true)");
-module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, 0400); +module_param_named_unsafe(enable_ppgtt, i915.enable_ppgtt, int, S_IRUSR); MODULE_PARM_DESC(enable_ppgtt, "Override PPGTT usage. " "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with extended address space)");
-module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, 0400); +module_param_named_unsafe(enable_execlists, i915.enable_execlists, int, S_IRUSR); MODULE_PARM_DESC(enable_execlists, "Override execlists usage. " "(-1=auto [default], 0=disabled, 1=enabled)");
-module_param_named_unsafe(enable_psr, i915.enable_psr, int, 0600); +module_param_named_unsafe(enable_psr, i915.enable_psr, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_psr, "Enable PSR " "(0=disabled, 1=enabled - link mode chosen per-platform, 2=force link-standby mode, 3=force link-off mode) " "Default: -1 (use per-chip default)");
-module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, 0400); +module_param_named_unsafe(preliminary_hw_support, i915.preliminary_hw_support, int, S_IRUSR); MODULE_PARM_DESC(preliminary_hw_support, "Enable preliminary hardware support.");
-module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, 0400); +module_param_named_unsafe(disable_power_well, i915.disable_power_well, int, S_IRUSR); MODULE_PARM_DESC(disable_power_well, "Disable display power wells when possible " "(-1=auto [default], 0=power wells always on, 1=power wells disabled when possible)");
-module_param_named_unsafe(enable_ips, i915.enable_ips, int, 0600); +module_param_named_unsafe(enable_ips, i915.enable_ips, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
-module_param_named(fastboot, i915.fastboot, bool, 0600); +module_param_named(fastboot, i915.fastboot, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time (default: false)");
-module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600); +module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(prefault_disable, "Disable page prefaulting for pread/pwrite/reloc (default:false). " "For developers only.");
-module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, 0600); +module_param_named_unsafe(load_detect_test, i915.load_detect_test, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(load_detect_test, "Force-enable the VGA load detect code for testing (default:false). " "For developers only.");
-module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, 0600); +module_param_named_unsafe(invert_brightness, i915.invert_brightness, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness " "(-1 force normal, 0 machine defaults, 1 force inversion), please " @@ -166,47 +166,47 @@ MODULE_PARM_DESC(invert_brightness, "to dri-devel@lists.freedesktop.org, if your machine needs it. " "It will then be included in an upcoming module version.");
-module_param_named(disable_display, i915.disable_display, bool, 0400); +module_param_named(disable_display, i915.disable_display, bool, S_IRUSR); MODULE_PARM_DESC(disable_display, "Disable display (default: false)");
-module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, 0600); +module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_cmd_parser, "Enable command parsing (1=enabled [default], 0=disabled)");
-module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, 0600); +module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
-module_param_named(mmio_debug, i915.mmio_debug, int, 0600); +module_param_named(mmio_debug, i915.mmio_debug, int, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(mmio_debug, "Enable the MMIO debug code for the first N failures (default: off). " "This may negatively affect performance.");
-module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, 0600); +module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(verbose_state_checks, "Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions.");
-module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0600); +module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(nuclear_pageflip, "Force atomic modeset functionality; asynchronous mode is not yet supported. (default: false).");
/* WA to get away with the default setting in VBT for early platforms.Will be removed */ -module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0400); +module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, S_IRUSR); MODULE_PARM_DESC(edp_vswing, "Ignore/Override vswing pre-emph table selection from VBT " "(0=use value from vbt [default], 1=low power swing(200mV)," "2=default swing(400mV))");
-module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, 0400); +module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, bool, S_IRUSR); MODULE_PARM_DESC(enable_guc_submission, "Enable GuC submission (default:false)");
-module_param_named(guc_log_level, i915.guc_log_level, int, 0400); +module_param_named(guc_log_level, i915.guc_log_level, int, S_IRUSR); MODULE_PARM_DESC(guc_log_level, "GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
-module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, 0600); +module_param_named_unsafe(enable_dp_mst, i915.enable_dp_mst, bool, S_IRUSR | S_IWUSR); MODULE_PARM_DESC(enable_dp_mst, "Enable multi-stream transport (MST) for new DisplayPort sinks. (default: true)"); -module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, 0400); +module_param_named_unsafe(inject_load_failure, i915.inject_load_failure, uint, S_IRUSR); MODULE_PARM_DESC(inject_load_failure, "Force an error after a number of failure check points (0:disabled (default), N:force failure at the Nth failure check point)"); -- 2.9.2
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Aug 02, 2016 at 02:37:37PM +0300, Ville Syrjälä wrote:
On Tue, Aug 02, 2016 at 06:48:47PM +0800, Baole Ni wrote:
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/i915/i915_params.c | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 1779f02..7184e06 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -60,22 +60,22 @@ struct i915_params i915 __read_mostly = { .inject_load_failure = 0, };
-module_param_named(modeset, i915.modeset, int, 0400); +module_param_named(modeset, i915.modeset, int, S_IRUSR);
At least I can't read those macros. Octal is much clearer IMO.
It's also easier to grep for, say, 644, rather than formulating a regex with all possible ordering permutations of these macros.
Best regards,
Lukas
dri-devel@lists.freedesktop.org
-
Baole Ni -
Daniel Vetter -
Jani Nikula -
Lukas Wunner -
Ville Syrjälä