Re: Re: New radeonfb, mostly untested

[Kronos <kronos@kronoz.cjb.net>]

Il Tue, Sep 09, 2003 at 09:37:17PM +0200, Benjamin Herrenschmidt ha scritto: 
> > 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:
> 
> >+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;
}

> > +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