Messages in this thread | | | Subject | Re: [PATCH] staging: vt6655: Fix line wrapping in rf.c file | From | Karolina Drobnik <> | Date | Tue, 19 Oct 2021 11:59:56 +0100 |
| |
Hi,
Thank you very much for your comments.
On Mon, 2021-10-18 at 17:12 +0200, Greg KH wrote: > Also, these are all just fine as-is for now. A better way to make > these lines smaller is to use better variable and function names > that are shorter and make sense :)
I have v2 ready but I'm not sure, given the Joe's patch, if my solution is a satisfactory one. I didn't jump on such refactoring as I'm still learning about the codebase/process and didn't want to muddle the waters (...more than I do already).
Greg, what would you prefer? Should I back up with my patch, pick something else and let Joe's patch be merged?
Also, I have a question about the patch if that's ok :)
On Mon, 2021-10-18 at 22:56 -0700, Joe Perches wrote: > Maybe some refactoring like: > --- > drivers/staging/vt6655/rf.c | 38 > ++++++++++++++++++-------------------- > 1 file changed, 18 insertions(+), 20 deletions(-) > > diff --git a/drivers/staging/vt6655/rf.c > b/drivers/staging/vt6655/rf.c > index 0dae593c6944f..7beb0cd5a62df 100644 > --- a/drivers/staging/vt6655/rf.c > +++ b/drivers/staging/vt6655/rf.c > @@ -680,16 +680,19 @@ bool RFvWriteWakeProgSyn(struct vnt_private > *priv, unsigned char byRFType, > u16 uChannel) > { > void __iomem *iobase = priv->PortOffset; > - int ii; > + int i; > + unsigned short idx = MISCFIFO_SYNDATA_IDX; > unsigned char byInitCount = 0; > unsigned char bySleepCount = 0; > + const unsigned long *data; > > + uChannel--; > VNSvOutPortW(iobase + MAC_REG_MISCFFNDEX, 0);
I see that you introduced `uChannel--` to further tidy up the lines with `[uChannel - 1]`. In general, is there anything wrong with indexing like `i - 1`? What's the preference here? DRY things up as much as possible?
I'm asking because when I was reading this line, at first, it wasn't clear to me why we could decrement it (example though: "Was this modified earlier? Do we need to "correct" it?").
Thanks, Karolina
| |