[PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[Jeremy Lacomis]

This patch to ddk750_sii164.h fixes line length warnings found by the
checkpatch.pl script and reformats comments uniformly.

Signed-off-by: Jeremy Lacomis <j.lacomis@gmail.com>
---
 drivers/staging/sm750fb/ddk750_sii164.h | 57 +++++++++++++--------------------
 1 file changed, 22 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
index 2e9a88cd6af3..393a3c5be3ae 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.h
+++ b/drivers/staging/sm750fb/ddk750_sii164.h
@@ -4,15 +4,20 @@
 
 #define USE_DVICHIP
 
-/* Hot Plug detection mode structure */
+/*
+ * Hot Plug detection mode structure:
+ *      Disable Hot Plug output bit (always high).
+ *      Use Monitor Detect Interrupt bit.
+ *      Use Receiver Sense detect bit.
+ *      Use Hot Plug detect bit.
+ */
 enum sii164_hot_plug_mode {
-	SII164_HOTPLUG_DISABLE = 0,         /* Disable Hot Plug output bit (always high). */
-	SII164_HOTPLUG_USE_MDI,             /* Use Monitor Detect Interrupt bit. */
-	SII164_HOTPLUG_USE_RSEN,            /* Use Receiver Sense detect bit. */
-	SII164_HOTPLUG_USE_HTPLG            /* Use Hot Plug detect bit. */
+	SII164_HOTPLUG_DISABLE = 0,
+	SII164_HOTPLUG_USE_MDI,
+	SII164_HOTPLUG_USE_RSEN,
+	SII164_HOTPLUG_USE_HTPLG
 };
 
-
 /* Silicon Image SiI164 chip prototype */
 long sii164InitChip(unsigned char edgeSelect,
 		    unsigned char busSelect,
@@ -28,7 +33,6 @@ long sii164InitChip(unsigned char edgeSelect,
 unsigned short sii164GetVendorID(void);
 unsigned short sii164GetDeviceID(void);
 
-
 #ifdef SII164_FULL_FUNCTIONS
 void sii164ResetChip(void);
 char *sii164GetChipString(void);
@@ -39,35 +43,26 @@ unsigned char sii164CheckInterrupt(void);
 void sii164ClearInterrupt(void);
 #endif
 /*
- * below register definition is used for
+ * The below register definition is used for the
  * Silicon Image SiI164 DVI controller chip
  */
-/*
- * Vendor ID registers
- */
+
+/* Vendor ID registers */
 #define SII164_VENDOR_ID_LOW                        0x00
 #define SII164_VENDOR_ID_HIGH                       0x01
 
-/*
- * Device ID registers
- */
+/* Device ID registers */
 #define SII164_DEVICE_ID_LOW                        0x02
 #define SII164_DEVICE_ID_HIGH                       0x03
 
-/*
- * Device Revision
- */
+/* Device Revision */
 #define SII164_DEVICE_REVISION                      0x04
 
-/*
- * Frequency Limitation registers
- */
+/* Frequency Limitation registers */
 #define SII164_FREQUENCY_LIMIT_LOW                  0x06
 #define SII164_FREQUENCY_LIMIT_HIGH                 0x07
 
-/*
- * Power Down and Input Signal Configuration registers
- */
+/* Power Down and Input Signal Configuration registers */
 #define SII164_CONFIGURATION                        0x08
 
 /* Power down (PD) */
@@ -95,9 +90,7 @@ void sii164ClearInterrupt(void);
 #define SII164_CONFIGURATION_VSYNC_FORCE_LOW        0x00
 #define SII164_CONFIGURATION_VSYNC_AS_IS            0x20
 
-/*
- * Detection registers
- */
+/* Detection registers */
 #define SII164_DETECT                               0x09
 
 /* Monitor Detect Interrupt (MDI) */
@@ -127,9 +120,7 @@ void sii164ClearInterrupt(void);
 #define SII164_DETECT_MONITOR_SENSE_OUTPUT_HTPLG    0x30
 #define SII164_DETECT_MONITOR_SENSE_OUTPUT_FLAG     0x30
 
-/*
- * Skewing registers
- */
+/* Skewing registers */
 #define SII164_DESKEW                               0x0A
 
 /* General Purpose Input (CTL[3:1]) */
@@ -149,14 +140,10 @@ void sii164ClearInterrupt(void);
 #define SII164_DESKEW_7_STEP                        0xC0
 #define SII164_DESKEW_8_STEP                        0xE0
 
-/*
- * User Configuration Data registers (CFG 7:0)
- */
+/* User Configuration Data registers (CFG 7:0) */
 #define SII164_USER_CONFIGURATION                   0x0B
 
-/*
- * PLL registers
- */
+/* PLL registers */
 #define SII164_PLL                                  0x0C
 
 /* PLL Filter Value (PLLF) */
-- 
2.11.0

[PATCH 2/2] staging: sm750b: Fix coding style issues in sm750_accel.c

[Jeremy Lacomis]

This is a patch to sm750_accel.c that fixes 80-character line length
warnings found by checkpatch.pl. It also fixes some grammatical errors
in comments and moves parameter-specific comments from inline to before
the function.

Signed-off-by: Jeremy Lacomis <j.lacomis@gmail.com>
---
 drivers/staging/sm750fb/sm750_accel.c | 184 ++++++++++++++++++----------------
 1 file changed, 98 insertions(+), 86 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 1035e91e7cd3..085e5064188f 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+/* SPDX-License-Identifier: GPL-2.0 */
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -68,11 +68,10 @@ void sm750_hw_de_init(struct lynx_accel *accel)
 }
 
 /*
- * set2dformat only be called from setmode functions
- * but if you need dual framebuffer driver,need call set2dformat
- * every time you use 2d function
+ * set2dformat can only be called from setmode functions, but if you need a dual
+ * framebuffer driver, set2dformat must be called every time a 2D function is
+ * used
  */
-
 void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt)
 {
 	u32 reg;
@@ -94,7 +93,7 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
 
 	if (accel->de_wait() != 0) {
 		/*
-		 * int time wait and always busy,seems hardware
+		 * int time wait and always busy, seems hardware
 		 * got something error
 		 */
 		pr_debug("De engine always busy\n");
@@ -130,80 +129,85 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
 	return 0;
 }
 
-int sm750_hw_copyarea(
-struct lynx_accel *accel,
-unsigned int sBase,  /* Address of source: offset in frame buffer */
-unsigned int sPitch, /* Pitch value of source surface in BYTE */
-unsigned int sx,
-unsigned int sy,     /* Starting coordinate of source surface */
-unsigned int dBase,  /* Address of destination: offset in frame buffer */
-unsigned int dPitch, /* Pitch value of destination surface in BYTE */
-unsigned int Bpp,    /* Color depth of destination surface */
-unsigned int dx,
-unsigned int dy,     /* Starting coordinate of destination surface */
-unsigned int width,
-unsigned int height, /* width and height of rectangle in pixel value */
-unsigned int rop2)   /* ROP value */
+/*
+ * Notes on these parameters:
+ *         sBase: Address of the source offset in the frame buffer
+ *         sPitch: Pitch value of the source surface in BYTE
+ *         sx, sy: Starting coordinate of the source surface
+ *         dBase: Address of the destination offset in the frame buffer
+ *         dPitch: Pitch value of the destination surface in BYTE
+ *         Bpp: Color depth of the destination surface
+ *         dx, dy: Starting coordinate of the destination surface
+ *         width, height: Dimensions of the rectangle in pixels
+ *         rop2: ROP value
+ */
+int sm750_hw_copyarea(struct lynx_accel *accel, unsigned int sBase,
+		      unsigned int sPitch, unsigned int sx, unsigned int sy,
+		      unsigned int dBase, unsigned int dPitch, unsigned int Bpp,
+		      unsigned int dx, unsigned int dy, unsigned int width,
+		      unsigned int height, unsigned int rop2)
 {
 	unsigned int nDirection, de_ctrl;
 
 	nDirection = LEFT_TO_RIGHT;
-	/* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left */
+	/*
+	 * Direction of ROP2 operation:
+	 *         1  = Left to Right
+	 *         -1 = Right to Left
+	 */
 	de_ctrl = 0;
 
-	/* If source and destination are the same surface, need to check for overlay cases */
+	/*
+	 * If the source and destination are the same surface, need to check for
+	 * overlay cases
+	 */
 	if (sBase == dBase && sPitch == dPitch) {
 		/* Determine direction of operation */
-		if (sy < dy) {
-			/* +----------+
-			 * |S         |
-			 * |   +----------+
-			 * |   |      |   |
-			 * |   |      |   |
-			 * +---|------+   |
-			 * |         D|
-			 * +----------+
-			 */
 
+		/* +----------+
+		 * |S         |
+		 * |   +----------+
+		 * |   |      |   |
+		 * |   |      |   |
+		 * +---|------+   |
+		 * |         D|
+		 * +----------+
+		 */
+		if (sy < dy) {
 			nDirection = BOTTOM_TO_TOP;
-		} else if (sy > dy) {
-			/* +----------+
-			 * |D         |
-			 * |   +----------+
-			 * |   |      |   |
-			 * |   |      |   |
-			 * +---|------+   |
-			 * |         S|
-			 * +----------+
-			 */
 
+		/* +----------+
+		 * |D         |
+		 * |   +----------+
+		 * |   |      |   |
+		 * |   |      |   |
+		 * +---|------+   |
+		 * |         S|
+		 * +----------+
+		 */
+		} else if (sy > dy) {
 			nDirection = TOP_TO_BOTTOM;
-		} else {
-			/* sy == dy */
-
-			if (sx <= dx) {
-				/* +------+---+------+
-				 * |S     |   |     D|
-				 * |      |   |      |
-				 * |      |   |      |
-				 * |      |   |      |
-				 * +------+---+------+
-				 */
 
+		} else {
+			/* +------+---+------+
+			 * |S     |   |     D|
+			 * |      |   |      |
+			 * |      |   |      |
+			 * |      |   |      |
+			 * +------+---+------+
+			 */
+			if (sx <= dx)
 				nDirection = RIGHT_TO_LEFT;
-			} else {
-			/* sx > dx */
-
-				/* +------+---+------+
-				 * |D     |   |     S|
-				 * |      |   |      |
-				 * |      |   |      |
-				 * |      |   |      |
-				 * +------+---+------+
-				 */
 
+			/* +------+---+------+
+			 * |D     |   |     S|
+			 * |      |   |      |
+			 * |      |   |      |
+			 * |      |   |      |
+			 * +------+---+------+
+			 */
+			else
 				nDirection = LEFT_TO_RIGHT;
-			}
 		}
 	}
 
@@ -288,20 +292,26 @@ static unsigned int deGetTransparency(struct lynx_accel *accel)
 	return de_ctrl;
 }
 
-int sm750_hw_imageblit(struct lynx_accel *accel,
-		 const char *pSrcbuf, /* pointer to start of source buffer in system memory */
-		 u32 srcDelta,          /* Pitch value (in bytes) of the source buffer, +ive means top down and -ive mean button up */
-		 u32 startBit, /* Mono data can start at any bit in a byte, this value should be 0 to 7 */
-		 u32 dBase,    /* Address of destination: offset in frame buffer */
-		 u32 dPitch,   /* Pitch value of destination surface in BYTE */
-		 u32 bytePerPixel,      /* Color depth of destination surface */
-		 u32 dx,
-		 u32 dy,       /* Starting coordinate of destination surface */
-		 u32 width,
-		 u32 height,   /* width and height of rectangle in pixel value */
-		 u32 fColor,   /* Foreground color (corresponding to a 1 in the monochrome data */
-		 u32 bColor,   /* Background color (corresponding to a 0 in the monochrome data */
-		 u32 rop2)     /* ROP value */
+/*
+ * Notes on these parameters:
+ *         pSrcbuf: Start of the source buffer in system memory
+ *         srcDelta: Pitch value of the source buffer in bytes. A positive value
+ *                   means top-down, while a negative value means bottom-up
+ *         startBit: Mono data can start at any bit in a byte, this value should
+ *                   be in the range 0-7
+ *         dBase: Address of the destination offset in the frame buffer
+ *         dPitch: Pitch value of the destination surface in BYTE
+ *         bytePerPixel: Color depth of the destination surface
+ *         dx, dy: Starting coordinates of the destination surface
+ *         width, height: Dimensions of the rectangle in pixels
+ *         fColor: Foreground color (cooresponding to a 1 in monochrome data)
+ *         bColor: Background color (corresponding to a 0 in monochrome data)
+ *         rop2: ROP value
+ */
+int sm750_hw_imageblit(struct lynx_accel *accel, const char *pSrcbuf,
+		       u32 srcDelta, u32 startBit, u32 dBase, u32 dPitch,
+		       u32 bytePerPixel, u32 dx, u32 dy, u32 width, u32 height,
+		       u32 fColor, u32 bColor, u32 rop2)
 {
 	unsigned int ulBytesPerScan;
 	unsigned int ul4BytesPerScan;
@@ -310,7 +320,8 @@ int sm750_hw_imageblit(struct lynx_accel *accel,
 	unsigned char ajRemain[4];
 	int i, j;
 
-	startBit &= 7; /* Just make sure the start bit is within legal range */
+    /* Make sure that the start bit is within legal range */
+	startBit &= 7;
 	ulBytesPerScan = (width + startBit + 7) / 8;
 	ul4BytesPerScan = ulBytesPerScan & ~3;
 	ulBytesRemain = ulBytesPerScan & 3;
@@ -324,9 +335,9 @@ int sm750_hw_imageblit(struct lynx_accel *accel,
 	 */
 	write_dpr(accel, DE_WINDOW_SOURCE_BASE, 0);
 
-	/* 2D Destination Base.
-	 * It is an address offset (128 bit aligned)
-	 * from the beginning of frame buffer.
+	/*
+	 * 2D Destination Base.
+	 * 128 bit aligned offset from the beginning of frame buffer.
 	 */
 	write_dpr(accel, DE_WINDOW_DESTINATION_BASE, dBase);
 
@@ -352,8 +363,7 @@ int sm750_hw_imageblit(struct lynx_accel *accel,
 
 	 /*
 	  * Note: For 2D Source in Host Write, only X_K1_MONO field is needed,
-	  * and Y_K2 field is not used.
-	  * For mono bitmap, use startBit for X_K1.
+	  * and Y_K2 field is not used. For mono bitmap, use startBit for X_K1.
 	  */
 	write_dpr(accel, DE_SOURCE,
 		  (startBit << DE_SOURCE_X_K1_SHIFT) &
@@ -380,10 +390,12 @@ int sm750_hw_imageblit(struct lynx_accel *accel,
 	for (i = 0; i < height; i++) {
 		/* For each line, send the data in chunks of 4 bytes */
 		for (j = 0; j < (ul4BytesPerScan / 4); j++)
-			write_dpPort(accel, *(unsigned int *)(pSrcbuf + (j * 4)));
+			write_dpPort(accel,
+				     *(unsigned int *)(pSrcbuf + (j * 4)));
 
 		if (ulBytesRemain) {
-			memcpy(ajRemain, pSrcbuf+ul4BytesPerScan, ulBytesRemain);
+			memcpy(ajRemain, pSrcbuf + ul4BytesPerScan,
+			       ulBytesRemain);
 			write_dpPort(accel, *(unsigned int *)ajRemain);
 		}
 
-- 
2.11.0

Re: [PATCH 2/2] staging: sm750b: Fix coding style issues in sm750_accel.c

[Joe Perches]

On Sat, 2017-11-25 at 13:26 -0500, Jeremy Lacomis wrote:
> This is a patch to sm750_accel.c that fixes 80-character line length
> warnings found by checkpatch.pl. It also fixes some grammatical errors
> in comments and moves parameter-specific comments from inline to before
> the function.
[]
> + * Notes on these parameters:
> + *         sBase: Address of the source offset in the frame buffer
> + *         sPitch: Pitch value of the source surface in BYTE
> + *         sx, sy: Starting coordinate of the source surface
> + *         dBase: Address of the destination offset in the frame buffer
> + *         dPitch: Pitch value of the destination surface in BYTE
> + *         Bpp: Color depth of the destination surface
> + *         dx, dy: Starting coordinate of the destination surface
> + *         width, height: Dimensions of the rectangle in pixels
> + *         rop2: ROP value

Use kernel-doc format instead.

Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[Dan Carpenter]

On Sat, Nov 25, 2017 at 01:26:35PM -0500, Jeremy Lacomis wrote:
> This patch to ddk750_sii164.h fixes line length warnings found by the
> checkpatch.pl script and reformats comments uniformly.
> 
> Signed-off-by: Jeremy Lacomis <j.lacomis@gmail.com>
> ---
>  drivers/staging/sm750fb/ddk750_sii164.h | 57 +++++++++++++--------------------
>  1 file changed, 22 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
> index 2e9a88cd6af3..393a3c5be3ae 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.h
> +++ b/drivers/staging/sm750fb/ddk750_sii164.h
> @@ -4,15 +4,20 @@
>  
>  #define USE_DVICHIP
>  
> -/* Hot Plug detection mode structure */
> +/*
> + * Hot Plug detection mode structure:
> + *      Disable Hot Plug output bit (always high).
> + *      Use Monitor Detect Interrupt bit.
> + *      Use Receiver Sense detect bit.
> + *      Use Hot Plug detect bit.
> + */
>  enum sii164_hot_plug_mode {
> -	SII164_HOTPLUG_DISABLE = 0,         /* Disable Hot Plug output bit (always high). */
> -	SII164_HOTPLUG_USE_MDI,             /* Use Monitor Detect Interrupt bit. */
> -	SII164_HOTPLUG_USE_RSEN,            /* Use Receiver Sense detect bit. */
> -	SII164_HOTPLUG_USE_HTPLG            /* Use Hot Plug detect bit. */
> +	SII164_HOTPLUG_DISABLE = 0,
> +	SII164_HOTPLUG_USE_MDI,
> +	SII164_HOTPLUG_USE_RSEN,
> +	SII164_HOTPLUG_USE_HTPLG
>  };

I feel like this makes it less readable.  The original was better.

regards,
dan carpenter

Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[Geert Uytterhoeven]

On Mon, Nov 27, 2017 at 10:46 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> On Sat, Nov 25, 2017 at 01:26:35PM -0500, Jeremy Lacomis wrote:
>> This patch to ddk750_sii164.h fixes line length warnings found by the
>> checkpatch.pl script and reformats comments uniformly.
>>
>> Signed-off-by: Jeremy Lacomis <j.lacomis@gmail.com>
>> ---
>>  drivers/staging/sm750fb/ddk750_sii164.h | 57 +++++++++++++--------------------
>>  1 file changed, 22 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
>> index 2e9a88cd6af3..393a3c5be3ae 100644
>> --- a/drivers/staging/sm750fb/ddk750_sii164.h
>> +++ b/drivers/staging/sm750fb/ddk750_sii164.h
>> @@ -4,15 +4,20 @@
>>
>>  #define USE_DVICHIP
>>
>> -/* Hot Plug detection mode structure */
>> +/*
>> + * Hot Plug detection mode structure:
>> + *      Disable Hot Plug output bit (always high).
>> + *      Use Monitor Detect Interrupt bit.
>> + *      Use Receiver Sense detect bit.
>> + *      Use Hot Plug detect bit.
>> + */
>>  enum sii164_hot_plug_mode {
>> -     SII164_HOTPLUG_DISABLE = 0,         /* Disable Hot Plug output bit (always high). */
>> -     SII164_HOTPLUG_USE_MDI,             /* Use Monitor Detect Interrupt bit. */
>> -     SII164_HOTPLUG_USE_RSEN,            /* Use Receiver Sense detect bit. */
>> -     SII164_HOTPLUG_USE_HTPLG            /* Use Hot Plug detect bit. */
>> +     SII164_HOTPLUG_DISABLE = 0,
>> +     SII164_HOTPLUG_USE_MDI,
>> +     SII164_HOTPLUG_USE_RSEN,
>> +     SII164_HOTPLUG_USE_HTPLG
>>  };
>
> I feel like this makes it less readable.  The original was better.

And more error prone.

NAKed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Please use common sense when following checkpatch.pl's advise.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[Jeremy Lacomis]

On Mon, Nov 27, 2017 at 11:45:00AM +0100, Geert Uytterhoeven wrote:
> On Mon, Nov 27, 2017 at 10:46 AM, Dan Carpenter
> <dan.carpenter@oracle.com> wrote:
> > On Sat, Nov 25, 2017 at 01:26:35PM -0500, Jeremy Lacomis wrote:
> >> This patch to ddk750_sii164.h fixes line length warnings found by the
> >> checkpatch.pl script and reformats comments uniformly.
> >>
> >> Signed-off-by: Jeremy Lacomis <j.lacomis@gmail.com>
> >> ---
> >>  drivers/staging/sm750fb/ddk750_sii164.h | 57 +++++++++++++--------------------
> >>  1 file changed, 22 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
> >> index 2e9a88cd6af3..393a3c5be3ae 100644
> >> --- a/drivers/staging/sm750fb/ddk750_sii164.h
> >> +++ b/drivers/staging/sm750fb/ddk750_sii164.h
> >> @@ -4,15 +4,20 @@
> >>
> >>  #define USE_DVICHIP
> >>
> >> -/* Hot Plug detection mode structure */
> >> +/*
> >> + * Hot Plug detection mode structure:
> >> + *      Disable Hot Plug output bit (always high).
> >> + *      Use Monitor Detect Interrupt bit.
> >> + *      Use Receiver Sense detect bit.
> >> + *      Use Hot Plug detect bit.
> >> + */
> >>  enum sii164_hot_plug_mode {
> >> -     SII164_HOTPLUG_DISABLE = 0,         /* Disable Hot Plug output bit (always high). */
> >> -     SII164_HOTPLUG_USE_MDI,             /* Use Monitor Detect Interrupt bit. */
> >> -     SII164_HOTPLUG_USE_RSEN,            /* Use Receiver Sense detect bit. */
> >> -     SII164_HOTPLUG_USE_HTPLG            /* Use Hot Plug detect bit. */
> >> +     SII164_HOTPLUG_DISABLE = 0,
> >> +     SII164_HOTPLUG_USE_MDI,
> >> +     SII164_HOTPLUG_USE_RSEN,
> >> +     SII164_HOTPLUG_USE_HTPLG
> >>  };
> >
> > I feel like this makes it less readable.  The original was better.
> 
> And more error prone.

Thanks for taking the time to review my patch, I'm just getting into kernel
development and am trying to do as little disservice as possible while learning
the process.

For something like this, would using the kernel-doc format make more sense, or
should the original inline comments be kept? I found the original difficult to
read because my terminal was just small enough that this didn't fit.

Thanks,
- Jeremy

Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

[Greg KH]

On Mon, Nov 27, 2017 at 09:37:27AM -0500, Jeremy Lacomis wrote:
> On Mon, Nov 27, 2017 at 11:45:00AM +0100, Geert Uytterhoeven wrote:
> > On Mon, Nov 27, 2017 at 10:46 AM, Dan Carpenter
> > <dan.carpenter@oracle.com> wrote:
> > > On Sat, Nov 25, 2017 at 01:26:35PM -0500, Jeremy Lacomis wrote:
> > >> This patch to ddk750_sii164.h fixes line length warnings found by the
> > >> checkpatch.pl script and reformats comments uniformly.
> > >>
> > >> Signed-off-by: Jeremy Lacomis <j.lacomis@gmail.com>
> > >> ---
> > >>  drivers/staging/sm750fb/ddk750_sii164.h | 57 +++++++++++++--------------------
> > >>  1 file changed, 22 insertions(+), 35 deletions(-)
> > >>
> > >> diff --git a/drivers/staging/sm750fb/ddk750_sii164.h b/drivers/staging/sm750fb/ddk750_sii164.h
> > >> index 2e9a88cd6af3..393a3c5be3ae 100644
> > >> --- a/drivers/staging/sm750fb/ddk750_sii164.h
> > >> +++ b/drivers/staging/sm750fb/ddk750_sii164.h
> > >> @@ -4,15 +4,20 @@
> > >>
> > >>  #define USE_DVICHIP
> > >>
> > >> -/* Hot Plug detection mode structure */
> > >> +/*
> > >> + * Hot Plug detection mode structure:
> > >> + *      Disable Hot Plug output bit (always high).
> > >> + *      Use Monitor Detect Interrupt bit.
> > >> + *      Use Receiver Sense detect bit.
> > >> + *      Use Hot Plug detect bit.
> > >> + */
> > >>  enum sii164_hot_plug_mode {
> > >> -     SII164_HOTPLUG_DISABLE = 0,         /* Disable Hot Plug output bit (always high). */
> > >> -     SII164_HOTPLUG_USE_MDI,             /* Use Monitor Detect Interrupt bit. */
> > >> -     SII164_HOTPLUG_USE_RSEN,            /* Use Receiver Sense detect bit. */
> > >> -     SII164_HOTPLUG_USE_HTPLG            /* Use Hot Plug detect bit. */
> > >> +     SII164_HOTPLUG_DISABLE = 0,
> > >> +     SII164_HOTPLUG_USE_MDI,
> > >> +     SII164_HOTPLUG_USE_RSEN,
> > >> +     SII164_HOTPLUG_USE_HTPLG
> > >>  };
> > >
> > > I feel like this makes it less readable.  The original was better.
> > 
> > And more error prone.
> 
> Thanks for taking the time to review my patch, I'm just getting into kernel
> development and am trying to do as little disservice as possible while learning
> the process.
> 
> For something like this, would using the kernel-doc format make more sense, or
> should the original inline comments be kept? I found the original difficult to
> read because my terminal was just small enough that this didn't fit.

kernel-doc might be good, but it also might just be worth it to leave it
alone as it might be too much work.

thanks,

greg k-h