* [PATCH 1/8] pxa: un-nest pxafb_parse_options() to cleanup the coding style issue
@ 2008-02-27 2:33 eric miao
2008-03-06 7:13 ` Andrew Morton
0 siblings, 1 reply; 2+ messages in thread
From: eric miao @ 2008-02-27 2:33 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-fbdev-devel
From e946f577d3f869abcc587629e2fdd5f1563ce1df Mon Sep 17 00:00:00 2001
From: eric miao <eric.miao@marvell.com>
Date: Fri, 22 Feb 2008 11:29:08 +0800
Subject: [PATCH] pxa: un-nest pxafb_parse_options() to cleanup the
coding style issue
pxafb_parse_options() has very long lines exceeding far beyond 80
characters, which makes the function looks bad. Un-nest it into
smaller functions and use a temporary string for only what has been
overridden instead of the whole dev_info() message to reduce the
line a bit more.
Signed-off-by: eric miao <eric.miao@marvell.com>
---
drivers/video/pxafb.c | 306 ++++++++++++++++++++++++++----------------------
1 files changed, 166 insertions(+), 140 deletions(-)
diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
index 97facb1..4eaaed5 100644
--- a/drivers/video/pxafb.c
+++ b/drivers/video/pxafb.c
@@ -1210,154 +1210,180 @@ static struct pxafb_info * __init
pxafb_init_fbinfo(struct device *dev)
}
#ifdef CONFIG_FB_PXA_PARAMETERS
-static int __init pxafb_parse_options(struct device *dev, char *options)
+static int parse_opt_mode(struct device *dev, const char *this_opt)
{
struct pxafb_mach_info *inf = dev->platform_data;
+
+ const char *name = this_opt+5;
+ unsigned int namelen = strlen(name);
+ int res_specified = 0, bpp_specified = 0;
+ unsigned int xres = 0, yres = 0, bpp = 0;
+ int yres_specified = 0;
+ int i;
+ for (i = namelen-1; i >= 0; i--) {
+ switch (name[i]) {
+ case '-':
+ namelen = i;
+ if (!bpp_specified && !yres_specified) {
+ bpp = simple_strtoul(&name[i+1], NULL, 0);
+ bpp_specified = 1;
+ } else
+ goto done;
+ break;
+ case 'x':
+ if (!yres_specified) {
+ yres = simple_strtoul(&name[i+1], NULL, 0);
+ yres_specified = 1;
+ } else
+ goto done;
+ break;
+ case '0' ... '9':
+ break;
+ default:
+ goto done;
+ }
+ }
+ if (i < 0 && yres_specified) {
+ xres = simple_strtoul(name, NULL, 0);
+ res_specified = 1;
+ }
+done:
+ if (res_specified) {
+ dev_info(dev, "overriding resolution: %dx%d\n", xres, yres);
+ inf->modes[0].xres = xres; inf->modes[0].yres = yres;
+ }
+ if (bpp_specified)
+ switch (bpp) {
+ case 1:
+ case 2:
+ case 4:
+ case 8:
+ case 16:
+ inf->modes[0].bpp = bpp;
+ dev_info(dev, "overriding bit depth: %d\n", bpp);
+ break;
+ default:
+ dev_err(dev, "Depth %d is not valid\n", bpp);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int parse_opt(struct device *dev, char *this_opt)
+{
+ struct pxafb_mach_info *inf = dev->platform_data;
+ struct pxafb_mode_info *mode = &inf->modes[0];
+ char s[64];
+
+ s[0] = '\0';
+
+ if (!strncmp(this_opt, "mode:", 5)) {
+ return parse_opt_mode(dev, this_opt);
+ } else if (!strncmp(this_opt, "pixclock:", 9)) {
+ mode->pixclock = simple_strtoul(this_opt+9, NULL, 0);
+ sprintf(s, "pixclock: %ld\n", mode->pixclock);
+ } else if (!strncmp(this_opt, "left:", 5)) {
+ mode->left_margin = simple_strtoul(this_opt+5, NULL, 0);
+ sprintf(s, "left: %u\n", mode->left_margin);
+ } else if (!strncmp(this_opt, "right:", 6)) {
+ mode->right_margin = simple_strtoul(this_opt+6, NULL, 0);
+ sprintf(s, "right: %u\n", mode->right_margin);
+ } else if (!strncmp(this_opt, "upper:", 6)) {
+ mode->upper_margin = simple_strtoul(this_opt+6, NULL, 0);
+ sprintf(s, "upper: %u\n", mode->upper_margin);
+ } else if (!strncmp(this_opt, "lower:", 6)) {
+ mode->lower_margin = simple_strtoul(this_opt+6, NULL, 0);
+ sprintf(s, "lower: %u\n", mode->lower_margin);
+ } else if (!strncmp(this_opt, "hsynclen:", 9)) {
+ mode->hsync_len = simple_strtoul(this_opt+9, NULL, 0);
+ sprintf(s, "hsynclen: %u\n", mode->hsync_len);
+ } else if (!strncmp(this_opt, "vsynclen:", 9)) {
+ mode->vsync_len = simple_strtoul(this_opt+9, NULL, 0);
+ sprintf(s, "vsynclen: %u\n", mode->vsync_len);
+ } else if (!strncmp(this_opt, "hsync:", 6)) {
+ if (simple_strtoul(this_opt+6, NULL, 0) == 0) {
+ sprintf(s, "hsync: Active Low\n");
+ mode->sync &= ~FB_SYNC_HOR_HIGH_ACT;
+ } else {
+ sprintf(s, "hsync: Active High\n");
+ mode->sync |= FB_SYNC_HOR_HIGH_ACT;
+ }
+ } else if (!strncmp(this_opt, "vsync:", 6)) {
+ if (simple_strtoul(this_opt+6, NULL, 0) == 0) {
+ sprintf(s, "vsync: Active Low\n");
+ mode->sync &= ~FB_SYNC_VERT_HIGH_ACT;
+ } else {
+ sprintf(s, "vsync: Active High\n");
+ mode->sync |= FB_SYNC_VERT_HIGH_ACT;
+ }
+ } else if (!strncmp(this_opt, "dpc:", 4)) {
+ if (simple_strtoul(this_opt+4, NULL, 0) == 0) {
+ sprintf(s, "double pixel clock: false\n");
+ inf->lccr3 &= ~LCCR3_DPC;
+ } else {
+ sprintf(s, "double pixel clock: true\n");
+ inf->lccr3 |= LCCR3_DPC;
+ }
+ } else if (!strncmp(this_opt, "outputen:", 9)) {
+ if (simple_strtoul(this_opt+9, NULL, 0) == 0) {
+ sprintf(s, "output enable: active low\n");
+ inf->lccr3 = (inf->lccr3 & ~LCCR3_OEP) | LCCR3_OutEnL;
+ } else {
+ sprintf(s, "output enable: active high\n");
+ inf->lccr3 = (inf->lccr3 & ~LCCR3_OEP) | LCCR3_OutEnH;
+ }
+ } else if (!strncmp(this_opt, "pixclockpol:", 12)) {
+ if (simple_strtoul(this_opt+12, NULL, 0) == 0) {
+ sprintf(s, "pixel clock polarity: falling edge\n");
+ inf->lccr3 = (inf->lccr3 & ~LCCR3_PCP) | LCCR3_PixFlEdg;
+ } else {
+ sprintf(s, "pixel clock polarity: rising edge\n");
+ inf->lccr3 = (inf->lccr3 & ~LCCR3_PCP) | LCCR3_PixRsEdg;
+ }
+ } else if (!strncmp(this_opt, "color", 5)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_CMS) | LCCR0_Color;
+ } else if (!strncmp(this_opt, "mono", 4)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_CMS) | LCCR0_Mono;
+ } else if (!strncmp(this_opt, "active", 6)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_PAS) | LCCR0_Act;
+ } else if (!strncmp(this_opt, "passive", 7)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_PAS) | LCCR0_Pas;
+ } else if (!strncmp(this_opt, "single", 6)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_SDS) | LCCR0_Sngl;
+ } else if (!strncmp(this_opt, "dual", 4)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_SDS) | LCCR0_Dual;
+ } else if (!strncmp(this_opt, "4pix", 4)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_DPD) | LCCR0_4PixMono;
+ } else if (!strncmp(this_opt, "8pix", 4)) {
+ inf->lccr0 = (inf->lccr0 & ~LCCR0_DPD) | LCCR0_8PixMono;
+ } else {
+ dev_err(dev, "unknown option: %s\n", this_opt);
+ return -EINVAL;
+ }
+
+ if (s[0] != '\0')
+ dev_info(dev, "override %s", s);
+
+ return 0;
+}
+
+static int __init pxafb_parse_options(struct device *dev, char *options)
+{
char *this_opt;
+ int ret;
- if (!options || !*options)
- return 0;
+ if (!options || !*options)
+ return 0;
dev_dbg(dev, "options are \"%s\"\n", options ? options : "null");
/* could be made table driven or similar?... */
- while ((this_opt = strsep(&options, ",")) != NULL) {
- if (!strncmp(this_opt, "mode:", 5)) {
- const char *name = this_opt+5;
- unsigned int namelen = strlen(name);
- int res_specified = 0, bpp_specified = 0;
- unsigned int xres = 0, yres = 0, bpp = 0;
- int yres_specified = 0;
- int i;
- for (i = namelen-1; i >= 0; i--) {
- switch (name[i]) {
- case '-':
- namelen = i;
- if (!bpp_specified && !yres_specified) {
- bpp = simple_strtoul(&name[i+1], NULL, 0);
- bpp_specified = 1;
- } else
- goto done;
- break;
- case 'x':
- if (!yres_specified) {
- yres = simple_strtoul(&name[i+1], NULL, 0);
- yres_specified = 1;
- } else
- goto done;
- break;
- case '0' ... '9':
- break;
- default:
- goto done;
- }
- }
- if (i < 0 && yres_specified) {
- xres = simple_strtoul(name, NULL, 0);
- res_specified = 1;
- }
- done:
- if (res_specified) {
- dev_info(dev, "overriding resolution: %dx%d\n", xres, yres);
- inf->modes[0].xres = xres; inf->modes[0].yres = yres;
- }
- if (bpp_specified)
- switch (bpp) {
- case 1:
- case 2:
- case 4:
- case 8:
- case 16:
- inf->modes[0].bpp = bpp;
- dev_info(dev, "overriding bit depth: %d\n", bpp);
- break;
- default:
- dev_err(dev, "Depth %d is not valid\n", bpp);
- }
- } else if (!strncmp(this_opt, "pixclock:", 9)) {
- inf->modes[0].pixclock =
simple_strtoul(this_opt+9, NULL, 0);
- dev_info(dev, "override pixclock: %ld\n", inf->modes[0].pixclock);
- } else if (!strncmp(this_opt, "left:", 5)) {
- inf->modes[0].left_margin =
simple_strtoul(this_opt+5, NULL, 0);
- dev_info(dev, "override left: %u\n", inf->modes[0].left_margin);
- } else if (!strncmp(this_opt, "right:", 6)) {
- inf->modes[0].right_margin =
simple_strtoul(this_opt+6, NULL, 0);
- dev_info(dev, "override right: %u\n", inf->modes[0].right_margin);
- } else if (!strncmp(this_opt, "upper:", 6)) {
- inf->modes[0].upper_margin =
simple_strtoul(this_opt+6, NULL, 0);
- dev_info(dev, "override upper: %u\n", inf->modes[0].upper_margin);
- } else if (!strncmp(this_opt, "lower:", 6)) {
- inf->modes[0].lower_margin =
simple_strtoul(this_opt+6, NULL, 0);
- dev_info(dev, "override lower: %u\n", inf->modes[0].lower_margin);
- } else if (!strncmp(this_opt, "hsynclen:", 9)) {
- inf->modes[0].hsync_len =
simple_strtoul(this_opt+9, NULL, 0);
- dev_info(dev, "override hsynclen: %u\n", inf->modes[0].hsync_len);
- } else if (!strncmp(this_opt, "vsynclen:", 9)) {
- inf->modes[0].vsync_len =
simple_strtoul(this_opt+9, NULL, 0);
- dev_info(dev, "override vsynclen: %u\n", inf->modes[0].vsync_len);
- } else if (!strncmp(this_opt, "hsync:", 6)) {
- if (simple_strtoul(this_opt+6, NULL, 0) == 0) {
- dev_info(dev, "override hsync: Active Low\n");
- inf->modes[0].sync &= ~FB_SYNC_HOR_HIGH_ACT;
- } else {
- dev_info(dev, "override hsync: Active High\n");
- inf->modes[0].sync |= FB_SYNC_HOR_HIGH_ACT;
- }
- } else if (!strncmp(this_opt, "vsync:", 6)) {
- if (simple_strtoul(this_opt+6, NULL, 0) == 0) {
- dev_info(dev, "override vsync: Active Low\n");
- inf->modes[0].sync &= ~FB_SYNC_VERT_HIGH_ACT;
- } else {
- dev_info(dev, "override vsync: Active High\n");
- inf->modes[0].sync |= FB_SYNC_VERT_HIGH_ACT;
- }
- } else if (!strncmp(this_opt, "dpc:", 4)) {
- if (simple_strtoul(this_opt+4, NULL, 0) == 0) {
- dev_info(dev, "override double pixel clock: false\n");
- inf->lccr3 &= ~LCCR3_DPC;
- } else {
- dev_info(dev, "override double pixel clock: true\n");
- inf->lccr3 |= LCCR3_DPC;
- }
- } else if (!strncmp(this_opt, "outputen:", 9)) {
- if (simple_strtoul(this_opt+9, NULL, 0) == 0) {
- dev_info(dev, "override output enable: active low\n");
- inf->lccr3 = (inf->lccr3 & ~LCCR3_OEP) | LCCR3_OutEnL;
- } else {
- dev_info(dev, "override output enable: active high\n");
- inf->lccr3 = (inf->lccr3 & ~LCCR3_OEP) | LCCR3_OutEnH;
- }
- } else if (!strncmp(this_opt, "pixclockpol:", 12)) {
- if (simple_strtoul(this_opt+12, NULL, 0) == 0) {
- dev_info(dev, "override pixel clock polarity: falling edge\n");
- inf->lccr3 = (inf->lccr3 & ~LCCR3_PCP) | LCCR3_PixFlEdg;
- } else {
- dev_info(dev, "override pixel clock polarity: rising edge\n");
- inf->lccr3 = (inf->lccr3 & ~LCCR3_PCP) | LCCR3_PixRsEdg;
- }
- } else if (!strncmp(this_opt, "color", 5)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_CMS) | LCCR0_Color;
- } else if (!strncmp(this_opt, "mono", 4)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_CMS) | LCCR0_Mono;
- } else if (!strncmp(this_opt, "active", 6)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_PAS) | LCCR0_Act;
- } else if (!strncmp(this_opt, "passive", 7)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_PAS) | LCCR0_Pas;
- } else if (!strncmp(this_opt, "single", 6)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_SDS) | LCCR0_Sngl;
- } else if (!strncmp(this_opt, "dual", 4)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_SDS) | LCCR0_Dual;
- } else if (!strncmp(this_opt, "4pix", 4)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_DPD) | LCCR0_4PixMono;
- } else if (!strncmp(this_opt, "8pix", 4)) {
- inf->lccr0 = (inf->lccr0 & ~LCCR0_DPD) | LCCR0_8PixMono;
- } else {
- dev_err(dev, "unknown option: %s\n", this_opt);
- return -EINVAL;
- }
- }
- return 0;
-
+ while ((this_opt = strsep(&options, ",")) != NULL) {
+ ret = parse_opt(dev, this_opt);
+ if (ret)
+ return ret;
+ }
+ return 0;
}
#endif
--
1.5.3.8
--
Cheers
- eric
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH 1/8] pxa: un-nest pxafb_parse_options() to cleanup the coding style issue
2008-02-27 2:33 [PATCH 1/8] pxa: un-nest pxafb_parse_options() to cleanup the coding style issue eric miao
@ 2008-03-06 7:13 ` Andrew Morton
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2008-03-06 7:13 UTC (permalink / raw)
To: linux-fbdev-devel; +Cc: eric miao, linux-kernel
On Wed, 27 Feb 2008 10:33:10 +0800 "eric miao" <eric.y.miao@gmail.com> wrote:
> >From e946f577d3f869abcc587629e2fdd5f1563ce1df Mon Sep 17 00:00:00 2001
> From: eric miao <eric.miao@marvell.com>
> Date: Fri, 22 Feb 2008 11:29:08 +0800
> Subject: [PATCH] pxa: un-nest pxafb_parse_options() to cleanup the
> coding style issue
>
> pxafb_parse_options() has very long lines exceeding far beyond 80
> characters, which makes the function looks bad. Un-nest it into
> smaller functions and use a temporary string for only what has been
> overridden instead of the whole dev_info() message to reduce the
> line a bit more.
>
These patches were all badly wordwrapped. Please refresh them against the
latest mainline kernel, fix your email client and then resend them, being
sure to cc myself.
Thanks.
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2008-03-06 7:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-27 2:33 [PATCH 1/8] pxa: un-nest pxafb_parse_options() to cleanup the coding style issue eric miao
2008-03-06 7:13 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).