Bruno Prémont wrote:

Exactly, thats why I would prefer to see all that initialization moved
out of registrer_framebuffer() into a init_framebuffer().

I'm okay with that, but I'm not crazy about it.

For simplicity any driver that uses framebuffer_alloc() should not need
to additionally call init_framebuffer() - framebuffer_alloc() should
call it just before returning.

My patch does that.

All those drivers that don't call framebuffer_alloc() would then have to
call init_framebuffer().

Well, that would then mean that it's easier to move initialization *into* register_framebuffer().

I think register_framebuffer() is a bit too late to do the initialisation
of mutexes and other class state because driver cannot use the same code
for HW setup before registration and after registration as at least the lock
is in undefined state.

How many drivers actually do that?  The only thing that framebuffer_alloc() initializes in fb_info is the 'device' field, which is just a parameter passed into framebuffer_alloc(), so the driver already knows what that value is.  I can't find any examples of a driver doing what you're talking about.

It should be easy to modify any driver to stop using the fields of fb_info before register_framebuffer() is called.  In fact, I would say it's bad programming to use the fb_info object before the framebuffer is registered.

In pseudo-code my though would be:


// driver using their own memory allocation

struct driver_data {
   ...
   struct fb_info fb;
   ...
}

int driver_init() {
   struct driver_data *data;
   ...
   memset(&data->fb, 0, sizeof(data->fb));
   init_framebuffer(&data->fb);
   // fill in driver's settings for fb
   ...
   register_framebuffer(&data->fb);
   ...
}

// driver using alloc_framebuffer()

struct driver_data {
   ...
   struct fb_info *fb;
   ...
}

int driver_init() {
   struct driver_data *data;
   ...
   data->fb = alloc_framebuffer(0); // this implicitly calls init_framebuffer();
   // fill in driver's settings for fb
   ...
   register_framebuffer(&data->fb);
   ...
}

This is pretty much what my patch does.  But the more I think about it, the more I'm in favor of moving all initialization into register_framebuffer().

-- 
Timur Tabi
Linux kernel developer at Freescale