2015-02-09 12:56:00

by Parmeshwr Prasad

[permalink] [raw]
Subject: [PATCH] Trivial patch in efifb.c to solve common indent issue and indent error

Hi All,

Please review this patch.
this patch is aimed to solve some indentation issue. It has also solved
three trivial error in efifb.c file.

And I have also changed printk with pr_err, pr_info ... at respective places.


>From c49139fac1d15fe2da80d06e2a79eb8be7c079a7 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Mon, 9 Feb 2015 07:33:59 -0500
Subject: [PATCH] Trival patch: improved indentation, and removed some ERROR
from code

Signed-off-by: Parmeshwr Prasad <[email protected]>
---
drivers/video/fbdev/efifb.c | 158 +++++++++++++++++++++++---------------------
1 file changed, 84 insertions(+), 74 deletions(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..5c03035 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -17,29 +17,28 @@
#include <video/vga.h>
#include <asm/sysfb.h>

-static bool request_mem_succeeded = false;
+static bool request_mem_succeeded;

static struct fb_var_screeninfo efifb_defined = {
- .activate = FB_ACTIVATE_NOW,
- .height = -1,
- .width = -1,
- .right_margin = 32,
- .upper_margin = 16,
- .lower_margin = 4,
- .vsync_len = 4,
- .vmode = FB_VMODE_NONINTERLACED,
+ .activate = FB_ACTIVATE_NOW,
+ .height = -1,
+ .width = -1,
+ .right_margin = 32,
+ .upper_margin = 16,
+ .lower_margin = 4,
+ .vsync_len = 4,
+ .vmode = FB_VMODE_NONINTERLACED,
};

static struct fb_fix_screeninfo efifb_fix = {
- .id = "EFI VGA",
- .type = FB_TYPE_PACKED_PIXELS,
- .accel = FB_ACCEL_NONE,
- .visual = FB_VISUAL_TRUECOLOR,
+ .id = "EFI VGA",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .accel = FB_ACCEL_NONE,
+ .visual = FB_VISUAL_TRUECOLOR,
};
static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
- unsigned blue, unsigned transp,
- struct fb_info *info)
+ unsigned blue, unsigned transp, struct fb_info *info)
{
/*
* Set a single color register. The values supplied are
@@ -52,13 +51,13 @@ static int efifb_setcolreg(unsigned regno, unsigned red,
unsigned green,
return 1;

if (regno < 16) {
- red >>= 8;
+ red >>= 8;
green >>= 8;
- blue >>= 8;
- ((u32 *)(info->pseudo_palette))[regno] =
- (red << info->var.red.offset) |
- (green << info->var.green.offset) |
- (blue << info->var.blue.offset);
+ blue >>= 8;
+ ((u32 *) (info->pseudo_palette))[regno] =
+ (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset);
}
return 0;
}
@@ -74,12 +73,12 @@ static void efifb_destroy(struct fb_info *info)
}

static struct fb_ops efifb_ops = {
- .owner = THIS_MODULE,
- .fb_destroy = efifb_destroy,
- .fb_setcolreg = efifb_setcolreg,
- .fb_fillrect = cfb_fillrect,
- .fb_copyarea = cfb_copyarea,
- .fb_imageblit = cfb_imageblit,
+ .owner = THIS_MODULE,
+ .fb_destroy = efifb_destroy,
+ .fb_setcolreg = efifb_setcolreg,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit,
};

static int efifb_setup(char *options)
@@ -89,25 +88,35 @@ static int efifb_setup(char *options)

if (options && *options) {
while ((this_opt = strsep(&options, ",")) != NULL) {
- if (!*this_opt) continue;
+ if (!*this_opt)
+ continue;

for (i = 0; i < M_UNKNOWN; i++) {
if (efifb_dmi_list[i].base != 0 &&
- !strcmp(this_opt,
efifb_dmi_list[i].optname)) {
- screen_info.lfb_base =
efifb_dmi_list[i].base;
- screen_info.lfb_linelength =
efifb_dmi_list[i].stride;
- screen_info.lfb_width =
efifb_dmi_list[i].width;
- screen_info.lfb_height =
efifb_dmi_list[i].height;
+ !strcmp(this_opt,
+ efifb_dmi_list[i].optname)) {
+ screen_info.lfb_base =
+ efifb_dmi_list[i].base;
+ screen_info.lfb_linelength =
+ efifb_dmi_list[i].stride;
+ screen_info.lfb_width =
+ efifb_dmi_list[i].width;
+ screen_info.lfb_height =
+ efifb_dmi_list[i].height;
}
}
if (!strncmp(this_opt, "base:", 5))
- screen_info.lfb_base =
simple_strtoul(this_opt+5, NULL, 0);
+ screen_info.lfb_base =
+ simple_strtoul(this_opt + 5, NULL, 0);
else if (!strncmp(this_opt, "stride:", 7))
- screen_info.lfb_linelength =
simple_strtoul(this_opt+7, NULL, 0) * 4;
+ screen_info.lfb_linelength =
+ simple_strtoul(this_opt + 7, NULL, 0) * 4;
else if (!strncmp(this_opt, "height:", 7))
- screen_info.lfb_height =
simple_strtoul(this_opt+7, NULL, 0);
+ screen_info.lfb_height =
+ simple_strtoul(this_opt + 7, NULL, 0);
else if (!strncmp(this_opt, "width:", 6))
- screen_info.lfb_width =
simple_strtoul(this_opt+6, NULL, 0);
+ screen_info.lfb_width =
+ simple_strtoul(this_opt + 6, NULL, 0);
}
}

@@ -142,10 +151,10 @@ static int efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
- printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
+ pr_err("efifb: invalid framebuffer address\n");
return -ENODEV;
}
- printk(KERN_INFO "efifb: probing for efifb\n");
+ pr_info("efifb: probing for efifb\n");

/* just assume they're all unset if any are */
if (!screen_info.blue_size) {
@@ -181,7 +190,7 @@ static int efifb_probe(struct platform_device *dev)
* use for efifb. With modern cards it is no
* option to simply use size_total as that
* wastes plenty of kernel address space. */
- size_remap = size_vmode * 2;
+ size_remap = size_vmode * 2;
if (size_remap > size_total)
size_remap = size_total;
if (size_remap % PAGE_SIZE)
@@ -193,14 +202,14 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
+ pr_warn(
"efifb: cannot reserve video memory at 0x%lx\n",
- efifb_fix.smem_start);
+ efifb_fix.smem_start);
}

