Drivers: gpu: drm: Fix coccinelle and checkpatch issues

PATCH Superseded Georgiana Chelu 2 participants 6 messages Oct 14, 2017 – Oct 17, 2017

Subsystems (2)

DRM drivers Radeon and amdgpu DRM drivers

lore

Share & tools

lore

Georgiana Chelu

author

2017-10-14 12:55 UTC #0 lore raw

cover Drivers: gpu: drm: Fix coccinelle and checkpatch issues

The first two patches fix coccinelle issues and the last patch fix a checkpatch issue.

First patch: The return value of kzalloc does not need a cast because the assignment operator will take care of this.

Second patch: The 'result' variable does not change its value until the end of the function. Instead of using the variable, return its default value.

Third patch: Placing the constant on the right side will make the code easier to read, as most of the people read from right to left.

Georgiana Chelu (3):
Drivers: gpu: drm: Don't cast kzalloc() return value
Drivers: gpu: drm: Remove useless variable
Drivers: gpu: drm: Place the constant on the right side of the test

 .../amd/powerplay/hwmgr/vega10_processpptables.c   | 35 ++++++++--------------
 1 file changed, 13 insertions(+), 22 deletions(-)

Signature · 1 line

2.11.0

Georgiana Chelu

author

2017-10-14 12:55 UTC #1 lore raw

PATCH 1/3 Drivers: gpu: drm: Don't cast kzalloc() return value

The kzalloc function returns a void pointer and the assignment operator converts it to the type of pointer it is assigned to. Therefore, there is no need to cast.

Issue found by coccinelle script:
WARNING: casting value returned by memory allocation function to <struct type> is useless.

Signed-off-by Georgiana Chelu <[email protected]>

---
 .../amd/powerplay/hwmgr/vega10_processpptables.c   | 24 ++++++++--------------
 1 file changed, 8 insertions(+), 16 deletions(-)

drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c +8 ~~-16~~

