lkml.org 
[lkml]   [2020]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Forest Bond <>,Greg Kroah-Hartman <>,,
From
Date
On Mon, 2020-06-08 at 22:58 +0000, Rodolfo C Villordo wrote:
> On Mon, Jun 08, 2020 at 01:41:11AM -0700, Joe Perches wrote:
> > On Mon, 2020-06-08 at 07:59 +0200, Julia Lawall wrote:
> > > On Mon, 8 Jun 2020, Al Viro wrote:
> > >
> > > > On Sun, Jun 07, 2020 at 10:41:56PM +0000, Rodolfo C. Villordo wrote:
> > > > > Multiple line over 80 characters fixes by splitting in multiple lines.
> > > > > Warning found by checkpatch.pl
> > > >
> > > > I doubt that checkpatch.pl can catch the real problems there:
> > > >
> > > > * Hungarian Notation Sucks. Really.
> > > > * so does CamelCase, especially for wonders like s_uGetRTSCTSRsvTime
>
> Yes, I agree with that.
>
> > > Rodolfo,
> > >
> > > If you work hard with Coccinelle and python scripting, it can help with
> > > the first two problems.
> >
> > These VIA vt6655/vt6656 drivers have been in staging for more than
> > a decade. There are relatively few checkpatch coding style
> > cleanups to do but there are many overall style issues to resolve.
> >
>
> Yes, vt6655/rxtx.c needs lots of work. I was avoiding submit bigger changes
> because this is my second patch submission.
>
> Thank you all for the comments. I'm really sorry for the odd subject.
>
> How should I move forward with this?
>
> 1 - Update this patch with the changes pointed by Dan Carpenter?

Keep your changes small until you really know how this
style of linux kernel staging changes is done.

> 2 - Do a more elaborated and bigger change, like suggested by Al Viro
> and Joe Perches?

A patch series is much preferred to a single large change.

If you decide to refactor various functions, please do that
in separate, discrete patches.

Adding a #define and doing a sed like:

$ sed -i 's/(BY_AL2230_REG_LEN << 3) + IFREGCTL_REGW/AL2230_RLEN_CTL/' drivers/staging/vt6655/*.[ch]

should be a single patch.

And if you do that, another should be done for AL7230

$ sed -i 's/(BY_AL7230_REG_LEN << 3) + IFREGCTL_REGW/AL7230_RLEN_CTL/' drivers/staging/vt6655/*.[ch]

etc...

Maybe the #define BY_AL2230_REG_LEN should be 0x17 so that
the << 3 is more obviously constrained to the low byte

Maybe the + uses in the macros should be bitwise |.

Go wild after you figure out the process, just keep your
patches to obvious, small and verifiable changes.


\
 
 \ /
  Last update: 2020-06-09 01:35    [W:0.103 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site