info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("efifb: cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -216,46 +225,46 @@ static int efifb_probe(struct platform_device *dev)
info->apertures->ranges[0].base = efifb_fix.smem_start;
info->apertures->ranges[0].size = size_remap;

- info->screen_base = ioremap_wc(efifb_fix.smem_start,
efifb_fix.smem_len);
+ info->screen_base =
+ ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
- "0x%x @ 0x%lx\n",
- efifb_fix.smem_len, efifb_fix.smem_start);
+ pr_err("efifb: abort, cannot ioremap video memory "
+ "0x%x @ 0x%lx\n",
+ efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}

- printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
+ pr_info("efifb: framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
efifb_fix.smem_start, info->screen_base,
- size_remap/1024, size_total/1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ size_remap / 1024, size_total / 1024);
+ pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);

efifb_defined.xres_virtual = efifb_defined.xres;
- efifb_defined.yres_virtual = efifb_fix.smem_len /
- efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length;
+ pr_info("efifb: scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;

/* some dummy values for timing to make fbset happy */
- efifb_defined.pixclock = 10000000 / efifb_defined.xres *
- 1000 / efifb_defined.yres;
- efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
- efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
-
- efifb_defined.red.offset = screen_info.red_pos;
- efifb_defined.red.length = screen_info.red_size;
- efifb_defined.green.offset = screen_info.green_pos;
- efifb_defined.green.length = screen_info.green_size;
- efifb_defined.blue.offset = screen_info.blue_pos;
- efifb_defined.blue.length = screen_info.blue_size;
+ efifb_defined.pixclock = 10000000 / efifb_defined.xres *
+ 1000 / efifb_defined.yres;
+ efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
+ efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
+
+ efifb_defined.red.offset = screen_info.red_pos;
+ efifb_defined.red.length = screen_info.red_size;
+ efifb_defined.green.offset = screen_info.green_pos;
+ efifb_defined.green.length = screen_info.green_size;
+ efifb_defined.blue.offset = screen_info.blue_pos;
+ efifb_defined.blue.length = screen_info.blue_size;
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;

- printk(KERN_INFO "efifb: %s: "
+ pr_info("efifb: %s: "
"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
"Truecolor",
screen_info.rsvd_size,
@@ -264,10 +273,9 @@ static int efifb_probe(struct platform_device *dev)
screen_info.blue_size,
screen_info.rsvd_pos,
screen_info.red_pos,
- screen_info.green_pos,
- screen_info.blue_pos);
+ screen_info.green_pos, screen_info.blue_pos);

- efifb_fix.ypanstep = 0;
+ efifb_fix.ypanstep = 0;
efifb_fix.ywrapstep = 0;

info->fbops = &efifb_ops;
@@ -275,12 +283,14 @@ static int efifb_probe(struct platform_device *dev)
info->fix = efifb_fix;
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;

- if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
- printk(KERN_ERR "efifb: cannot allocate colormap\n");
+ err = fb_alloc_cmap(&info->cmap, 256, 0);
+ if ((err < 0) {
+ pr_err("efifb: cannot allocate colormap\n");
goto err_unmap;
}
- if ((err = register_framebuffer(info)) < 0) {
- printk(KERN_ERR "efifb: cannot register framebuffer\n");
+ err = register_framebuffer(info);
+ if (err < 0) {
+ pr_err("efifb: cannot register framebuffer\n");
goto err_fb_dealoc;
}
fb_info(info, "%s frame buffer device\n", info->fix.id);
@@ -310,8 +320,8 @@ static int efifb_remove(struct platform_device *pdev)

static struct platform_driver efifb_driver = {
.driver = {
- .name = "efi-framebuffer",
- },
+ .name = "efi-framebuffer",
+ },
.probe = efifb_probe,
.remove = efifb_remove,
};
--
1.9.3


2015-02-09 14:25:24

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH] Trivial patch in efifb.c to solve common indent issue and indent error

Hi,

Thanks for the patch.

On Mon, Feb 9, 2015 at 12:55 PM, Parmeshwr Prasad
<[email protected]> wrote:
> Hi All,
>
> Please review this patch.
> this patch is aimed to solve some indentation issue. It has also solved
> three trivial error in efifb.c file.
>
> And I have also changed printk with pr_err, pr_info ... at respective places.
>
>
> From c49139fac1d15fe2da80d06e2a79eb8be7c079a7 Mon Sep 17 00:00:00 2001
> From: Parmeshwr Prasad <[email protected]>
> Date: Mon, 9 Feb 2015 07:33:59 -0500
> Subject: [PATCH] Trival patch: improved indentation, and removed some ERROR
> from code
>
> Signed-off-by: Parmeshwr Prasad <[email protected]>
> ---

1: did you use git to send this patch ?
2: did you checkpatch it (I see some issues)?
3: break this patch into 2 one fixing the 3 issues and other fixing
the indentation.
4: have proper commit message.

Cheers,
--Prabhakar Lad

2015-02-10 08:06:33

by Parmeshwr Prasad

[permalink] [raw]
Subject: Re: [PATCH1/2] Trivial patch in efifb.c to solve common indent issue and indent error

I have created two patch now.
0001-Trival-patch-improved-indentation-in-efifb.c-file.patch:
This patch solves indentation issue in efifb.c file.
This don't has any code change.
I checked with checkpatch.pl it has some warning to remove some old functions in
efifb.c. my changes are not to address that. That changes I can try in my next
patch.
0002-Trivial-patch-In-this-patch-printk-is-replaced-by-pr.patch:
This is addressing following changes:
1- It has removed "quoted string split across lines" warning.
2- There was static initialization of request_mem_succeeded, which is removed.
3- Assignment in if condition, this is fixed.

This one I have checked with checkpatch.pl it don't has any warning or comment.


I am sending these patches with mutt.

>From 8ce800b5f5a048109014994bcfc4fae2ef9cc271 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Tue, 10 Feb 2015 00:33:30 -0500
Subject: [PATCH] Trival patch: improved indentation in efifb.c file

Signed-off-by: Parmeshwr Prasad <[email protected]>
---
drivers/video/fbdev/efifb.c | 128 +++++++++++++++++++++++---------------------
1 file changed, 68 insertions(+), 60 deletions(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..d757d0f 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -20,26 +20,25 @@
static bool request_mem_succeeded = false;

static struct fb_var_screeninfo efifb_defined = {
- .activate = FB_ACTIVATE_NOW,
- .height = -1,
- .width = -1,
- .right_margin = 32,
- .upper_margin = 16,
- .lower_margin = 4,
- .vsync_len = 4,
- .vmode = FB_VMODE_NONINTERLACED,
+ .activate = FB_ACTIVATE_NOW,
+ .height = -1,
+ .width = -1,
+ .right_margin = 32,
+ .upper_margin = 16,
+ .lower_margin = 4,
+ .vsync_len = 4,
+ .vmode = FB_VMODE_NONINTERLACED,
};

static struct fb_fix_screeninfo efifb_fix = {
- .id = "EFI VGA",
- .type = FB_TYPE_PACKED_PIXELS,
- .accel = FB_ACCEL_NONE,
- .visual = FB_VISUAL_TRUECOLOR,
+ .id = "EFI VGA",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .accel = FB_ACCEL_NONE,
+ .visual = FB_VISUAL_TRUECOLOR,
};

static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
- unsigned blue, unsigned transp,
- struct fb_info *info)
+ unsigned blue, unsigned transp, struct fb_info *info)
{
/*
* Set a single color register. The values supplied are
@@ -52,13 +51,13 @@ static int efifb_setcolreg(unsigned regno, unsigned red,
unsigned green,
return 1;

if (regno < 16) {
- red >>= 8;
+ red >>= 8;
green >>= 8;
- blue >>= 8;
- ((u32 *)(info->pseudo_palette))[regno] =
- (red << info->var.red.offset) |
- (green << info->var.green.offset) |
- (blue << info->var.blue.offset);
+ blue >>= 8;
+ ((u32 *) (info->pseudo_palette))[regno] =
+ (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset);
}
return 0;
}
@@ -74,12 +73,12 @@ static void efifb_destroy(struct fb_info *info)
}

static struct fb_ops efifb_ops = {
- .owner = THIS_MODULE,
- .fb_destroy = efifb_destroy,
- .fb_setcolreg = efifb_setcolreg,
- .fb_fillrect = cfb_fillrect,
- .fb_copyarea = cfb_copyarea,
- .fb_imageblit = cfb_imageblit,
+ .owner = THIS_MODULE,
+ .fb_destroy = efifb_destroy,
+ .fb_setcolreg = efifb_setcolreg,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit,
};

static int efifb_setup(char *options)
@@ -89,25 +88,35 @@ static int efifb_setup(char *options)

if (options && *options) {
while ((this_opt = strsep(&options, ",")) != NULL) {
- if (!*this_opt) continue;
+ if (!*this_opt)
+ continue;

for (i = 0; i < M_UNKNOWN; i++) {
if (efifb_dmi_list[i].base != 0 &&
- !strcmp(this_opt,
efifb_dmi_list[i].optname)) {
- screen_info.lfb_base =
efifb_dmi_list[i].base;
- screen_info.lfb_linelength =
efifb_dmi_list[i].stride;
- screen_info.lfb_width =
efifb_dmi_list[i].width;
- screen_info.lfb_height =
efifb_dmi_list[i].height;
+ !strcmp(this_opt,
+ efifb_dmi_list[i].optname)) {
+ screen_info.lfb_base =
+ efifb_dmi_list[i].base;
+ screen_info.lfb_linelength =
+ efifb_dmi_list[i].stride;
+ screen_info.lfb_width =
+ efifb_dmi_list[i].width;
+ screen_info.lfb_height =
+ efifb_dmi_list[i].height;
}
}
if (!strncmp(this_opt, "base:", 5))
- screen_info.lfb_base =
simple_strtoul(this_opt+5, NULL, 0);
+ screen_info.lfb_base =
+ simple_strtoul(this_opt + 5, NULL, 0);
else if (!strncmp(this_opt, "stride:", 7))
- screen_info.lfb_linelength =
simple_strtoul(this_opt+7, NULL, 0) * 4;
+ screen_info.lfb_linelength =
+ simple_strtoul(this_opt + 7, NULL, 0) * 4;
else if (!strncmp(this_opt, "height:", 7))
- screen_info.lfb_height =
simple_strtoul(this_opt+7, NULL, 0);
+ screen_info.lfb_height =
+ simple_strtoul(this_opt + 7, NULL, 0);
else if (!strncmp(this_opt, "width:", 6))
- screen_info.lfb_width =
simple_strtoul(this_opt+6, NULL, 0);
+ screen_info.lfb_width =
+ simple_strtoul(this_opt + 6, NULL, 0);
}
}

@@ -181,7 +190,7 @@ static int efifb_probe(struct platform_device *dev)
* use for efifb. With modern cards it is no
* option to simply use size_total as that
* wastes plenty of kernel address space. */
- size_remap = size_vmode * 2;
+ size_remap = size_vmode * 2;
if (size_remap > size_total)
size_remap = size_total;
if (size_remap % PAGE_SIZE)
@@ -195,7 +204,7 @@ static int efifb_probe(struct platform_device *dev)
spaces our resource handlers simply don't know about */
printk(KERN_WARNING
"efifb: cannot reserve video memory at 0x%lx\n",
- efifb_fix.smem_start);
+ efifb_fix.smem_start);
}

