| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Jan 2010 10:39:54 +0100
From: Jean Delvare <khali@...ux-fr.org>
To: Peter Hüwe <PeterHuewe@....de>
Cc: Joe Perches <joe@...ches.com>,
Petr Vandrovec <vandrove@...cvut.cz>,
Andrew Morton <akpm@...ux-foundation.org>,
Krzysztof Helt <krzysztof.h1@...pl>,
linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org,
kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v3] video/matrox: Checkpatch cleanups for
matroxfb_crtc2.c
Hi Peter,
On Mon, 18 Jan 2010 22:42:28 +0100, Peter Hüwe wrote:
> From: Peter Huewe <peterhuewe@....de>
> Date: Mon, 18 Jan 2010 19:21:02 +0100
>
> This patch fixes all 77 errors reported by checkpatch - no functional
> change was made. The line over 80 chars warnings were left untouched.
Not really untouched: your patch adds or preserves 9 of these:
WARNING: line over 80 characters
#134: FILE: drivers/video/matrox/matroxfb_crtc2.c:88:
+ if (minfo->outputs[1].mode != MATROXFB_OUTPUT_MODE_MONITOR)
WARNING: line over 80 characters
#158: FILE: drivers/video/matrox/matroxfb_crtc2.c:124:
+ mga_outl(0x3C2C, pos); /* field #1 vmemory start */
WARNING: line over 80 characters
#275: FILE: drivers/video/matrox/matroxfb_crtc2.c:315:
+static int matroxfb_dh_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
WARNING: line over 80 characters
#307: FILE: drivers/video/matrox/matroxfb_crtc2.c:358:
+ pos = (m2info->fbcon.var.yoffset * m2info->fbcon.var.xres_virtual +
WARNING: line over 80 characters
#308: FILE: drivers/video/matrox/matroxfb_crtc2.c:359:
+ m2info->fbcon.var.xoffset) * m2info->fbcon.var.bits_per_pixel >> 3;
WARNING: line over 80 characters
#519: FILE: drivers/video/matrox/matroxfb_crtc2.c:490:
+ if ((minfo->outputs[2].src == MATROXFB_SRC_CRTC1) && tmp)
WARNING: line over 80 characters
#525: FILE: drivers/video/matrox/matroxfb_crtc2.c:496:
+ if (minfo->outputs[out].src != MATROXFB_SRC_CRTC2) {
WARNING: line over 80 characters
#527: FILE: drivers/video/matrox/matroxfb_crtc2.c:498:
+ minfo->outputs[out].src = MATROXFB_SRC_CRTC2;
WARNING: line over 80 characters
#532: FILE: drivers/video/matrox/matroxfb_crtc2.c:500:
+ } else if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2) {
total: 0 errors, 9 warnings, 685 lines checked
Some of them could be easily avoided.
>
> Signed-off-by: Peter Huewe <peterhuewe@....de>
> ---
> KernelVersion: 2.6.33-rc4
> Patch against Linus' Tree.
>
> drivers/video/matrox/matroxfb_crtc2.c | 432 +++++++++++++++++----------------
> 1 files changed, 220 insertions(+), 212 deletions(-)
>
> diff --git a/drivers/video/matrox/matroxfb_crtc2.c b/drivers/video/matrox/matroxfb_crtc2.c
> index 78414ba..946f5b7 100644
> --- a/drivers/video/matrox/matroxfb_crtc2.c
> +++ b/drivers/video/matrox/matroxfb_crtc2.c
> @@ -27,7 +27,8 @@ MODULE_PARM_DESC(mem, "Memory size reserved for dualhead (default=8MB)");
> /* **************************************************** */
>
> static int matroxfb_dh_setcolreg(unsigned regno, unsigned red, unsigned green,
> - unsigned blue, unsigned transp, struct fb_info* info) {
> + unsigned blue, unsigned transp, struct fb_info *info)
> +{
> u_int32_t col;
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
>
> @@ -48,36 +49,35 @@ static int matroxfb_dh_setcolreg(unsigned regno, unsigned red, unsigned green,
> (transp << m2info->fbcon.var.transp.offset);
>
> switch (m2info->fbcon.var.bits_per_pixel) {
> - case 16:
> - m2info->cmap[regno] = col | (col << 16);
> - break;
> - case 32:
> - m2info->cmap[regno] = col;
> - break;
> + case 16:
> + m2info->cmap[regno] = col | (col << 16);
> + break;
> + case 32:
> + m2info->cmap[regno] = col;
> + break;
> }
> return 0;
> #undef m2info
> }
>
> -static void matroxfb_dh_restore(struct matroxfb_dh_fb_info* m2info,
> - struct my_timming* mt,
> - int mode,
> - unsigned int pos) {
> +static void matroxfb_dh_restore(struct matroxfb_dh_fb_info *m2info,
> + struct my_timming *mt, int mode, unsigned int pos)
> +{
> u_int32_t tmp;
> u_int32_t datactl;
> struct matrox_fb_info *minfo = m2info->primary_dev;
>
> switch (mode) {
> - case 15:
> - tmp = 0x00200000;
> - break;
> - case 16:
> - tmp = 0x00400000;
> - break;
> -/* case 32: */
> - default:
> - tmp = 0x00800000;
> - break;
> + case 15:
> + tmp = 0x00200000;
> + break;
> + case 16:
> + tmp = 0x00400000;
> + break;
> + /* case 32: */
> + default:
> + tmp = 0x00800000;
> + break;
> }
> tmp |= 0x00000001; /* enable CRTC2 */
> datactl = 0;
> @@ -85,9 +85,9 @@ static void matroxfb_dh_restore(struct matroxfb_dh_fb_info* m2info,
> if (minfo->devflags.g450dac) {
> tmp |= 0x00000006; /* source from secondary pixel PLL */
> /* no vidrst when in monitor mode */
> - if (minfo->outputs[1].mode != MATROXFB_OUTPUT_MODE_MONITOR) {
> + if (minfo->outputs[1].mode != MATROXFB_OUTPUT_MODE_MONITOR)
> tmp |= 0xC0001000; /* Enable H/V vidrst */
> - }
> +
I wouldn't add a blank line there.
> } else {
> tmp |= 0x00000002; /* source from VDOCLK */
> tmp |= 0xC0000000; /* enable vvidrst & hvidrst */
> @@ -97,9 +97,9 @@ static void matroxfb_dh_restore(struct matroxfb_dh_fb_info* m2info,
> tmp |= 0x00000004; /* source from pixclock */
> /* PIXPLL is our clock source */
> }
> - if (minfo->outputs[0].src == MATROXFB_SRC_CRTC2) {
> + if (minfo->outputs[0].src == MATROXFB_SRC_CRTC2)
> tmp |= 0x00100000; /* connect CRTC2 to DAC */
> - }
> +
Nor here.
> if (mt->interlaced) {
> tmp |= 0x02000000; /* interlaced, second field is bigger, as G450 apparently ignores it */
> mt->VDisplay >>= 1;
> @@ -121,7 +121,7 @@ static void matroxfb_dh_restore(struct matroxfb_dh_fb_info* m2info,
> u_int32_t linelen = m2info->fbcon.var.xres_virtual * (m2info->fbcon.var.bits_per_pixel >> 3);
> if (tmp & 0x02000000) {
> /* field #0 is smaller, so... */
> - mga_outl(0x3C2C, pos); /* field #1 vmemory start */
> + mga_outl(0x3C2C, pos); /* field #1 vmemory start */
Not sure what had to be fixed here, but the cure seems worse.
> mga_outl(0x3C28, pos + linelen); /* field #0 vmemory start */
> linelen <<= 1;
> m2info->interlaced = 1;
> @@ -140,9 +140,8 @@ static void matroxfb_dh_restore(struct matroxfb_dh_fb_info* m2info,
> unsigned int nl;
> unsigned int lastl = 0;
>
> - while ((nl = mga_inl(0x3C48) & 0xFFF) >= lastl) {
> + while ((nl = mga_inl(0x3C48) & 0xFFF) >= lastl)
> lastl = nl;
> - }
> }
> }
> mga_outl(0x3C10, tmp);
> @@ -156,15 +155,17 @@ static void matroxfb_dh_restore(struct matroxfb_dh_fb_info* m2info,
> mga_outl(0x3C44, tmp);
> }
>
> -static void matroxfb_dh_disable(struct matroxfb_dh_fb_info* m2info) {
> +static void matroxfb_dh_disable(struct matroxfb_dh_fb_info *m2info)
> +{
> struct matrox_fb_info *minfo = m2info->primary_dev;
>
> mga_outl(0x3C10, 0x00000004); /* disable CRTC2, CRTC1->DAC1, PLL as clock source */
> minfo->hw.crtc2.ctl = 0x00000004;
> }
>
> -static void matroxfb_dh_pan_var(struct matroxfb_dh_fb_info* m2info,
> - struct fb_var_screeninfo* var) {
> +static void matroxfb_dh_pan_var(struct matroxfb_dh_fb_info *m2info,
> + struct fb_var_screeninfo *var)
> +{
> unsigned int pos;
> unsigned int linelen;
> unsigned int pixelsize;
> @@ -184,21 +185,23 @@ static void matroxfb_dh_pan_var(struct matroxfb_dh_fb_info* m2info,
> }
> }
>
> -static int matroxfb_dh_decode_var(struct matroxfb_dh_fb_info* m2info,
> - struct fb_var_screeninfo* var,
> - int *visual,
> - int *video_cmap_len,
> - int *mode) {
> +static int matroxfb_dh_decode_var(struct matroxfb_dh_fb_info *m2info,
> + struct fb_var_screeninfo *var, int *visual, int *video_cmap_len,
> + int *mode)
> +{
> unsigned int mask;
> unsigned int memlen;
> unsigned int vramlen;
>
> switch (var->bits_per_pixel) {
> - case 16: mask = 0x1F;
> - break;
> - case 32: mask = 0x0F;
> - break;
> - default: return -EINVAL;
> + case 16:
> + mask = 0x1F;
> + break;
> + case 32:
> + mask = 0x0F;
> + break;
> + default:
> + return -EINVAL;
> }
> vramlen = m2info->video.len_usable;
> if (var->yres_virtual < var->yres)
> @@ -258,33 +261,33 @@ static int matroxfb_dh_decode_var(struct matroxfb_dh_fb_info* m2info,
> return 0;
> }
>
> -static int matroxfb_dh_open(struct fb_info* info, int user) {
> +static int matroxfb_dh_open(struct fb_info *info, int user)
> +{
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
> struct matrox_fb_info *minfo = m2info->primary_dev;
>
> if (minfo) {
> int err;
>
> - if (minfo->dead) {
> + if (minfo->dead)
> return -ENXIO;
> - }
> +
> err = minfo->fbops.fb_open(&minfo->fbcon, user);
> - if (err) {
> + if (err)
> return err;
> - }
> }
> return 0;
> #undef m2info
> }
>
> -static int matroxfb_dh_release(struct fb_info* info, int user) {
> +static int matroxfb_dh_release(struct fb_info *info, int user)
> +{
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
> int err = 0;
> struct matrox_fb_info *minfo = m2info->primary_dev;
>
> - if (minfo) {
> + if (minfo)
> err = minfo->fbops.fb_release(&minfo->fbcon, user);
> - }
> return err;
> #undef m2info
> }
> @@ -309,7 +312,8 @@ static void matroxfb_dh_init_fix(struct matroxfb_dh_fb_info *m2info)
> fix->accel = 0; /* no accel... */
> }
>
> -static int matroxfb_dh_check_var(struct fb_var_screeninfo* var, struct fb_info* info) {
> +static int matroxfb_dh_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> +{
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
> int visual;
> int cmap_len;
> @@ -319,16 +323,18 @@ static int matroxfb_dh_check_var(struct fb_var_screeninfo* var, struct fb_info*
> #undef m2info
> }
>
> -static int matroxfb_dh_set_par(struct fb_info* info) {
> +static int matroxfb_dh_set_par(struct fb_info *info)
> +{
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
> int visual;
> int cmap_len;
> int mode;
> int err;
> - struct fb_var_screeninfo* var = &info->var;
> + struct fb_var_screeninfo *var = &info->var;
> struct matrox_fb_info *minfo = m2info->primary_dev;
>
> - if ((err = matroxfb_dh_decode_var(m2info, var, &visual, &cmap_len, &mode)) != 0)
> + err = matroxfb_dh_decode_var(m2info, var, &visual, &cmap_len, &mode);
> + if (err != 0)
> return err;
> /* cmap */
> {
> @@ -349,38 +355,37 @@ static int matroxfb_dh_set_par(struct fb_info* info) {
> /* CRTC2 delay */
> mt.delay = 34;
>
> - pos = (m2info->fbcon.var.yoffset * m2info->fbcon.var.xres_virtual + m2info->fbcon.var.xoffset) * m2info->fbcon.var.bits_per_pixel >> 3;
> + pos = (m2info->fbcon.var.yoffset * m2info->fbcon.var.xres_virtual +
> + m2info->fbcon.var.xoffset) * m2info->fbcon.var.bits_per_pixel >> 3;
This is overly indented.
> pos += m2info->video.offbase;
> cnt = 0;
> down_read(&minfo->altout.lock);
> for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2) {
> cnt++;
> - if (minfo->outputs[out].output->compute) {
> + if (minfo->outputs[out].output->compute)
> minfo->outputs[out].output->compute(minfo->outputs[out].data, &mt);
> - }
> }
> }
> minfo->crtc2.pixclock = mt.pixclock;
> minfo->crtc2.mnp = mt.mnp;
> up_read(&minfo->altout.lock);
> - if (cnt) {
> + if (cnt)
> matroxfb_dh_restore(m2info, &mt, mode, pos);
> - } else {
> + else
> matroxfb_dh_disable(m2info);
> - }
> DAC1064_global_init(minfo);
> DAC1064_global_restore(minfo);
> down_read(&minfo->altout.lock);
> for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2 &&
> - minfo->outputs[out].output->program) {
> + minfo->outputs[out].output->program) {
This is wrong, please revert.
> minfo->outputs[out].output->program(minfo->outputs[out].data);
> }
> }
> for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2 &&
> - minfo->outputs[out].output->start) {
> + minfo->outputs[out].output->start) {
Same here.
> minfo->outputs[out].output->start(minfo->outputs[out].data);
> }
> }
> @@ -391,14 +396,18 @@ static int matroxfb_dh_set_par(struct fb_info* info) {
> #undef m2info
> }
>
> -static int matroxfb_dh_pan_display(struct fb_var_screeninfo* var, struct fb_info* info) {
> +static int matroxfb_dh_pan_display(struct fb_var_screeninfo *var,
> + struct fb_info *info)
> +{
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
> matroxfb_dh_pan_var(m2info, var);
> return 0;
> #undef m2info
> }
>
> -static int matroxfb_dh_get_vblank(const struct matroxfb_dh_fb_info* m2info, struct fb_vblank* vblank) {
> +static int matroxfb_dh_get_vblank(const struct matroxfb_dh_fb_info *m2info,
> + struct fb_vblank *vblank)
> +{
> struct matrox_fb_info *minfo = m2info->primary_dev;
>
> matroxfb_enable_irq(minfo, 0);
> @@ -410,11 +419,11 @@ static int matroxfb_dh_get_vblank(const struct matroxfb_dh_fb_info* m2info, stru
> if (vblank->vcount >= m2info->fbcon.var.yres)
> vblank->flags |= FB_VBLANK_VBLANKING;
> if (test_bit(0, &minfo->irq_flags)) {
> - vblank->flags |= FB_VBLANK_HAVE_COUNT;
> - /* Only one writer, aligned int value...
> - it should work without lock and without atomic_t */
> + vblank->flags |= FB_VBLANK_HAVE_COUNT;
> + /* Only one writer, aligned int value...
> + it should work without lock and without atomic_t */
You shouldn't have removed the 3 spaces before the second line of
comment.
> vblank->count = minfo->crtc2.vsync.cnt;
> - }
> + }
> return 0;
> }
>
> @@ -428,133 +437,126 @@ static int matroxfb_dh_ioctl(struct fb_info *info,
> DBG(__func__)
>
> switch (cmd) {
> - case FBIOGET_VBLANK:
> - {
> - struct fb_vblank vblank;
> - int err;
> -
> - err = matroxfb_dh_get_vblank(m2info, &vblank);
> - if (err)
> - return err;
> - if (copy_to_user((void __user *)arg, &vblank, sizeof(vblank)))
> - return -EFAULT;
> - return 0;
> - }
> - case FBIO_WAITFORVSYNC:
> - {
> - u_int32_t crt;
> + case FBIOGET_VBLANK: {
> + struct fb_vblank vblank;
> + int err;
>
> - if (get_user(crt, (u_int32_t __user *)arg))
> - return -EFAULT;
> + err = matroxfb_dh_get_vblank(m2info, &vblank);
> + if (err)
> + return err;
> + if (copy_to_user((void __user *)arg, &vblank, sizeof(vblank)))
> + return -EFAULT;
> + return 0;
> + }
> + case FBIO_WAITFORVSYNC: {
> + u_int32_t crt;
>
> - if (crt != 0)
> - return -ENODEV;
> - return matroxfb_wait_for_sync(minfo, 1);
> - }
> - case MATROXFB_SET_OUTPUT_MODE:
> - case MATROXFB_GET_OUTPUT_MODE:
> - case MATROXFB_GET_ALL_OUTPUTS:
> - {
> - return minfo->fbcon.fbops->fb_ioctl(&minfo->fbcon, cmd, arg);
> - }
> - case MATROXFB_SET_OUTPUT_CONNECTION:
> - {
> - u_int32_t tmp;
> - int out;
> - int changes;
> -
> - if (get_user(tmp, (u_int32_t __user *)arg))
> - return -EFAULT;
> - for (out = 0; out < 32; out++) {
> - if (tmp & (1 << out)) {
> - if (out >= MATROXFB_MAX_OUTPUTS)
> - return -ENXIO;
> - if (!minfo->outputs[out].output)
> - return -ENXIO;
> - switch (minfo->outputs[out].src) {
> - case MATROXFB_SRC_NONE:
> - case MATROXFB_SRC_CRTC2:
> - break;
> - default:
> - return -EBUSY;
> - }
> - }
> - }
> - if (minfo->devflags.panellink) {
> - if (tmp & MATROXFB_OUTPUT_CONN_DFP)
> - return -EINVAL;
> - if ((minfo->outputs[2].src == MATROXFB_SRC_CRTC1) && tmp)
> - return -EBUSY;
> - }
> - changes = 0;
> - for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> - if (tmp & (1 << out)) {
> - if (minfo->outputs[out].src != MATROXFB_SRC_CRTC2) {
> - changes = 1;
> - minfo->outputs[out].src = MATROXFB_SRC_CRTC2;
> - }
> - } else if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2) {
> - changes = 1;
> - minfo->outputs[out].src = MATROXFB_SRC_NONE;
> - }
> + if (get_user(crt, (u_int32_t __user *)arg))
> + return -EFAULT;
> +
> + if (crt != 0)
> + return -ENODEV;
> + return matroxfb_wait_for_sync(minfo, 1);
> + }
> + case MATROXFB_SET_OUTPUT_MODE:
> + case MATROXFB_GET_OUTPUT_MODE:
> + case MATROXFB_GET_ALL_OUTPUTS:
> + return minfo->fbcon.fbops->fb_ioctl(&minfo->fbcon, cmd, arg);
> + case MATROXFB_SET_OUTPUT_CONNECTION: {
> + u_int32_t tmp;
> + int out;
> + int changes;
> +
> + if (get_user(tmp, (u_int32_t __user *)arg))
> + return -EFAULT;
> + for (out = 0; out < 32; out++) {
> + if (tmp & (1 << out)) {
> + if (out >= MATROXFB_MAX_OUTPUTS)
> + return -ENXIO;
> + if (!minfo->outputs[out].output)
> + return -ENXIO;
> + switch (minfo->outputs[out].src) {
> + case MATROXFB_SRC_NONE:
> + case MATROXFB_SRC_CRTC2:
> + break;
> + default:
> + return -EBUSY;
> }
> - if (!changes)
> - return 0;
> - matroxfb_dh_set_par(info);
> - return 0;
> }
> - case MATROXFB_GET_OUTPUT_CONNECTION:
> - {
> - u_int32_t conn = 0;
> - int out;
> -
> - for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> - if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2) {
> - conn |= 1 << out;
> - }
> + }
> + if (minfo->devflags.panellink) {
> + if (tmp & MATROXFB_OUTPUT_CONN_DFP)
> + return -EINVAL;
> + if ((minfo->outputs[2].src == MATROXFB_SRC_CRTC1) && tmp)
> + return -EBUSY;
> + }
> + changes = 0;
> + for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> + if (tmp & (1 << out)) {
> + if (minfo->outputs[out].src != MATROXFB_SRC_CRTC2) {
> + changes = 1;
> + minfo->outputs[out].src = MATROXFB_SRC_CRTC2;
> }
> - if (put_user(conn, (u_int32_t __user *)arg))
> - return -EFAULT;
> - return 0;
> + } else if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2) {
> + changes = 1;
> + minfo->outputs[out].src = MATROXFB_SRC_NONE;
> }
> - case MATROXFB_GET_AVAILABLE_OUTPUTS:
> - {
> - u_int32_t tmp = 0;
> - int out;
> -
> - for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> - if (minfo->outputs[out].output) {
> - switch (minfo->outputs[out].src) {
> - case MATROXFB_SRC_NONE:
> - case MATROXFB_SRC_CRTC2:
> - tmp |= 1 << out;
> - break;
> - }
> - }
> - }
> - if (minfo->devflags.panellink) {
> - tmp &= ~MATROXFB_OUTPUT_CONN_DFP;
> - if (minfo->outputs[2].src == MATROXFB_SRC_CRTC1) {
> - tmp = 0;
> - }
> + }
> + if (!changes)
> + return 0;
> + matroxfb_dh_set_par(info);
> + return 0;
> + }
> + case MATROXFB_GET_OUTPUT_CONNECTION: {
> + u_int32_t conn = 0;
> + int out;
> +
> + for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> + if (minfo->outputs[out].src == MATROXFB_SRC_CRTC2)
> + conn |= 1 << out;
> + }
> + if (put_user(conn, (u_int32_t __user *)arg))
> + return -EFAULT;
> + return 0;
> + }
> + case MATROXFB_GET_AVAILABLE_OUTPUTS: {
> + u_int32_t tmp = 0;
> + int out;
> +
> + for (out = 0; out < MATROXFB_MAX_OUTPUTS; out++) {
> + if (minfo->outputs[out].output) {
> + switch (minfo->outputs[out].src) {
> + case MATROXFB_SRC_NONE:
> + case MATROXFB_SRC_CRTC2:
> + tmp |= 1 << out;
> + break;
> }
> - if (put_user(tmp, (u_int32_t __user *)arg))
> - return -EFAULT;
> - return 0;
> }
> + }
> + if (minfo->devflags.panellink) {
> + tmp &= ~MATROXFB_OUTPUT_CONN_DFP;
> + if (minfo->outputs[2].src == MATROXFB_SRC_CRTC1)
> + tmp = 0;
> + }
> + if (put_user(tmp, (u_int32_t __user *)arg))
> + return -EFAULT;
> + return 0;
> + }
> }
> return -ENOTTY;
> #undef m2info
> }
>
> -static int matroxfb_dh_blank(int blank, struct fb_info* info) {
> +static int matroxfb_dh_blank(int blank, struct fb_info *info)
> +{
> #define m2info (container_of(info, struct matroxfb_dh_fb_info, fbcon))
> switch (blank) {
> - case 1:
> - case 2:
> - case 3:
> - case 4:
> - default:;
> + case 1:
> + case 2:
> + case 3:
> + case 4:
> + default:
> + break;
> }
This piece of code is terribly useful ;)
> /* do something... */
> return 0;
> @@ -568,7 +570,7 @@ static struct fb_ops matroxfb_dh_ops = {
> .fb_check_var = matroxfb_dh_check_var,
> .fb_set_par = matroxfb_dh_set_par,
> .fb_setcolreg = matroxfb_dh_setcolreg,
> - .fb_pan_display =matroxfb_dh_pan_display,
> + .fb_pan_display = matroxfb_dh_pan_display,
> .fb_blank = matroxfb_dh_blank,
> .fb_ioctl = matroxfb_dh_ioctl,
> .fb_fillrect = cfb_fillrect,
> @@ -577,34 +579,34 @@ static struct fb_ops matroxfb_dh_ops = {
> };
>
> static struct fb_var_screeninfo matroxfb_dh_defined = {
> - 640,480,640,480,/* W,H, virtual W,H */
> - 0,0, /* offset */
> - 32, /* depth */
> - 0, /* gray */
> - {0,0,0}, /* R */
> - {0,0,0}, /* G */
> - {0,0,0}, /* B */
> - {0,0,0}, /* alpha */
> - 0, /* nonstd */
> - FB_ACTIVATE_NOW,
> - -1,-1, /* display size */
> - 0, /* accel flags */
> - 39721L,48L,16L,33L,10L,
> - 96L,2,0, /* no sync info */
> - FB_VMODE_NONINTERLACED,
> - 0, {0,0,0,0,0}
> + 640, 480, 640, 480,/* W,H, virtual W,H */
One space between "," and "/*" would be welcome. Or even better, one
tab, and align all the other comments below on that one.
> + 0, 0, /* offset */
> + 32, /* depth */
> + 0, /* gray */
> + {0, 0, 0}, /* R */
> + {0, 0, 0}, /* G */
> + {0, 0, 0}, /* B */
> + {0, 0, 0}, /* alpha */
> + 0, /* nonstd */
> + FB_ACTIVATE_NOW,
> + -1, -1, /* display size */
> + 0, /* accel flags */
> + 39721L, 48L, 16L, 33L, 10L,
> + 96L, 2, 0, /* no sync info */
> + FB_VMODE_NONINTERLACED,
> + 0, {0, 0, 0, 0, 0}
> };
>
> static int matroxfb_dh_regit(const struct matrox_fb_info *minfo,
> - struct matroxfb_dh_fb_info *m2info)
> + struct matroxfb_dh_fb_info *m2info)
The original indentation wasn't that bad.
> {
> #define minfo (m2info->primary_dev)
> - void* oldcrtc2;
> + void *oldcrtc2;
>
> m2info->fbcon.fbops = &matroxfb_dh_ops;
> m2info->fbcon.flags = FBINFO_FLAG_DEFAULT;
> m2info->fbcon.flags |= FBINFO_HWACCEL_XPAN |
> - FBINFO_HWACCEL_YPAN;
> + FBINFO_HWACCEL_YPAN;
No, please revert.
> m2info->fbcon.pseudo_palette = m2info->cmap;
> fb_alloc_cmap(&m2info->fbcon.cmap, 256, 1);
>
> @@ -630,9 +632,9 @@ static int matroxfb_dh_regit(const struct matrox_fb_info *minfo,
> m2info->mmio.len = minfo->mmio.len;
>
> matroxfb_dh_init_fix(m2info);
> - if (register_framebuffer(&m2info->fbcon)) {
> + if (register_framebuffer(&m2info->fbcon))
> return -ENXIO;
> - }
> +
> if (!m2info->initialized)
> fb_set_var(&m2info->fbcon, &matroxfb_dh_defined);
> down_write(&minfo->crtc2.lock);
> @@ -649,7 +651,8 @@ static int matroxfb_dh_regit(const struct matrox_fb_info *minfo,
>
> /* ************************** */
>
> -static int matroxfb_dh_registerfb(struct matroxfb_dh_fb_info* m2info) {
> +static int matroxfb_dh_registerfb(struct matroxfb_dh_fb_info *m2info)
> +{
> #define minfo (m2info->primary_dev)
> if (matroxfb_dh_regit(minfo, m2info)) {
> printk(KERN_ERR "matroxfb_crtc2: secondary head failed to register\n");
> @@ -662,11 +665,12 @@ static int matroxfb_dh_registerfb(struct matroxfb_dh_fb_info* m2info) {
> #undef minfo
> }
>
> -static void matroxfb_dh_deregisterfb(struct matroxfb_dh_fb_info* m2info) {
> +static void matroxfb_dh_deregisterfb(struct matroxfb_dh_fb_info *m2info)
> +{
> #define minfo (m2info->primary_dev)
> if (m2info->fbcon_registered) {
> int id;
> - struct matroxfb_dh_fb_info* crtc2;
> + struct matroxfb_dh_fb_info *crtc2;
>
> down_write(&minfo->crtc2.lock);
> crtc2 = minfo->crtc2.info;
> @@ -689,8 +693,9 @@ static void matroxfb_dh_deregisterfb(struct matroxfb_dh_fb_info* m2info) {
> #undef minfo
> }
>
> -static void* matroxfb_crtc2_probe(struct matrox_fb_info* minfo) {
> - struct matroxfb_dh_fb_info* m2info;
> +static void *matroxfb_crtc2_probe(struct matrox_fb_info *minfo)
> +{
> + struct matroxfb_dh_fb_info *m2info;
>
> /* hardware is CRTC2 incapable... */
> if (!minfo->devflags.crtc2)
> @@ -709,17 +714,19 @@ static void* matroxfb_crtc2_probe(struct matrox_fb_info* minfo) {
> return m2info;
> }
>
> -static void matroxfb_crtc2_remove(struct matrox_fb_info* minfo, void* crtc2) {
> +static void matroxfb_crtc2_remove(struct matrox_fb_info *minfo, void *crtc2)
> +{
> matroxfb_dh_deregisterfb(crtc2);
> kfree(crtc2);
> }
>
> static struct matroxfb_driver crtc2 = {
> - .name = "Matrox G400 CRTC2",
> - .probe = matroxfb_crtc2_probe,
> - .remove = matroxfb_crtc2_remove };
> + .name = "Matrox G400 CRTC2",
> + .probe = matroxfb_crtc2_probe,
> + .remove = matroxfb_crtc2_remove };
I tend to prefer ".name<tabs>=<space>foo,", but up to you.
The trailing "};" should go on its own line.
>
> -static int matroxfb_crtc2_init(void) {
> +static int matroxfb_crtc2_init(void)
> +{
> if (fb_get_options("matrox_crtc2fb", NULL))
> return -ENODEV;
>
> @@ -727,7 +734,8 @@ static int matroxfb_crtc2_init(void) {
> return 0;
> }
>
> -static void matroxfb_crtc2_exit(void) {
> +static void matroxfb_crtc2_exit(void)
> +{
> matroxfb_unregister_driver(&crtc2);
> }
>
The rest looks reasonable.
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists