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. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
