From: Joe Perches <joe@perches.com>
To: Arushi Singhal <arushisinghal19971997@gmail.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>,
arnaud.patard@rtp-net.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
outreachy-kernel@googlegroups.com
Subject: Re: [Outreachy kernel] [PATCH] staging: xgifb: correct the multiple line dereference to fix coding stye errors
Date: Tue, 21 Feb 2017 20:33:38 -0800 [thread overview]
Message-ID: <1487738018.14159.1.camel@perches.com> (raw)
In-Reply-To: <CA+XqjF-bsKnXFkFN_SGToYTvMdWHX_CCas_BvdtT9UOb04hkCQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
On Wed, 2017-02-22 at 09:46 +0530, Arushi Singhal wrote:
> On Wed, Feb 22, 2017 at 12:12 AM, Joe Perches <joe@perches.com> wrote:
>
> > On Tue, 2017-02-21 at 18:40 +0100, Julia Lawall wrote:
> > >
> > > On Tue, 21 Feb 2017, Arushi Singhal wrote:
> > >
> > > > Error was reported by checkpatch.pl as
> > > > WARNING: Avoid multiple line dereference...
> > > > if there is boolean operator then it is fixed by Splitting line at
> > > > boolean operator.
> > >
> > > This is massively execeeding the 80 character boundary, and not for
> > > something trivial like a string. Maybe the code can be reorganized in
> > > some other way.
> >
> > The easiest way to do that is to change the test above it
> > to reduce indentation from
> >
> > if (xgifb_info->display2 == XGIFB_DISP_TV &&
> > xgifb_info->hasVB == HASVB_301) {
> > [code...];
> > }
> > }
> >
> > to
> >
> > if (xgifb_info->display2 != XGIFB_DISP_TV ||
> > xgifb_in
> > fo->hasVB != HASVB_301)
> > return;
> >
> > [code...];
> > }
> >
> >
>
> Hi
> So basically you are saying that cut one word and put the other part of the
> word in the next line.
No, evolution is a horrible email client
that wraps lines oddly sometimes and sends
out different content than it displays in
its on-screen editor.
What I am saying is the block after the test
can be unindented one level by inverting the
test and using return.
It should be:
if (xgifb_info->display2 != XGIFB_DISP_TV ||
xgifb_in> > fo->hasVB != HASVB_301)
return;
[code...];
I sent you a separate attachment privately a
few hours ago but I'll attach it here too.
[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 7434 bytes --]
drivers/staging/xgifb/XGI_main_26.c | 256 +++++++++++++-----------------------
1 file changed, 92 insertions(+), 164 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 6930f7eb741b..0df7fb2b8892 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -731,179 +731,107 @@ static void XGIfb_post_setmode(struct xgifb_video_info *xgifb_info)
xgifb_reg_and(XGISR, IND_SIS_RAMDAC_CONTROL, ~0x04);
- if (xgifb_info->display2 == XGIFB_DISP_TV &&
- xgifb_info->hasVB == HASVB_301) {
- reg = xgifb_reg_get(XGIPART4, 0x01);
+ if (xgifb_info->display2 != XGIFB_DISP_TV ||
+ xgifb_info->hasVB != HASVB_301)
+ return;
- if (reg < 0xB0) { /* Set filter for XGI301 */
- int filter_tb;
+ reg = xgifb_reg_get(XGIPART4, 0x01);
+ if (reg < 0xB0) { /* Set filter for XGI301 */
+ int filter_tb;
- switch (xgifb_info->video_width) {
- case 320:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 4 : 12;
- break;
- case 640:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 5 : 13;
- break;
- case 720:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 6 : 14;
- break;
- case 800:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 7 : 15;
- break;
- default:
- filter_tb = 0;
- filter = -1;
- break;
- }
- xgifb_reg_or(XGIPART1,
- SIS_CRT2_WENABLE_315,
- 0x01);
+ switch (xgifb_info->video_width) {
+ case 320:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 4 : 12;
+ break;
+ case 640:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 5 : 13;
+ break;
+ case 720:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 6 : 14;
+ break;
+ case 800:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 7 : 15;
+ break;
+ default:
+ filter_tb = 0;
+ filter = -1;
+ break;
+ }
+ xgifb_reg_or(XGIPART1, SIS_CRT2_WENABLE_315, 0x01);
- if (xgifb_info->TV_type == TVMODE_NTSC) {
- xgifb_reg_and(XGIPART2, 0x3a, 0x1f);
-
- if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
- xgifb_reg_and(XGIPART2, 0x30, 0xdf);
-
- } else if (xgifb_info->TV_plug
- == TVPLUG_COMPOSITE) {
- xgifb_reg_or(XGIPART2, 0x30, 0x20);
-
- switch (xgifb_info->video_width) {
- case 640:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xEB);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x04);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x25);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x18);
- break;
- case 720:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xEE);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x0C);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x22);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x08);
- break;
- case 800:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xEB);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x15);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x25);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0xF6);
- break;
- }
- }
+ if (xgifb_info->TV_type == TVMODE_NTSC) {
+ xgifb_reg_and(XGIPART2, 0x3a, 0x1f);
- } else if (xgifb_info->TV_type == TVMODE_PAL) {
- xgifb_reg_and(XGIPART2, 0x3A, 0x1F);
-
- if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
- xgifb_reg_and(XGIPART2, 0x30, 0xDF);
-
- } else if (xgifb_info->TV_plug
- == TVPLUG_COMPOSITE) {
- xgifb_reg_or(XGIPART2, 0x30, 0x20);
-
- switch (xgifb_info->video_width) {
- case 640:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xF1);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0xF7);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x1F);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x32);
- break;
- case 720:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xF3);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x00);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x1D);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x20);
- break;
- case 800:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xFC);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0xFB);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x14);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x2A);
- break;
- }
+ if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
+ xgifb_reg_and(XGIPART2, 0x30, 0xdf);
+
+ } else if (xgifb_info->TV_plug == TVPLUG_COMPOSITE) {
+ xgifb_reg_or(XGIPART2, 0x30, 0x20);
+
+ switch (xgifb_info->video_width) {
+ case 640:
+ xgifb_reg_set(XGIPART2, 0x35, 0xEB);
+ xgifb_reg_set(XGIPART2, 0x36, 0x04);
+ xgifb_reg_set(XGIPART2, 0x37, 0x25);
+ xgifb_reg_set(XGIPART2, 0x38, 0x18);
+ break;
+ case 720:
+ xgifb_reg_set(XGIPART2, 0x35, 0xEE);
+ xgifb_reg_set(XGIPART2, 0x36, 0x0C);
+ xgifb_reg_set(XGIPART2, 0x37, 0x22);
+ xgifb_reg_set(XGIPART2, 0x38, 0x08);
+ break;
+ case 800:
+ xgifb_reg_set(XGIPART2, 0x35, 0xEB);
+ xgifb_reg_set(XGIPART2, 0x36, 0x15);
+ xgifb_reg_set(XGIPART2, 0x37, 0x25);
+ xgifb_reg_set(XGIPART2, 0x38, 0xF6);
+ break;
}
}
- if ((filter >= 0) && (filter <= 7)) {
- pr_debug("FilterTable[%d]-%d: %*ph\n",
- filter_tb, filter,
- 4, XGI_TV_filter[filter_tb].
- filter[filter]);
- xgifb_reg_set(
- XGIPART2,
- 0x35,
- (XGI_TV_filter[filter_tb].
- filter[filter][0]));
- xgifb_reg_set(
- XGIPART2,
- 0x36,
- (XGI_TV_filter[filter_tb].
- filter[filter][1]));
- xgifb_reg_set(
- XGIPART2,
- 0x37,
- (XGI_TV_filter[filter_tb].
- filter[filter][2]));
- xgifb_reg_set(
- XGIPART2,
- 0x38,
- (XGI_TV_filter[filter_tb].
- filter[filter][3]));
+ } else if (xgifb_info->TV_type == TVMODE_PAL) {
+ xgifb_reg_and(XGIPART2, 0x3A, 0x1F);
+
+ if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
+ xgifb_reg_and(XGIPART2, 0x30, 0xDF);
+
+ } else if (xgifb_info->TV_plug == TVPLUG_COMPOSITE) {
+ xgifb_reg_or(XGIPART2, 0x30, 0x20);
+
+ switch (xgifb_info->video_width) {
+ case 640:
+ xgifb_reg_set(XGIPART2, 0x35, 0xF1);
+ xgifb_reg_set(XGIPART2, 0x36, 0xF7);
+ xgifb_reg_set(XGIPART2, 0x37, 0x1F);
+ xgifb_reg_set(XGIPART2, 0x38, 0x32);
+ break;
+ case 720:
+ xgifb_reg_set(XGIPART2, 0x35, 0xF3);
+ xgifb_reg_set(XGIPART2, 0x36, 0x00);
+ xgifb_reg_set(XGIPART2, 0x37, 0x1D);
+ xgifb_reg_set(XGIPART2, 0x38, 0x20);
+ break;
+ case 800:
+ xgifb_reg_set(XGIPART2, 0x35, 0xFC);
+ xgifb_reg_set(XGIPART2, 0x36, 0xFB);
+ xgifb_reg_set(XGIPART2, 0x37, 0x14);
+ xgifb_reg_set(XGIPART2, 0x38, 0x2A);
+ break;
+ }
}
}
+
+ if (filter >= 0 && filter <= 7) {
+ const u8 *f = XGI_TV_filter[filter_tb].filter[filter];
+
+ pr_debug("FilterTable[%d]-%d: %*ph\n",
+ filter_tb, filter, 4, f);
+ xgifb_reg_set(XGIPART2, 0x35, f[0]);
+ xgifb_reg_set(XGIPART2, 0x36, f[1]);
+ xgifb_reg_set(XGIPART2, 0x37, f[2]);
+ xgifb_reg_set(XGIPART2, 0x38, f[3]);
+ }
}
}
prev parent reply other threads:[~2017-02-22 4:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-21 17:19 [PATCH] staging: xgifb: correct the multiple line dereference to fix coding stye errors Arushi Singhal
2017-02-21 17:40 ` [Outreachy kernel] " Julia Lawall
2017-02-21 18:42 ` Joe Perches
[not found] ` <CA+XqjF-bsKnXFkFN_SGToYTvMdWHX_CCas_BvdtT9UOb04hkCQ@mail.gmail.com>
2017-02-22 4:33 ` Joe Perches [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1487738018.14159.1.camel@perches.com \
--to=joe@perches.com \
--cc=arnaud.patard@rtp-net.org \
--cc=arushisinghal19971997@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=julia.lawall@lip6.fr \
--cc=linux-kernel@vger.kernel.org \
--cc=outreachy-kernel@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox