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 at intel.com> > Signed-off-by: Baole Ni <baolex.ni at 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 at 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 at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC