staging: sm750fb: Fix multiple checkpatch warnings

PATCH In review Rehas Sachdeva 3 participants 12 messages Sep 15, 2016 – Sep 16, 2016

Subsystems (2)

Staging Staging - silicon motion SM750 frame buffer driver

lore

Share & tools

lore

Rehas Sachdeva

author

2016-09-15 21:31 UTC #0 lore raw

cover staging: sm750fb: Fix multiple checkpatch warnings

Fixes multiple checkpatch warnings in sm750fb driver.

Rehas Sachdeva (5):
staging: sm750fb: Add blank line after declarations
staging: sm750fb: Change 'x != NULL' to 'x'
staging: sm750fb: Remove multiple assignments
staging: sm750fb: Change 'uint32_t' to 'u32'
staging: sm750fb: Add spaces around '|'

 drivers/staging/sm750fb/ddk750_chip.h    |  3 +++
 drivers/staging/sm750fb/ddk750_display.h | 45 ++++++++++++++++----------------
 drivers/staging/sm750fb/ddk750_mode.h    |  2 ++
 drivers/staging/sm750fb/ddk750_power.h   |  1 +
 drivers/staging/sm750fb/sm750.c          | 14 ++++++----
 drivers/staging/sm750fb/sm750_cursor.c   |  3 +++
 drivers/staging/sm750fb/sm750_hw.c       |  2 +-
 7 files changed, 42 insertions(+), 28 deletions(-)

Signature · 1 line

2.7.4

Rehas Sachdeva

author

2016-09-15 21:32 UTC #1 lore raw

PATCH 1/5 staging: sm750fb: Add blank line after declarations

It is preferred to add a blank line after function, struct, union and enum declarations for better readability. Issue detected by checkpatch.

Signed-off-by Rehas Sachdeva <[email protected]>

---
 drivers/staging/sm750fb/ddk750_chip.h    | 3 +++
 drivers/staging/sm750fb/ddk750_display.h | 1 +
 drivers/staging/sm750fb/ddk750_mode.h    | 2 ++
 drivers/staging/sm750fb/ddk750_power.h   | 1 +
 drivers/staging/sm750fb/sm750_cursor.c   | 3 +++
 5 files changed, 10 insertions(+)

drivers/staging/sm750fb/ddk750_chip.h +3 -0

index 0891384..5ef8aad 100644
@@ -14,6 +14,7 @@ typedef enum _logical_chip_type_t {
 	SM750,
 	SM750LE,
 }
