Re: Forest Bond <forest@xxxxxxxxxxxxxxxxxxx>,Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>,devel@xxxxxxxxxxxxxxxxxxxx,linux-kernel@xxxxxxxxxxxxxxx

[Rodolfo C Villordo]

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.