[PATCH 0/5] staging: sm750fb: Fix multiple checkpatch warnings

[PATCH 0/5] staging: sm750fb: Fix multiple checkpatch warnings

[Rehas Sachdeva]

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(-)

-- 
2.7.4

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

[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 <aquannie@gmail.com>
---
 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(+)

diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
index 0891384..5ef8aad 100644
--- a/drivers/staging/sm750fb/ddk750_chip.h
+++ b/drivers/staging/sm750fb/ddk750_chip.h
@@ -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 */
diff --git a/drivers/staging/sm750fb/ddk750_display.h b/drivers/staging/sm750fb/ddk750_display.h
index ca35aa1..04b8eba 100644
--- a/drivers/staging/sm750fb/ddk750_display.h
+++ b/drivers/staging/sm750fb/ddk750_display.h
@@ -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);
diff --git a/drivers/staging/sm750fb/ddk750_mode.h b/drivers/staging/sm750fb/ddk750_mode.h
index e846dc2..453c294 100644
--- a/drivers/staging/sm750fb/ddk750_mode.h
+++ b/drivers/staging/sm750fb/ddk750_mode.h
@@ -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);
diff --git a/drivers/staging/sm750fb/ddk750_power.h b/drivers/staging/sm750fb/ddk750_power.h
index 5963691..c777d67 100644
--- a/drivers/staging/sm750fb/ddk750_power.h
+++ b/drivers/staging/sm750fb/ddk750_power.h
@@ -7,6 +7,7 @@ typedef enum _DPMS_t {
 	crtDPMS_SUSPEND = 0x2,
 	crtDPMS_OFF = 0x3,
 }
+
 DPMS_t;
 
 #define setDAC(off) {							\
diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index d622d65..77563dc 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -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)
 {
-- 
2.7.4

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

[Rehas Sachdeva]

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

Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 6ed004e..7d90e25 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -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;
-- 
2.7.4

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

[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 <aquannie@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 7d90e25..9f1b591 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -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;
-- 
2.7.4

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

[Rehas Sachdeva]

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

Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
---
 drivers/staging/sm750fb/sm750_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 1de9f81..a376d17 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -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)) {
-- 
2.7.4

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

[Rehas Sachdeva]

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

Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
---
 drivers/staging/sm750fb/ddk750_display.h | 44 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_display.h b/drivers/staging/sm750fb/ddk750_display.h
index 04b8eba..f69d798 100644
--- a/drivers/staging/sm750fb/ddk750_display.h
+++ b/drivers/staging/sm750fb/ddk750_display.h
@@ -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;
-- 
2.7.4

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

[Greg Kroah-Hartman]

On Fri, Sep 16, 2016 at 03:02:21AM +0530, Rehas Sachdeva wrote:
> 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 <aquannie@gmail.com>
> ---
>  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(+)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
> index 0891384..5ef8aad 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.h
> +++ b/drivers/staging/sm750fb/ddk750_chip.h
> @@ -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

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

[Greg Kroah-Hartman]

On Fri, Sep 16, 2016 at 03:03:02AM +0530, Rehas Sachdeva wrote:
> Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue
> detected by checkpatch.
> 
> Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 7d90e25..9f1b591 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -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

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

[Greg Kroah-Hartman]

On Fri, Sep 16, 2016 at 03:03:52AM +0530, Rehas Sachdeva wrote:
> Adds spaces on either side of a '|'. Issue dound by checkpatch.
> 
> Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
> ---
>  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

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

[Julia Lawall]

On Fri, 16 Sep 2016, Greg Kroah-Hartman wrote:

> On Fri, Sep 16, 2016 at 03:02:21AM +0530, Rehas Sachdeva wrote:
> > 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 <aquannie@gmail.com>
> > ---
> >  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(+)
> >
> > diff --git a/drivers/staging/sm750fb/ddk750_chip.h b/drivers/staging/sm750fb/ddk750_chip.h
> > index 0891384..5ef8aad 100644
> > --- a/drivers/staging/sm750fb/ddk750_chip.h
> > +++ b/drivers/staging/sm750fb/ddk750_chip.h
> > @@ -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,

Rehas,

Maybe you could get rid of the typedef instead.

julia

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

[Rehas Sachdeva]

On Fri, Sep 16, 2016 at 09:48:59AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Sep 16, 2016 at 03:03:02AM +0530, Rehas Sachdeva wrote:
> > Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue
> > detected by checkpatch.
> > 
> > Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
> > ---
> >  drivers/staging/sm750fb/sm750.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> > index 7d90e25..9f1b591 100644
> > --- a/drivers/staging/sm750fb/sm750.c
> > +++ b/drivers/staging/sm750fb/sm750.c
> > @@ -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

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

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

[Greg Kroah-Hartman]

On Fri, Sep 16, 2016 at 06:04:39PM +0530, Rehas Sachdeva wrote:
> On Fri, Sep 16, 2016 at 09:48:59AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Sep 16, 2016 at 03:03:02AM +0530, Rehas Sachdeva wrote:
> > > Fixes multiple assignments of the form 'x = y = k' to 'x = k; y = k;'. Issue
> > > detected by checkpatch.
> > > 
> > > Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
> > > ---
> > >  drivers/staging/sm750fb/sm750.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> > > index 7d90e25..9f1b591 100644
> > > --- a/drivers/staging/sm750fb/sm750.c
> > > +++ b/drivers/staging/sm750fb/sm750.c
> > > @@ -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
> 
> 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