On Mon, Jun 08, 2020 at 04:33:06PM -0700, Joe Perches wrote: > On Mon, 2020-06-08 at 22:58 +0000, Rodolfo C Villordo wrote: > > > > 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. > Thank you so much for your comments Joe. I'll take off this patch and focus on these other changes. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
