[PATCH] fb: split out framebuffer initialization from allocation

[PATCH] fb: split out framebuffer initialization from allocation

[Timur Tabi]

Introduce functions framebuffer_init() and framebuffer_cleanup() to allow
the initialization of a user-allocated fb_info object.

framebuffer_alloc() allows for appending a private data structure when it
allocates the fb_info object.  However, a driver that registers multiple
framebuffers for one device may also need another private data structure
for the device itself.  framebuffer_init() allows such drivers to store
the fb_info objects in the device-specific private data structure,
thereby simplifying memory allocation.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fbsysfs.c |   45 +++++++++++++++++++++++++++++++++++++++------
 include/linux/fb.h      |    2 ++
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index 67afa9c..77d1c83 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -24,6 +24,29 @@
 #define FB_SYSFS_FLAG_ATTR 1
 
 /**
+ * framebuffer_init - initialize a frame buffer info object
+ *
+ * @info: pointer to the fb_info object
+ * @dev: pointer to the device for this fb, this can be NULL
+ *
+ * Initializes a frame buffer info object.  The object should be zeroed-out
+ * before calling this function, because only some fields are initialized.
+ *
+ * Use this function for fb_info objects that are not allocated via
+ * framebuffer_alloc(). The object must be cleaned up with
+ * framebuffer_cleanup() before its memory is deallocated.
+ */
+void framebuffer_init(struct fb_info *info, struct device *dev)
+{
+	info->device = dev;
+
+#ifdef CONFIG_FB_BACKLIGHT
+	mutex_init(&info->bl_curve_mutex);
+#endif
+}
+EXPORT_SYMBOL(framebuffer_init);
+
+/**
  * framebuffer_alloc - creates a new frame buffer info structure
  *
  * @size: size of driver private data, can be zero
@@ -35,6 +58,7 @@
  *
  * Returns the new structure, or NULL if an error occurred.
  *
+ * The object must be deallocated with framebuffer_release().
  */
 struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 {
@@ -57,11 +81,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 	if (size)
 		info->par = p + fb_info_size;
 
-	info->device = dev;
-
-#ifdef CONFIG_FB_BACKLIGHT
-	mutex_init(&info->bl_curve_mutex);
-#endif
+	framebuffer_init(info, dev);
 
 	return info;
 #undef PADDING
@@ -70,6 +90,19 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 EXPORT_SYMBOL(framebuffer_alloc);
 
 /**
+ * framebuffer_cleanup - cleans up a frame buffer info object
+ *
+ * @info: frame buffer info object
+ *
+ * Cleans up an fb_info object that was initialized with framebuffer_init().
+ */
+void framebuffer_cleanup(struct fb_info *info)
+{
+	kfree(info->apertures);
+}
+EXPORT_SYMBOL(framebuffer_cleanup);
+
+/**
  * framebuffer_release - marks the structure available for freeing
  *
  * @info: frame buffer info structure
@@ -80,7 +113,7 @@ EXPORT_SYMBOL(framebuffer_alloc);
  */
 void framebuffer_release(struct fb_info *info)
 {
-	kfree(info->apertures);
+	framebuffer_cleanup(info);
 	kfree(info);
 }
 EXPORT_SYMBOL(framebuffer_release);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 1d6836c..c2347fb 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -1068,6 +1068,8 @@ static inline bool fb_be_math(struct fb_info *info)
 /* drivers/video/fbsysfs.c */
 extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
 extern void framebuffer_release(struct fb_info *info);
+extern void framebuffer_init(struct fb_info *info, struct device *dev);
+extern void framebuffer_cleanup(struct fb_info *info);
 extern int fb_init_device(struct fb_info *fb_info);
 extern void fb_cleanup_device(struct fb_info *head);
 extern void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max);
-- 
1.7.3.4

Re: [PATCH] fb: split out framebuffer initialization from allocation

[Timur Tabi]

Timur Tabi wrote:
> Introduce functions framebuffer_init() and framebuffer_cleanup() to allow
> the initialization of a user-allocated fb_info object.
> 
> framebuffer_alloc() allows for appending a private data structure when it
> allocates the fb_info object.  However, a driver that registers multiple
> framebuffers for one device may also need another private data structure
> for the device itself.  framebuffer_init() allows such drivers to store
> the fb_info objects in the device-specific private data structure,
> thereby simplifying memory allocation.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>

