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
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
