[PATCH] back out fbdev sysfs support

[PATCH] back out fbdev sysfs support

[Christoph Hellwig]

Hi Linus,

this patch backs out James' sysfs support for fbdev again.  It
introduces a big, race for every driver not converted to
framebuffer_{alloc,release} (that is every driver but Ben's new
radeonfb).

I've left in framebuffer_{alloc,release} as stubs so drivers can be
converted to it gradually and once all drivers are done it can be
enabled again.

<rant>
James, what about pushing the 2GB worth of fbdev driver fixes in your
tree to Linus so people actually get working fb support again instead
of adding new holes?  A maintainers job can't be to apply patches to
his personal CVS repository and sitting on them forever
</rant>


--- 1.82/drivers/video/fbmem.c	Thu Feb 12 18:14:53 2004
+++ edited/drivers/video/fbmem.c	Fri Feb 13 06:00:42 2004
@@ -1228,9 +1228,6 @@
 			break;
 	fb_info->node = i;
 	
-	if (fb_add_class_device(fb_info))
-		return -EINVAL;
-	
 	if (fb_info->pixmap.addr == NULL) {
 		fb_info->pixmap.addr = kmalloc(FBPIXMAPSIZE, GFP_KERNEL);
 		if (fb_info->pixmap.addr) {
@@ -1279,7 +1276,6 @@
 		kfree(fb_info->pixmap.addr);
 	registered_fb[i]=NULL;
 	num_registered_fb--;
-	class_device_del(&fb_info->class_dev);
 	return 0;
 }
 
@@ -1307,8 +1303,6 @@
 	if (register_chrdev(FB_MAJOR,"fb",&fb_fops))
 		printk("unable to get major %d for fb devs\n", FB_MAJOR);
 
-	class_register(&fb_class);
-	
 #ifdef CONFIG_FB_OF
 	if (ofonly) {
 		offb_init();
--- 1.1/drivers/video/fbsysfs.c	Fri Feb  6 12:14:39 2004
+++ edited/drivers/video/fbsysfs.c	Fri Feb 13 08:04:18 2004
@@ -9,50 +9,15 @@
  *	2 of the License, or (at your option) any later version.
  */
 
-#include <linux/config.h>
+/*
+ * Note:  currently there's only stubs for framebuffer_alloc and
+ * framebuffer_release here.  The reson for that is that until all drivers
+ * are converted to use it a sysfsification will open OOPSable races.
+ */
+
 #include <linux/kernel.h>
 #include <linux/fb.h>
 
-#define to_fb_info(class) container_of(class, struct fb_info, class_dev)
-
-static void release_fb_info(struct class_device *class_dev)
-{
-	struct fb_info *info = to_fb_info(class_dev);
-
-	/* This doesn't harm */
-	fb_dealloc_cmap(&info->cmap);
-
-	kfree(info);
-}
-
-struct class fb_class = {
-	.name 		= "graphics",
-	.release 	= &release_fb_info,
-};
-
-static ssize_t show_dev(struct class_device *class_dev, char *buf)
-{
-	struct fb_info *info = to_fb_info(class_dev);
-
-	return sprintf(buf, "%u:%u\n", FB_MAJOR, info->node);
-}
-
-static CLASS_DEVICE_ATTR(dev, S_IRUGO, show_dev, NULL);
-
-int fb_add_class_device(struct fb_info *info)
-{
-	int retval;
-
-	info->class_dev.class = &fb_class;
-	snprintf(info->class_dev.class_id, BUS_ID_SIZE, "fb%d",
-		 info->node);
-	retval = class_device_register(&info->class_dev);
-	if (retval)
-		return retval;
-	return class_device_create_file(&info->class_dev,
-					&class_device_attr_dev);
-}
-
 /**
  * framebuffer_alloc - creates a new frame buffer info structure
  *
@@ -82,7 +47,6 @@
 		return NULL;
 	memset(p, 0, fb_info_size + size);
 	info = (struct fb_info *) p;
-	info->class_dev.dev = dev;
 
 	if (size)
 		info->par = p + fb_info_size;
@@ -103,7 +67,7 @@
  */
 void framebuffer_release(struct fb_info *info)
 {
-	class_device_put(&info->class_dev);
+	kfree(info);
 }
 
 EXPORT_SYMBOL(framebuffer_release);
--- 1.57/include/linux/fb.h	Fri Feb 13 16:19:26 2004
+++ edited/include/linux/fb.h	Fri Feb 13 08:04:27 2004
@@ -448,7 +448,6 @@
 	char *screen_base;		/* Virtual address */
 	struct vc_data *display_fg;	/* Console visible on this display */
 	int currcon;			/* Current VC. */
-	struct class_device class_dev;	/* Sysfs data */	
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 
 	/* From here on everything is device dependent */
 	void *par;	
@@ -533,9 +532,6 @@
 /* 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 int fb_add_class_device(struct fb_info *info);
-
-extern struct class fb_class;
 
 /* drivers/video/fbmon.c */
 #define FB_MAXTIMINGS       0

Re: [PATCH] back out fbdev sysfs support

[Linus Torvalds]

On Sat, 14 Feb 2004, Christoph Hellwig wrote:
> 
> <rant>
> James, what about pushing the 2GB worth of fbdev driver fixes in your
> tree to Linus so people actually get working fb support again instead
> of adding new holes?

Sorry, but at this point I WOULD NOT EVEN TAKE IT ANY MORE.

That's just how I work: if somebody maintains his own tree and builds up a 
lot of patches, that's _his_ problem. I'm not going to replace things 
totally unless there is some really fundamental reason I would have to. 
And quite frankly, the most common "fundamental reason" is that the 
maintainer has not done his job.

I want controlled patches that do one thing at a time. Not a 2GB untested 
dump.

>  A maintainers job can't be to apply patches to
> his personal CVS repository and sitting on them forever
> </rant>

.. and once he has patches, he can't just "dump" them out, either.

These things need to be done in a timely fashion, incrementally, one thing 
at a time. Anything else does not work.

And btw, for anybody who is impacted by this: you are encouraged to help. 
If you have a machine that works with some out-of-tree code but does 
_not_ work with the in-tree code, send a patch that fixes JUST THAT BUG.

Because if James can't trickle them in, somebody else will have to. That's 
what happened with the new radeon driver.

		Linus

Re: [PATCH] back out fbdev sysfs support

[Christoph Hellwig]

On Sat, Feb 14, 2004 at 09:02:12AM -0800, Linus Torvalds wrote:
> Sorry, but at this point I WOULD NOT EVEN TAKE IT ANY MORE.
> 
> That's just how I work: if somebody maintains his own tree and builds up a 
> lot of patches, that's _his_ problem. I'm not going to replace things 
> totally unless there is some really fundamental reason I would have to. 
> And quite frankly, the most common "fundamental reason" is that the 
> maintainer has not done his job.
> 
> I want controlled patches that do one thing at a time. Not a 2GB untested 
> dump.

Hey, that's not what I meant.  Sorry for the silly language.  I'm the
last one to argue in favour of a single big diff.  But all the driver
fixes are easily separatable, in fact benh probably still has all the
individual diffs for all the mac drivers.

What I meant is that the FB maintainer should try to get the existing
fixes merged before adding dubious features.

Re: [PATCH] back out fbdev sysfs support

[Dave Jones]

On Sat, Feb 14, 2004 at 06:06:00PM +0100, Christoph Hellwig wrote:

 > What I meant is that the FB maintainer should try to get the existing
 > fixes merged before adding dubious features.

Whilst on the subject, why does the fb code need sysfs support
anyway ?

		Dave

Re: [PATCH] back out fbdev sysfs support

[Benjamin Herrenschmidt]

On Sun, 2004-02-15 at 04:28, Dave Jones wrote:
> On Sat, Feb 14, 2004 at 06:06:00PM +0100, Christoph Hellwig wrote:
> 
>  > What I meant is that the FB maintainer should try to get the existing
>  > fixes merged before adding dubious features.
> 
> Whilst on the subject, why does the fb code need sysfs support
> anyway ?

We want a class device so the drivers can expose some informations like
the monitor probing data (EDID), allowed modes list etc... to userland.

I now have access to these infos in some drivers like radeonfb that do
full monitor probing, and the fbdev "API" provides no way to get that
down to userland (for use by xserver or MOL for example). I could just
add some cases to the existing ioctl API, but I don't like that ;)

Ben.

Re: [PATCH] back out fbdev sysfs support

[viro]

On Sat, Feb 14, 2004 at 09:02:12AM -0800, Linus Torvalds wrote:
> These things need to be done in a timely fashion, incrementally, one thing 
> at a time. Anything else does not work.
> 
> And btw, for anybody who is impacted by this: you are encouraged to help. 
> If you have a machine that works with some out-of-tree code but does 
> _not_ work with the in-tree code, send a patch that fixes JUST THAT BUG.
> 
> Because if James can't trickle them in, somebody else will have to. That's 
> what happened with the new radeon driver.

Where's James' repository, BTW?  I could help with split-and-reorder on
that one...

Re: [PATCH] back out fbdev sysfs support

[Paul Mundt]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

On Sat, Feb 14, 2004 at 05:58:13PM +0000, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > Because if James can't trickle them in, somebody else will have to. That's 
> > what happened with the new radeon driver.
> 
> Where's James' repository, BTW?  I could help with split-and-reorder on
> that one...

bk://fbdev.bkbits.net/fbdev-2.6


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] back out fbdev sysfs support

[Benjamin Herrenschmidt]

On Sun, 2004-02-15 at 04:02, Linus Torvalds wrote:
> On Sat, 14 Feb 2004, Christoph Hellwig wrote:
> > 
> > <rant>
> > James, what about pushing the 2GB worth of fbdev driver fixes in your
> > tree to Linus so people actually get working fb support again instead
> > of adding new holes?
> 
> Sorry, but at this point I WOULD NOT EVEN TAKE IT ANY MORE.
> 
> That's just how I work: if somebody maintains his own tree and builds up a 
> lot of patches, that's _his_ problem. I'm not going to replace things 
> totally unless there is some really fundamental reason I would have to. 
> And quite frankly, the most common "fundamental reason" is that the 
> maintainer has not done his job.
> 
> I want controlled patches that do one thing at a time. Not a 2GB untested 
> dump.

I'll send you/andrew individually the drivers I control and the ones I
already fixed in james tree (5 or 6 drivers)

James: The fbcon & cursor changes must get in asap. There are races that
I fixed, without the changes, those races will be in 2.6.3.

Ben.

Re: [PATCH] back out fbdev sysfs support

[James Simmons]

> I'll send you/andrew individually the drivers I control and the ones I
> already fixed in james tree (5 or 6 drivers)

I have drivers as well which I need to send. 
 
> James: The fbcon & cursor changes must get in asap. There are races that
> I fixed, without the changes, those races will be in 2.6.3.

I just sent out a patch for people to try and look over before I submit 
it.

Re: [PATCH] back out fbdev sysfs support

[Benjamin Herrenschmidt]

On Tue, 2004-02-17 at 09:49, James Simmons wrote:
> > I'll send you/andrew individually the drivers I control and the ones I
> > already fixed in james tree (5 or 6 drivers)
> 
> I have drivers as well which I need to send. 
>  
> > James: The fbcon & cursor changes must get in asap. There are races that
> > I fixed, without the changes, those races will be in 2.6.3.
> 
> I just sent out a patch for people to try and look over before I submit 
> it.

Double check with what I already commited.

Ben.

Re: [PATCH] back out fbdev sysfs support

[James Simmons]

> > > James: The fbcon & cursor changes must get in asap. There are races that
> > > I fixed, without the changes, those races will be in 2.6.3.
> > 
> > I just sent out a patch for people to try and look over before I submit 
> > it.
> 
> Double check with what I already commited.

It against Linus latest tree which included your submitted changes.

Re: [PATCH] back out fbdev sysfs support

[James Simmons]

> <rant>
> James, what about pushing the 2GB worth of fbdev driver fixes in your
> tree to Linus so people actually get working fb support again instead
> of adding new holes?  A maintainers job can't be to apply patches to
> his personal CVS repository and sitting on them forever
> </rant>

I don't have 2GB of patches. I have way less than that. Almost every core 
change is in now. I'm very cautious and I rather have drivers tested for a 
long time.