index e343df190375..d968c3834481 100644
@@ -291,8 +291,7 @@ static int get_mm_clock_voltage_table(
 	table_size = sizeof(uint32_t) +
 			sizeof(phm_ppt_v1_mm_clock_voltage_dependency_record) *
 			mm_dependency_table->ucNumEntries;
-	mm_table = (phm_ppt_v1_mm_clock_voltage_dependency_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	mm_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!mm_table)
 		return -ENOMEM;

@@ -519,8 +518,7 @@ static int get_socclk_voltage_dependency_table(
 			sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
 			clk_dep_table->ucNumEntries;
 
-	clk_table = (phm_ppt_v1_clock_voltage_dependency_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	clk_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!clk_table)
 		return -ENOMEM;

@@ -554,8 +552,7 @@ static int get_mclk_voltage_dependency_table(
 			sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
 			mclk_dep_table->ucNumEntries;
 
-	mclk_table = (phm_ppt_v1_clock_voltage_dependency_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	mclk_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!mclk_table)
 		return -ENOMEM;

@@ -596,8 +593,7 @@ static int get_gfxclk_voltage_dependency_table(
 			sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
 			clk_dep_table->ucNumEntries;
 
-	clk_table = (struct phm_ppt_v1_clock_voltage_dependency_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	clk_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!clk_table)
 		return -ENOMEM;

@@ -663,8 +659,7 @@ static int get_pix_clk_voltage_dependency_table(
 			sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
 			clk_dep_table->ucNumEntries;
 
-	clk_table = (struct phm_ppt_v1_clock_voltage_dependency_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	clk_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!clk_table)
 		return -ENOMEM;

@@ -728,8 +723,7 @@ static int get_dcefclk_voltage_dependency_table(
 			sizeof(phm_ppt_v1_clock_voltage_dependency_record) *
 			num_entries;
 
-	clk_table = (struct phm_ppt_v1_clock_voltage_dependency_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	clk_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!clk_table)
 		return -ENOMEM;

@@ -772,8 +766,7 @@ static int get_pcie_table(struct pp_hwmgr *hwmgr,
 			sizeof(struct phm_ppt_v1_pcie_record) *
 			atom_pcie_table->ucNumEntries;
 
-	pcie_table = (struct phm_ppt_v1_pcie_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	pcie_table = kzalloc(table_size, GFP_KERNEL);
 
 	if (!pcie_table)
 		return -ENOMEM;

@@ -1026,8 +1019,7 @@ static int get_vddc_lookup_table(
 	table_size = sizeof(uint32_t) +
 			sizeof(phm_ppt_v1_voltage_lookup_record) * max_levels;
 
-	table = (phm_ppt_v1_voltage_lookup_table *)
-			kzalloc(table_size, GFP_KERNEL);
+	table = kzalloc(table_size, GFP_KERNEL);
 
 	if (NULL == table)
 		return -ENOMEM;

Signature · 1 line

2.11.0

Georgiana Chelu

author

2017-10-14 12:55 UTC #2 lore raw

PATCH 2/3 Drivers: gpu: drm: Remove useless variable

In reply to Georgiana Chelu

The result variable is initialized at the beginning of the function, but its value does not change during the function execution. Thus, remove the variable and return the SUCCESS value, which is 0.

Issue found by coccinelle script:
* Unneeded variable: "result". Return "0"

Signed-off-by Georgiana Chelu <[email protected]>

---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c +1 -2

index d968c3834481..769ac11a9215 100644
@@ -1174,7 +1174,6 @@ int vega10_pp_tables_initialize(struct pp_hwmgr *hwmgr)
 
 static int vega10_pp_tables_uninitialize(struct pp_hwmgr *hwmgr)
 {
-	int result = 0;
 	struct phm_ppt_v2_information *pp_table_info =
 			(struct phm_ppt_v2_information *)(hwmgr->pptable);

@@ -1217,7 +1216,7 @@ static int vega10_pp_tables_uninitialize(struct pp_hwmgr *hwmgr)
 	kfree(hwmgr->pptable);
 	hwmgr->pptable = NULL;
 
-	return result;
+	return 0;
 }
 
 const struct pp_table_func vega10_pptable_funcs = {

Signature · 1 line

2.11.0

Georgiana Chelu

author

2017-10-14 12:55 UTC #3 lore raw

PATCH 3/3 Drivers: gpu: drm: Place the constant on the right side of the test

In reply to Georgiana Chelu

Move the constant on the right side of the comparison in order to make the code easier to read.

Issue found by checkpatch script:
* WARNING: Comparisons should place the constant on the right side of the test

Signed-off-by Georgiana Chelu <[email protected]>

---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c +4 -4

index 769ac11a9215..f14c7611fad3 100644
@@ -1021,7 +1021,7 @@ static int get_vddc_lookup_table(
 
 	table = kzalloc(table_size, GFP_KERNEL);
 
-	if (NULL == table)
+	if (table == NULL)
 		return -ENOMEM;
 
 	table->count = vddc_lookup_pp_tables->ucNumEntries;

@@ -1130,12 +1130,12 @@ int vega10_pp_tables_initialize(struct pp_hwmgr *hwmgr)
 
 	hwmgr->pptable = kzalloc(sizeof(struct phm_ppt_v2_information), GFP_KERNEL);
 
-	PP_ASSERT_WITH_CODE((NULL != hwmgr->pptable),
+	PP_ASSERT_WITH_CODE((hwmgr->pptable != NULL),
 			    "Failed to allocate hwmgr->pptable!", return -ENOMEM);
 
 	powerplay_table = get_powerplay_table(hwmgr);
 
-	PP_ASSERT_WITH_CODE((NULL != powerplay_table),
+	PP_ASSERT_WITH_CODE((powerplay_table != NULL),
 		"Missing PowerPlay Table!", return -1);
 
 	result = check_powerplay_tables(hwmgr, powerplay_table);

@@ -1229,7 +1229,7 @@ int vega10_get_number_of_powerplay_table_entries(struct pp_hwmgr *hwmgr)
 	const ATOM_Vega10_State_Array *state_arrays;
 	const ATOM_Vega10_POWERPLAYTABLE *pp_table = get_powerplay_table(hwmgr);
 
-	PP_ASSERT_WITH_CODE((NULL != pp_table),
+	PP_ASSERT_WITH_CODE((pp_table != NULL),
 			"Missing PowerPlay Table!", return -1);
 	PP_ASSERT_WITH_CODE((pp_table->sHeader.format_revision >=
 			ATOM_Vega10_TABLE_REVISION_VEGA10),

Signature · 1 line

2.11.0

Sean Paul

2017-10-16 18:39 UTC #4 lore raw

In reply to Georgiana Chelu

Quoted 9 lines · Georgiana Chelu

The first two patches fix coccinelle issues and the last patch fix a checkpatch issue.
First patch: The return value of kzalloc does not need a cast because the assignment operator will take care of this.
Second patch: The 'result' variable does not change its value until the end of the function. Instead of using the variable, return its default value.

Quoted 8 lines

Third patch: Placing the constant on the right side will make the code easier to read, as most of the people read from right to left.
Georgiana Chelu (3):
Drivers: gpu: drm: Don't cast kzalloc() return value
Drivers: gpu: drm: Remove useless variable
Drivers: gpu: drm: Place the constant on the right side of the test

Hi Georgiana,
Thank you for your patches.

A couple things:
- Please look at git log for the file(s) you're changing when choosing your subject prefix. In this instance, it should be "drm/amd/powerplay:"
- Please cc [email protected] on drm patches
- Please post the cocci script or the path to it when submitting cocci-generated patches.

I look forward to your v2!

Sean

Quoted 12 lines

 .../amd/powerplay/hwmgr/vega10_processpptables.c   | 35 ++++++++--------------
 1 file changed, 13 insertions(+), 22 deletions(-)
--
2.11.0
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1507985001.git.georgiana.chelu93%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Georgiana Chelu

author

2017-10-17 20:21 UTC #5 lore raw

Quoted 31 lines · Sean Paul

Quoted 9 lines · Georgiana Chelu
The first two patches fix coccinelle issues and the last patch fix a checkpatch issue.
First patch: The return value of kzalloc does not need a cast because the assignment operator will take care of this.
Second patch: The 'result' variable does not change its value until the end of the function. Instead of using the variable, return its default value.
Quoted 8 lines
Third patch: Placing the constant on the right side will make the code easier to read, as most of the people read from right to left.
Georgiana Chelu (3):
Drivers: gpu: drm: Don't cast kzalloc() return value
Drivers: gpu: drm: Remove useless variable
Drivers: gpu: drm: Place the constant on the right side of the test
Hi Georgiana,
Thank you for your patches.
A couple things:
- Please look at git log for the file(s) you're changing when choosing your subject prefix. In this instance, it should be "drm/amd/powerplay:"
- Please cc [email protected] on drm patches
- Please post the cocci script or the path to it when submitting cocci-generated patches.

Quoted 2 lines

I look forward to your v2!

Hi Sean,

Thank you for the review! I will resend the patches soon.

Georgiana

Quoted 15 lines

Sean
Quoted 12 lines
 .../amd/powerplay/hwmgr/vega10_processpptables.c   | 35 ++++++++--------------
 1 file changed, 13 insertions(+), 22 deletions(-)
--
2.11.0
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1507985001.git.georgiana.chelu93%40gmail.com.
For more options, visit https://groups.google.com/d/optout.