Florian,

Any comments on this patch?  If you're okay with the change, I want to take advantage of it in my framebuffer driver.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] fb: split out framebuffer initialization from allocation

[Florian Tobias Schandinat]

Hi Timur,

On 11/17/2011 08:19 PM, Timur Tabi wrote:
> Timur Tabi wrote:
>> Introduce functions framebuffer_init() and framebuffer_cleanup() to allow
>> the initialization of a user-allocated fb_info object.
>>
>> framebuffer_alloc() allows for appending a private data structure when it
>> allocates the fb_info object.  However, a driver that registers multiple
>> framebuffers for one device may also need another private data structure
>> for the device itself.  framebuffer_init() allows such drivers to store
>> the fb_info objects in the device-specific private data structure,
>> thereby simplifying memory allocation.
>>
>> Signed-off-by: Timur Tabi <timur@freescale.com>
> 
> Florian,
> 
> Any comments on this patch?  If you're okay with the change, I want to take
> advantage of it in my framebuffer driver.

Of course you want, otherwise I'd be wondering why you are sending this patch
at all.

But I don't see any advantages of your approach. Instead of pointers to fb_info
with this patch you could embed fb_info directly in your data structure but that
is barely a difference for a programmer I'd think. You'd still have to call your
new functions on init/exit so the amount of function calls needed is the same
with or without the patch (I could see an advantage if alloc and release were
pure memory allocations). Or is this all about handling the case when fb_alloc
fails?
Historically some drivers don't even call alloc but have their own fb_info and
call only register. I do not want to add yet another way of doing framebuffer
initialization unless you can clearly show its benefits.


Best regards,

Florian Tobias Schandinat

Re: [PATCH] fb: split out framebuffer initialization from

[Bruno Prémont]

Hi Florian, Timur,

On Sat, 19 November 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> On 11/17/2011 08:19 PM, Timur Tabi wrote:
> > Timur Tabi wrote:
> >> Introduce functions framebuffer_init() and framebuffer_cleanup() to allow
> >> the initialization of a user-allocated fb_info object.
> >>
> >> framebuffer_alloc() allows for appending a private data structure when it
> >> allocates the fb_info object.  However, a driver that registers multiple
> >> framebuffers for one device may also need another private data structure
> >> for the device itself.  framebuffer_init() allows such drivers to store
> >> the fb_info objects in the device-specific private data structure,
> >> thereby simplifying memory allocation.
> >>
> >> Signed-off-by: Timur Tabi <timur@freescale.com>
> > 
> > Florian,
> > 
> > Any comments on this patch?  If you're okay with the change, I want to take
> > advantage of it in my framebuffer driver.
> 
> Of course you want, otherwise I'd be wondering why you are sending this patch
> at all.
> 
> But I don't see any advantages of your approach. Instead of pointers to fb_info
> with this patch you could embed fb_info directly in your data structure but that
> is barely a difference for a programmer I'd think. You'd still have to call your
> new functions on init/exit so the amount of function calls needed is the same
> with or without the patch (I could see an advantage if alloc and release were
> pure memory allocations). Or is this all about handling the case when fb_alloc
> fails?
> Historically some drivers don't even call alloc but have their own fb_info and
> call only register. I do not want to add yet another way of doing framebuffer
> initialization unless you can clearly show its benefits.

Wouldn't it even make sense to move some more of the initialization of fb_info
out of fb registration into this new init funtion? (I'm thinking about initializing
mutexes and the like)

This way fb_info could be used before being registered. Registration would then
be reduced to makeing the framebuffer visible to userspace and listed in
registered_fb[].

This way framebuffer_alloc() would be no more that kzalloc(), framebuffer_init()
would setup all "non-zero" fields of fb_info (including setup of all mutexes, one
of which is currently being done by framebuffer_alloc() and the rest by
do_register_framebuffer()!).

Bruno

> Best regards,
> 
> Florian Tobias Schandinat

Re: [PATCH] fb: split out framebuffer initialization from allocation

[Florian Tobias Schandinat]

Hi Bruno,

On 11/19/2011 11:47 AM, Bruno Prémont wrote:
> Hi Florian, Timur,
> 
> On Sat, 19 November 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
>> On 11/17/2011 08:19 PM, Timur Tabi wrote:
>>> Timur Tabi wrote:
>>>> Introduce functions framebuffer_init() and framebuffer_cleanup() to allow
>>>> the initialization of a user-allocated fb_info object.
>>>>
>>>> framebuffer_alloc() allows for appending a private data structure when it
>>>> allocates the fb_info object.  However, a driver that registers multiple
>>>> framebuffers for one device may also need another private data structure
>>>> for the device itself.  framebuffer_init() allows such drivers to store
>>>> the fb_info objects in the device-specific private data structure,
>>>> thereby simplifying memory allocation.
>>>>
>>>> Signed-off-by: Timur Tabi <timur@freescale.com>
>>>
>>> Florian,
>>>
>>> Any comments on this patch?  If you're okay with the change, I want to take
>>> advantage of it in my framebuffer driver.
>>
>> Of course you want, otherwise I'd be wondering why you are sending this patch
>> at all.
>>
>> But I don't see any advantages of your approach. Instead of pointers to fb_info
>> with this patch you could embed fb_info directly in your data structure but that
>> is barely a difference for a programmer I'd think. You'd still have to call your
>> new functions on init/exit so the amount of function calls needed is the same
>> with or without the patch (I could see an advantage if alloc and release were
>> pure memory allocations). Or is this all about handling the case when fb_alloc
>> fails?
>> Historically some drivers don't even call alloc but have their own fb_info and
>> call only register. I do not want to add yet another way of doing framebuffer
>> initialization unless you can clearly show its benefits.
> 
> Wouldn't it even make sense to move some more of the initialization of fb_info
> out of fb registration into this new init funtion? (I'm thinking about initializing
> mutexes and the like)

This would require someone going through all framebuffers and change them to use
the new fb_init function if they don't use fb_alloc. I'd accept this patch as an
improvement iff someone would do this. Than the number of variation of doing
framebuffer initialization would remain the same. Of course the same should be
done for fb_cleanup.


Best regards,

Florian Tobias Schandinat

> This way fb_info could be used before being registered. Registration would then
> be reduced to makeing the framebuffer visible to userspace and listed in
> registered_fb[].
> 
> This way framebuffer_alloc() would be no more that kzalloc(), framebuffer_init()
> would setup all "non-zero" fields of fb_info (including setup of all mutexes, one
> of which is currently being done by framebuffer_alloc() and the rest by
> do_register_framebuffer()!).
> 
> Bruno
> 
>> Best regards,
>>
>> Florian Tobias Schandinat
> 

Re: [PATCH] fb: split out framebuffer initialization from

[Bruno Prémont]

Hi Florian,

On Sat, 19 November 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> On 11/19/2011 11:47 AM, Bruno Prémont wrote:
> > On Sat, 19 November 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> >> On 11/17/2011 08:19 PM, Timur Tabi wrote:
> >>> Timur Tabi wrote:
> >>>> Introduce functions framebuffer_init() and framebuffer_cleanup() to allow
> >>>> the initialization of a user-allocated fb_info object.
> >>>>
> >>>> framebuffer_alloc() allows for appending a private data structure when it
> >>>> allocates the fb_info object.  However, a driver that registers multiple
> >>>> framebuffers for one device may also need another private data structure
> >>>> for the device itself.  framebuffer_init() allows such drivers to store
> >>>> the fb_info objects in the device-specific private data structure,
> >>>> thereby simplifying memory allocation.
> >>>>
> >>>> Signed-off-by: Timur Tabi <timur@freescale.com>
> >>>
> >>> Florian,
> >>>
> >>> Any comments on this patch?  If you're okay with the change, I want to take
> >>> advantage of it in my framebuffer driver.
> >>
> >> Of course you want, otherwise I'd be wondering why you are sending this patch
> >> at all.
> >>
> >> But I don't see any advantages of your approach. Instead of pointers to fb_info
> >> with this patch you could embed fb_info directly in your data structure but that
> >> is barely a difference for a programmer I'd think. You'd still have to call your
> >> new functions on init/exit so the amount of function calls needed is the same
> >> with or without the patch (I could see an advantage if alloc and release were
> >> pure memory allocations). Or is this all about handling the case when fb_alloc
> >> fails?
> >> Historically some drivers don't even call alloc but have their own fb_info and
> >> call only register. I do not want to add yet another way of doing framebuffer
> >> initialization unless you can clearly show its benefits.
> > 
> > Wouldn't it even make sense to move some more of the initialization of fb_info
> > out of fb registration into this new init funtion? (I'm thinking about initializing
> > mutexes and the like)
> 
> This would require someone going through all framebuffers and change them to use
> the new fb_init function if they don't use fb_alloc. I'd accept this patch as an
> improvement iff someone would do this. Than the number of variation of doing
> framebuffer initialization would remain the same. Of course the same should be
> done for fb_cleanup.

