linux-mips
[Top] [All Lists]

Re: [PATCH] Impact video driver for SGI Indigo2

To: Sergei Shtylyov <sshtylyov@mvista.com>
Subject: Re: [PATCH] Impact video driver for SGI Indigo2
From: peter fuerst <post@pfrst.de>
Date: Tue, 13 Sep 2011 11:39:20 +0200 (CEST)
Cc: linux-mips@linux-mips.org
Reply-to: post@pfrst.de
Sender: linux-mips-bounce@linux-mips.org


Hi Sergei,

On Mon, 12 Sep 2011, Sergei Shtylyov wrote:

Date: Mon, 12 Sep 2011 13:56:36 +0400
From: Sergei Shtylyov <sshtylyov@mvista.com>
To: post@pfrst.de
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org,
    attilio.fiandrotti@gmail.com
Subject: Re: [PATCH] Impact video driver for SGI Indigo2

...
framebuffer device. Without the support of PCI & AGP.

  It looks like the patch is spoiled as I'm seeing two spaces at the start
of line when looking at the message source.

hmmm, that's a strange problem. The two spaces are not in the diff-file
read into the eMail and are not displayed by the MUA (pine 4.64). But
indeed, where's a leading space in the diff, there's an additional space
inserted into the eMail-body. Have to find out the best way to suppress
this behaviour...



...

  There are alos empty lines after each file in the patch -- which
shouldn't be there.

These were intended for readability (reviewability :), but i can remove
them easily (of course).


diff --git a/drivers/video/Makefile b/drivers/video/Makefile
...

  The above should be a part of the driver patch, as you can't add Makefile
targets fow which no source files exist yet.


Do you suggest to submit the ip22-setup.c-, impact.h-, impact.c-parts
alone in a first patch and then, in a separate follow-up patch, the
Kconfig- and Makefile-parts, or just to reorder the parts in this single
patch?

diff --git a/arch/mips/sgi-ip22/ip22-setup.c
...
+ if (sgi_gfxaddr < 0x1f000000 || 0x1fa00000 <= sgi_gfxaddr)

  Immediate should generally be the right operand in comparison.

This can be changed, should the "if (a is outside the range...)"
notation be confusing or the like.

...

  I lack the time to review such a large driver, unfortunately...

WBR, Sergei


with kind regards

peter


<Prev in Thread] Current Thread [Next in Thread>