All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Suspend2 Merge: Driver model patches 2/2
@ 2004-09-16 10:58 Nigel Cunningham
  2004-09-16 14:28 ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Nigel Cunningham @ 2004-09-16 10:58 UTC (permalink / raw)
  To: Andrew Morton, Patrick Mochel, Pavel Machek; +Cc: Linux Kernel Mailing List

Hi.

This simple helper adds support for finding a class given its name. I
use this to locate the frame buffer drivers and move them to the
keep-alive tree while suspending other drivers.

Regards,

Nigel

diff -ruN linux-2.6.9-rc1/drivers/base/class.c software-suspend-linux-2.6.9-rc1-rev3/drivers/base/class.c
--- linux-2.6.9-rc1/drivers/base/class.c	2004-09-07 21:58:30.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/base/class.c	2004-09-09 19:36:24.000000000 +1000
@@ -460,6 +460,20 @@
 	kobject_put(&class_dev->kobj);
 }
 
+struct class * class_find(char * name)
+{
+	struct class * this_class;
+
+	if (!name)
+		return NULL;
+
+	list_for_each_entry(this_class, &class_subsys.kset.list, subsys.kset.kobj.entry) {
+		if (!(strcmp(this_class->name, name)))
+			return this_class;
+	}
+
+	return NULL;
+}
 
 int class_interface_register(struct class_interface *class_intf)
 {
@@ -542,3 +556,5 @@
 
 EXPORT_SYMBOL(class_interface_register);
 EXPORT_SYMBOL(class_interface_unregister);
+
+EXPORT_SYMBOL(class_find);
diff -ruN linux-2.6.9-rc1/include/linux/device.h software-suspend-linux-2.6.9-rc1-rev3/include/linux/device.h
--- linux-2.6.9-rc1/include/linux/device.h	2004-09-07 21:58:59.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/include/linux/device.h	2004-09-09 19:36:24.000000000 +1000
@@ -163,6 +163,7 @@
 
 extern struct class * class_get(struct class *);
 extern void class_put(struct class *);
+extern struct class * class_find(char * name);
 
 
 struct class_attribute {

-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 10:58 [PATCH] Suspend2 Merge: Driver model patches 2/2 Nigel Cunningham
@ 2004-09-16 14:28 ` Greg KH
  2004-09-16 22:18   ` Nigel Cunningham
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2004-09-16 14:28 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

On Thu, Sep 16, 2004 at 08:58:51PM +1000, Nigel Cunningham wrote:
> 
> This simple helper adds support for finding a class given its name. I
> use this to locate the frame buffer drivers and move them to the
> keep-alive tree while suspending other drivers.
> 
> +struct class * class_find(char * name)
> +{
> +	struct class * this_class;
> +
> +	if (!name)
> +		return NULL;
> +
> +	list_for_each_entry(this_class, &class_subsys.kset.list, subsys.kset.kobj.entry) {
> +		if (!(strcmp(this_class->name, name)))
> +			return this_class;
> +	}
> +
> +	return NULL;
> +}

Ick, no.  I've been over this before with the fb people, and am not going
to accept this patch (nevermind that it's broken...)  See the lkml
archives for more info on why I don't like this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 14:28 ` Greg KH
@ 2004-09-16 22:18   ` Nigel Cunningham
  2004-09-16 22:35     ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Nigel Cunningham @ 2004-09-16 22:18 UTC (permalink / raw)
  To: Greg KH
  Cc: Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

Hi.

On Fri, 2004-09-17 at 00:28, Greg KH wrote:
> On Thu, Sep 16, 2004 at 08:58:51PM +1000, Nigel Cunningham wrote:
> > 
> > This simple helper adds support for finding a class given its name. I
> > use this to locate the frame buffer drivers and move them to the
> > keep-alive tree while suspending other drivers.
> > 
> > +struct class * class_find(char * name)
> > +{
> > +	struct class * this_class;
> > +
> > +	if (!name)
> > +		return NULL;
> > +
> > +	list_for_each_entry(this_class, &class_subsys.kset.list, subsys.kset.kobj.entry) {
> > +		if (!(strcmp(this_class->name, name)))
> > +			return this_class;
> > +	}
> > +
> > +	return NULL;
> > +}
> 
> Ick, no.  I've been over this before with the fb people, and am not going
> to accept this patch (nevermind that it's broken...)  See the lkml
> archives for more info on why I don't like this.

Please excuse my ignorance but I don't see how it's broken (their patch
just fills in a field that was left blank previously), and this patch
just makes use of that change. What's the point to device_class if we
don't use it?

That said, I do agree with using Pavel's new enum that includes
_SNAPSHOT and can see that it's a cleaner way in that it requires less
knowledge on suspend's part of what it wants to stay alive.

Regards,

Nigel

-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 22:18   ` Nigel Cunningham
@ 2004-09-16 22:35     ` Greg KH
  2004-09-16 22:49       ` Nigel Cunningham
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2004-09-16 22:35 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

On Fri, Sep 17, 2004 at 08:18:47AM +1000, Nigel Cunningham wrote:
> On Fri, 2004-09-17 at 00:28, Greg KH wrote:
> > On Thu, Sep 16, 2004 at 08:58:51PM +1000, Nigel Cunningham wrote:
> > > 
> > > This simple helper adds support for finding a class given its name. I
> > > use this to locate the frame buffer drivers and move them to the
> > > keep-alive tree while suspending other drivers.
> > > 
> > > +struct class * class_find(char * name)
> > > +{
> > > +	struct class * this_class;
> > > +
> > > +	if (!name)
> > > +		return NULL;
> > > +
> > > +	list_for_each_entry(this_class, &class_subsys.kset.list, subsys.kset.kobj.entry) {
> > > +		if (!(strcmp(this_class->name, name)))
> > > +			return this_class;
> > > +	}
> > > +
> > > +	return NULL;
> > > +}
> > 
> > Ick, no.  I've been over this before with the fb people, and am not going
> > to accept this patch (nevermind that it's broken...)  See the lkml
> > archives for more info on why I don't like this.
> 
> Please excuse my ignorance but I don't see how it's broken

This function, as written is very broken.  I will not accept it.  Not to
mention the fact that the functionality this function proposes to offer
is not needed either.

> (their patch just fills in a field that was left blank previously),

What patch?

> and this patch just makes use of that change. What's the point to
> device_class if we don't use it?

I don't see a use of device_class in this function.  I'm confused.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 22:35     ` Greg KH
@ 2004-09-16 22:49       ` Nigel Cunningham
  2004-09-16 23:07         ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Nigel Cunningham @ 2004-09-16 22:49 UTC (permalink / raw)
  To: Greg KH
  Cc: Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

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

Hi.

On Fri, 2004-09-17 at 08:35, Greg KH wrote:
> > > Ick, no.  I've been over this before with the fb people, and am not going
> > > to accept this patch (nevermind that it's broken...)  See the lkml
> > > archives for more info on why I don't like this.
> > 
> > Please excuse my ignorance but I don't see how it's broken
> 
> This function, as written is very broken.  I will not accept it.  Not to

What's broken? (I want to learn what I've done wrong that I'm not
seeing).

> mention the fact that the functionality this function proposes to offer
> is not needed either.
> 
> > (their patch just fills in a field that was left blank previously),
> 
> What patch?

Attached. Sorry if I wrongly assumed this was the patch you're talking
about.

> > and this patch just makes use of that change. What's the point to
> > device_class if we don't use it?
> 
> I don't see a use of device_class in this function.  I'm confused.

This patch finds the device_class that the frame buffer drivers
register. It gets called by suspend code I haven't posted yet, which
then moves the drivers in this class from one pm tree to another so that
the frame buffer drivers don't get suspended until it's time for the
atomic snapshot and can be resumed afterwards while we write the rest of
the image, without resuming all drivers. Given Pavel's work with the new
_SNAPSHOT flag, I guess this won't eventually be needed (provided, of
course that drivers do the right thing when called with _SNAPSHOT).

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.

[-- Attachment #2: 204-frame-buffer-class-support --]
[-- Type: text/x-patch, Size: 12788 bytes --]

diff -ruN linux-2.6.9-rc1/drivers/video/aty/atyfb_base.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/aty/atyfb_base.c
--- linux-2.6.9-rc1/drivers/video/aty/atyfb_base.c	2004-09-07 21:58:51.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/aty/atyfb_base.c	2004-09-09 19:36:24.000000000 +1000
@@ -1972,7 +1972,7 @@
 
 			info->fix = atyfb_fix;
 			info->par = default_par;
-
+			info->device = &pdev->dev;
 #ifdef __sparc__
 			/*
 			 * Map memory-mapped registers.
diff -ruN linux-2.6.9-rc1/drivers/video/chipsfb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/chipsfb.c
--- linux-2.6.9-rc1/drivers/video/chipsfb.c	2004-09-07 21:58:51.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/chipsfb.c	2004-09-09 19:36:24.000000000 +1000
@@ -416,7 +416,7 @@
 		release_mem_region(addr, size);
 		return -ENOMEM;
 	}
-
+	p->device = &dp->dev;
 	init_chips(p, addr);
 
 #ifdef CONFIG_PMAC_PBOOK
diff -ruN linux-2.6.9-rc1/drivers/video/cyber2000fb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/cyber2000fb.c
--- linux-2.6.9-rc1/drivers/video/cyber2000fb.c	2004-09-07 21:58:51.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/cyber2000fb.c	2004-09-09 19:36:24.000000000 +1000
@@ -1399,6 +1399,8 @@
 		cfb->fb.var.xres, cfb->fb.var.yres,
 		h_sync / 1000, h_sync % 1000, v_sync);
 
+	if (cfb->dev)
+		cfb->fb.device = &cfb->dev->dev;
 	err = register_framebuffer(&cfb->fb);
 
 failed:
diff -ruN linux-2.6.9-rc1/drivers/video/fbmem.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/fbmem.c
--- linux-2.6.9-rc1/drivers/video/fbmem.c	2004-09-07 21:58:51.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/fbmem.c	2004-09-09 19:36:24.000000000 +1000
@@ -1444,7 +1444,8 @@
 			break;
 	fb_info->node = i;
 
-	c = class_simple_device_add(fb_class, MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
+	c = class_simple_device_add(fb_class, MKDEV(FB_MAJOR, i),
+				    fb_info->device, "fb%d", i);
 	if (IS_ERR(c)) {
 		/* Not fatal */
 		printk(KERN_WARNING "Unable to create class_device for framebuffer %d; errno = %ld\n", i, PTR_ERR(c));
diff -ruN linux-2.6.9-rc1/drivers/video/fbsysfs.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/fbsysfs.c
--- linux-2.6.9-rc1/drivers/video/fbsysfs.c	2004-02-18 19:16:01.000000000 +1100
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/fbsysfs.c	2004-09-09 19:36:24.000000000 +1000
@@ -51,6 +51,8 @@
 	if (size)
 		info->par = p + fb_info_size;
 
+	info->device = dev;
+
 	return info;
 #undef PADDING
 #undef BYTES_PER_LONG
diff -ruN linux-2.6.9-rc1/drivers/video/i810/i810_main.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/i810/i810_main.c
--- linux-2.6.9-rc1/drivers/video/i810/i810_main.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/i810/i810_main.c	2004-09-09 19:36:24.000000000 +1000
@@ -1855,20 +1855,13 @@
 	int i, err = -1, vfreq, hfreq, pixclock;
 
 	i = 0;
-	if (!(info = kmalloc(sizeof(struct fb_info), GFP_KERNEL))) {
-		i810fb_release_resource(info, par);
-		return -ENOMEM;
-	}
-	memset(info, 0, sizeof(struct fb_info));
 
-	if(!(par = kmalloc(sizeof(struct i810fb_par), GFP_KERNEL))) {
-		i810fb_release_resource(info, par);
+	info = framebuffer_alloc(sizeof(struct i810fb_par), &dev->dev);
+	if (!info)
 		return -ENOMEM;
-	}
-	memset(par, 0, sizeof(struct i810fb_par));
 
+	par = (struct i810fb_par *) info->par;
 	par->dev = dev;
-	info->par = par;
 
 	if (!(info->pixmap.addr = kmalloc(64*1024, GFP_KERNEL))) {
 		i810fb_release_resource(info, par);
@@ -1941,38 +1934,36 @@
 static void i810fb_release_resource(struct fb_info *info, 
 				    struct i810fb_par *par)
 {
-	if (par) {
-		unset_mtrr(par);
-		if (par->drm_agp) {
-			drm_agp_t *agp = par->drm_agp;
-			struct gtt_data *gtt = &par->i810_gtt;
-
-			if (par->i810_gtt.i810_cursor_memory) 
-				agp->free_memory(gtt->i810_cursor_memory);
-			if (par->i810_gtt.i810_fb_memory) 
-				agp->free_memory(gtt->i810_fb_memory);
-
-			inter_module_put("drm_agp");
-			par->drm_agp = NULL;
-		}
-
-		if (par->mmio_start_virtual) 
-			iounmap(par->mmio_start_virtual);
-		if (par->aperture.virtual) 
-			iounmap(par->aperture.virtual);
-
-		if (par->res_flags & FRAMEBUFFER_REQ)
-			release_mem_region(par->aperture.physical, 
-					   par->aperture.size);
-		if (par->res_flags & MMIO_REQ)
-			release_mem_region(par->mmio_start_phys, MMIO_SIZE);
+	unset_mtrr(par);
+	if (par->drm_agp) {
+		drm_agp_t *agp = par->drm_agp;
+		struct gtt_data *gtt = &par->i810_gtt;
+
+		if (par->i810_gtt.i810_cursor_memory)
+			agp->free_memory(gtt->i810_cursor_memory);
+		if (par->i810_gtt.i810_fb_memory)
+			agp->free_memory(gtt->i810_fb_memory);
+
+		inter_module_put("drm_agp");
+		par->drm_agp = NULL;
+	}
+
+	if (par->mmio_start_virtual)
+		iounmap(par->mmio_start_virtual);
+	if (par->aperture.virtual)
+		iounmap(par->aperture.virtual);
+
+	if (par->res_flags & FRAMEBUFFER_REQ)
+		release_mem_region(par->aperture.physical,
+				   par->aperture.size);
+	if (par->res_flags & MMIO_REQ)
+		release_mem_region(par->mmio_start_phys, MMIO_SIZE);
 
-		if (par->res_flags & PCI_DEVICE_ENABLED)
-			pci_disable_device(par->dev); 
+	if (par->res_flags & PCI_DEVICE_ENABLED)
+		pci_disable_device(par->dev);
+
+	framebuffer_release(info);
 
-		kfree(par);
-	}
-	kfree(info);
 }
 
 static void __exit i810fb_remove_pci(struct pci_dev *dev)
diff -ruN linux-2.6.9-rc1/drivers/video/igafb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/igafb.c
--- linux-2.6.9-rc1/drivers/video/igafb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/igafb.c	2004-09-09 19:36:24.000000000 +1000
@@ -528,6 +528,7 @@
 	info->var = default_var;
 	info->fix = igafb_fix;
 	info->pseudo_palette = (void *)(par + 1);
+	info->device = &pdev->dev;
 
 	if (!iga_init(info, par)) {
 		iounmap((void *)par->io_base);
diff -ruN linux-2.6.9-rc1/drivers/video/imsttfb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/imsttfb.c
--- linux-2.6.9-rc1/drivers/video/imsttfb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/imsttfb.c	2004-09-09 19:36:24.000000000 +1000
@@ -1524,6 +1524,7 @@
 	par->cmap_regs = (__u8 *)ioremap(addr + 0x840000, 0x1000);
 	info->par = par;
 	info->pseudo_palette = (void *) (par + 1);
+	info->device = &pdev->dev;
 	init_imstt(info);
 
 	pci_set_drvdata(pdev, info);
diff -ruN linux-2.6.9-rc1/drivers/video/kyro/fbdev.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/kyro/fbdev.c
--- linux-2.6.9-rc1/drivers/video/kyro/fbdev.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/kyro/fbdev.c	2004-09-09 19:36:24.000000000 +1000
@@ -735,6 +735,7 @@
 
 	fb_memset(info->screen_base, 0, size);
 
+	info->device = &pdev->dev;
 	if (register_framebuffer(info) < 0)
 		goto out_unmap;
 
diff -ruN linux-2.6.9-rc1/drivers/video/matrox/matroxfb_base.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/matrox/matroxfb_base.c
--- linux-2.6.9-rc1/drivers/video/matrox/matroxfb_base.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/matrox/matroxfb_base.c	2004-09-09 19:36:24.000000000 +1000
@@ -1864,6 +1864,7 @@
 /* We do not have to set currcon to 0... register_framebuffer do it for us on first console
  * and we do not want currcon == 0 for subsequent framebuffers */
 
+	ACCESS_FBINFO(fbcon).device = &ACCESS_FBINFO(pcidev)->dev;
 	if (register_framebuffer(&ACCESS_FBINFO(fbcon)) < 0) {
 		goto failVideoIO;
 	}
diff -ruN linux-2.6.9-rc1/drivers/video/pvr2fb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/pvr2fb.c
--- linux-2.6.9-rc1/drivers/video/pvr2fb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/pvr2fb.c	2004-09-09 19:36:24.000000000 +1000
@@ -939,6 +939,7 @@
 
 	pvr2_fix.mmio_start	= pci_resource_start(pdev, 1);
 	pvr2_fix.mmio_len	= pci_resource_len(pdev, 1);
+	fbinfo->device = &pdev->dev;
 
 	return pvr2fb_common_init();
 }
diff -ruN linux-2.6.9-rc1/drivers/video/radeonfb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/radeonfb.c
--- linux-2.6.9-rc1/drivers/video/radeonfb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/radeonfb.c	2004-09-09 19:36:24.000000000 +1000
@@ -3040,7 +3040,7 @@
 	pci_set_drvdata(pdev, rinfo);
 	rinfo->next = board_list;
 	board_list = rinfo;
-
+	((struct fb_info *) rinfo)->device = &pdev->dev;
 	if (register_framebuffer ((struct fb_info *) rinfo) < 0) {
 		printk ("radeonfb: could not register framebuffer\n");
 		iounmap ((void*)rinfo->fb_base);
diff -ruN linux-2.6.9-rc1/drivers/video/riva/fbdev.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/riva/fbdev.c
--- linux-2.6.9-rc1/drivers/video/riva/fbdev.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/riva/fbdev.c	2004-09-09 19:36:24.000000000 +1000
@@ -1850,21 +1850,17 @@
 	NVTRACE_ENTER();
 	assert(pd != NULL);
 
-	info = kmalloc(sizeof(struct fb_info), GFP_KERNEL);
+	info = framebuffer_alloc(sizeof(struct riva_par), &pd->dev);
+
 	if (!info)
 		goto err_out;
 
-	default_par = kmalloc(sizeof(struct riva_par), GFP_KERNEL);
-	if (!default_par)
-		goto err_out_kfree;
-
-	memset(info, 0, sizeof(struct fb_info));
-	memset(default_par, 0, sizeof(struct riva_par));
+	default_par = (struct riva_par *) info->par;
 	default_par->pdev = pd;
 
 	info->pixmap.addr = kmalloc(64 * 1024, GFP_KERNEL);
 	if (info->pixmap.addr == NULL)
-		goto err_out_kfree1;
+		goto err_out_kfree;
 	memset(info->pixmap.addr, 0, 64 * 1024);
 
 	if (pci_enable_device(pd)) {
@@ -1888,7 +1884,7 @@
 
 	if(default_par->riva.Architecture == 0) {
 		printk(KERN_ERR PFX "unknown NV_ARCH\n");
-		goto err_out_kfree1;
+		goto err_out_free_base0;
 	}
 	if(default_par->riva.Architecture == NV_ARCH_10 ||
 	   default_par->riva.Architecture == NV_ARCH_20 ||
@@ -1994,7 +1990,6 @@
 	fb_destroy_modedb(info->monspecs.modedb);
 	info->monspecs.modedb_len = 0;
 	info->monspecs.modedb = NULL;
-
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR PFX
 			"error registering riva framebuffer\n");
@@ -2033,10 +2028,8 @@
 	pci_disable_device(pd);
 err_out_enable:
 	kfree(info->pixmap.addr);
-err_out_kfree1:
-	kfree(default_par);
 err_out_kfree:
-	kfree(info);
+	framebuffer_release(info);
 err_out:
 	return -ENODEV;
 }
@@ -2070,8 +2063,7 @@
 	pci_release_regions(pd);
 	pci_disable_device(pd);
 	kfree(info->pixmap.addr);
-	kfree(par);
-	kfree(info);
+	framebuffer_release(info);
 	pci_set_drvdata(pd, NULL);
 	NVTRACE_LEAVE();
 }
diff -ruN linux-2.6.9-rc1/drivers/video/sstfb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/sstfb.c
--- linux-2.6.9-rc1/drivers/video/sstfb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/sstfb.c	2004-09-09 19:36:24.000000000 +1000
@@ -1507,6 +1507,7 @@
 	fb_alloc_cmap(&info->cmap, 256, 0);
 
 	/* register fb */
+	info->device = &pdev->dev;
 	if (register_framebuffer(info) < 0) {
 		eprintk("can't register framebuffer.\n");
 		goto fail;
diff -ruN linux-2.6.9-rc1/drivers/video/tgafb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/tgafb.c
--- linux-2.6.9-rc1/drivers/video/tgafb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/tgafb.c	2004-09-09 19:36:24.000000000 +1000
@@ -1454,6 +1454,7 @@
 	tgafb_set_par(&all->info);
 	tgafb_init_fix(&all->info);
 
+	all->info.device = &pdev->dev;
 	if (register_framebuffer(&all->info) < 0) {
 		printk(KERN_ERR "tgafb: Could not register framebuffer\n");
 		ret = -EINVAL;
diff -ruN linux-2.6.9-rc1/drivers/video/tridentfb.c software-suspend-linux-2.6.9-rc1-rev3/drivers/video/tridentfb.c
--- linux-2.6.9-rc1/drivers/video/tridentfb.c	2004-09-07 21:58:52.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/drivers/video/tridentfb.c	2004-09-09 19:36:24.000000000 +1000
@@ -1164,6 +1164,7 @@
 		default_var.accel_flags &= ~FB_ACCELF_TEXT;
 	default_var.activate |= FB_ACTIVATE_NOW;
 	fb_info.var = default_var;
+	fb_info.device = &dev->dev;
 	if (register_framebuffer(&fb_info) < 0) {
 		output("Could not register Trident framebuffer\n");
 		return -EINVAL;
diff -ruN linux-2.6.9-rc1/include/linux/fb.h software-suspend-linux-2.6.9-rc1-rev3/include/linux/fb.h
--- linux-2.6.9-rc1/include/linux/fb.h	2004-09-07 21:58:59.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/include/linux/fb.h	2004-09-09 19:36:24.000000000 +1000
@@ -595,6 +595,7 @@
 	struct fb_pixmap sprite;	/* Cursor hardware mapper */
 	struct fb_cmap cmap;		/* Current cmap */
 	struct fb_ops *fbops;
+	struct device *device;
 	char *screen_base;		/* Virtual address */
 	int currcon;			/* Current VC. */
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 22:49       ` Nigel Cunningham
@ 2004-09-16 23:07         ` Greg KH
  2004-09-16 23:19           ` Nigel Cunningham
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2004-09-16 23:07 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

On Fri, Sep 17, 2004 at 08:49:07AM +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Fri, 2004-09-17 at 08:35, Greg KH wrote:
> > > > Ick, no.  I've been over this before with the fb people, and am not going
> > > > to accept this patch (nevermind that it's broken...)  See the lkml
> > > > archives for more info on why I don't like this.
> > > 
> > > Please excuse my ignorance but I don't see how it's broken
> > 
> > This function, as written is very broken.  I will not accept it.  Not to
> 
> What's broken? (I want to learn what I've done wrong that I'm not
> seeing).

 - No locking when traversing the list.
 - Reference count needs to be bumped before returning a pointer to the
   object you found.

> > mention the fact that the functionality this function proposes to offer
> > is not needed either.
> > 
> > > (their patch just fills in a field that was left blank previously),
> > 
> > What patch?
> 
> Attached. Sorry if I wrongly assumed this was the patch you're talking
> about.

Ah, no, I've never seen this one, thanks.  But it looks sane, I don't
have a problem with it (sysfs will like it, it's not a suspend specific
patch at all.)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 23:07         ` Greg KH
@ 2004-09-16 23:19           ` Nigel Cunningham
  2004-09-17 19:40             ` Pavel Machek
  0 siblings, 1 reply; 8+ messages in thread
From: Nigel Cunningham @ 2004-09-16 23:19 UTC (permalink / raw)
  To: Greg KH
  Cc: Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

Hi.

On Fri, 2004-09-17 at 09:07, Greg KH wrote:
> > What's broken? (I want to learn what I've done wrong that I'm not
> > seeing).
> 
>  - No locking when traversing the list.
>  - Reference count needs to be bumped before returning a pointer to the
>    object you found.

Ah. Fair enough. I haven't seen any problems because the locking is more
abstract: processes are frozen when this runs for suspend. I'll fix it,
but won't bother resubmitting it because Pavel's changes should obsolete
this stuff.

> > > mention the fact that the functionality this function proposes to offer
> > > is not needed either.
> > > 
> > > > (their patch just fills in a field that was left blank previously),
> > > 
> > > What patch?
> > 
> > Attached. Sorry if I wrongly assumed this was the patch you're talking
> > about.
> 
> Ah, no, I've never seen this one, thanks.  But it looks sane, I don't
> have a problem with it (sysfs will like it, it's not a suspend specific
> patch at all.)

Antonio posted it to LKML last week IIRC, which is why I didn't include
it in the device driver patches. Given Pavel's changes (again), I'm in
two minds as to whether its needed. It's clearly the right thing to do,
but not needed at the moment. Then again, as we noted already, the whole
device_class thing doesn't get a lot of use at the moment.

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Suspend2 Merge: Driver model patches 2/2
  2004-09-16 23:19           ` Nigel Cunningham
@ 2004-09-17 19:40             ` Pavel Machek
  0 siblings, 0 replies; 8+ messages in thread
From: Pavel Machek @ 2004-09-17 19:40 UTC (permalink / raw)
  To: Nigel Cunningham
  Cc: Greg KH, Andrew Morton, Patrick Mochel, Pavel Machek,
	Linux Kernel Mailing List

Hi!

> > Ah, no, I've never seen this one, thanks.  But it looks sane, I don't
> > have a problem with it (sysfs will like it, it's not a suspend specific
> > patch at all.)
> 
> Antonio posted it to LKML last week IIRC, which is why I didn't include
> it in the device driver patches. Given Pavel's changes (again), I'm in
> two minds as to whether its needed. It's clearly the right thing to do,
> but not needed at the moment. Then again, as we noted already, the whole

If it is not needed right now, go for simple solution and drop
that patch. Interested people can find it in list archives.
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2004-09-17 20:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-16 10:58 [PATCH] Suspend2 Merge: Driver model patches 2/2 Nigel Cunningham
2004-09-16 14:28 ` Greg KH
2004-09-16 22:18   ` Nigel Cunningham
2004-09-16 22:35     ` Greg KH
2004-09-16 22:49       ` Nigel Cunningham
2004-09-16 23:07         ` Greg KH
2004-09-16 23:19           ` Nigel Cunningham
2004-09-17 19:40             ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.