info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
@@ -216,11 +225,12 @@ static int efifb_probe(struct platform_device *dev)
info->apertures->ranges[0].base = efifb_fix.smem_start;
info->apertures->ranges[0].size = size_remap;

- info->screen_base = ioremap_wc(efifb_fix.smem_start,
efifb_fix.smem_len);
+ info->screen_base =
+ ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
if (!info->screen_base) {
printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
- "0x%x @ 0x%lx\n",
- efifb_fix.smem_len, efifb_fix.smem_start);
+ "0x%x @ 0x%lx\n",
+ efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}
@@ -228,30 +238,29 @@ static int efifb_probe(struct platform_device *dev)
printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
efifb_fix.smem_start, info->screen_base,
- size_remap/1024, size_total/1024);
+ size_remap / 1024, size_total / 1024);
printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);

efifb_defined.xres_virtual = efifb_defined.xres;
- efifb_defined.yres_virtual = efifb_fix.smem_len /
- efifb_fix.line_length;
+ efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length;
printk(KERN_INFO "efifb: scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;

/* some dummy values for timing to make fbset happy */
- efifb_defined.pixclock = 10000000 / efifb_defined.xres *
- 1000 / efifb_defined.yres;
- efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
- efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
-
- efifb_defined.red.offset = screen_info.red_pos;
- efifb_defined.red.length = screen_info.red_size;
- efifb_defined.green.offset = screen_info.green_pos;
- efifb_defined.green.length = screen_info.green_size;
- efifb_defined.blue.offset = screen_info.blue_pos;
- efifb_defined.blue.length = screen_info.blue_size;
+ efifb_defined.pixclock = 10000000 / efifb_defined.xres *
+ 1000 / efifb_defined.yres;
+ efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
+ efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
+
+ efifb_defined.red.offset = screen_info.red_pos;
+ efifb_defined.red.length = screen_info.red_size;
+ efifb_defined.green.offset = screen_info.green_pos;
+ efifb_defined.green.length = screen_info.green_size;
+ efifb_defined.blue.offset = screen_info.blue_pos;
+ efifb_defined.blue.length = screen_info.blue_size;
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;

@@ -264,10 +273,9 @@ static int efifb_probe(struct platform_device *dev)
screen_info.blue_size,
screen_info.rsvd_pos,
screen_info.red_pos,
- screen_info.green_pos,
- screen_info.blue_pos);
+ screen_info.green_pos, screen_info.blue_pos);

