* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 19:43 ` Alex Deucher
0 siblings, 0 replies; 33+ messages in thread
From: Alex Deucher @ 2020-11-02 19:43 UTC (permalink / raw)
To: Deepak R Varma
Cc: David Airlie, Greg KH, LKML, Maling list - DRI developers,
Melissa Wen, amd-gfx list, Daniel Vetter, Daniel Vetter,
Alex Deucher, Christian König
On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>
> Initializing global variable to 0 or NULL is not necessary and should
> be avoided. Issue reported by checkpatch script as:
> ERROR: do not initialise globals to 0 (or NULL).
I agree that this is technically correct, but a lot of people don't
seem to know that so we get a lot of comments about this code for the
variables that are not explicitly set. Seems less confusing to
initialize them even if it not necessary. I don't have a particularly
strong opinion on it however.
Alex
>
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 46 ++++++++++++-------------
> drivers/gpu/drm/amd/amdgpu/atom.c | 4 +--
> 2 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 8ab6126ff70c..6de94c46bc91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -94,16 +94,16 @@
> #define KMS_DRIVER_MINOR 40
> #define KMS_DRIVER_PATCHLEVEL 0
>
> -int amdgpu_vram_limit = 0;
> -int amdgpu_vis_vram_limit = 0;
> +int amdgpu_vram_limit;
> +int amdgpu_vis_vram_limit;
> int amdgpu_gart_size = -1; /* auto */
> int amdgpu_gtt_size = -1; /* auto */
> int amdgpu_moverate = -1; /* auto */
> -int amdgpu_benchmarking = 0;
> -int amdgpu_testing = 0;
> +int amdgpu_benchmarking;
> +int amdgpu_testing;
> int amdgpu_audio = -1;
> -int amdgpu_disp_priority = 0;
> -int amdgpu_hw_i2c = 0;
> +int amdgpu_disp_priority;
> +int amdgpu_hw_i2c;
> int amdgpu_pcie_gen2 = -1;
> int amdgpu_msi = -1;
> char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
> @@ -113,43 +113,43 @@ int amdgpu_aspm = -1;
> int amdgpu_runtime_pm = -1;
> uint amdgpu_ip_block_mask = 0xffffffff;
> int amdgpu_bapm = -1;
> -int amdgpu_deep_color = 0;
> +int amdgpu_deep_color;
> int amdgpu_vm_size = -1;
> int amdgpu_vm_fragment_size = -1;
> int amdgpu_vm_block_size = -1;
> -int amdgpu_vm_fault_stop = 0;
> -int amdgpu_vm_debug = 0;
> +int amdgpu_vm_fault_stop;
> +int amdgpu_vm_debug;
> int amdgpu_vm_update_mode = -1;
> -int amdgpu_exp_hw_support = 0;
> +int amdgpu_exp_hw_support;
> int amdgpu_dc = -1;
> int amdgpu_sched_jobs = 32;
> int amdgpu_sched_hw_submission = 2;
> -uint amdgpu_pcie_gen_cap = 0;
> -uint amdgpu_pcie_lane_cap = 0;
> +uint amdgpu_pcie_gen_cap;
> +uint amdgpu_pcie_lane_cap;
> uint amdgpu_cg_mask = 0xffffffff;
> uint amdgpu_pg_mask = 0xffffffff;
> uint amdgpu_sdma_phase_quantum = 32;
> -char *amdgpu_disable_cu = NULL;
> -char *amdgpu_virtual_display = NULL;
> +char *amdgpu_disable_cu;
> +char *amdgpu_virtual_display;
> /* OverDrive(bit 14) disabled by default*/
> uint amdgpu_pp_feature_mask = 0xffffbfff;
> -uint amdgpu_force_long_training = 0;
> -int amdgpu_job_hang_limit = 0;
> +uint amdgpu_force_long_training;
> +int amdgpu_job_hang_limit;
> int amdgpu_lbpw = -1;
> int amdgpu_compute_multipipe = -1;
> int amdgpu_gpu_recovery = -1; /* auto */
> -int amdgpu_emu_mode = 0;
> -uint amdgpu_smu_memory_pool_size = 0;
> +int amdgpu_emu_mode;
> +uint amdgpu_smu_memory_pool_size;
> /* FBC (bit 0) disabled by default*/
> -uint amdgpu_dc_feature_mask = 0;
> -uint amdgpu_dc_debug_mask = 0;
> +uint amdgpu_dc_feature_mask;
> +uint amdgpu_dc_debug_mask;
> int amdgpu_async_gfx_ring = 1;
> -int amdgpu_mcbp = 0;
> +int amdgpu_mcbp;
> int amdgpu_discovery = -1;
> -int amdgpu_mes = 0;
> +int amdgpu_mes;
> int amdgpu_noretry = -1;
> int amdgpu_force_asic_type = -1;
> -int amdgpu_tmz = 0;
> +int amdgpu_tmz;
> int amdgpu_reset_method = -1; /* auto */
> int amdgpu_num_kcq = -1;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 696e97ab77eb..46c00ee580b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -66,7 +66,7 @@ typedef struct {
> bool abort;
> } atom_exec_context;
>
> -int amdgpu_atom_debug = 0;
> +int amdgpu_atom_debug;
> static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t * params);
> int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * params);
>
> @@ -88,7 +88,7 @@ static int atom_dst_to_src[8][4] = {
> };
> static int atom_def_dst[8] = { 0, 0, 1, 2, 0, 1, 2, 3 };
>
> -static int debug_depth = 0;
> +static int debug_depth;
> #ifdef ATOM_DEBUG
> static void debug_print_spaces(int n)
> {
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 19:43 ` Alex Deucher
0 siblings, 0 replies; 33+ messages in thread
From: Alex Deucher @ 2020-11-02 19:43 UTC (permalink / raw)
To: Deepak R Varma
Cc: David Airlie, Greg KH, LKML, Maling list - DRI developers,
Melissa Wen, amd-gfx list, Daniel Vetter, Alex Deucher,
Christian König
On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>
> Initializing global variable to 0 or NULL is not necessary and should
> be avoided. Issue reported by checkpatch script as:
> ERROR: do not initialise globals to 0 (or NULL).
I agree that this is technically correct, but a lot of people don't
seem to know that so we get a lot of comments about this code for the
variables that are not explicitly set. Seems less confusing to
initialize them even if it not necessary. I don't have a particularly
strong opinion on it however.
Alex
>
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 46 ++++++++++++-------------
> drivers/gpu/drm/amd/amdgpu/atom.c | 4 +--
> 2 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 8ab6126ff70c..6de94c46bc91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -94,16 +94,16 @@
> #define KMS_DRIVER_MINOR 40
> #define KMS_DRIVER_PATCHLEVEL 0
>
> -int amdgpu_vram_limit = 0;
> -int amdgpu_vis_vram_limit = 0;
> +int amdgpu_vram_limit;
> +int amdgpu_vis_vram_limit;
> int amdgpu_gart_size = -1; /* auto */
> int amdgpu_gtt_size = -1; /* auto */
> int amdgpu_moverate = -1; /* auto */
> -int amdgpu_benchmarking = 0;
> -int amdgpu_testing = 0;
> +int amdgpu_benchmarking;
> +int amdgpu_testing;
> int amdgpu_audio = -1;
> -int amdgpu_disp_priority = 0;
> -int amdgpu_hw_i2c = 0;
> +int amdgpu_disp_priority;
> +int amdgpu_hw_i2c;
> int amdgpu_pcie_gen2 = -1;
> int amdgpu_msi = -1;
> char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
> @@ -113,43 +113,43 @@ int amdgpu_aspm = -1;
> int amdgpu_runtime_pm = -1;
> uint amdgpu_ip_block_mask = 0xffffffff;
> int amdgpu_bapm = -1;
> -int amdgpu_deep_color = 0;
> +int amdgpu_deep_color;
> int amdgpu_vm_size = -1;
> int amdgpu_vm_fragment_size = -1;
> int amdgpu_vm_block_size = -1;
> -int amdgpu_vm_fault_stop = 0;
> -int amdgpu_vm_debug = 0;
> +int amdgpu_vm_fault_stop;
> +int amdgpu_vm_debug;
> int amdgpu_vm_update_mode = -1;
> -int amdgpu_exp_hw_support = 0;
> +int amdgpu_exp_hw_support;
> int amdgpu_dc = -1;
> int amdgpu_sched_jobs = 32;
> int amdgpu_sched_hw_submission = 2;
> -uint amdgpu_pcie_gen_cap = 0;
> -uint amdgpu_pcie_lane_cap = 0;
> +uint amdgpu_pcie_gen_cap;
> +uint amdgpu_pcie_lane_cap;
> uint amdgpu_cg_mask = 0xffffffff;
> uint amdgpu_pg_mask = 0xffffffff;
> uint amdgpu_sdma_phase_quantum = 32;
> -char *amdgpu_disable_cu = NULL;
> -char *amdgpu_virtual_display = NULL;
> +char *amdgpu_disable_cu;
> +char *amdgpu_virtual_display;
> /* OverDrive(bit 14) disabled by default*/
> uint amdgpu_pp_feature_mask = 0xffffbfff;
> -uint amdgpu_force_long_training = 0;
> -int amdgpu_job_hang_limit = 0;
> +uint amdgpu_force_long_training;
> +int amdgpu_job_hang_limit;
> int amdgpu_lbpw = -1;
> int amdgpu_compute_multipipe = -1;
> int amdgpu_gpu_recovery = -1; /* auto */
> -int amdgpu_emu_mode = 0;
> -uint amdgpu_smu_memory_pool_size = 0;
> +int amdgpu_emu_mode;
> +uint amdgpu_smu_memory_pool_size;
> /* FBC (bit 0) disabled by default*/
> -uint amdgpu_dc_feature_mask = 0;
> -uint amdgpu_dc_debug_mask = 0;
> +uint amdgpu_dc_feature_mask;
> +uint amdgpu_dc_debug_mask;
> int amdgpu_async_gfx_ring = 1;
> -int amdgpu_mcbp = 0;
> +int amdgpu_mcbp;
> int amdgpu_discovery = -1;
> -int amdgpu_mes = 0;
> +int amdgpu_mes;
> int amdgpu_noretry = -1;
> int amdgpu_force_asic_type = -1;
> -int amdgpu_tmz = 0;
> +int amdgpu_tmz;
> int amdgpu_reset_method = -1; /* auto */
> int amdgpu_num_kcq = -1;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 696e97ab77eb..46c00ee580b1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -66,7 +66,7 @@ typedef struct {
> bool abort;
> } atom_exec_context;
>
> -int amdgpu_atom_debug = 0;
> +int amdgpu_atom_debug;
> static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t * params);
> int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * params);
>
> @@ -88,7 +88,7 @@ static int atom_dst_to_src[8][4] = {
> };
> static int atom_def_dst[8] = { 0, 0, 1, 2, 0, 1, 2, 3 };
>
> -static int debug_depth = 0;
> +static int debug_depth;
> #ifdef ATOM_DEBUG
> static void debug_print_spaces(int n)
> {
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-02 19:43 ` Alex Deucher
(?)
@ 2020-11-02 20:06 ` Christian König
-1 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-02 20:06 UTC (permalink / raw)
To: Alex Deucher, Deepak R Varma
Cc: David Airlie, Greg KH, LKML, Maling list - DRI developers,
Melissa Wen, amd-gfx list, Daniel Vetter, Daniel Vetter,
Alex Deucher, Christian König
Am 02.11.20 um 20:43 schrieb Alex Deucher:
> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>> Initializing global variable to 0 or NULL is not necessary and should
>> be avoided. Issue reported by checkpatch script as:
>> ERROR: do not initialise globals to 0 (or NULL).
> I agree that this is technically correct, but a lot of people don't
> seem to know that so we get a lot of comments about this code for the
> variables that are not explicitly set. Seems less confusing to
> initialize them even if it not necessary. I don't have a particularly
> strong opinion on it however.
Agree with Alex.
Especially for the module parameters we should have a explicit init
value for documentation purposes, even when it is 0.
Christian.
>
> Alex
>
>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 46 ++++++++++++-------------
>> drivers/gpu/drm/amd/amdgpu/atom.c | 4 +--
>> 2 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 8ab6126ff70c..6de94c46bc91 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -94,16 +94,16 @@
>> #define KMS_DRIVER_MINOR 40
>> #define KMS_DRIVER_PATCHLEVEL 0
>>
>> -int amdgpu_vram_limit = 0;
>> -int amdgpu_vis_vram_limit = 0;
>> +int amdgpu_vram_limit;
>> +int amdgpu_vis_vram_limit;
>> int amdgpu_gart_size = -1; /* auto */
>> int amdgpu_gtt_size = -1; /* auto */
>> int amdgpu_moverate = -1; /* auto */
>> -int amdgpu_benchmarking = 0;
>> -int amdgpu_testing = 0;
>> +int amdgpu_benchmarking;
>> +int amdgpu_testing;
>> int amdgpu_audio = -1;
>> -int amdgpu_disp_priority = 0;
>> -int amdgpu_hw_i2c = 0;
>> +int amdgpu_disp_priority;
>> +int amdgpu_hw_i2c;
>> int amdgpu_pcie_gen2 = -1;
>> int amdgpu_msi = -1;
>> char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
>> @@ -113,43 +113,43 @@ int amdgpu_aspm = -1;
>> int amdgpu_runtime_pm = -1;
>> uint amdgpu_ip_block_mask = 0xffffffff;
>> int amdgpu_bapm = -1;
>> -int amdgpu_deep_color = 0;
>> +int amdgpu_deep_color;
>> int amdgpu_vm_size = -1;
>> int amdgpu_vm_fragment_size = -1;
>> int amdgpu_vm_block_size = -1;
>> -int amdgpu_vm_fault_stop = 0;
>> -int amdgpu_vm_debug = 0;
>> +int amdgpu_vm_fault_stop;
>> +int amdgpu_vm_debug;
>> int amdgpu_vm_update_mode = -1;
>> -int amdgpu_exp_hw_support = 0;
>> +int amdgpu_exp_hw_support;
>> int amdgpu_dc = -1;
>> int amdgpu_sched_jobs = 32;
>> int amdgpu_sched_hw_submission = 2;
>> -uint amdgpu_pcie_gen_cap = 0;
>> -uint amdgpu_pcie_lane_cap = 0;
>> +uint amdgpu_pcie_gen_cap;
>> +uint amdgpu_pcie_lane_cap;
>> uint amdgpu_cg_mask = 0xffffffff;
>> uint amdgpu_pg_mask = 0xffffffff;
>> uint amdgpu_sdma_phase_quantum = 32;
>> -char *amdgpu_disable_cu = NULL;
>> -char *amdgpu_virtual_display = NULL;
>> +char *amdgpu_disable_cu;
>> +char *amdgpu_virtual_display;
>> /* OverDrive(bit 14) disabled by default*/
>> uint amdgpu_pp_feature_mask = 0xffffbfff;
>> -uint amdgpu_force_long_training = 0;
>> -int amdgpu_job_hang_limit = 0;
>> +uint amdgpu_force_long_training;
>> +int amdgpu_job_hang_limit;
>> int amdgpu_lbpw = -1;
>> int amdgpu_compute_multipipe = -1;
>> int amdgpu_gpu_recovery = -1; /* auto */
>> -int amdgpu_emu_mode = 0;
>> -uint amdgpu_smu_memory_pool_size = 0;
>> +int amdgpu_emu_mode;
>> +uint amdgpu_smu_memory_pool_size;
>> /* FBC (bit 0) disabled by default*/
>> -uint amdgpu_dc_feature_mask = 0;
>> -uint amdgpu_dc_debug_mask = 0;
>> +uint amdgpu_dc_feature_mask;
>> +uint amdgpu_dc_debug_mask;
>> int amdgpu_async_gfx_ring = 1;
>> -int amdgpu_mcbp = 0;
>> +int amdgpu_mcbp;
>> int amdgpu_discovery = -1;
>> -int amdgpu_mes = 0;
>> +int amdgpu_mes;
>> int amdgpu_noretry = -1;
>> int amdgpu_force_asic_type = -1;
>> -int amdgpu_tmz = 0;
>> +int amdgpu_tmz;
>> int amdgpu_reset_method = -1; /* auto */
>> int amdgpu_num_kcq = -1;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
>> index 696e97ab77eb..46c00ee580b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
>> @@ -66,7 +66,7 @@ typedef struct {
>> bool abort;
>> } atom_exec_context;
>>
>> -int amdgpu_atom_debug = 0;
>> +int amdgpu_atom_debug;
>> static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t * params);
>> int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * params);
>>
>> @@ -88,7 +88,7 @@ static int atom_dst_to_src[8][4] = {
>> };
>> static int atom_def_dst[8] = { 0, 0, 1, 2, 0, 1, 2, 3 };
>>
>> -static int debug_depth = 0;
>> +static int debug_depth;
>> #ifdef ATOM_DEBUG
>> static void debug_print_spaces(int n)
>> {
>> --
>> 2.25.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 20:06 ` Christian König
0 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-02 20:06 UTC (permalink / raw)
To: Alex Deucher, Deepak R Varma
Cc: David Airlie, Greg KH, LKML, amd-gfx list, Melissa Wen,
Maling list - DRI developers, Daniel Vetter, Daniel Vetter,
Alex Deucher, Christian König
Am 02.11.20 um 20:43 schrieb Alex Deucher:
> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>> Initializing global variable to 0 or NULL is not necessary and should
>> be avoided. Issue reported by checkpatch script as:
>> ERROR: do not initialise globals to 0 (or NULL).
> I agree that this is technically correct, but a lot of people don't
> seem to know that so we get a lot of comments about this code for the
> variables that are not explicitly set. Seems less confusing to
> initialize them even if it not necessary. I don't have a particularly
> strong opinion on it however.
Agree with Alex.
Especially for the module parameters we should have a explicit init
value for documentation purposes, even when it is 0.
Christian.
>
> Alex
>
>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 46 ++++++++++++-------------
>> drivers/gpu/drm/amd/amdgpu/atom.c | 4 +--
>> 2 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 8ab6126ff70c..6de94c46bc91 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -94,16 +94,16 @@
>> #define KMS_DRIVER_MINOR 40
>> #define KMS_DRIVER_PATCHLEVEL 0
>>
>> -int amdgpu_vram_limit = 0;
>> -int amdgpu_vis_vram_limit = 0;
>> +int amdgpu_vram_limit;
>> +int amdgpu_vis_vram_limit;
>> int amdgpu_gart_size = -1; /* auto */
>> int amdgpu_gtt_size = -1; /* auto */
>> int amdgpu_moverate = -1; /* auto */
>> -int amdgpu_benchmarking = 0;
>> -int amdgpu_testing = 0;
>> +int amdgpu_benchmarking;
>> +int amdgpu_testing;
>> int amdgpu_audio = -1;
>> -int amdgpu_disp_priority = 0;
>> -int amdgpu_hw_i2c = 0;
>> +int amdgpu_disp_priority;
>> +int amdgpu_hw_i2c;
>> int amdgpu_pcie_gen2 = -1;
>> int amdgpu_msi = -1;
>> char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
>> @@ -113,43 +113,43 @@ int amdgpu_aspm = -1;
>> int amdgpu_runtime_pm = -1;
>> uint amdgpu_ip_block_mask = 0xffffffff;
>> int amdgpu_bapm = -1;
>> -int amdgpu_deep_color = 0;
>> +int amdgpu_deep_color;
>> int amdgpu_vm_size = -1;
>> int amdgpu_vm_fragment_size = -1;
>> int amdgpu_vm_block_size = -1;
>> -int amdgpu_vm_fault_stop = 0;
>> -int amdgpu_vm_debug = 0;
>> +int amdgpu_vm_fault_stop;
>> +int amdgpu_vm_debug;
>> int amdgpu_vm_update_mode = -1;
>> -int amdgpu_exp_hw_support = 0;
>> +int amdgpu_exp_hw_support;
>> int amdgpu_dc = -1;
>> int amdgpu_sched_jobs = 32;
>> int amdgpu_sched_hw_submission = 2;
>> -uint amdgpu_pcie_gen_cap = 0;
>> -uint amdgpu_pcie_lane_cap = 0;
>> +uint amdgpu_pcie_gen_cap;
>> +uint amdgpu_pcie_lane_cap;
>> uint amdgpu_cg_mask = 0xffffffff;
>> uint amdgpu_pg_mask = 0xffffffff;
>> uint amdgpu_sdma_phase_quantum = 32;
>> -char *amdgpu_disable_cu = NULL;
>> -char *amdgpu_virtual_display = NULL;
>> +char *amdgpu_disable_cu;
>> +char *amdgpu_virtual_display;
>> /* OverDrive(bit 14) disabled by default*/
>> uint amdgpu_pp_feature_mask = 0xffffbfff;
>> -uint amdgpu_force_long_training = 0;
>> -int amdgpu_job_hang_limit = 0;
>> +uint amdgpu_force_long_training;
>> +int amdgpu_job_hang_limit;
>> int amdgpu_lbpw = -1;
>> int amdgpu_compute_multipipe = -1;
>> int amdgpu_gpu_recovery = -1; /* auto */
>> -int amdgpu_emu_mode = 0;
>> -uint amdgpu_smu_memory_pool_size = 0;
>> +int amdgpu_emu_mode;
>> +uint amdgpu_smu_memory_pool_size;
>> /* FBC (bit 0) disabled by default*/
>> -uint amdgpu_dc_feature_mask = 0;
>> -uint amdgpu_dc_debug_mask = 0;
>> +uint amdgpu_dc_feature_mask;
>> +uint amdgpu_dc_debug_mask;
>> int amdgpu_async_gfx_ring = 1;
>> -int amdgpu_mcbp = 0;
>> +int amdgpu_mcbp;
>> int amdgpu_discovery = -1;
>> -int amdgpu_mes = 0;
>> +int amdgpu_mes;
>> int amdgpu_noretry = -1;
>> int amdgpu_force_asic_type = -1;
>> -int amdgpu_tmz = 0;
>> +int amdgpu_tmz;
>> int amdgpu_reset_method = -1; /* auto */
>> int amdgpu_num_kcq = -1;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
>> index 696e97ab77eb..46c00ee580b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
>> @@ -66,7 +66,7 @@ typedef struct {
>> bool abort;
>> } atom_exec_context;
>>
>> -int amdgpu_atom_debug = 0;
>> +int amdgpu_atom_debug;
>> static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t * params);
>> int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * params);
>>
>> @@ -88,7 +88,7 @@ static int atom_dst_to_src[8][4] = {
>> };
>> static int atom_def_dst[8] = { 0, 0, 1, 2, 0, 1, 2, 3 };
>>
>> -static int debug_depth = 0;
>> +static int debug_depth;
>> #ifdef ATOM_DEBUG
>> static void debug_print_spaces(int n)
>> {
>> --
>> 2.25.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 20:06 ` Christian König
0 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-02 20:06 UTC (permalink / raw)
To: Alex Deucher, Deepak R Varma
Cc: David Airlie, Greg KH, LKML, amd-gfx list, Melissa Wen,
Maling list - DRI developers, Daniel Vetter, Alex Deucher,
Christian König
Am 02.11.20 um 20:43 schrieb Alex Deucher:
> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>> Initializing global variable to 0 or NULL is not necessary and should
>> be avoided. Issue reported by checkpatch script as:
>> ERROR: do not initialise globals to 0 (or NULL).
> I agree that this is technically correct, but a lot of people don't
> seem to know that so we get a lot of comments about this code for the
> variables that are not explicitly set. Seems less confusing to
> initialize them even if it not necessary. I don't have a particularly
> strong opinion on it however.
Agree with Alex.
Especially for the module parameters we should have a explicit init
value for documentation purposes, even when it is 0.
Christian.
>
> Alex
>
>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 46 ++++++++++++-------------
>> drivers/gpu/drm/amd/amdgpu/atom.c | 4 +--
>> 2 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 8ab6126ff70c..6de94c46bc91 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -94,16 +94,16 @@
>> #define KMS_DRIVER_MINOR 40
>> #define KMS_DRIVER_PATCHLEVEL 0
>>
>> -int amdgpu_vram_limit = 0;
>> -int amdgpu_vis_vram_limit = 0;
>> +int amdgpu_vram_limit;
>> +int amdgpu_vis_vram_limit;
>> int amdgpu_gart_size = -1; /* auto */
>> int amdgpu_gtt_size = -1; /* auto */
>> int amdgpu_moverate = -1; /* auto */
>> -int amdgpu_benchmarking = 0;
>> -int amdgpu_testing = 0;
>> +int amdgpu_benchmarking;
>> +int amdgpu_testing;
>> int amdgpu_audio = -1;
>> -int amdgpu_disp_priority = 0;
>> -int amdgpu_hw_i2c = 0;
>> +int amdgpu_disp_priority;
>> +int amdgpu_hw_i2c;
>> int amdgpu_pcie_gen2 = -1;
>> int amdgpu_msi = -1;
>> char amdgpu_lockup_timeout[AMDGPU_MAX_TIMEOUT_PARAM_LENGTH];
>> @@ -113,43 +113,43 @@ int amdgpu_aspm = -1;
>> int amdgpu_runtime_pm = -1;
>> uint amdgpu_ip_block_mask = 0xffffffff;
>> int amdgpu_bapm = -1;
>> -int amdgpu_deep_color = 0;
>> +int amdgpu_deep_color;
>> int amdgpu_vm_size = -1;
>> int amdgpu_vm_fragment_size = -1;
>> int amdgpu_vm_block_size = -1;
>> -int amdgpu_vm_fault_stop = 0;
>> -int amdgpu_vm_debug = 0;
>> +int amdgpu_vm_fault_stop;
>> +int amdgpu_vm_debug;
>> int amdgpu_vm_update_mode = -1;
>> -int amdgpu_exp_hw_support = 0;
>> +int amdgpu_exp_hw_support;
>> int amdgpu_dc = -1;
>> int amdgpu_sched_jobs = 32;
>> int amdgpu_sched_hw_submission = 2;
>> -uint amdgpu_pcie_gen_cap = 0;
>> -uint amdgpu_pcie_lane_cap = 0;
>> +uint amdgpu_pcie_gen_cap;
>> +uint amdgpu_pcie_lane_cap;
>> uint amdgpu_cg_mask = 0xffffffff;
>> uint amdgpu_pg_mask = 0xffffffff;
>> uint amdgpu_sdma_phase_quantum = 32;
>> -char *amdgpu_disable_cu = NULL;
>> -char *amdgpu_virtual_display = NULL;
>> +char *amdgpu_disable_cu;
>> +char *amdgpu_virtual_display;
>> /* OverDrive(bit 14) disabled by default*/
>> uint amdgpu_pp_feature_mask = 0xffffbfff;
>> -uint amdgpu_force_long_training = 0;
>> -int amdgpu_job_hang_limit = 0;
>> +uint amdgpu_force_long_training;
>> +int amdgpu_job_hang_limit;
>> int amdgpu_lbpw = -1;
>> int amdgpu_compute_multipipe = -1;
>> int amdgpu_gpu_recovery = -1; /* auto */
>> -int amdgpu_emu_mode = 0;
>> -uint amdgpu_smu_memory_pool_size = 0;
>> +int amdgpu_emu_mode;
>> +uint amdgpu_smu_memory_pool_size;
>> /* FBC (bit 0) disabled by default*/
>> -uint amdgpu_dc_feature_mask = 0;
>> -uint amdgpu_dc_debug_mask = 0;
>> +uint amdgpu_dc_feature_mask;
>> +uint amdgpu_dc_debug_mask;
>> int amdgpu_async_gfx_ring = 1;
>> -int amdgpu_mcbp = 0;
>> +int amdgpu_mcbp;
>> int amdgpu_discovery = -1;
>> -int amdgpu_mes = 0;
>> +int amdgpu_mes;
>> int amdgpu_noretry = -1;
>> int amdgpu_force_asic_type = -1;
>> -int amdgpu_tmz = 0;
>> +int amdgpu_tmz;
>> int amdgpu_reset_method = -1; /* auto */
>> int amdgpu_num_kcq = -1;
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
>> index 696e97ab77eb..46c00ee580b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
>> @@ -66,7 +66,7 @@ typedef struct {
>> bool abort;
>> } atom_exec_context;
>>
>> -int amdgpu_atom_debug = 0;
>> +int amdgpu_atom_debug;
>> static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t * params);
>> int amdgpu_atom_execute_table(struct atom_context *ctx, int index, uint32_t * params);
>>
>> @@ -88,7 +88,7 @@ static int atom_dst_to_src[8][4] = {
>> };
>> static int atom_def_dst[8] = { 0, 0, 1, 2, 0, 1, 2, 3 };
>>
>> -static int debug_depth = 0;
>> +static int debug_depth;
>> #ifdef ATOM_DEBUG
>> static void debug_print_spaces(int n)
>> {
>> --
>> 2.25.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-02 20:06 ` Christian König
(?)
@ 2020-11-03 6:53 ` Greg KH
-1 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 6:53 UTC (permalink / raw)
To: christian.koenig
Cc: Alex Deucher, Deepak R Varma, David Airlie, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Daniel Vetter, Alex Deucher
On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > Initializing global variable to 0 or NULL is not necessary and should
> > > be avoided. Issue reported by checkpatch script as:
> > > ERROR: do not initialise globals to 0 (or NULL).
> > I agree that this is technically correct, but a lot of people don't
> > seem to know that so we get a lot of comments about this code for the
> > variables that are not explicitly set. Seems less confusing to
> > initialize them even if it not necessary. I don't have a particularly
> > strong opinion on it however.
>
> Agree with Alex.
>
> Especially for the module parameters we should have a explicit init value
> for documentation purposes, even when it is 0.
Why is this one tiny driver somehow special compared to the entire rest
of the kernel? (hint, it isn't...)
Please follow the normal coding style rules, there's no reason to ignore
them unless you like to constantly reject patches like this that get
sent to you.
thnaks,
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 6:53 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 6:53 UTC (permalink / raw)
To: christian.koenig
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list, Daniel Vetter, Alex Deucher
On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > Initializing global variable to 0 or NULL is not necessary and should
> > > be avoided. Issue reported by checkpatch script as:
> > > ERROR: do not initialise globals to 0 (or NULL).
> > I agree that this is technically correct, but a lot of people don't
> > seem to know that so we get a lot of comments about this code for the
> > variables that are not explicitly set. Seems less confusing to
> > initialize them even if it not necessary. I don't have a particularly
> > strong opinion on it however.
>
> Agree with Alex.
>
> Especially for the module parameters we should have a explicit init value
> for documentation purposes, even when it is 0.
Why is this one tiny driver somehow special compared to the entire rest
of the kernel? (hint, it isn't...)
Please follow the normal coding style rules, there's no reason to ignore
them unless you like to constantly reject patches like this that get
sent to you.
thnaks,
greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 6:53 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 6:53 UTC (permalink / raw)
To: christian.koenig
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list
On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > Initializing global variable to 0 or NULL is not necessary and should
> > > be avoided. Issue reported by checkpatch script as:
> > > ERROR: do not initialise globals to 0 (or NULL).
> > I agree that this is technically correct, but a lot of people don't
> > seem to know that so we get a lot of comments about this code for the
> > variables that are not explicitly set. Seems less confusing to
> > initialize them even if it not necessary. I don't have a particularly
> > strong opinion on it however.
>
> Agree with Alex.
>
> Especially for the module parameters we should have a explicit init value
> for documentation purposes, even when it is 0.
Why is this one tiny driver somehow special compared to the entire rest
of the kernel? (hint, it isn't...)
Please follow the normal coding style rules, there's no reason to ignore
them unless you like to constantly reject patches like this that get
sent to you.
thnaks,
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-03 6:53 ` Greg KH
(?)
@ 2020-11-02 20:48 ` Christian König
-1 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-02 20:48 UTC (permalink / raw)
To: Greg KH
Cc: Alex Deucher, Deepak R Varma, David Airlie, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Daniel Vetter, Alex Deucher
Am 03.11.20 um 07:53 schrieb Greg KH:
> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
>> Am 02.11.20 um 20:43 schrieb Alex Deucher:
>>> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>> Initializing global variable to 0 or NULL is not necessary and should
>>>> be avoided. Issue reported by checkpatch script as:
>>>> ERROR: do not initialise globals to 0 (or NULL).
>>> I agree that this is technically correct, but a lot of people don't
>>> seem to know that so we get a lot of comments about this code for the
>>> variables that are not explicitly set. Seems less confusing to
>>> initialize them even if it not necessary. I don't have a particularly
>>> strong opinion on it however.
>> Agree with Alex.
>>
>> Especially for the module parameters we should have a explicit init value
>> for documentation purposes, even when it is 0.
> Why is this one tiny driver somehow special compared to the entire rest
> of the kernel? (hint, it isn't...)
And it certainly shouldn't :)
> Please follow the normal coding style rules, there's no reason to ignore
> them unless you like to constantly reject patches like this that get
> sent to you.
Yeah, that's a rather good point.
Not a particular strong opinion on this either, but when something
global is set to 0 people usually do this to emphases that it is
important that it is zero.
Regards,
Christian.
>
> thnaks,
>
> greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 20:48 ` Christian König
0 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-02 20:48 UTC (permalink / raw)
To: Greg KH
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list, Daniel Vetter, Alex Deucher
Am 03.11.20 um 07:53 schrieb Greg KH:
> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
>> Am 02.11.20 um 20:43 schrieb Alex Deucher:
>>> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>> Initializing global variable to 0 or NULL is not necessary and should
>>>> be avoided. Issue reported by checkpatch script as:
>>>> ERROR: do not initialise globals to 0 (or NULL).
>>> I agree that this is technically correct, but a lot of people don't
>>> seem to know that so we get a lot of comments about this code for the
>>> variables that are not explicitly set. Seems less confusing to
>>> initialize them even if it not necessary. I don't have a particularly
>>> strong opinion on it however.
>> Agree with Alex.
>>
>> Especially for the module parameters we should have a explicit init value
>> for documentation purposes, even when it is 0.
> Why is this one tiny driver somehow special compared to the entire rest
> of the kernel? (hint, it isn't...)
And it certainly shouldn't :)
> Please follow the normal coding style rules, there's no reason to ignore
> them unless you like to constantly reject patches like this that get
> sent to you.
Yeah, that's a rather good point.
Not a particular strong opinion on this either, but when something
global is set to 0 people usually do this to emphases that it is
important that it is zero.
Regards,
Christian.
>
> thnaks,
>
> greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 20:48 ` Christian König
0 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-02 20:48 UTC (permalink / raw)
To: Greg KH
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list
Am 03.11.20 um 07:53 schrieb Greg KH:
> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
>> Am 02.11.20 um 20:43 schrieb Alex Deucher:
>>> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>> Initializing global variable to 0 or NULL is not necessary and should
>>>> be avoided. Issue reported by checkpatch script as:
>>>> ERROR: do not initialise globals to 0 (or NULL).
>>> I agree that this is technically correct, but a lot of people don't
>>> seem to know that so we get a lot of comments about this code for the
>>> variables that are not explicitly set. Seems less confusing to
>>> initialize them even if it not necessary. I don't have a particularly
>>> strong opinion on it however.
>> Agree with Alex.
>>
>> Especially for the module parameters we should have a explicit init value
>> for documentation purposes, even when it is 0.
> Why is this one tiny driver somehow special compared to the entire rest
> of the kernel? (hint, it isn't...)
And it certainly shouldn't :)
> Please follow the normal coding style rules, there's no reason to ignore
> them unless you like to constantly reject patches like this that get
> sent to you.
Yeah, that's a rather good point.
Not a particular strong opinion on this either, but when something
global is set to 0 people usually do this to emphases that it is
important that it is zero.
Regards,
Christian.
>
> thnaks,
>
> greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-02 20:48 ` Christian König
(?)
@ 2020-11-03 7:53 ` Greg KH
-1 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 7:53 UTC (permalink / raw)
To: Christian König
Cc: Alex Deucher, Deepak R Varma, David Airlie, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Daniel Vetter, Alex Deucher
On Mon, Nov 02, 2020 at 09:48:25PM +0100, Christian König wrote:
> Am 03.11.20 um 07:53 schrieb Greg KH:
> > On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > > > Initializing global variable to 0 or NULL is not necessary and should
> > > > > be avoided. Issue reported by checkpatch script as:
> > > > > ERROR: do not initialise globals to 0 (or NULL).
> > > > I agree that this is technically correct, but a lot of people don't
> > > > seem to know that so we get a lot of comments about this code for the
> > > > variables that are not explicitly set. Seems less confusing to
> > > > initialize them even if it not necessary. I don't have a particularly
> > > > strong opinion on it however.
> > > Agree with Alex.
> > >
> > > Especially for the module parameters we should have a explicit init value
> > > for documentation purposes, even when it is 0.
> > Why is this one tiny driver somehow special compared to the entire rest
> > of the kernel? (hint, it isn't...)
>
> And it certainly shouldn't :)
>
> > Please follow the normal coding style rules, there's no reason to ignore
> > them unless you like to constantly reject patches like this that get
> > sent to you.
>
> Yeah, that's a rather good point.
>
> Not a particular strong opinion on this either, but when something global is
> set to 0 people usually do this to emphases that it is important that it is
> zero.
Again, no, that's not what we have been doing in the kernel for the past
20+ years. If you do not set it to anything, we all know it is
important for it to be set to 0. Otherwise we would explicitly set it
to something else. And if we don't care, then that too doesn't matter
so we let it be 0 by not initializing it, it doesn't matter.
I think this very change is what started the whole "kernel janitor"
movement all those years ago, because it was easily proven that this
simple change saved both time and memory.
This shouldn't even be an argument we are having anymore...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 7:53 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 7:53 UTC (permalink / raw)
To: Christian König
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list, Daniel Vetter, Alex Deucher
On Mon, Nov 02, 2020 at 09:48:25PM +0100, Christian König wrote:
> Am 03.11.20 um 07:53 schrieb Greg KH:
> > On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > > > Initializing global variable to 0 or NULL is not necessary and should
> > > > > be avoided. Issue reported by checkpatch script as:
> > > > > ERROR: do not initialise globals to 0 (or NULL).
> > > > I agree that this is technically correct, but a lot of people don't
> > > > seem to know that so we get a lot of comments about this code for the
> > > > variables that are not explicitly set. Seems less confusing to
> > > > initialize them even if it not necessary. I don't have a particularly
> > > > strong opinion on it however.
> > > Agree with Alex.
> > >
> > > Especially for the module parameters we should have a explicit init value
> > > for documentation purposes, even when it is 0.
> > Why is this one tiny driver somehow special compared to the entire rest
> > of the kernel? (hint, it isn't...)
>
> And it certainly shouldn't :)
>
> > Please follow the normal coding style rules, there's no reason to ignore
> > them unless you like to constantly reject patches like this that get
> > sent to you.
>
> Yeah, that's a rather good point.
>
> Not a particular strong opinion on this either, but when something global is
> set to 0 people usually do this to emphases that it is important that it is
> zero.
Again, no, that's not what we have been doing in the kernel for the past
20+ years. If you do not set it to anything, we all know it is
important for it to be set to 0. Otherwise we would explicitly set it
to something else. And if we don't care, then that too doesn't matter
so we let it be 0 by not initializing it, it doesn't matter.
I think this very change is what started the whole "kernel janitor"
movement all those years ago, because it was easily proven that this
simple change saved both time and memory.
This shouldn't even be an argument we are having anymore...
thanks,
greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 7:53 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 7:53 UTC (permalink / raw)
To: Christian König
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list
On Mon, Nov 02, 2020 at 09:48:25PM +0100, Christian König wrote:
> Am 03.11.20 um 07:53 schrieb Greg KH:
> > On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > > > Initializing global variable to 0 or NULL is not necessary and should
> > > > > be avoided. Issue reported by checkpatch script as:
> > > > > ERROR: do not initialise globals to 0 (or NULL).
> > > > I agree that this is technically correct, but a lot of people don't
> > > > seem to know that so we get a lot of comments about this code for the
> > > > variables that are not explicitly set. Seems less confusing to
> > > > initialize them even if it not necessary. I don't have a particularly
> > > > strong opinion on it however.
> > > Agree with Alex.
> > >
> > > Especially for the module parameters we should have a explicit init value
> > > for documentation purposes, even when it is 0.
> > Why is this one tiny driver somehow special compared to the entire rest
> > of the kernel? (hint, it isn't...)
>
> And it certainly shouldn't :)
>
> > Please follow the normal coding style rules, there's no reason to ignore
> > them unless you like to constantly reject patches like this that get
> > sent to you.
>
> Yeah, that's a rather good point.
>
> Not a particular strong opinion on this either, but when something global is
> set to 0 people usually do this to emphases that it is important that it is
> zero.
Again, no, that's not what we have been doing in the kernel for the past
20+ years. If you do not set it to anything, we all know it is
important for it to be set to 0. Otherwise we would explicitly set it
to something else. And if we don't care, then that too doesn't matter
so we let it be 0 by not initializing it, it doesn't matter.
I think this very change is what started the whole "kernel janitor"
movement all those years ago, because it was easily proven that this
simple change saved both time and memory.
This shouldn't even be an argument we are having anymore...
thanks,
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-03 7:53 ` Greg KH
(?)
@ 2020-11-03 8:23 ` Christian König
-1 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-03 8:23 UTC (permalink / raw)
To: Greg KH
Cc: Alex Deucher, Deepak R Varma, David Airlie, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Daniel Vetter, Alex Deucher
Am 03.11.20 um 08:53 schrieb Greg KH:
> On Mon, Nov 02, 2020 at 09:48:25PM +0100, Christian König wrote:
>> Am 03.11.20 um 07:53 schrieb Greg KH:
>>> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
>>>> Am 02.11.20 um 20:43 schrieb Alex Deucher:
>>>>> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>>>> Initializing global variable to 0 or NULL is not necessary and should
>>>>>> be avoided. Issue reported by checkpatch script as:
>>>>>> ERROR: do not initialise globals to 0 (or NULL).
>>>>> I agree that this is technically correct, but a lot of people don't
>>>>> seem to know that so we get a lot of comments about this code for the
>>>>> variables that are not explicitly set. Seems less confusing to
>>>>> initialize them even if it not necessary. I don't have a particularly
>>>>> strong opinion on it however.
>>>> Agree with Alex.
>>>>
>>>> Especially for the module parameters we should have a explicit init value
>>>> for documentation purposes, even when it is 0.
>>> Why is this one tiny driver somehow special compared to the entire rest
>>> of the kernel? (hint, it isn't...)
>> And it certainly shouldn't :)
>>
>>> Please follow the normal coding style rules, there's no reason to ignore
>>> them unless you like to constantly reject patches like this that get
>>> sent to you.
>> Yeah, that's a rather good point.
>>
>> Not a particular strong opinion on this either, but when something global is
>> set to 0 people usually do this to emphases that it is important that it is
>> zero.
> Again, no, that's not what we have been doing in the kernel for the past
> 20+ years. If you do not set it to anything, we all know it is
> important for it to be set to 0. Otherwise we would explicitly set it
> to something else. And if we don't care, then that too doesn't matter
> so we let it be 0 by not initializing it, it doesn't matter.
>
> I think this very change is what started the whole "kernel janitor"
> movement all those years ago, because it was easily proven that this
> simple change saved both time and memory.
Ok, well that is even better because it is a technical argument.
You have convinced me, the patch is Reviewed-by: Christian König
<christian.koenig@amd.com>.
Regards,
Christian.
>
> This shouldn't even be an argument we are having anymore...
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 8:23 ` Christian König
0 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-03 8:23 UTC (permalink / raw)
To: Greg KH
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list, Daniel Vetter, Alex Deucher
Am 03.11.20 um 08:53 schrieb Greg KH:
> On Mon, Nov 02, 2020 at 09:48:25PM +0100, Christian König wrote:
>> Am 03.11.20 um 07:53 schrieb Greg KH:
>>> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
>>>> Am 02.11.20 um 20:43 schrieb Alex Deucher:
>>>>> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>>>> Initializing global variable to 0 or NULL is not necessary and should
>>>>>> be avoided. Issue reported by checkpatch script as:
>>>>>> ERROR: do not initialise globals to 0 (or NULL).
>>>>> I agree that this is technically correct, but a lot of people don't
>>>>> seem to know that so we get a lot of comments about this code for the
>>>>> variables that are not explicitly set. Seems less confusing to
>>>>> initialize them even if it not necessary. I don't have a particularly
>>>>> strong opinion on it however.
>>>> Agree with Alex.
>>>>
>>>> Especially for the module parameters we should have a explicit init value
>>>> for documentation purposes, even when it is 0.
>>> Why is this one tiny driver somehow special compared to the entire rest
>>> of the kernel? (hint, it isn't...)
>> And it certainly shouldn't :)
>>
>>> Please follow the normal coding style rules, there's no reason to ignore
>>> them unless you like to constantly reject patches like this that get
>>> sent to you.
>> Yeah, that's a rather good point.
>>
>> Not a particular strong opinion on this either, but when something global is
>> set to 0 people usually do this to emphases that it is important that it is
>> zero.
> Again, no, that's not what we have been doing in the kernel for the past
> 20+ years. If you do not set it to anything, we all know it is
> important for it to be set to 0. Otherwise we would explicitly set it
> to something else. And if we don't care, then that too doesn't matter
> so we let it be 0 by not initializing it, it doesn't matter.
>
> I think this very change is what started the whole "kernel janitor"
> movement all those years ago, because it was easily proven that this
> simple change saved both time and memory.
Ok, well that is even better because it is a technical argument.
You have convinced me, the patch is Reviewed-by: Christian König
<christian.koenig@amd.com>.
Regards,
Christian.
>
> This shouldn't even be an argument we are having anymore...
>
> thanks,
>
> greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 8:23 ` Christian König
0 siblings, 0 replies; 33+ messages in thread
From: Christian König @ 2020-11-03 8:23 UTC (permalink / raw)
To: Greg KH
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, Alex Deucher,
amd-gfx list
Am 03.11.20 um 08:53 schrieb Greg KH:
> On Mon, Nov 02, 2020 at 09:48:25PM +0100, Christian König wrote:
>> Am 03.11.20 um 07:53 schrieb Greg KH:
>>> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
>>>> Am 02.11.20 um 20:43 schrieb Alex Deucher:
>>>>> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>>>> Initializing global variable to 0 or NULL is not necessary and should
>>>>>> be avoided. Issue reported by checkpatch script as:
>>>>>> ERROR: do not initialise globals to 0 (or NULL).
>>>>> I agree that this is technically correct, but a lot of people don't
>>>>> seem to know that so we get a lot of comments about this code for the
>>>>> variables that are not explicitly set. Seems less confusing to
>>>>> initialize them even if it not necessary. I don't have a particularly
>>>>> strong opinion on it however.
>>>> Agree with Alex.
>>>>
>>>> Especially for the module parameters we should have a explicit init value
>>>> for documentation purposes, even when it is 0.
>>> Why is this one tiny driver somehow special compared to the entire rest
>>> of the kernel? (hint, it isn't...)
>> And it certainly shouldn't :)
>>
>>> Please follow the normal coding style rules, there's no reason to ignore
>>> them unless you like to constantly reject patches like this that get
>>> sent to you.
>> Yeah, that's a rather good point.
>>
>> Not a particular strong opinion on this either, but when something global is
>> set to 0 people usually do this to emphases that it is important that it is
>> zero.
> Again, no, that's not what we have been doing in the kernel for the past
> 20+ years. If you do not set it to anything, we all know it is
> important for it to be set to 0. Otherwise we would explicitly set it
> to something else. And if we don't care, then that too doesn't matter
> so we let it be 0 by not initializing it, it doesn't matter.
>
> I think this very change is what started the whole "kernel janitor"
> movement all those years ago, because it was easily proven that this
> simple change saved both time and memory.
Ok, well that is even better because it is a technical argument.
You have convinced me, the patch is Reviewed-by: Christian König
<christian.koenig@amd.com>.
Regards,
Christian.
>
> This shouldn't even be an argument we are having anymore...
>
> thanks,
>
> greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-03 6:53 ` Greg KH
(?)
@ 2020-11-03 14:50 ` Deucher, Alexander
-1 siblings, 0 replies; 33+ messages in thread
From: Deucher, Alexander @ 2020-11-03 14:50 UTC (permalink / raw)
To: Greg KH, Koenig, Christian
Cc: Alex Deucher, Deepak R Varma, David Airlie, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Daniel Vetter
[AMD Public Use]
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, November 3, 2020 1:53 AM
> To: Koenig, Christian <Christian.Koenig@amd.com>
> Cc: Alex Deucher <alexdeucher@gmail.com>; Deepak R Varma
> <mh12gx2825@gmail.com>; David Airlie <airlied@linux.ie>; LKML <linux-
> kernel@vger.kernel.org>; Maling list - DRI developers <dri-
> devel@lists.freedesktop.org>; Melissa Wen <melissa.srw@gmail.com>;
> amd-gfx list <amd-gfx@lists.freedesktop.org>; Daniel Vetter
> <daniel@ffwll.ch>; Daniel Vetter <daniel.vetter@ffwll.ch>; Deucher,
> Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or
> NULL
>
> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma
> <mh12gx2825@gmail.com> wrote:
> > > > Initializing global variable to 0 or NULL is not necessary and
> > > > should be avoided. Issue reported by checkpatch script as:
> > > > ERROR: do not initialise globals to 0 (or NULL).
> > > I agree that this is technically correct, but a lot of people don't
> > > seem to know that so we get a lot of comments about this code for
> > > the variables that are not explicitly set. Seems less confusing to
> > > initialize them even if it not necessary. I don't have a
> > > particularly strong opinion on it however.
> >
> > Agree with Alex.
> >
> > Especially for the module parameters we should have a explicit init
> > value for documentation purposes, even when it is 0.
>
> Why is this one tiny driver somehow special compared to the entire rest of
> the kernel? (hint, it isn't...)
>
> Please follow the normal coding style rules, there's no reason to ignore them
> unless you like to constantly reject patches like this that get sent to you.
>
I'll apply the patch, but as a data point, this is the first time I've gotten a patch to make this change. I get several bug reports or patches every year to explicitly set values to global variables because users assume they are not initialized. So it will always be a trade off as to which patches you want to NACK.
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 14:50 ` Deucher, Alexander
0 siblings, 0 replies; 33+ messages in thread
From: Deucher, Alexander @ 2020-11-03 14:50 UTC (permalink / raw)
To: Greg KH, Koenig, Christian
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Alex Deucher
[AMD Public Use]
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, November 3, 2020 1:53 AM
> To: Koenig, Christian <Christian.Koenig@amd.com>
> Cc: Alex Deucher <alexdeucher@gmail.com>; Deepak R Varma
> <mh12gx2825@gmail.com>; David Airlie <airlied@linux.ie>; LKML <linux-
> kernel@vger.kernel.org>; Maling list - DRI developers <dri-
> devel@lists.freedesktop.org>; Melissa Wen <melissa.srw@gmail.com>;
> amd-gfx list <amd-gfx@lists.freedesktop.org>; Daniel Vetter
> <daniel@ffwll.ch>; Daniel Vetter <daniel.vetter@ffwll.ch>; Deucher,
> Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or
> NULL
>
> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma
> <mh12gx2825@gmail.com> wrote:
> > > > Initializing global variable to 0 or NULL is not necessary and
> > > > should be avoided. Issue reported by checkpatch script as:
> > > > ERROR: do not initialise globals to 0 (or NULL).
> > > I agree that this is technically correct, but a lot of people don't
> > > seem to know that so we get a lot of comments about this code for
> > > the variables that are not explicitly set. Seems less confusing to
> > > initialize them even if it not necessary. I don't have a
> > > particularly strong opinion on it however.
> >
> > Agree with Alex.
> >
> > Especially for the module parameters we should have a explicit init
> > value for documentation purposes, even when it is 0.
>
> Why is this one tiny driver somehow special compared to the entire rest of
> the kernel? (hint, it isn't...)
>
> Please follow the normal coding style rules, there's no reason to ignore them
> unless you like to constantly reject patches like this that get sent to you.
>
I'll apply the patch, but as a data point, this is the first time I've gotten a patch to make this change. I get several bug reports or patches every year to explicitly set values to global variables because users assume they are not initialized. So it will always be a trade off as to which patches you want to NACK.
Alex
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 14:50 ` Deucher, Alexander
0 siblings, 0 replies; 33+ messages in thread
From: Deucher, Alexander @ 2020-11-03 14:50 UTC (permalink / raw)
To: Greg KH, Koenig, Christian
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list
[AMD Public Use]
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, November 3, 2020 1:53 AM
> To: Koenig, Christian <Christian.Koenig@amd.com>
> Cc: Alex Deucher <alexdeucher@gmail.com>; Deepak R Varma
> <mh12gx2825@gmail.com>; David Airlie <airlied@linux.ie>; LKML <linux-
> kernel@vger.kernel.org>; Maling list - DRI developers <dri-
> devel@lists.freedesktop.org>; Melissa Wen <melissa.srw@gmail.com>;
> amd-gfx list <amd-gfx@lists.freedesktop.org>; Daniel Vetter
> <daniel@ffwll.ch>; Daniel Vetter <daniel.vetter@ffwll.ch>; Deucher,
> Alexander <Alexander.Deucher@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or
> NULL
>
> On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma
> <mh12gx2825@gmail.com> wrote:
> > > > Initializing global variable to 0 or NULL is not necessary and
> > > > should be avoided. Issue reported by checkpatch script as:
> > > > ERROR: do not initialise globals to 0 (or NULL).
> > > I agree that this is technically correct, but a lot of people don't
> > > seem to know that so we get a lot of comments about this code for
> > > the variables that are not explicitly set. Seems less confusing to
> > > initialize them even if it not necessary. I don't have a
> > > particularly strong opinion on it however.
> >
> > Agree with Alex.
> >
> > Especially for the module parameters we should have a explicit init
> > value for documentation purposes, even when it is 0.
>
> Why is this one tiny driver somehow special compared to the entire rest of
> the kernel? (hint, it isn't...)
>
> Please follow the normal coding style rules, there's no reason to ignore them
> unless you like to constantly reject patches like this that get sent to you.
>
I'll apply the patch, but as a data point, this is the first time I've gotten a patch to make this change. I get several bug reports or patches every year to explicitly set values to global variables because users assume they are not initialized. So it will always be a trade off as to which patches you want to NACK.
Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-03 14:50 ` Deucher, Alexander
(?)
@ 2020-11-03 15:34 ` Greg KH
-1 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 15:34 UTC (permalink / raw)
To: Deucher, Alexander
Cc: Koenig, Christian, Alex Deucher, Deepak R Varma, David Airlie,
LKML, Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Daniel Vetter
On Tue, Nov 03, 2020 at 02:50:40PM +0000, Deucher, Alexander wrote:
> [AMD Public Use]
>
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Tuesday, November 3, 2020 1:53 AM
> > To: Koenig, Christian <Christian.Koenig@amd.com>
> > Cc: Alex Deucher <alexdeucher@gmail.com>; Deepak R Varma
> > <mh12gx2825@gmail.com>; David Airlie <airlied@linux.ie>; LKML <linux-
> > kernel@vger.kernel.org>; Maling list - DRI developers <dri-
> > devel@lists.freedesktop.org>; Melissa Wen <melissa.srw@gmail.com>;
> > amd-gfx list <amd-gfx@lists.freedesktop.org>; Daniel Vetter
> > <daniel@ffwll.ch>; Daniel Vetter <daniel.vetter@ffwll.ch>; Deucher,
> > Alexander <Alexander.Deucher@amd.com>
> > Subject: Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or
> > NULL
> >
> > On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma
> > <mh12gx2825@gmail.com> wrote:
> > > > > Initializing global variable to 0 or NULL is not necessary and
> > > > > should be avoided. Issue reported by checkpatch script as:
> > > > > ERROR: do not initialise globals to 0 (or NULL).
> > > > I agree that this is technically correct, but a lot of people don't
> > > > seem to know that so we get a lot of comments about this code for
> > > > the variables that are not explicitly set. Seems less confusing to
> > > > initialize them even if it not necessary. I don't have a
> > > > particularly strong opinion on it however.
> > >
> > > Agree with Alex.
> > >
> > > Especially for the module parameters we should have a explicit init
> > > value for documentation purposes, even when it is 0.
> >
> > Why is this one tiny driver somehow special compared to the entire rest of
> > the kernel? (hint, it isn't...)
> >
> > Please follow the normal coding style rules, there's no reason to ignore them
> > unless you like to constantly reject patches like this that get sent to you.
> >
>
> I'll apply the patch, but as a data point, this is the first time I've
> gotten a patch to make this change. I get several bug reports or
> patches every year to explicitly set values to global variables
> because users assume they are not initialized. So it will always be a
> trade off as to which patches you want to NACK.
Are you all not running your patches through checkpatch.pl to tell you
simple things like this? If no, I suggest you start doing that :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 15:34 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 15:34 UTC (permalink / raw)
To: Deucher, Alexander
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list,
Daniel Vetter, Alex Deucher, Koenig, Christian
On Tue, Nov 03, 2020 at 02:50:40PM +0000, Deucher, Alexander wrote:
> [AMD Public Use]
>
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Tuesday, November 3, 2020 1:53 AM
> > To: Koenig, Christian <Christian.Koenig@amd.com>
> > Cc: Alex Deucher <alexdeucher@gmail.com>; Deepak R Varma
> > <mh12gx2825@gmail.com>; David Airlie <airlied@linux.ie>; LKML <linux-
> > kernel@vger.kernel.org>; Maling list - DRI developers <dri-
> > devel@lists.freedesktop.org>; Melissa Wen <melissa.srw@gmail.com>;
> > amd-gfx list <amd-gfx@lists.freedesktop.org>; Daniel Vetter
> > <daniel@ffwll.ch>; Daniel Vetter <daniel.vetter@ffwll.ch>; Deucher,
> > Alexander <Alexander.Deucher@amd.com>
> > Subject: Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or
> > NULL
> >
> > On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma
> > <mh12gx2825@gmail.com> wrote:
> > > > > Initializing global variable to 0 or NULL is not necessary and
> > > > > should be avoided. Issue reported by checkpatch script as:
> > > > > ERROR: do not initialise globals to 0 (or NULL).
> > > > I agree that this is technically correct, but a lot of people don't
> > > > seem to know that so we get a lot of comments about this code for
> > > > the variables that are not explicitly set. Seems less confusing to
> > > > initialize them even if it not necessary. I don't have a
> > > > particularly strong opinion on it however.
> > >
> > > Agree with Alex.
> > >
> > > Especially for the module parameters we should have a explicit init
> > > value for documentation purposes, even when it is 0.
> >
> > Why is this one tiny driver somehow special compared to the entire rest of
> > the kernel? (hint, it isn't...)
> >
> > Please follow the normal coding style rules, there's no reason to ignore them
> > unless you like to constantly reject patches like this that get sent to you.
> >
>
> I'll apply the patch, but as a data point, this is the first time I've
> gotten a patch to make this change. I get several bug reports or
> patches every year to explicitly set values to global variables
> because users assume they are not initialized. So it will always be a
> trade off as to which patches you want to NACK.
Are you all not running your patches through checkpatch.pl to tell you
simple things like this? If no, I suggest you start doing that :)
thanks,
greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-03 15:34 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-03 15:34 UTC (permalink / raw)
To: Deucher, Alexander
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML,
Maling list - DRI developers, Melissa Wen, amd-gfx list, Koenig,
Christian
On Tue, Nov 03, 2020 at 02:50:40PM +0000, Deucher, Alexander wrote:
> [AMD Public Use]
>
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Tuesday, November 3, 2020 1:53 AM
> > To: Koenig, Christian <Christian.Koenig@amd.com>
> > Cc: Alex Deucher <alexdeucher@gmail.com>; Deepak R Varma
> > <mh12gx2825@gmail.com>; David Airlie <airlied@linux.ie>; LKML <linux-
> > kernel@vger.kernel.org>; Maling list - DRI developers <dri-
> > devel@lists.freedesktop.org>; Melissa Wen <melissa.srw@gmail.com>;
> > amd-gfx list <amd-gfx@lists.freedesktop.org>; Daniel Vetter
> > <daniel@ffwll.ch>; Daniel Vetter <daniel.vetter@ffwll.ch>; Deucher,
> > Alexander <Alexander.Deucher@amd.com>
> > Subject: Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or
> > NULL
> >
> > On Mon, Nov 02, 2020 at 09:06:21PM +0100, Christian König wrote:
> > > Am 02.11.20 um 20:43 schrieb Alex Deucher:
> > > > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma
> > <mh12gx2825@gmail.com> wrote:
> > > > > Initializing global variable to 0 or NULL is not necessary and
> > > > > should be avoided. Issue reported by checkpatch script as:
> > > > > ERROR: do not initialise globals to 0 (or NULL).
> > > > I agree that this is technically correct, but a lot of people don't
> > > > seem to know that so we get a lot of comments about this code for
> > > > the variables that are not explicitly set. Seems less confusing to
> > > > initialize them even if it not necessary. I don't have a
> > > > particularly strong opinion on it however.
> > >
> > > Agree with Alex.
> > >
> > > Especially for the module parameters we should have a explicit init
> > > value for documentation purposes, even when it is 0.
> >
> > Why is this one tiny driver somehow special compared to the entire rest of
> > the kernel? (hint, it isn't...)
> >
> > Please follow the normal coding style rules, there's no reason to ignore them
> > unless you like to constantly reject patches like this that get sent to you.
> >
>
> I'll apply the patch, but as a data point, this is the first time I've
> gotten a patch to make this change. I get several bug reports or
> patches every year to explicitly set values to global variables
> because users assume they are not initialized. So it will always be a
> trade off as to which patches you want to NACK.
Are you all not running your patches through checkpatch.pl to tell you
simple things like this? If no, I suggest you start doing that :)
thanks,
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-02 19:43 ` Alex Deucher
(?)
@ 2020-11-02 20:10 ` Greg KH
-1 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-02 20:10 UTC (permalink / raw)
To: Alex Deucher
Cc: Deepak R Varma, Alex Deucher, Christian König, David Airlie,
Daniel Vetter, amd-gfx list, Maling list - DRI developers, LKML,
Melissa Wen, Daniel Vetter
On Mon, Nov 02, 2020 at 02:43:45PM -0500, Alex Deucher wrote:
> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> >
> > Initializing global variable to 0 or NULL is not necessary and should
> > be avoided. Issue reported by checkpatch script as:
> > ERROR: do not initialise globals to 0 (or NULL).
>
> I agree that this is technically correct, but a lot of people don't
> seem to know that so we get a lot of comments about this code for the
> variables that are not explicitly set. Seems less confusing to
> initialize them even if it not necessary. I don't have a particularly
> strong opinion on it however.
The kernel coding style is to do it this way. You even save space and
time by doing it as well :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 20:10 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-02 20:10 UTC (permalink / raw)
To: Alex Deucher
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML, amd-gfx list,
Melissa Wen, Maling list - DRI developers, Daniel Vetter,
Alex Deucher, Christian König
On Mon, Nov 02, 2020 at 02:43:45PM -0500, Alex Deucher wrote:
> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> >
> > Initializing global variable to 0 or NULL is not necessary and should
> > be avoided. Issue reported by checkpatch script as:
> > ERROR: do not initialise globals to 0 (or NULL).
>
> I agree that this is technically correct, but a lot of people don't
> seem to know that so we get a lot of comments about this code for the
> variables that are not explicitly set. Seems less confusing to
> initialize them even if it not necessary. I don't have a particularly
> strong opinion on it however.
The kernel coding style is to do it this way. You even save space and
time by doing it as well :)
thanks,
greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 20:10 ` Greg KH
0 siblings, 0 replies; 33+ messages in thread
From: Greg KH @ 2020-11-02 20:10 UTC (permalink / raw)
To: Alex Deucher
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML, amd-gfx list,
Melissa Wen, Maling list - DRI developers, Alex Deucher,
Christian König
On Mon, Nov 02, 2020 at 02:43:45PM -0500, Alex Deucher wrote:
> On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> >
> > Initializing global variable to 0 or NULL is not necessary and should
> > be avoided. Issue reported by checkpatch script as:
> > ERROR: do not initialise globals to 0 (or NULL).
>
> I agree that this is technically correct, but a lot of people don't
> seem to know that so we get a lot of comments about this code for the
> variables that are not explicitly set. Seems less confusing to
> initialize them even if it not necessary. I don't have a particularly
> strong opinion on it however.
The kernel coding style is to do it this way. You even save space and
time by doing it as well :)
thanks,
greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
2020-11-02 20:10 ` Greg KH
(?)
@ 2020-11-02 22:30 ` David Laight
-1 siblings, 0 replies; 33+ messages in thread
From: David Laight @ 2020-11-02 22:30 UTC (permalink / raw)
To: 'Greg KH', Alex Deucher
Cc: Deepak R Varma, Alex Deucher, Christian König, David Airlie,
Daniel Vetter, amd-gfx list, Maling list - DRI developers, LKML,
Melissa Wen, Daniel Vetter
From: Greg KH
> Sent: 02 November 2020 20:11
>
> On Mon, Nov 02, 2020 at 02:43:45PM -0500, Alex Deucher wrote:
> > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > >
> > > Initializing global variable to 0 or NULL is not necessary and should
> > > be avoided. Issue reported by checkpatch script as:
> > > ERROR: do not initialise globals to 0 (or NULL).
> >
> > I agree that this is technically correct, but a lot of people don't
> > seem to know that so we get a lot of comments about this code for the
> > variables that are not explicitly set. Seems less confusing to
> > initialize them even if it not necessary. I don't have a particularly
> > strong opinion on it however.
>
> The kernel coding style is to do it this way. You even save space and
> time by doing it as well :)
Uninitialised globals end up as 'named common' (variables that are
their own code section - from FORTRAN) until the final link puts
them into the .bss.
Globals initialised to 0 go into the .bss of the object file
being created.
So both end up in the final .bss.
If the code goes into a module you aren't allowed 'common' data
in a module to every global must be initialised.
I'm surprised checkpatch complains.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 22:30 ` David Laight
0 siblings, 0 replies; 33+ messages in thread
From: David Laight @ 2020-11-02 22:30 UTC (permalink / raw)
To: 'Greg KH', Alex Deucher
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML, amd-gfx list,
Melissa Wen, Maling list - DRI developers, Daniel Vetter,
Alex Deucher, Christian König
From: Greg KH
> Sent: 02 November 2020 20:11
>
> On Mon, Nov 02, 2020 at 02:43:45PM -0500, Alex Deucher wrote:
> > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > >
> > > Initializing global variable to 0 or NULL is not necessary and should
> > > be avoided. Issue reported by checkpatch script as:
> > > ERROR: do not initialise globals to 0 (or NULL).
> >
> > I agree that this is technically correct, but a lot of people don't
> > seem to know that so we get a lot of comments about this code for the
> > variables that are not explicitly set. Seems less confusing to
> > initialize them even if it not necessary. I don't have a particularly
> > strong opinion on it however.
>
> The kernel coding style is to do it this way. You even save space and
> time by doing it as well :)
Uninitialised globals end up as 'named common' (variables that are
their own code section - from FORTRAN) until the final link puts
them into the .bss.
Globals initialised to 0 go into the .bss of the object file
being created.
So both end up in the final .bss.
If the code goes into a module you aren't allowed 'common' data
in a module to every global must be initialised.
I'm surprised checkpatch complains.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH] drm/amdgpu: do not initialise global variables to 0 or NULL
@ 2020-11-02 22:30 ` David Laight
0 siblings, 0 replies; 33+ messages in thread
From: David Laight @ 2020-11-02 22:30 UTC (permalink / raw)
To: 'Greg KH', Alex Deucher
Cc: Deepak R Varma, David Airlie, Daniel Vetter, LKML, amd-gfx list,
Melissa Wen, Maling list - DRI developers, Alex Deucher,
Christian König
From: Greg KH
> Sent: 02 November 2020 20:11
>
> On Mon, Nov 02, 2020 at 02:43:45PM -0500, Alex Deucher wrote:
> > On Mon, Nov 2, 2020 at 1:42 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > >
> > > Initializing global variable to 0 or NULL is not necessary and should
> > > be avoided. Issue reported by checkpatch script as:
> > > ERROR: do not initialise globals to 0 (or NULL).
> >
> > I agree that this is technically correct, but a lot of people don't
> > seem to know that so we get a lot of comments about this code for the
> > variables that are not explicitly set. Seems less confusing to
> > initialize them even if it not necessary. I don't have a particularly
> > strong opinion on it however.
>
> The kernel coding style is to do it this way. You even save space and
> time by doing it as well :)
Uninitialised globals end up as 'named common' (variables that are
their own code section - from FORTRAN) until the final link puts
them into the .bss.
Globals initialised to 0 go into the .bss of the object file
being created.
So both end up in the final .bss.
If the code goes into a module you aren't allowed 'common' data
in a module to every global must be initialised.
I'm surprised checkpatch complains.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 33+ messages in thread