+
 logical_chip_type_t;
 
 typedef enum _clock_type_t {

@@ -23,6 +24,7 @@ typedef enum _clock_type_t {
 	VGA0_PLL,
 	VGA1_PLL,
 }
+
 clock_type_t;
 
 typedef struct _pll_value_t {

@@ -35,6 +37,7 @@ typedef struct _pll_value_t {
 	unsigned long OD;
 	unsigned long POD;
 }
+
 pll_value_t;
 
 /* input struct to initChipParam() function */

drivers/staging/sm750fb/ddk750_display.h +1 -0

index ca35aa1..04b8eba 100644
@@ -97,6 +97,7 @@ typedef enum _disp_output_t {
 	do_CRT_PRI = CRT_2_PRI|PRI_TP_ON|DPMS_ON|DAC_ON,
 	do_CRT_SEC = CRT_2_SEC|SEC_TP_ON|DPMS_ON|DAC_ON,
 }
+
 disp_output_t;
 
 void ddk750_setLogicalDispOut(disp_output_t);

drivers/staging/sm750fb/ddk750_mode.h +2 -0

index e846dc2..453c294 100644
@@ -7,6 +7,7 @@ typedef enum _spolarity_t {
 	POS = 0, /* positive */
 	NEG, /* negative */
 }
+
 spolarity_t;

@@ -33,6 +34,7 @@ typedef struct _mode_parameter_t {
 	/* Clock Phase. This clock phase only applies to Panel. */
 	spolarity_t clock_phase_polarity;
 }
+
 mode_parameter_t;
 
 int ddk750_setModeTiming(mode_parameter_t *, clock_type_t);

drivers/staging/sm750fb/ddk750_power.h +1 -0

index 5963691..c777d67 100644
@@ -7,6 +7,7 @@ typedef enum _DPMS_t {
 	crtDPMS_SUSPEND = 0x2,
 	crtDPMS_OFF = 0x3,
 }
+
 DPMS_t;
 
 #define setDAC(off) {							\

drivers/staging/sm750fb/sm750_cursor.c +3 -0

index d622d65..77563dc 100644
@@ -54,6 +54,7 @@ void hw_cursor_enable(struct lynx_cursor *cursor)
 	reg = (cursor->offset & HWC_ADDRESS_ADDRESS_MASK) | HWC_ADDRESS_ENABLE;
 	POKE32(HWC_ADDRESS, reg);
 }
+
 void hw_cursor_disable(struct lynx_cursor *cursor)
 {
 	POKE32(HWC_ADDRESS, 0);

@@ -65,6 +66,7 @@ void hw_cursor_setSize(struct lynx_cursor *cursor,
 	cursor->w = w;
 	cursor->h = h;
 }
+
 void hw_cursor_setPos(struct lynx_cursor *cursor,
 						int x, int y)
 {

@@ -74,6 +76,7 @@ void hw_cursor_setPos(struct lynx_cursor *cursor,
 		(x & HWC_LOCATION_X_MASK));
 	POKE32(HWC_LOCATION, reg);
 }
+
 void hw_cursor_setColor(struct lynx_cursor *cursor,
 						u32 fg, u32 bg)
 {

Signature · 1 line

2.7.4

Rehas Sachdeva

author

2016-09-15 21:32 UTC #4 lore raw

PATCH 2/5 staging: sm750fb: Change 'x != NULL' to 'x'

In reply to Rehas Sachdeva

Changes the explicit comparison to NULL from 'x != NULL' to 'x'. Issue detected by checkpatch.

Signed-off-by Rehas Sachdeva <[email protected]>

---
 drivers/staging/sm750fb/sm750.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/sm750fb/sm750.c +1 -1

index 6ed004e..7d90e25 100644
@@ -1176,7 +1176,7 @@ static int __init lynxfb_setup(char *options)
 		else {
 			strcat(tmp, opt);
 			tmp += strlen(opt);
-			if (options != NULL)
+			if (options)
 				*tmp++ = ':';
 			else
 				*tmp++ = 0;

Signature · 1 line

2.7.4

Rehas Sachdeva

author

2016-09-15 21:33 UTC #5 lore raw

PATCH 3/5 staging: sm750fb: Remove multiple assignments

In reply to Rehas Sachdeva

Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue detected by checkpatch.

Signed-off-by Rehas Sachdeva <[email protected]>

---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

drivers/staging/sm750fb/sm750.c +8 -4

index 7d90e25..9f1b591 100644
@@ -365,7 +365,8 @@ static int lynxfb_ops_set_par(struct fb_info *info)
 		ret = -EINVAL;
 		break;
 	}
-	var->height = var->width = -1;
+	var->height = -1;
+	var->width = -1;
 	var->accel_flags = 0;/*FB_ACCELF_TEXT;*/
 
 	if (ret) {

@@ -558,7 +559,8 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
 		pr_err("bpp %d not supported\n", var->bits_per_pixel);
 		return -EINVAL;
 	}
-	var->height = var->width = -1;
+	var->height = -1;
+	var->width = -1;
 	var->accel_flags = 0;/* FB_ACCELF_TEXT; */
 
 	/* check if current fb's video memory big enought to hold the onscreen*/

@@ -781,7 +783,8 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
 		0x800f0 + (int)crtc->channel * 0x140;
 
 	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
-	crtc->cursor.maxH = crtc->cursor.maxW = 64;
+	crtc->cursor.maxH = 64;
+	crtc->cursor.maxW = 64;
 	crtc->cursor.size = crtc->cursor.maxH * crtc->cursor.maxW * 2 / 8;
 	crtc->cursor.vstart = sm750_dev->pvMem + crtc->cursor.offset;

@@ -1067,7 +1070,8 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
 	if (!sm750_dev)
 		return err;
 
-	sm750_dev->fbinfo[0] = sm750_dev->fbinfo[1] = NULL;
+	sm750_dev->fbinfo[0] = NULL;
+	sm750_dev->fbinfo[1] = NULL;
 	sm750_dev->devid = pdev->device;
 	sm750_dev->revid = pdev->revision;
 	sm750_dev->pdev = pdev;

Signature · 1 line

2.7.4

Rehas Sachdeva

author

2016-09-15 21:33 UTC #9 lore raw

PATCH 4/5 staging: sm750fb: Change 'uint32_t' to 'u32'

In reply to Rehas Sachdeva

It is preferred to use 'u32' instead of 'uint32_t' for unsigned int. Issue detected by checkpatch.

Signed-off-by Rehas Sachdeva <[email protected]>

---
 drivers/staging/sm750fb/sm750_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/sm750fb/sm750_hw.c +1 -1

index 1de9f81..a376d17 100644
@@ -534,7 +534,7 @@ int hw_sm750_pan_display(struct lynxfb_crtc *crtc,
 			 const struct fb_var_screeninfo *var,
 			 const struct fb_info *info)
 {
-	uint32_t total;
+	u32 total;
 	/* check params */
 	if ((var->xoffset + var->xres > var->xres_virtual) ||
 	    (var->yoffset + var->yres > var->yres_virtual)) {

Signature · 1 line

2.7.4

Rehas Sachdeva

author

2016-09-15 21:34 UTC #10 lore raw

PATCH 5/5 staging: sm750fb: Add spaces around '|'

In reply to Rehas Sachdeva

Adds spaces on either side of a '|'. Issue dound by checkpatch.

Signed-off-by Rehas Sachdeva <[email protected]>

---
 drivers/staging/sm750fb/ddk750_display.h | 44 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

drivers/staging/sm750fb/ddk750_display.h +22 ~~-22~~

index 04b8eba..f69d798 100644
@@ -8,8 +8,8 @@
 #define PNL_2_OFFSET 0
 #define PNL_2_MASK (3 << PNL_2_OFFSET)
 #define PNL_2_USAGE	(PNL_2_MASK << 16)
-#define PNL_2_PRI	((0 << PNL_2_OFFSET)|PNL_2_USAGE)
-#define PNL_2_SEC	((2 << PNL_2_OFFSET)|PNL_2_USAGE)
+#define PNL_2_PRI	((0 << PNL_2_OFFSET) | PNL_2_USAGE)
+#define PNL_2_SEC	((2 << PNL_2_OFFSET) | PNL_2_USAGE)
 
 
 /* primary timing & plane enable bit

@@ -19,8 +19,8 @@
 #define PRI_TP_OFFSET 4
 #define PRI_TP_MASK BIT(PRI_TP_OFFSET)
 #define PRI_TP_USAGE (PRI_TP_MASK << 16)
-#define PRI_TP_ON ((0x1 << PRI_TP_OFFSET)|PRI_TP_USAGE)
-#define PRI_TP_OFF ((0x0 << PRI_TP_OFFSET)|PRI_TP_USAGE)
+#define PRI_TP_ON ((0x1 << PRI_TP_OFFSET) | PRI_TP_USAGE)
+#define PRI_TP_OFF ((0x0 << PRI_TP_OFFSET) | PRI_TP_USAGE)
 
 
 /* panel sequency status

@@ -29,8 +29,8 @@
 #define PNL_SEQ_OFFSET 6
 #define PNL_SEQ_MASK BIT(PNL_SEQ_OFFSET)
 #define PNL_SEQ_USAGE (PNL_SEQ_MASK << 16)
-#define PNL_SEQ_ON (BIT(PNL_SEQ_OFFSET)|PNL_SEQ_USAGE)
-#define PNL_SEQ_OFF ((0 << PNL_SEQ_OFFSET)|PNL_SEQ_USAGE)
+#define PNL_SEQ_ON (BIT(PNL_SEQ_OFFSET) | PNL_SEQ_USAGE)
+#define PNL_SEQ_OFF ((0 << PNL_SEQ_OFFSET) | PNL_SEQ_USAGE)
 
 /* dual digital output
 	80000[19]

@@ -38,8 +38,8 @@
 #define DUAL_TFT_OFFSET 8
 #define DUAL_TFT_MASK BIT(DUAL_TFT_OFFSET)
 #define DUAL_TFT_USAGE (DUAL_TFT_MASK << 16)
-#define DUAL_TFT_ON (BIT(DUAL_TFT_OFFSET)|DUAL_TFT_USAGE)
-#define DUAL_TFT_OFF ((0 << DUAL_TFT_OFFSET)|DUAL_TFT_USAGE)
+#define DUAL_TFT_ON (BIT(DUAL_TFT_OFFSET) | DUAL_TFT_USAGE)
+#define DUAL_TFT_OFF ((0 << DUAL_TFT_OFFSET) | DUAL_TFT_USAGE)
 
 /* secondary timing & plane enable bit
 	1:80200[8] & 80200[2] on

@@ -48,8 +48,8 @@
 #define SEC_TP_OFFSET 5
 #define SEC_TP_MASK BIT(SEC_TP_OFFSET)
 #define SEC_TP_USAGE (SEC_TP_MASK << 16)
-#define SEC_TP_ON  ((0x1 << SEC_TP_OFFSET)|SEC_TP_USAGE)
-#define SEC_TP_OFF ((0x0 << SEC_TP_OFFSET)|SEC_TP_USAGE)
+#define SEC_TP_ON  ((0x1 << SEC_TP_OFFSET) | SEC_TP_USAGE)
+#define SEC_TP_OFF ((0x0 << SEC_TP_OFFSET) | SEC_TP_USAGE)
 
 /* crt path select
 	80200[19:18]

@@ -57,8 +57,8 @@
 #define CRT_2_OFFSET 2
 #define CRT_2_MASK (3 << CRT_2_OFFSET)
 #define CRT_2_USAGE (CRT_2_MASK << 16)
-#define CRT_2_PRI ((0x0 << CRT_2_OFFSET)|CRT_2_USAGE)
-#define CRT_2_SEC ((0x2 << CRT_2_OFFSET)|CRT_2_USAGE)
+#define CRT_2_PRI ((0x0 << CRT_2_OFFSET) | CRT_2_USAGE)
+#define CRT_2_SEC ((0x2 << CRT_2_OFFSET) | CRT_2_USAGE)
 
 
 /* DAC affect both DVI and DSUB

@@ -67,8 +67,8 @@
 #define DAC_OFFSET 7
 #define DAC_MASK BIT(DAC_OFFSET)
 #define DAC_USAGE (DAC_MASK << 16)
-#define DAC_ON ((0x0 << DAC_OFFSET)|DAC_USAGE)
-#define DAC_OFF ((0x1 << DAC_OFFSET)|DAC_USAGE)
+#define DAC_ON ((0x0 << DAC_OFFSET) | DAC_USAGE)
+#define DAC_OFF ((0x1 << DAC_OFFSET) | DAC_USAGE)
 
 /* DPMS only affect D-SUB head
 	0[31:30]

@@ -76,8 +76,8 @@
 #define DPMS_OFFSET 9
 #define DPMS_MASK (3 << DPMS_OFFSET)
 #define DPMS_USAGE (DPMS_MASK << 16)
-#define DPMS_OFF ((3 << DPMS_OFFSET)|DPMS_USAGE)
-#define DPMS_ON ((0 << DPMS_OFFSET)|DPMS_USAGE)
+#define DPMS_OFF ((3 << DPMS_OFFSET) | DPMS_USAGE)
+#define DPMS_ON ((0 << DPMS_OFFSET) | DPMS_USAGE)

@@ -86,16 +86,16 @@
 	CRT means crt path DSUB
 */
 typedef enum _disp_output_t {
-	do_LCD1_PRI = PNL_2_PRI|PRI_TP_ON|PNL_SEQ_ON|DAC_ON,
-	do_LCD1_SEC = PNL_2_SEC|SEC_TP_ON|PNL_SEQ_ON|DAC_ON,
-	do_LCD2_PRI = CRT_2_PRI|PRI_TP_ON|DUAL_TFT_ON,
-	do_LCD2_SEC = CRT_2_SEC|SEC_TP_ON|DUAL_TFT_ON,
+	do_LCD1_PRI = PNL_2_PRI | PRI_TP_ON | PNL_SEQ_ON | DAC_ON,
+	do_LCD1_SEC = PNL_2_SEC | SEC_TP_ON | PNL_SEQ_ON | DAC_ON,
+	do_LCD2_PRI = CRT_2_PRI | PRI_TP_ON | DUAL_TFT_ON,
+	do_LCD2_SEC = CRT_2_SEC | SEC_TP_ON | DUAL_TFT_ON,
 	/*
 	do_DSUB_PRI = CRT_2_PRI|PRI_TP_ON|DPMS_ON|DAC_ON,
 	do_DSUB_SEC = CRT_2_SEC|SEC_TP_ON|DPMS_ON|DAC_ON,
 	*/
-	do_CRT_PRI = CRT_2_PRI|PRI_TP_ON|DPMS_ON|DAC_ON,
-	do_CRT_SEC = CRT_2_SEC|SEC_TP_ON|DPMS_ON|DAC_ON,
+	do_CRT_PRI = CRT_2_PRI | PRI_TP_ON | DPMS_ON | DAC_ON,
+	do_CRT_SEC = CRT_2_SEC | SEC_TP_ON | DPMS_ON | DAC_ON,
 }
 
 disp_output_t;

Signature · 1 line

2.7.4

Greg Kroah-Hartman

maintainer

2016-09-16 07:48 UTC #2 lore raw

Re: [PATCH 1/5] staging: sm750fb: Add blank line after declarations

In reply to Rehas Sachdeva

Quoted 17 lines · Rehas Sachdeva

It is preferred to add a blank line after function, struct, union and enum declarations for better readability. Issue detected by checkpatch.
Signed-off-by Rehas Sachdeva <[email protected]>
---
 drivers/staging/sm750fb/ddk750_chip.h    | 3 +++
 drivers/staging/sm750fb/ddk750_display.h | 1 +
 drivers/staging/sm750fb/ddk750_mode.h    | 2 ++
 drivers/staging/sm750fb/ddk750_power.h   | 1 +
 drivers/staging/sm750fb/sm750_cursor.c   | 3 +++
 5 files changed, 10 insertions(+)
drivers/staging/sm750fb/ddk750_chip.h +0 -0
index 0891384..5ef8aad 100644

drivers/staging/sm750fb/ddk750_chip.h typedef enum _logical_chip_type_t { · line 18

Quoted 5 lines

drivers/staging/sm750fb/ddk750_chip.h +1 -0

@@ -14,6 +14,7 @@ typedef enum _logical_chip_type_t {
 	SM750,
 	SM750LE,
 }
+
 logical_chip_type_t;

Please look at what the C code is doing here, does this change now make sense? Why add a blank line in the middle of a typedef definition?

Yes, the code isn't the best, but this is not the correct fix for this issue, remember checkpatch is a dumb script, sometimes it tells you the totally wrong thing to do, you always have to look at, and understand, the code before you change it.

sorry, I can't take this patch,

greg k-h

Greg Kroah-Hartman

maintainer

2016-09-16 07:48 UTC #6 lore raw

Re: [PATCH 3/5] staging: sm750fb: Remove multiple assignments

In reply to Rehas Sachdeva

Quoted 13 lines · Rehas Sachdeva

Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue detected by checkpatch.
Signed-off-by Rehas Sachdeva <[email protected]>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/staging/sm750fb/sm750.c +0 -0
index 7d90e25..9f1b591 100644

drivers/staging/sm750fb/sm750.c static int lynxfb_ops_set_par(struct fb_info *info) · line 369

Quoted 6 lines

drivers/staging/sm750fb/sm750.c +2 -1

@@ -365,7 +365,8 @@ static int lynxfb_ops_set_par(struct fb_info *info)
 		ret = -EINVAL;
 		break;
 	}
-	var->height = var->width = -1;
+	var->height = -1;
+	var->width = -1;

that's an odd change, isn't the first version of all of these more readable?

thanks,

greg k-h

Greg Kroah-Hartman

maintainer

2016-09-16 07:50 UTC #11 lore raw

Re: [PATCH 5/5] staging: sm750fb: Add spaces around '|'

In reply to Rehas Sachdeva

Quoted 6 lines · Rehas Sachdeva

Adds spaces on either side of a '|'. Issue dound by checkpatch.
Signed-off-by Rehas Sachdeva <[email protected]>
---
 drivers/staging/sm750fb/ddk750_display.h | 44 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

This patch didn't apply cleanly, can you refresh it against my latest tree and resend?

thanks,

greg k-h

Julia Lawall

2016-09-16 07:53 UTC #3 lore raw

Re: [Outreachy kernel] Re: [PATCH 1/5] staging: sm750fb: Add blank line after declarations

In reply to Greg Kroah-Hartman

Quoted 27 lines · Greg Kroah-Hartman

Quoted 17 lines · Rehas Sachdeva
It is preferred to add a blank line after function, struct, union and enum declarations for better readability. Issue detected by checkpatch.
Signed-off-by Rehas Sachdeva <[email protected]>
---
 drivers/staging/sm750fb/ddk750_chip.h    | 3 +++
 drivers/staging/sm750fb/ddk750_display.h | 1 +
 drivers/staging/sm750fb/ddk750_mode.h    | 2 ++
 drivers/staging/sm750fb/ddk750_power.h   | 1 +
 drivers/staging/sm750fb/sm750_cursor.c   | 3 +++
 5 files changed, 10 insertions(+)
drivers/staging/sm750fb/ddk750_chip.h +0 -0
index 0891384..5ef8aad 100644
drivers/staging/sm750fb/ddk750_chip.h typedef enum _logical_chip_type_t { · line 18
Quoted 5 lines
drivers/staging/sm750fb/ddk750_chip.h +1 -0
@@ -14,6 +14,7 @@ typedef enum _logical_chip_type_t {
 	SM750,
 	SM750LE,
 }
+
 logical_chip_type_t;
Please look at what the C code is doing here, does this change now make sense? Why add a blank line in the middle of a typedef definition?

Quoted 6 lines

Yes, the code isn't the best, but this is not the correct fix for this issue, remember checkpatch is a dumb script, sometimes it tells you the totally wrong thing to do, you always have to look at, and understand, the code before you change it.
sorry, I can't take this patch,

Rehas,

Maybe you could get rid of the typedef instead.

julia

Rehas Sachdeva

author

2016-09-16 12:34 UTC #7 lore raw

Re: [PATCH 3/5] staging: sm750fb: Remove multiple assignments

In reply to Greg Kroah-Hartman

Quoted 24 lines · Greg Kroah-Hartman

Quoted 13 lines · Rehas Sachdeva
Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue detected by checkpatch.
Signed-off-by Rehas Sachdeva <[email protected]>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/staging/sm750fb/sm750.c +0 -0
index 7d90e25..9f1b591 100644
drivers/staging/sm750fb/sm750.c static int lynxfb_ops_set_par(struct fb_info *info) · line 369
Quoted 6 lines
drivers/staging/sm750fb/sm750.c +2 -1
@@ -365,7 +365,8 @@ static int lynxfb_ops_set_par(struct fb_info *info)
 		ret = -EINVAL;
 		break;
 	}
-	var->height = var->width = -1;
+	var->height = -1;
+	var->width = -1;
that's an odd change, isn't the first version of all of these more readable?

Quoted 3 lines

thanks,
greg k-h

Hi Greg,

This is an odd change indeed, something I saw only in sm750fb, among so many other drivers. Does that mean that for the sake of uniformity, we should go ahead with this? Although the first version looks better somehow.

Rehas

Greg Kroah-Hartman

maintainer

2016-09-16 12:54 UTC #8 lore raw

Re: [PATCH 3/5] staging: sm750fb: Remove multiple assignments

Quoted 27 lines · Rehas Sachdeva

Quoted 26 lines · Greg Kroah-Hartman
Quoted 13 lines · Rehas Sachdeva
Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue detected by checkpatch.
Signed-off-by Rehas Sachdeva <[email protected]>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/staging/sm750fb/sm750.c +0 -0
index 7d90e25..9f1b591 100644
drivers/staging/sm750fb/sm750.c static int lynxfb_ops_set_par(struct fb_info *info) · line 369
Quoted 6 lines
drivers/staging/sm750fb/sm750.c +2 -1
@@ -365,7 +365,8 @@ static int lynxfb_ops_set_par(struct fb_info *info)
 		ret = -EINVAL;
 		break;
 	}
-	var->height = var->width = -1;
+	var->height = -1;
+	var->width = -1;
that's an odd change, isn't the first version of all of these more readable?
thanks,

Quoted 7 lines

Quoted 1 line
greg k-h
Hi Greg,
This is an odd change indeed, something I saw only in sm750fb, among so many other drivers. Does that mean that for the sake of uniformity, we should go ahead with this? Although the first version looks better somehow.

I recommend just leaving it alone, thanks.

greg k-h