diff mbox

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

Message ID 20171125182636.15245-1-j.lacomis@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeremy Lacomis Nov. 25, 2017, 6:26 p.m. UTC
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(-)

Comments

Dan Carpenter Nov. 27, 2017, 9:46 a.m. UTC | #1
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

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Nov. 27, 2017, 10:45 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy Lacomis Nov. 27, 2017, 2:37 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH Nov. 28, 2017, 12:57 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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) */