I agree, though (in a first step) alloc_framebuffer() could call init_framebuffer()
by itself (same goes for cleanup on release) so that only those doing the memory
allocation manually have to be changed (though I'm wondering if those are not already
wrong with regard to fb_info.bl_curve_mutex which is being initialized in
alloc_framebuffer() - but then again they will not use backlight).

I might give it a try once I'm no more busy with system administration.

Bruno

> > This way fb_info could be used before being registered. Registration would then
> > be reduced to makeing the framebuffer visible to userspace and listed in
> > registered_fb[].
> > 
> > This way framebuffer_alloc() would be no more that kzalloc(), framebuffer_init()
> > would setup all "non-zero" fields of fb_info (including setup of all mutexes, one
> > of which is currently being done by framebuffer_alloc() and the rest by
> > do_register_framebuffer()!).

Re: [PATCH] fb: split out framebuffer initialization from allocation

[Timur Tabi]

Florian Tobias Schandinat wrote:

>> Any comments on this patch?  If you're okay with the change, I want to take
>> advantage of it in my framebuffer driver.
> 
> Of course you want, otherwise I'd be wondering why you are sending this patch
> at all.

I frequently have to ping maintainers on patches that I've posted.  I've lost count how many times I've been told, "Oh sorry, I forgot all about your patch".

> But I don't see any advantages of your approach. Instead of pointers to fb_info
> with this patch you could embed fb_info directly in your data structure but that
> is barely a difference for a programmer I'd think. You'd still have to call your
> new functions on init/exit so the amount of function calls needed is the same
> with or without the patch (I could see an advantage if alloc and release were
> pure memory allocations). Or is this all about handling the case when fb_alloc
> fails?

It's all about reducing the number of kmalloc calls.  Here's an example of what my per-device data structure looks like:

struct fsl_diu_data {
	dma_addr_t phys;
	struct fb_info fsl_diu_info[NUM_AOIS];	<-----
	struct mfb_info mfb[NUM_AOIS];
	struct device_attribute dev_attr;
	unsigned int irq;
	int fb_enabled;
	enum fsl_diu_monitor_port monitor_port;
	struct diu __iomem *diu_reg;
	spinlock_t reg_lock;
	u8 dummy_aoi[4 * 4 * 4];
	struct diu_ad dummy_ad __aligned(8);
	struct diu_ad ad[NUM_AOIS] __aligned(8);
	u8 gamma[256 * 3] __aligned(32);
	u8 cursor[MAX_CURS * MAX_CURS * 2] __aligned(32);
} __aligned(32);

So this way, I have all of the memory I need allocated in one spot.  I know need to allocate one block of memory.

> Historically some drivers don't even call alloc but have their own fb_info and
> call only register.

That's exactly the same thing I want to do.  Do these drivers also initialize info->device and info->bl_curve_mutex by themselves?  How would they know if the fb_info structure got new members that also need to be initialized?

Perhaps we need to move the fb_info initialization code from framebuffer_alloc() into register_framebuffer()?  

> I do not want to add yet another way of doing framebuffer
> initialization unless you can clearly show its benefits.

I think the problem is that we already have two ways that framebuffers are initialized.

What do you think about this:


diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index 67afa9c..ba47a38 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -57,12 +57,6 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 	if (size)
 		info->par = p + fb_info_size;
 
-	info->device = dev;
-
-#ifdef CONFIG_FB_BACKLIGHT
-	mutex_init(&info->bl_curve_mutex);
-#endif
-
 	return info;
 #undef PADDING
 #undef BYTES_PER_LONG


diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index ad93629..ea35bf8 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1591,6 +1591,12 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	mutex_init(&fb_info->lock);
 	mutex_init(&fb_info->mm_lock);
 
+	fb_info->device = dev;
+
+#ifdef CONFIG_FB_BACKLIGHT
+	mutex_init(&fb_info->bl_curve_mutex);
+#endif
+
 	fb_info->dev = device_create(fb_class, fb_info->device,
 				     MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
 	if (IS_ERR(fb_info->dev)) {


There are a few instances where drivers use info->device after calling framebuffer_alloc() but before calling register_framebuffer().  These drivers will need to be fixed.  But otherwise, I think this is a good idea.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] fb: split out framebuffer initialization from allocation

[Timur Tabi]

Bruno Prémont wrote:

> Wouldn't it even make sense to move some more of the initialization of fb_info
> out of fb registration into this new init funtion? (I'm thinking about initializing
> mutexes and the like)

I was thinking the opposite.  See my reply to Florian's message.

> This way fb_info could be used before being registered. Registration would then
> be reduced to makeing the framebuffer visible to userspace and listed in
> registered_fb[].
> 
> This way framebuffer_alloc() would be no more that kzalloc(), framebuffer_init()
> would setup all "non-zero" fields of fb_info (including setup of all mutexes, one
> of which is currently being done by framebuffer_alloc() and the rest by
> do_register_framebuffer()!).

I think the problem is that it's unclear what the difference is between framebuffer_alloc() and register_framebuffer().  The problem is that register_framebuffer() also initializes the fb_info structure.  So some initialization is done in framebuffer_alloc(), some is done in register_framebuffer(), and some is done by the driver.  Not only that, but drivers that don't call framebuffer_alloc() can't really know what needs to be initialized before calling register_framebuffer().

Why is fb_info->lock initialized in register_framebuffer(), but fb_info->bl_curve_mutex is initialized in framebuffer_alloc()?  They're both mutexes.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] fb: split out framebuffer initialization from

[Bruno Prémont]

On Mon, 21 November 2011 Timur Tabi wrote:
> Bruno Prémont wrote:
> 
> > Wouldn't it even make sense to move some more of the initialization of fb_info
> > out of fb registration into this new init funtion? (I'm thinking about initializing
> > mutexes and the like)
> 
> I was thinking the opposite.  See my reply to Florian's message.

Well, in there you don't mention a reason to put it in
register_framebuffer() rather than collect initialization into
a new function for exactly that task.

> > This way fb_info could be used before being registered. Registration would then
> > be reduced to makeing the framebuffer visible to userspace and listed in
> > registered_fb[].
> > 
> > This way framebuffer_alloc() would be no more that kzalloc(), framebuffer_init()
> > would setup all "non-zero" fields of fb_info (including setup of all mutexes, one
> > of which is currently being done by framebuffer_alloc() and the rest by
> > do_register_framebuffer()!).
> 
> I think the problem is that it's unclear what the difference is between
> framebuffer_alloc() and register_framebuffer().  The problem is that
> register_framebuffer() also initializes the fb_info structure.  So some
> initialization is done in framebuffer_alloc(), some is done in register_framebuffer(),
> and some is done by the driver.  Not only that, but drivers that don't call
> framebuffer_alloc() can't really know what needs to be initialized before
> calling register_framebuffer().
> 
> Why is fb_info->lock initialized in register_framebuffer(), but
> fb_info->bl_curve_mutex is initialized in framebuffer_alloc()?
>  They're both mutexes.

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

For simplicity any driver that uses framebuffer_alloc() should not need
to additionally call init_framebuffer() - framebuffer_alloc() should
call it just before returning.
All those drivers that don't call framebuffer_alloc() would then have to
call init_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.


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);
   ...
}




Bruno

Re: [PATCH] fb: split out framebuffer initialization from allocation

[Timur Tabi]

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