Re: [PATCH] staging: vt6655: Fix line wrapping in rf.c file
From: Karolina Drobnik <hidden>
Date: 2021-10-19 11:00:41
Also in:
lkml
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:
quoted hunk ↗ jump to hunk
Maybe some refactoring like: --- drivers/staging/vt6655/rf.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-)diff --git a/drivers/staging/vt6655/rf.cb/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