- efifb_fix.ypanstep = 0;
+ efifb_fix.ypanstep = 0;
efifb_fix.ywrapstep = 0;

info->fbops = &efifb_ops;
@@ -310,8 +318,8 @@ static int efifb_remove(struct platform_device *pdev)

static struct platform_driver efifb_driver = {
.driver = {
- .name = "efi-framebuffer",
- },
+ .name = "efi-framebuffer",
+ },
.probe = efifb_probe,
.remove = efifb_remove,
};
--
1.9.3


-Parmeshwr


On Mon, Feb 09, 2015 at 08:24:50AM -0600, Lad, Prabhakar wrote:
> Hi,
>
> Thanks for the patch.
>
> On Mon, Feb 9, 2015 at 12:55 PM, Parmeshwr Prasad
> <[email protected]> wrote:
> > Hi All,
> >
> > Please review this patch.
> > this patch is aimed to solve some indentation issue. It has also solved
> > three trivial error in efifb.c file.
> >
> > And I have also changed printk with pr_err, pr_info ... at respective places.
> >
> >
> > From c49139fac1d15fe2da80d06e2a79eb8be7c079a7 Mon Sep 17 00:00:00 2001
> > From: Parmeshwr Prasad <[email protected]>
> > Date: Mon, 9 Feb 2015 07:33:59 -0500
> > Subject: [PATCH] Trival patch: improved indentation, and removed some ERROR
> > from code
> >
> > Signed-off-by: Parmeshwr Prasad <[email protected]>
> > ---
>
> 1: did you use git to send this patch ?
> 2: did you checkpatch it (I see some issues)?
> 3: break this patch into 2 one fixing the 3 issues and other fixing
> the indentation.
> 4: have proper commit message.
>
> Cheers,
> --Prabhakar Lad

