linux-kernel - Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 28 Nov 2017 13:57:22 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Jeremy Lacomis <j.lacomis@...il.com>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        driverdevel <devel@...verdev.osuosl.org>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
        Teddy Wang <teddy.wang@...iconmotion.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sudip Mukherjee <sudipm.mukherjee@...il.com>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH 1/2] staging: sm750fb: Fix coding style in ddk750_sii164.h

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@...cle.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@...il.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