Re: [PATCH] fbdev updates.

[Russell King <rmk@arm.linux.org.uk>]

On Mon, Jul 01, 2002 at 10:48:41AM -0700, James Simmons wrote:
> Here are the latest updates to the framebuffer layer. Please test it outso
> I can push it as soon as possible to Linus.

The following comments are from a first review of sa1100fb.[ch] changes.

1. It would appear to break sa1100 inverse colourmap stuff.  There are LCD
   panels out there where it is a rather fundamental requirement to write
   the palette with "inverted" colourmap values.

2. I've no idea why you moved "lccr0" and "lccr3" in sa1100fb.h - this
   looks like noise to me.

3. I think you replaced "fbi" too many times:
	+/* Fake monspecs to fill in infonfo structure */

4. You're also merging in cpufreq changes from my tree in this patch;
   however I was going to send these to Linus along with the cpufreq
   submission so no problem.

5. I strongly disagree with your apparant decision to make the cpufreq
   part of the generic framebuffer core (by apparantly adding the notifier
   block to the core fb_info structure).  Firstly, you've broken sa1100fb.c
   by not including the relevant definition in fb_info (ok, so cpufreq
   stuff isn't in Linus' tree yet).  Secondly, it isn't something that all
   framebuffers require; its only required on SoC devices where the hardware
   designers have been stingy.  As such, we should NOT penalise the x86
   people by adding random useless garbage to structures that they're never
   going to use.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
No, I will not fix your computer.
http://thinkgeek.com/sf