2015-02-10 08:10:29

by Parmeshwr Prasad

[permalink] [raw]
Subject: Re: [PATCH2/2] Trivial patch in efifb.c to solve common indent issue and indent error

On Tue, Feb 10, 2015 at 02:06:24AM -0600, Prasad, Parmeshwr wrote:
This is secong patch.
This is addressing following changes:
1- It has removed "quoted string split across lines" warning.
2- There was static initialization of request_mem_succeeded, which is removed.
3- Assignment in if condition, this is fixed.

>From bf6328196341ea36ec79fba71b9b5c2c36d61043 Mon Sep 17 00:00:00 2001
From: Parmeshwr Prasad <[email protected]>
Date: Tue, 10 Feb 2015 02:49:26 -0500
Subject: [PATCH 2/2] Trivial patch: In this patch printk is replaced by pr_*
macros and static initialization of request_mem_succeeded is removed and
assignment in if condition is removed

Signed-off-by: Parmeshwr Prasad <[email protected]>
---
drivers/video/fbdev/efifb.c | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index d757d0f..133fbfb 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -17,8 +17,7 @@
#include <video/vga.h>
#include <asm/sysfb.h>

-static bool request_mem_succeeded = false;
-
+static bool request_mem_succeeded;
static struct fb_var_screeninfo efifb_defined = {
.activate = FB_ACTIVATE_NOW,
.height = -1,
@@ -30,6 +29,9 @@ static struct fb_var_screeninfo efifb_defined = {
.vmode = FB_VMODE_NONINTERLACED,
};

+
+
+
static struct fb_fix_screeninfo efifb_fix = {
.id = "EFI VGA",
.type = FB_TYPE_PACKED_PIXELS,
@@ -151,10 +153,10 @@ static int efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
- printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
+ pr_err("efifb: invalid framebuffer address\n");
return -ENODEV;
}
- printk(KERN_INFO "efifb: probing for efifb\n");
+ pr_info("efifb: probing for efifb\n");

/* just assume they're all unset if any are */
if (!screen_info.blue_size) {
@@ -202,14 +204,14 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
+ pr_warn(
"efifb: cannot reserve video memory at 0x%lx\n",
efifb_fix.smem_start);
}

info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("efifb: cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -228,25 +230,23 @@ static int efifb_probe(struct platform_device *dev)
info->screen_base =
ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
- "0x%x @ 0x%lx\n",
- efifb_fix.smem_len, efifb_fix.smem_start);
+ pr_err("efifb: abort, cannot ioremap video memory 0x%x @ 0x%lx
+ \n", efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}

- printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
- "using %dk, total %dk\n",
- efifb_fix.smem_start, info->screen_base,
+ pr_info("efifb: framebuffer at 0x%lx, mapped to 0x%p,using %dk,
total%dk\n"
+ , efifb_fix.smem_start, info->screen_base,
size_remap / 1024, size_total / 1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);

efifb_defined.xres_virtual = efifb_defined.xres;
efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ pr_info("efifb: scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;

/* some dummy values for timing to make fbset happy */
@@ -264,9 +264,7 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;

- printk(KERN_INFO "efifb: %s: "
- "size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
- "Truecolor",
+ pr_info("efifb: %s:ize=%d:%d:%d:%d, shift=%d:%d:%d:%d\n", "Truecolor",
screen_info.rsvd_size,
screen_info.red_size,
screen_info.green_size,
@@ -283,12 +281,14 @@ static int efifb_probe(struct platform_device *dev)
info->fix = efifb_fix;
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;

- if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
- printk(KERN_ERR "efifb: cannot allocate colormap\n");
+ err = fb_alloc_cmap(&info->cmap, 256, 0);
+ if (err < 0) {
+ pr_err("efifb: cannot allocate colormap\n");
goto err_unmap;
}
- if ((err = register_framebuffer(info)) < 0) {
- printk(KERN_ERR "efifb: cannot register framebuffer\n");
+ err = register_framebuffer(info);
+ if (err < 0) {
+ pr_err("efifb: cannot register framebuffer\n");
goto err_fb_dealoc;
}
fb_info(info, "%s frame buffer device\n", info->fix.id);
--
1.9.3

This don't has any checkpatch.pl comment/warning.

-Parmeshwr

> I have created two patch now.
> 0001-Trival-patch-improved-indentation-in-efifb.c-file.patch:
> This patch solves indentation issue in efifb.c file.
> This don't has any code change.
> I checked with checkpatch.pl it has some warning to remove some old functions in
> efifb.c. my changes are not to address that. That changes I can try in my next
> patch.
> 0002-Trivial-patch-In-this-patch-printk-is-replaced-by-pr.patch:
> This is addressing following changes:
> 1- It has removed "quoted string split across lines" warning.
> 2- There was static initialization of request_mem_succeeded, which is removed.
> 3- Assignment in if condition, this is fixed.
>
> This one I have checked with checkpatch.pl it don't has any warning or comment.
>
>
> I am sending these patches with mutt.
>
> From 8ce800b5f5a048109014994bcfc4fae2ef9cc271 Mon Sep 17 00:00:00 2001
> From: Parmeshwr Prasad <[email protected]>
> Date: Tue, 10 Feb 2015 00:33:30 -0500
> Subject: [PATCH] Trival patch: improved indentation in efifb.c file
>
> Signed-off-by: Parmeshwr Prasad <[email protected]>
> ---
> drivers/video/fbdev/efifb.c | 128 +++++++++++++++++++++++---------------------
> 1 file changed, 68 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 4bfff34..d757d0f 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -20,26 +20,25 @@
> static bool request_mem_succeeded = false;
>
> static struct fb_var_screeninfo efifb_defined = {
> - .activate = FB_ACTIVATE_NOW,
> - .height = -1,
> - .width = -1,
> - .right_margin = 32,
> - .upper_margin = 16,
> - .lower_margin = 4,
> - .vsync_len = 4,
> - .vmode = FB_VMODE_NONINTERLACED,
> + .activate = FB_ACTIVATE_NOW,
> + .height = -1,
> + .width = -1,
> + .right_margin = 32,
> + .upper_margin = 16,
> + .lower_margin = 4,
> + .vsync_len = 4,
> + .vmode = FB_VMODE_NONINTERLACED,
> };
>
> static struct fb_fix_screeninfo efifb_fix = {
> - .id = "EFI VGA",
> - .type = FB_TYPE_PACKED_PIXELS,
> - .accel = FB_ACCEL_NONE,
> - .visual = FB_VISUAL_TRUECOLOR,
> + .id = "EFI VGA",
> + .type = FB_TYPE_PACKED_PIXELS,
> + .accel = FB_ACCEL_NONE,
> + .visual = FB_VISUAL_TRUECOLOR,
> };
>
> static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
> - unsigned blue, unsigned transp,
> - struct fb_info *info)
> + unsigned blue, unsigned transp, struct fb_info *info)
> {
> /*
> * Set a single color register. The values supplied are
> @@ -52,13 +51,13 @@ static int efifb_setcolreg(unsigned regno, unsigned red,
> unsigned green,
> return 1;
>
> if (regno < 16) {
> - red >>= 8;
> + red >>= 8;
> green >>= 8;
> - blue >>= 8;
> - ((u32 *)(info->pseudo_palette))[regno] =
> - (red << info->var.red.offset) |
> - (green << info->var.green.offset) |
> - (blue << info->var.blue.offset);
> + blue >>= 8;
> + ((u32 *) (info->pseudo_palette))[regno] =
> + (red << info->var.red.offset) |
> + (green << info->var.green.offset) |
> + (blue << info->var.blue.offset);
> }
> return 0;
> }
> @@ -74,12 +73,12 @@ static void efifb_destroy(struct fb_info *info)
> }
>
> static struct fb_ops efifb_ops = {
> - .owner = THIS_MODULE,
> - .fb_destroy = efifb_destroy,
> - .fb_setcolreg = efifb_setcolreg,
> - .fb_fillrect = cfb_fillrect,
> - .fb_copyarea = cfb_copyarea,
> - .fb_imageblit = cfb_imageblit,
> + .owner = THIS_MODULE,
> + .fb_destroy = efifb_destroy,
> + .fb_setcolreg = efifb_setcolreg,
> + .fb_fillrect = cfb_fillrect,
> + .fb_copyarea = cfb_copyarea,
> + .fb_imageblit = cfb_imageblit,
> };
>
> static int efifb_setup(char *options)
> @@ -89,25 +88,35 @@ static int efifb_setup(char *options)
>
> if (options && *options) {
> while ((this_opt = strsep(&options, ",")) != NULL) {
> - if (!*this_opt) continue;
> + if (!*this_opt)
> + continue;
>
> for (i = 0; i < M_UNKNOWN; i++) {
> if (efifb_dmi_list[i].base != 0 &&
> - !strcmp(this_opt,
> efifb_dmi_list[i].optname)) {
> - screen_info.lfb_base =
> efifb_dmi_list[i].base;
> - screen_info.lfb_linelength =
> efifb_dmi_list[i].stride;
> - screen_info.lfb_width =
> efifb_dmi_list[i].width;
> - screen_info.lfb_height =
> efifb_dmi_list[i].height;
> + !strcmp(this_opt,
> + efifb_dmi_list[i].optname)) {
> + screen_info.lfb_base =
> + efifb_dmi_list[i].base;
> + screen_info.lfb_linelength =
> + efifb_dmi_list[i].stride;
> + screen_info.lfb_width =
> + efifb_dmi_list[i].width;
> + screen_info.lfb_height =
> + efifb_dmi_list[i].height;
> }
> }
> if (!strncmp(this_opt, "base:", 5))
> - screen_info.lfb_base =
> simple_strtoul(this_opt+5, NULL, 0);
> + screen_info.lfb_base =
> + simple_strtoul(this_opt + 5, NULL, 0);
> else if (!strncmp(this_opt, "stride:", 7))
> - screen_info.lfb_linelength =
> simple_strtoul(this_opt+7, NULL, 0) * 4;
> + screen_info.lfb_linelength =
> + simple_strtoul(this_opt + 7, NULL, 0) * 4;
> else if (!strncmp(this_opt, "height:", 7))
> - screen_info.lfb_height =
> simple_strtoul(this_opt+7, NULL, 0);
> + screen_info.lfb_height =
> + simple_strtoul(this_opt + 7, NULL, 0);
> else if (!strncmp(this_opt, "width:", 6))
> - screen_info.lfb_width =
> simple_strtoul(this_opt+6, NULL, 0);
> + screen_info.lfb_width =
> + simple_strtoul(this_opt + 6, NULL, 0);
> }
> }
>
> @@ -181,7 +190,7 @@ static int efifb_probe(struct platform_device *dev)
> * use for efifb. With modern cards it is no
> * option to simply use size_total as that
> * wastes plenty of kernel address space. */
> - size_remap = size_vmode * 2;
> + size_remap = size_vmode * 2;
> if (size_remap > size_total)
> size_remap = size_total;
> if (size_remap % PAGE_SIZE)
> @@ -195,7 +204,7 @@ static int efifb_probe(struct platform_device *dev)
> spaces our resource handlers simply don't know about */
> printk(KERN_WARNING
> "efifb: cannot reserve video memory at 0x%lx\n",
> - efifb_fix.smem_start);
> + efifb_fix.smem_start);
> }
>
> info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
> @@ -216,11 +225,12 @@ static int efifb_probe(struct platform_device *dev)
> info->apertures->ranges[0].base = efifb_fix.smem_start;
> info->apertures->ranges[0].size = size_remap;
>
> - info->screen_base = ioremap_wc(efifb_fix.smem_start,
> efifb_fix.smem_len);
> + info->screen_base =
> + ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
> if (!info->screen_base) {
> printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
> - "0x%x @ 0x%lx\n",
> - efifb_fix.smem_len, efifb_fix.smem_start);
> + "0x%x @ 0x%lx\n",
> + efifb_fix.smem_len, efifb_fix.smem_start);
> err = -EIO;
> goto err_release_fb;
> }
> @@ -228,30 +238,29 @@ static int efifb_probe(struct platform_device *dev)
> printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
> "using %dk, total %dk\n",
> efifb_fix.smem_start, info->screen_base,
> - size_remap/1024, size_total/1024);
> + size_remap / 1024, size_total / 1024);
> printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
> efifb_defined.xres, efifb_defined.yres,
> efifb_defined.bits_per_pixel, efifb_fix.line_length,
> screen_info.pages);
>
> efifb_defined.xres_virtual = efifb_defined.xres;
> - efifb_defined.yres_virtual = efifb_fix.smem_len /
> - efifb_fix.line_length;
> + efifb_defined.yres_virtual = efifb_fix.smem_len / efifb_fix.line_length;
> printk(KERN_INFO "efifb: scrolling: redraw\n");
> efifb_defined.yres_virtual = efifb_defined.yres;
>
> /* some dummy values for timing to make fbset happy */
> - efifb_defined.pixclock = 10000000 / efifb_defined.xres *
> - 1000 / efifb_defined.yres;
> - efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
> - efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
> -
> - efifb_defined.red.offset = screen_info.red_pos;
> - efifb_defined.red.length = screen_info.red_size;
> - efifb_defined.green.offset = screen_info.green_pos;
> - efifb_defined.green.length = screen_info.green_size;
> - efifb_defined.blue.offset = screen_info.blue_pos;
> - efifb_defined.blue.length = screen_info.blue_size;
> + efifb_defined.pixclock = 10000000 / efifb_defined.xres *
> + 1000 / efifb_defined.yres;
> + efifb_defined.left_margin = (efifb_defined.xres / 8) & 0xf8;
> + efifb_defined.hsync_len = (efifb_defined.xres / 8) & 0xf8;
> +
> + efifb_defined.red.offset = screen_info.red_pos;
> + efifb_defined.red.length = screen_info.red_size;
> + efifb_defined.green.offset = screen_info.green_pos;
> + efifb_defined.green.length = screen_info.green_size;
> + efifb_defined.blue.offset = screen_info.blue_pos;
> + efifb_defined.blue.length = screen_info.blue_size;
> efifb_defined.transp.offset = screen_info.rsvd_pos;
> efifb_defined.transp.length = screen_info.rsvd_size;
>
> @@ -264,10 +273,9 @@ static int efifb_probe(struct platform_device *dev)
> screen_info.blue_size,
> screen_info.rsvd_pos,
> screen_info.red_pos,
> - screen_info.green_pos,
> - screen_info.blue_pos);
> + screen_info.green_pos, screen_info.blue_pos);
>
> - efifb_fix.ypanstep = 0;
> + efifb_fix.ypanstep = 0;
> efifb_fix.ywrapstep = 0;
>
> info->fbops = &efifb_ops;
> @@ -310,8 +318,8 @@ static int efifb_remove(struct platform_device *pdev)
>
> static struct platform_driver efifb_driver = {
> .driver = {
> - .name = "efi-framebuffer",
> - },
> + .name = "efi-framebuffer",
> + },
> .probe = efifb_probe,
> .remove = efifb_remove,
> };
> --
> 1.9.3
>
>
> -Parmeshwr
>
>
> On Mon, Feb 09, 2015 at 08:24:50AM -0600, Lad, Prabhakar wrote:
> > Hi,
> >
> > Thanks for the patch.
> >
> > On Mon, Feb 9, 2015 at 12:55 PM, Parmeshwr Prasad
> > <[email protected]> wrote:
> > > Hi All,
> > >
> > > Please review this patch.
> > > this patch is aimed to solve some indentation issue. It has also solved
> > > three trivial error in efifb.c file.
> > >
> > > And I have also changed printk with pr_err, pr_info ... at respective places.
> > >
> > >
> > > From c49139fac1d15fe2da80d06e2a79eb8be7c079a7 Mon Sep 17 00:00:00 2001
> > > From: Parmeshwr Prasad <[email protected]>
> > > Date: Mon, 9 Feb 2015 07:33:59 -0500
> > > Subject: [PATCH] Trival patch: improved indentation, and removed some ERROR
> > > from code
> > >
> > > Signed-off-by: Parmeshwr Prasad <[email protected]>
> > > ---
> >
> > 1: did you use git to send this patch ?
> > 2: did you checkpatch it (I see some issues)?
> > 3: break this patch into 2 one fixing the 3 issues and other fixing
> > the indentation.
> > 4: have proper commit message.
> >
> > Cheers,
> > --Prabhakar Lad
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html