Re: Re: New radeonfb, mostly untested

Il Tue, Sep 09, 2003 at 09:37:17PM +0200, Benjamin Herrenschmidt ha scritto: 

quoted

Most of work consists in changing "." into "->" ;)
Changes to fbmem.c are very similar to Greg KH's patch:

You probably want to fix more drivers , that would help getting
it merged.

I sent radeonfb as an example. I've already fixed chipsfb.c, cyber2000fb.c,
hgafb.c, imsttfb.c, neofb.c, tdfxfb.c, tridentfb.c, vesafb.c and i810.

Some comments though:

quoted

+static void release_fb_info(struct class_device *class_dev) {
+       struct fb_info *info = to_fb_info(class_dev);
+
+       if (info->release)
+               info->release(info);
+       
+       kfree(info);
+}

Since you provide the implicit kfree(), 

Implicit? What do you mean?

you should also provide an
framebuffer_alloc() some for consistency imho. That would take the
size of the private structure like for netdevs and allocate both.

Like this?

struct fb_info *framebuffer_alloc(size_t size, struct device *dev) {
        struct fb_info *ret;

        ret = kmalloc(sizeof(struct fb_info) + size, GFP_KERNEL);
        if (!ret)
                return NULL;
        memset(ret, 0, sizeof(struct fb_info) + size);

        ret->class_dev.dev = dev;

        if (size)
                ret->par = ret + 1;

        return ret;
}

quoted

+static struct class fb_class = {
+	.name		= "video",

I'd rather use "display" here. "video" is too broad and will cause
confusion with multimedia stuff.

Ok.

Luca
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
K.R.O.N.O.S
Kinetic Replicant Optimized for Nocturnal Observation and Sabotage


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf