[PATCH] mach64 atari patch

[PATCH] mach64 atari patch

[James Simmons]

This patch makes the mach64 chip a platform device so sysfs can work with 
it. 

Signed-off-by: James Simmons <jsimmons@infradead.org>

diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/aty/atyfb_base.c fbdev-2.6/drivers/video/aty/atyfb_base.c
--- linus-2.6/drivers/video/aty/atyfb_base.c	2005-07-11 10:07:21.000000000 -0700
+++ fbdev-2.6/drivers/video/aty/atyfb_base.c	2005-07-27 08:41:24.000000000 -0700
@@ -244,9 +244,6 @@
      */
 
 static int aty_init(struct fb_info *info, const char *name);
-#ifdef CONFIG_ATARI
-static int store_video_par(char *videopar, unsigned char m64_num);
-#endif
 
 static struct crtc saved_crtc;
 static union aty_pll saved_pll;
@@ -321,10 +318,8 @@
 #endif
 
 #ifdef CONFIG_ATARI
-static unsigned int mach64_count __initdata = 0;
-static unsigned long phys_vmembase[FB_MAX] __initdata = { 0, };
-static unsigned long phys_size[FB_MAX] __initdata = { 0, };
-static unsigned long phys_guiregbase[FB_MAX] __initdata = { 0, };
+static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num)
+static LIST_HEAD(mach64_list);
 #endif
 
 /* top -> down is an evolution of mach64 chipset, any corrections? */
@@ -2170,8 +2165,6 @@
      *  Initialisation
      */
 
-static struct fb_info *fb_list = NULL;
-
 static int __init aty_init(struct fb_info *info, const char *name)
 {
 	struct atyfb_par *par = (struct atyfb_par *) info->par;
@@ -2562,8 +2555,6 @@
 	if (register_framebuffer(info) < 0)
 		goto aty_init_exit;
 
-	fb_list = info;
-
 	PRINTKI("fb%d: %s frame buffer device on %s\n",
 	       info->node, info->fix.id, name);
 	return 0;
@@ -2587,10 +2578,13 @@
 }
 
 #ifdef CONFIG_ATARI
-static int __init store_video_par(char *video_str, unsigned char m64_num)
+static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num)
 {
-	char *p;
 	unsigned long vmembase, size, guiregbase;
+	struct platform_device *atyfb_device;
+	struct mach64_device *device;
+	struct resource io[2];
+	char *p;
 
 	PRINTKI("store_video_par() '%s' \n", video_str);
 
@@ -2604,16 +2598,18 @@
 		goto mach64_invalid;
 	guiregbase = simple_strtoul(p, NULL, 0);
 
-	phys_vmembase[m64_num] = vmembase;
-	phys_size[m64_num] = size;
-	phys_guiregbase[m64_num] = guiregbase;
-	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size,
-	       guiregbase);
-	return 0;
+	io[0] = request_mem_region(vmembase, size, "atyfb");
+	io[1] = request_mem_region(guiregbase, 0x10000, "atyfb");
 
-      mach64_invalid:
-	phys_vmembase[m64_num] = 0;
-	return -1;
+	atyfb_device = platform_device_register_simple("atyfb", m64_num, io, 2);
+	if (IS_ERR(atyfb_device))
+		return PTR_ERR(atyfb_device);
+
+	device = kmalloc(sizeof(struct mach64_device), GFP_KERNEL);
+	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size, guiregbase);
+	return device;
+mach64_invalid:
+	return NULL;
 }
 #endif /* CONFIG_ATARI */
 
@@ -2679,17 +2675,10 @@
 static void aty_st_pal(u_int regno, u_int red, u_int green, u_int blue,
 		       const struct atyfb_par *par)
 {
-#ifdef CONFIG_ATARI
-	out_8(&par->aty_cmap_regs->windex, regno);
-	out_8(&par->aty_cmap_regs->lut, red);
-	out_8(&par->aty_cmap_regs->lut, green);
-	out_8(&par->aty_cmap_regs->lut, blue);
-#else
 	writeb(regno, &par->aty_cmap_regs->windex);
 	writeb(red, &par->aty_cmap_regs->lut);
 	writeb(green, &par->aty_cmap_regs->lut);
 	writeb(blue, &par->aty_cmap_regs->lut);
-#endif
 }
 
     /*
@@ -3456,47 +3445,43 @@
 
 #ifdef CONFIG_ATARI
 
-static int __devinit atyfb_atari_probe(void)
+static int __devinit atyfb_atari_probe(struct device *device)
 {
-	struct aty_par *par;
+	struct platform_device *dev = to_platform_device(device);
 	struct fb_info *info;
-	int m64_num;
+	struct aty_par *par;
+	int size = 0;
 	u32 clock_r;
 
-	for (m64_num = 0; m64_num < mach64_count; m64_num++) {
-		if (!phys_vmembase[m64_num] || !phys_size[m64_num] ||
-		    !phys_guiregbase[m64_num]) {
-		    PRINTKI("phys_*[%d] parameters not set => returning early. \n", m64_num);
-			continue;
-		}
-
-		info = framebuffer_alloc(sizeof(struct atyfb_par), NULL);
-		if (!info) {
-			PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
-			return -ENOMEM;
-		}
-		par = info->par;
+	info = framebuffer_alloc(sizeof(struct atyfb_par), &dev->dev);
+	if (!info) {
+		PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
+		return -ENOMEM;
+	}
+	par = info->par;
 
-		info->fix = atyfb_fix;
+	info->fix = atyfb_fix;
 
-		par->irq = (unsigned int) -1; /* something invalid */
+	par->irq = (unsigned int) -1; /* something invalid */
 
-		/*
-		 *  Map the video memory (physical address given) to somewhere in the
-		 *  kernel address space.
-		 */
-		info->screen_base = ioremap(phys_vmembase[m64_num], phys_size[m64_num]);
-		info->fix.smem_start = (unsigned long)info->screen_base; /* Fake! */
-		par->ati_regbase = ioremap(phys_guiregbase[m64_num], 0x10000) +
-						0xFC00ul;
-		info->fix.mmio_start = (unsigned long)par->ati_regbase; /* Fake! */
-
-		aty_st_le32(CLOCK_CNTL, 0x12345678, par);
-		clock_r = aty_ld_le32(CLOCK_CNTL, par);
-
-		switch (clock_r & 0x003F) {
-		case 0x12:
-			par->clk_wr_offset = 3; /*  */
+	/*
+	 *  Map the video memory (physical address given) to somewhere in the
+	 *  kernel address space.
+	 */
+	size = dev->resource[0].start - dev->resource[0].end;
+	info->fix.smem_start = dev->resource[0].start;
+	info->screen_base = ioremap(dev->resource[0], size);
+
+	size = 0x10000;
+	info->fix.mmio_start = dev->resource[1].start + 0xFC00ul;
+	par->ati_regbase = ioremap(dev->resource[1], size) + 0xFC00ul;
+
+	aty_st_le32(CLOCK_CNTL, 0x12345678, par);
+	clock_r = aty_ld_le32(CLOCK_CNTL, par);
+
+	switch (clock_r & 0x003F) {
+	case 0x12:
+		par->clk_wr_offset = 3; /*  */
 			break;
 		case 0x34:
 			par->clk_wr_offset = 2; /* Medusa ST-IO ISA Adapter etc. */
@@ -3595,6 +3580,23 @@
 
 #endif /* CONFIG_PCI */
 
+#ifdef	CONFIG_ATARI
+
+static struct device_driver atyfb_driver = {
+	.name		= "atyfb",
+	.probe		= atyfb_atari_probe,
+	.remove		= __devexit_p(atyfb_atari_remove),
+}
+
+static void __devexit atyfb_atari_remove(struct device *dev)
+{
+	struct fb_info *info = dev_get_drvdata(dev);
+
+	atyfb_remove(info);
+}
+
+#endif /* CONFIG_ATARI */
+
 #ifndef MODULE
 static int __init atyfb_setup(char *options)
 {
@@ -3650,15 +3652,15 @@
 		 * Why do we need this silly Mach64 argument?
 		 * We are already here because of mach64= so its redundant.
 		 */
-		else if (MACH_IS_ATARI
-			 && (!strncmp(this_opt, "Mach64:", 7))) {
-			static unsigned char m64_num;
-			static char mach64_str[80];
+		else if (MACH_IS_ATARI && (!strncmp(this_opt, "Mach64:", 7))) {
+			struct mach64_device *dev;
+			unsigned char m64_num = 0;
+			char mach64_str[80];
+
 			strlcpy(mach64_str, this_opt + 7, sizeof(mach64_str));
-			if (!store_video_par(mach64_str, m64_num)) {
-				m64_num++;
-				mach64_count = m64_num;
-			}
+			dev = store_video_par(mach64_str, m64_num++); 
+			if (dev != null)
+				list_add_tail(dev->node, &mach64_list);
 		}
 #endif
 		else
@@ -3670,6 +3672,8 @@
 
 static int __init atyfb_init(void)
 {
+    int retval = 0;	
+
 #ifndef MODULE
     char *option = NULL;
 
@@ -3679,16 +3683,34 @@
 #endif
 
 #ifdef CONFIG_PCI
-    pci_register_driver(&atyfb_driver);
+    retval = pci_register_driver(&atyfb_driver);
 #endif
 #ifdef CONFIG_ATARI
-    atyfb_atari_probe();
+    retval = driver_register(&atyfb_driver);
+    if (retval < 0) {
+	struct platform_device *device;
+
+	list_for_each_safe(device, &mach64_list, node) {
+		platform_device_unregister(&device->dev);
+		kfree(device);
+	}
+    }
 #endif
-    return 0;
+    return retval;
 }
 
 static void __exit atyfb_exit(void)
 {
+#ifdef CONFIG_ATARI
+	struct platform_device *device;
+
+	list_for_each_safe(device, &mach64_list, node) {
+		platform_device_unregister(&device->dev);
+		kfree(device);
+	}
+	driver_unregister(&atyfb_driver);
+#endif
+
 #ifdef CONFIG_PCI
 	pci_unregister_driver(&atyfb_driver);
 #endif
diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/aty/atyfb.h fbdev-2.6/drivers/video/aty/atyfb.h
--- linus-2.6/drivers/video/aty/atyfb.h	2005-07-11 10:07:21.000000000 -0700
+++ fbdev-2.6/drivers/video/aty/atyfb.h	2005-07-22 15:36:41.000000000 -0700
@@ -187,6 +187,13 @@
 #endif
 };
 
+#ifdef CONFIG_ATARI
+struct mach64_device {
+	struct list_head node;
+	struct platform_device *dev;
+};
+#endif
+
     /*
      *  ATI Mach64 features
      */


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] mach64 atari patch

[Geert Uytterhoeven]

On Sun, 7 Aug 2005, James Simmons wrote:
> This patch makes the mach64 chip a platform device so sysfs can work with 
> it. 
> 
> Signed-off-by: James Simmons <jsimmons@infradead.org>
> 
> --- linus-2.6/drivers/video/aty/atyfb_base.c	2005-07-11 10:07:21.000000000 -0700
> +++ fbdev-2.6/drivers/video/aty/atyfb_base.c	2005-07-27 08:41:24.000000000 -0700
> @@ -321,10 +318,8 @@
>  #endif
>  
>  #ifdef CONFIG_ATARI
> -static unsigned int mach64_count __initdata = 0;
> -static unsigned long phys_vmembase[FB_MAX] __initdata = { 0, };
> -static unsigned long phys_size[FB_MAX] __initdata = { 0, };
> -static unsigned long phys_guiregbase[FB_MAX] __initdata = { 0, };
> +static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num)

Missing semicolumn at end of line.

> @@ -2587,10 +2578,13 @@
>  }
>  
>  #ifdef CONFIG_ATARI
> -static int __init store_video_par(char *video_str, unsigned char m64_num)
> +static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num)
>  {
> -	char *p;
>  	unsigned long vmembase, size, guiregbase;
> +	struct platform_device *atyfb_device;
> +	struct mach64_device *device;
> +	struct resource io[2];
> +	char *p;
>  
>  	PRINTKI("store_video_par() '%s' \n", video_str);
>  
> @@ -2604,16 +2598,18 @@
>  		goto mach64_invalid;
>  	guiregbase = simple_strtoul(p, NULL, 0);
>  
> -	phys_vmembase[m64_num] = vmembase;
> -	phys_size[m64_num] = size;
> -	phys_guiregbase[m64_num] = guiregbase;
> -	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size,
> -	       guiregbase);
> -	return 0;
> +	io[0] = request_mem_region(vmembase, size, "atyfb");
> +	io[1] = request_mem_region(guiregbase, 0x10000, "atyfb");

1. request_mem_region(0 returns a pointer to a struct resource, not a struct
   resource.
2. What if request_mem_region() fails and returns NULL?

> -      mach64_invalid:
> -	phys_vmembase[m64_num] = 0;
> -	return -1;
> +	atyfb_device = platform_device_register_simple("atyfb", m64_num, io, 2);
> +	if (IS_ERR(atyfb_device))
> +		return PTR_ERR(atyfb_device);
                       ^^^^^^^
PTR_ERR() returns a long, but we should return a struct mach64_device *.

> @@ -2679,17 +2675,10 @@
>  static void aty_st_pal(u_int regno, u_int red, u_int green, u_int blue,
>  		       const struct atyfb_par *par)
>  {
> -#ifdef CONFIG_ATARI
> -	out_8(&par->aty_cmap_regs->windex, regno);
> -	out_8(&par->aty_cmap_regs->lut, red);
> -	out_8(&par->aty_cmap_regs->lut, green);
> -	out_8(&par->aty_cmap_regs->lut, blue);
> -#else
>  	writeb(regno, &par->aty_cmap_regs->windex);
>  	writeb(red, &par->aty_cmap_regs->lut);
>  	writeb(green, &par->aty_cmap_regs->lut);
>  	writeb(blue, &par->aty_cmap_regs->lut);
> -#endif

Why? writeb() does not exist, because there's no PCI bus.

>  }
>  
>      /*
> @@ -3456,47 +3445,43 @@
>  
>  #ifdef CONFIG_ATARI
>  
> -static int __devinit atyfb_atari_probe(void)
> +static int __devinit atyfb_atari_probe(struct device *device)
>  {
> -	struct aty_par *par;
> +	struct platform_device *dev = to_platform_device(device);
>  	struct fb_info *info;
> -	int m64_num;
> +	struct aty_par *par;
> +	int size = 0;
>  	u32 clock_r;
>  
> -	for (m64_num = 0; m64_num < mach64_count; m64_num++) {
> -		if (!phys_vmembase[m64_num] || !phys_size[m64_num] ||
> -		    !phys_guiregbase[m64_num]) {

The for-loop was removed....

> -		    PRINTKI("phys_*[%d] parameters not set => returning early. \n", m64_num);
> -			continue;
> -		}
> -
> -		info = framebuffer_alloc(sizeof(struct atyfb_par), NULL);
> -		if (!info) {
> -			PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
> -			return -ENOMEM;
> -		}
> -		par = info->par;
> +	info = framebuffer_alloc(sizeof(struct atyfb_par), &dev->dev);
> +	if (!info) {
> +		PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
> +		return -ENOMEM;
> +	}
> +	par = info->par;
>  
> -		info->fix = atyfb_fix;
> +	info->fix = atyfb_fix;
>  
> -		par->irq = (unsigned int) -1; /* something invalid */
> +	par->irq = (unsigned int) -1; /* something invalid */
>  
> -		/*
> -		 *  Map the video memory (physical address given) to somewhere in the
> -		 *  kernel address space.
> -		 */
> -		info->screen_base = ioremap(phys_vmembase[m64_num], phys_size[m64_num]);
> -		info->fix.smem_start = (unsigned long)info->screen_base; /* Fake! */
> -		par->ati_regbase = ioremap(phys_guiregbase[m64_num], 0x10000) +
> -						0xFC00ul;
> -		info->fix.mmio_start = (unsigned long)par->ati_regbase; /* Fake! */
> -
> -		aty_st_le32(CLOCK_CNTL, 0x12345678, par);
> -		clock_r = aty_ld_le32(CLOCK_CNTL, par);
> -
> -		switch (clock_r & 0x003F) {
> -		case 0x12:
> -			par->clk_wr_offset = 3; /*  */
> +	/*
> +	 *  Map the video memory (physical address given) to somewhere in the
> +	 *  kernel address space.
> +	 */
> +	size = dev->resource[0].start - dev->resource[0].end;
	       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	       dev->resource[0].start - dev->resource[0].end + 1;

> +	info->fix.smem_start = dev->resource[0].start;
> +	info->screen_base = ioremap(dev->resource[0], size);
                                    ^^^^^^^^^^^^^^^^
				    dev->resource[0].start

> +
> +	size = 0x10000;
> +	info->fix.mmio_start = dev->resource[1].start + 0xFC00ul;
> +	par->ati_regbase = ioremap(dev->resource[1], size) + 0xFC00ul;
                                   ^^^^^^^^^^^^^^^^
				   dev->resource[1].start

> +
> +	aty_st_le32(CLOCK_CNTL, 0x12345678, par);
> +	clock_r = aty_ld_le32(CLOCK_CNTL, par);
> +
> +	switch (clock_r & 0x003F) {
> +	case 0x12:
> +		par->clk_wr_offset = 3; /*  */
>  			break;
>  		case 0x34:
>  			par->clk_wr_offset = 2; /* Medusa ST-IO ISA Adapter etc. */

... but the closing brace for the for-loop is still there.

> @@ -3595,6 +3580,23 @@
>  
>  #endif /* CONFIG_PCI */
>  
> +#ifdef	CONFIG_ATARI
> +
> +static struct device_driver atyfb_driver = {
> +	.name		= "atyfb",
> +	.probe		= atyfb_atari_probe,
> +	.remove		= __devexit_p(atyfb_atari_remove),

Initialization from incompatible pointer type,
and atyfb_atari_remove is used before its definition, ...

> +}

Missing semicolumn

> +
> +static void __devexit atyfb_atari_remove(struct device *dev)
> +{
> +	struct fb_info *info = dev_get_drvdata(dev);
> +
> +	atyfb_remove(info);
> +}

... so you better move this one up.

> @@ -3650,15 +3652,15 @@
>  		 * Why do we need this silly Mach64 argument?
>  		 * We are already here because of mach64= so its redundant.
>  		 */
> -		else if (MACH_IS_ATARI
> -			 && (!strncmp(this_opt, "Mach64:", 7))) {
> -			static unsigned char m64_num;
> -			static char mach64_str[80];
> +		else if (MACH_IS_ATARI && (!strncmp(this_opt, "Mach64:", 7))) {
> +			struct mach64_device *dev;
> +			unsigned char m64_num = 0;
> +			char mach64_str[80];
> +
>  			strlcpy(mach64_str, this_opt + 7, sizeof(mach64_str));
> -			if (!store_video_par(mach64_str, m64_num)) {
> -				m64_num++;
> -				mach64_count = m64_num;
> -			}
> +			dev = store_video_par(mach64_str, m64_num++); 
> +			if (dev != null)
                                   ^^^^
				   NULL

> +				list_add_tail(dev->node, &mach64_list);

incompatible type for argument 1 of `list_add_tail'

> @@ -3679,16 +3683,34 @@
>  #endif
>  
>  #ifdef CONFIG_PCI
> -    pci_register_driver(&atyfb_driver);
> +    retval = pci_register_driver(&atyfb_driver);
>  #endif
>  #ifdef CONFIG_ATARI
> -    atyfb_atari_probe();
> +    retval = driver_register(&atyfb_driver);
> +    if (retval < 0) {
> +	struct platform_device *device;
> +
> +	list_for_each_safe(device, &mach64_list, node) {

`node' undeclared

> +		platform_device_unregister(&device->dev);
                                           ^^^^^^^^^^^^
					   device ??

> +		kfree(device);
> +	}
> +    }
>  #endif
> -    return 0;
> +    return retval;
>  }
>  
>  static void __exit atyfb_exit(void)
>  {
> +#ifdef CONFIG_ATARI
> +	struct platform_device *device;
> +
> +	list_for_each_safe(device, &mach64_list, node) {

`node' undeclared

> +		platform_device_unregister(&device->dev);
> +		kfree(device);
> +	}

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] mach64 atari patch

[Andrew Morton]

Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Sun, 7 Aug 2005, James Simmons wrote:
> > This patch makes the mach64 chip a platform device so sysfs can work with 
> > it. 
> > 
> > Signed-off-by: James Simmons <jsimmons@infradead.org>
> > 
> > --- linus-2.6/drivers/video/aty/atyfb_base.c	2005-07-11 10:07:21.000000000 -0700
> > +++ fbdev-2.6/drivers/video/aty/atyfb_base.c	2005-07-27 08:41:24.000000000 -0700
> > @@ -321,10 +318,8 @@
> >  #endif
> >  
> >  #ifdef CONFIG_ATARI
> > -static unsigned int mach64_count __initdata = 0;
> > -static unsigned long phys_vmembase[FB_MAX] __initdata = { 0, };
> > -static unsigned long phys_size[FB_MAX] __initdata = { 0, };
> > -static unsigned long phys_guiregbase[FB_MAX] __initdata = { 0, };
> > +static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num)
> 
> Missing semicolumn at end of line.
> 
> [etc]
>

Thanks, Geert.

James, I'll drop this one - it seems to need version 2.

I'll also drop fbdev-dont-allow-softcursor-use-from-userland.patch as there
seems to be quite a bit of controversy there.



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[James Simmons]

> 
> Thanks, Geert.
> 
> James, I'll drop this one - it seems to need version 2.

Yeah!!! I got no response earlier. Now I can fix the patch up.

> I'll also drop fbdev-dont-allow-softcursor-use-from-userland.patch as there
> seems to be quite a bit of controversy there.

The disagreement was how much to change it at one time. Some patches where 
just huge and did massive changes to all drivers. I wanted to break the patch 
up into little pieces. Please apply this patch otherwise we will be stuck 
where we are. 


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Antonino A. Daplas]

James Simmons wrote:
> 
>> I'll also drop fbdev-dont-allow-softcursor-use-from-userland.patch as there
>> seems to be quite a bit of controversy there.
> 
> The disagreement was how much to change it at one time. Some patches where 
> just huge and did massive changes to all drivers. I wanted to break the patch 
> up into little pieces. Please apply this patch otherwise we will be stuck 
> where we are. 
> 

Please, my patch (based from Jon's) is huge because it's removing > 200 lines
and it's moving softcursor.c from drivers/video to drivers/video/console where
it belongs. But it is _not_ intrusive.  In fact, the majority of the patch consists
of a single logical change which removes the line below from each driver

- .fb_cursor = softcursor;

and removes the line below from drivers/video/Kconfig.

- select FB_SOFT_CURSOR

Our proposal aims to make complex code simpler and big code smaller. Your patch,
on the other hand, although small, introduces another flag  FB_HWCURSOR_SOFTCURSOR,
which is redundant, and adds yet another level of complexity to an already too
complex code.

I already have the patch, but I'm not pushing it yet because of an ongoing
disagreement, in courtesy to other developers such as yourself.

Tony


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Jon Smirl]

On 8/7/05, Antonino A. Daplas <adaplas@gmail.com> wrote:
> James Simmons wrote:
> >
> >> I'll also drop fbdev-dont-allow-softcursor-use-from-userland.patch as there
> >> seems to be quite a bit of controversy there.
> >
> > The disagreement was how much to change it at one time. Some patches where
> > just huge and did massive changes to all drivers. I wanted to break the patch
> > up into little pieces. Please apply this patch otherwise we will be stuck
> > where we are.
> >

James, if you are really worried about our patch to move softcursor
into fbcon, why don't we put it into the -mm tree for a month without
moving it to the main kernel and see if anyone complains.

I am definitely of the belief that since softcursor can only be used
by fbcon then it should be moved into fbcon and all support for it be
dropped from fbdev. We can't leave the code as is since it interferes
with the use of the hardware cursors from user space.

I just don't think this patch is as complex as you are making it out
to be. It is large simply because the same action has been repeated
for each of the 65 drivers. The action is only a simple deletion of
two lines.

> Please, my patch (based from Jon's) is huge because it's removing > 200 lines
> and it's moving softcursor.c from drivers/video to drivers/video/console where
> it belongs. But it is _not_ intrusive.  In fact, the majority of the patch consists
> of a single logical change which removes the line below from each driver
> 
> - .fb_cursor = softcursor;
> 
> and removes the line below from drivers/video/Kconfig.
> 
> - select FB_SOFT_CURSOR
> 
> Our proposal aims to make complex code simpler and big code smaller. Your patch,
> on the other hand, although small, introduces another flag  FB_HWCURSOR_SOFTCURSOR,
> which is redundant, and adds yet another level of complexity to an already too
> complex code.
> 
> I already have the patch, but I'm not pushing it yet because of an ongoing
> disagreement, in courtesy to other developers such as yourself.
> 
> Tony

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[James Simmons]

> James, if you are really worried about our patch to move softcursor
> into fbcon, why don't we put it into the -mm tree for a month without
> moving it to the main kernel and see if anyone complains.
> 
> I am definitely of the belief that since softcursor can only be used
> by fbcon then it should be moved into fbcon and all support for it be
> dropped from fbdev. We can't leave the code as is since it interferes
> with the use of the hardware cursors from user space.
> 
> I just don't think this patch is as complex as you are making it out
> to be. It is large simply because the same action has been repeated
> for each of the 65 drivers. The action is only a simple deletion of
> two lines.

I'm not arguing over this. What is wrong with breaking this patch up!!!!!
I still have no answer to this!!!! I'm using the hardware flag because 
sometimes the driver that does have a hardware cursor wants to turn 
support off for it. 


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Jon Smirl]

On 8/8/05, James Simmons <jsimmons@infradead.org> wrote:
> 
> > James, if you are really worried about our patch to move softcursor
> > into fbcon, why don't we put it into the -mm tree for a month without
> > moving it to the main kernel and see if anyone complains.
> >
> > I am definitely of the belief that since softcursor can only be used
> > by fbcon then it should be moved into fbcon and all support for it be
> > dropped from fbdev. We can't leave the code as is since it interferes
> > with the use of the hardware cursors from user space.
> >
> > I just don't think this patch is as complex as you are making it out
> > to be. It is large simply because the same action has been repeated
> > for each of the 65 drivers. The action is only a simple deletion of
> > two lines.
> 
> I'm not arguing over this. What is wrong with breaking this patch up!!!!!
> I still have no answer to this!!!! I'm using the hardware flag because
> sometimes the driver that does have a hardware cursor wants to turn
> support off for it.

Set  .fb_cursor = NULL; to turn off the support.
 
-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[James Simmons]

> > > I just don't think this patch is as complex as you are making it out
> > > to be. It is large simply because the same action has been repeated
> > > for each of the 65 drivers. The action is only a simple deletion of
> > > two lines.
> > 
> > I'm not arguing over this. What is wrong with breaking this patch up!!!!!
> > I still have no answer to this!!!! I'm using the hardware flag because
> > sometimes the driver that does have a hardware cursor wants to turn
> > support off for it.
> 
> Set  .fb_cursor = NULL; to turn off the support.

So forbid hardware drivers that support hware cursors to never turn off 
there hardware cursor support!!!!!

Let see what drivers have a flag to control using the hardware cursor.

au1100fb	int nohwcursor
intelfb		int hwcursor
nvidia		int hwcur
cyberfb

You need to remove this!!!!

What I am arguing is that drivers with hardware cursor support should be 
able to turn off and on hardware cursor support. That is why the 
HWACCEL_CURSOR flag. Your test the fb_cursor field prevents this!!!




-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Jon Smirl]

On 8/8/05, James Simmons <jsimmons@infradead.org> wrote:
> 
> > > > I just don't think this patch is as complex as you are making it out
> > > > to be. It is large simply because the same action has been repeated
> > > > for each of the 65 drivers. The action is only a simple deletion of
> > > > two lines.
> > >
> > > I'm not arguing over this. What is wrong with breaking this patch up!!!!!
> > > I still have no answer to this!!!! I'm using the hardware flag because
> > > sometimes the driver that does have a hardware cursor wants to turn
> > > support off for it.
> >
> > Set  .fb_cursor = NULL; to turn off the support.
> 
> So forbid hardware drivers that support hware cursors to never turn off
> there hardware cursor support!!!!!
> 
> Let see what drivers have a flag to control using the hardware cursor.
> 
> au1100fb        int nohwcursor
> intelfb         int hwcursor
> nvidia          int hwcur
> cyberfb
> 
> You need to remove this!!!!
> 
> What I am arguing is that drivers with hardware cursor support should be
> able to turn off and on hardware cursor support. That is why the
> HWACCEL_CURSOR flag. Your test the fb_cursor field prevents this!!!

If you want fbconsole to use the software cusor instead of the
hardware cursor, that's between you and fbconsole to decide. Control
over that choice needs to be in fbconsole, not the base fbdev.

The fbdev drivers should just unconditionally offer the hardware
cursor if they support it. It is up to the user of the cursor to
choose whether to use it or ignore it.

-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[James Simmons]

> > au1100fb        int nohwcursor
> > intelfb         int hwcursor
> > nvidia          int hwcur
> > cyberfb
> > 
> > You need to remove this!!!!
> > 
> > What I am arguing is that drivers with hardware cursor support should be
> > able to turn off and on hardware cursor support. That is why the
> > HWACCEL_CURSOR flag. Your test the fb_cursor field prevents this!!!
> 
> If you want fbconsole to use the software cusor instead of the
> hardware cursor, that's between you and fbconsole to decide. Control
> over that choice needs to be in fbconsole, not the base fbdev.
> 
> The fbdev drivers should just unconditionally offer the hardware
> cursor if they support it. It is up to the user of the cursor to
> choose whether to use it or ignore it.

Finally you see the point I was making. I wanted it be very clear to every 
one here the implications of your changes. You remove the power to control 
the use of a hardware cursor from the driver. At this point driver writers 
need to speak up if they have no problem with this.

P.S
	The patch still needs to broken into smaller pieces.




-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Jon Smirl]

On 8/8/05, James Simmons <jsimmons@infradead.org> wrote:
> 
> > > au1100fb        int nohwcursor
> > > intelfb         int hwcursor
> > > nvidia          int hwcur
> > > cyberfb
> > >
> > > You need to remove this!!!!
> > >
> > > What I am arguing is that drivers with hardware cursor support should be
> > > able to turn off and on hardware cursor support. That is why the
> > > HWACCEL_CURSOR flag. Your test the fb_cursor field prevents this!!!
> >
> > If you want fbconsole to use the software cusor instead of the
> > hardware cursor, that's between you and fbconsole to decide. Control
> > over that choice needs to be in fbconsole, not the base fbdev.
> >
> > The fbdev drivers should just unconditionally offer the hardware
> > cursor if they support it. It is up to the user of the cursor to
> > choose whether to use it or ignore it.
> 
> Finally you see the point I was making. I wanted it be very clear to every
> one here the implications of your changes. You remove the power to control
> the use of a hardware cursor from the driver. At this point driver writers
> need to speak up if they have no problem with this.

I'm not working on fbconsole so it did not occur to me what your
issues was. My user space apps have always had control of whether they
used the hardware cursor or not.

You will need to ask Tony for a switch. Easiest way is to make it a
module parameter on fbconsole. That way it will appear in
/sys/module/fbconsole/parameters and you can use echo to set it from a
script. There is no need for an ioctl.

It only takes about five lines of code to implement this, something like this...

fbconsole.c

int use_hw_cursor = 0;
module_parm(use_hw_cursor);

if (fb_info->fb_cursor && use_hw_cursor)
  fb_info->fb_cursor(...)
else
  softcursor

> 
> P.S
>         The patch still needs to broken into smaller pieces.
> 
> 
> 


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Jon Smirl]

On 8/8/05, Jon Smirl <jonsmirl@gmail.com> wrote:
> On 8/8/05, James Simmons <jsimmons@infradead.org> wrote:
> >
> > > > au1100fb        int nohwcursor
> > > > intelfb         int hwcursor
> > > > nvidia          int hwcur
> > > > cyberfb
> > > >
> > > > You need to remove this!!!!
> > > >
> > > > What I am arguing is that drivers with hardware cursor support should be
> > > > able to turn off and on hardware cursor support. That is why the
> > > > HWACCEL_CURSOR flag. Your test the fb_cursor field prevents this!!!
> > >
> > > If you want fbconsole to use the software cusor instead of the
> > > hardware cursor, that's between you and fbconsole to decide. Control
> > > over that choice needs to be in fbconsole, not the base fbdev.
> > >
> > > The fbdev drivers should just unconditionally offer the hardware
> > > cursor if they support it. It is up to the user of the cursor to
> > > choose whether to use it or ignore it.
> >
> > Finally you see the point I was making. I wanted it be very clear to every
> > one here the implications of your changes. You remove the power to control
> > the use of a hardware cursor from the driver. At this point driver writers
> > need to speak up if they have no problem with this.
> 
> I'm not working on fbconsole so it did not occur to me what your
> issues was. My user space apps have always had control of whether they
> used the hardware cursor or not.
> 
> You will need to ask Tony for a switch. Easiest way is to make it a
> module parameter on fbconsole. That way it will appear in
> /sys/module/fbconsole/parameters and you can use echo to set it from a
> script. There is no need for an ioctl.

Personally I am not in favor of building hundreds of switches like
this into the kernel. If the hardware cursor doesn't work it should
either be fixed or disabled in the fbdev driver.  If you take that
philosophy there is no need for a switch on fbconsole.

I really don't like the "if it doesn't work for you here's how to turn
if off" switches. All they do is hide bugs.

> 
> It only takes about five lines of code to implement this, something like this...
> 
> fbconsole.c
> 
> int use_hw_cursor = 0;
> module_parm(use_hw_cursor);
> 
> if (fb_info->fb_cursor && use_hw_cursor)
>   fb_info->fb_cursor(...)
> else
>   softcursor
> 
> >
> > P.S
> >         The patch still needs to broken into smaller pieces.
> >
> >
> >
> 
> 
> --
> Jon Smirl
> jonsmirl@gmail.com
> 


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] mach64 atari patch

[Alexander Kern]

Am Sonntag, 7. August 2005 04:27 schrieb James Simmons:

As long it not breaks PCI based Mach64, I am happy
Acked-by: Alexander Kern <alex.kern@gmx.de>
> This patch makes the mach64 chip a platform device so sysfs can work with
> it.
>
> Signed-off-by: James Simmons <jsimmons@infradead.org>
>
> diff -urN -X /home/jsimmons/dontdiff
> linus-2.6/drivers/video/aty/atyfb_base.c
> fbdev-2.6/drivers/video/aty/atyfb_base.c ---
> linus-2.6/drivers/video/aty/atyfb_base.c	2005-07-11 10:07:21.000000000
> -0700 +++ fbdev-2.6/drivers/video/aty/atyfb_base.c	2005-07-27
> 08:41:24.000000000 -0700 @@ -244,9 +244,6 @@
>       */
>
>  static int aty_init(struct fb_info *info, const char *name);
> -#ifdef CONFIG_ATARI
> -static int store_video_par(char *videopar, unsigned char m64_num);
> -#endif
>
>  static struct crtc saved_crtc;
>  static union aty_pll saved_pll;
> @@ -321,10 +318,8 @@
>  #endif
>
>  #ifdef CONFIG_ATARI
> -static unsigned int mach64_count __initdata = 0;
> -static unsigned long phys_vmembase[FB_MAX] __initdata = { 0, };
> -static unsigned long phys_size[FB_MAX] __initdata = { 0, };
> -static unsigned long phys_guiregbase[FB_MAX] __initdata = { 0, };
> +static struct mach64_device* __init store_video_par(char *video_str,
> unsigned char m64_num) +static LIST_HEAD(mach64_list);
>  #endif
>
>  /* top -> down is an evolution of mach64 chipset, any corrections? */
> @@ -2170,8 +2165,6 @@
>       *  Initialisation
>       */
>
> -static struct fb_info *fb_list = NULL;
> -
>  static int __init aty_init(struct fb_info *info, const char *name)
>  {
>  	struct atyfb_par *par = (struct atyfb_par *) info->par;
> @@ -2562,8 +2555,6 @@
>  	if (register_framebuffer(info) < 0)
>  		goto aty_init_exit;
>
> -	fb_list = info;
> -
>  	PRINTKI("fb%d: %s frame buffer device on %s\n",
>  	       info->node, info->fix.id, name);
>  	return 0;
> @@ -2587,10 +2578,13 @@
>  }
>
>  #ifdef CONFIG_ATARI
> -static int __init store_video_par(char *video_str, unsigned char m64_num)
> +static struct mach64_device* __init store_video_par(char *video_str,
> unsigned char m64_num) {
> -	char *p;
>  	unsigned long vmembase, size, guiregbase;
> +	struct platform_device *atyfb_device;
> +	struct mach64_device *device;
> +	struct resource io[2];
> +	char *p;
>
>  	PRINTKI("store_video_par() '%s' \n", video_str);
>
> @@ -2604,16 +2598,18 @@
>  		goto mach64_invalid;
>  	guiregbase = simple_strtoul(p, NULL, 0);
>
> -	phys_vmembase[m64_num] = vmembase;
> -	phys_size[m64_num] = size;
> -	phys_guiregbase[m64_num] = guiregbase;
> -	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size,
> -	       guiregbase);
> -	return 0;
> +	io[0] = request_mem_region(vmembase, size, "atyfb");
> +	io[1] = request_mem_region(guiregbase, 0x10000, "atyfb");
>
> -      mach64_invalid:
> -	phys_vmembase[m64_num] = 0;
> -	return -1;
> +	atyfb_device = platform_device_register_simple("atyfb", m64_num, io, 2);
> +	if (IS_ERR(atyfb_device))
> +		return PTR_ERR(atyfb_device);
> +
> +	device = kmalloc(sizeof(struct mach64_device), GFP_KERNEL);
> +	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size,
> guiregbase); +	return device;
> +mach64_invalid:
> +	return NULL;
>  }
>  #endif /* CONFIG_ATARI */
>
> @@ -2679,17 +2675,10 @@
>  static void aty_st_pal(u_int regno, u_int red, u_int green, u_int blue,
>  		       const struct atyfb_par *par)
>  {
> -#ifdef CONFIG_ATARI
> -	out_8(&par->aty_cmap_regs->windex, regno);
> -	out_8(&par->aty_cmap_regs->lut, red);
> -	out_8(&par->aty_cmap_regs->lut, green);
> -	out_8(&par->aty_cmap_regs->lut, blue);
> -#else
>  	writeb(regno, &par->aty_cmap_regs->windex);
>  	writeb(red, &par->aty_cmap_regs->lut);
>  	writeb(green, &par->aty_cmap_regs->lut);
>  	writeb(blue, &par->aty_cmap_regs->lut);
> -#endif
>  }
>
>      /*
> @@ -3456,47 +3445,43 @@
>
>  #ifdef CONFIG_ATARI
>
> -static int __devinit atyfb_atari_probe(void)
> +static int __devinit atyfb_atari_probe(struct device *device)
>  {
> -	struct aty_par *par;
> +	struct platform_device *dev = to_platform_device(device);
>  	struct fb_info *info;
> -	int m64_num;
> +	struct aty_par *par;
> +	int size = 0;
>  	u32 clock_r;
>
> -	for (m64_num = 0; m64_num < mach64_count; m64_num++) {
> -		if (!phys_vmembase[m64_num] || !phys_size[m64_num] ||
> -		    !phys_guiregbase[m64_num]) {
> -		    PRINTKI("phys_*[%d] parameters not set => returning early. \n",
> m64_num); -			continue;
> -		}
> -
> -		info = framebuffer_alloc(sizeof(struct atyfb_par), NULL);
> -		if (!info) {
> -			PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
> -			return -ENOMEM;
> -		}
> -		par = info->par;
> +	info = framebuffer_alloc(sizeof(struct atyfb_par), &dev->dev);
> +	if (!info) {
> +		PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
> +		return -ENOMEM;
> +	}
> +	par = info->par;
>
> -		info->fix = atyfb_fix;
> +	info->fix = atyfb_fix;
>
> -		par->irq = (unsigned int) -1; /* something invalid */
> +	par->irq = (unsigned int) -1; /* something invalid */
>
> -		/*
> -		 *  Map the video memory (physical address given) to somewhere in the
> -		 *  kernel address space.
> -		 */
> -		info->screen_base = ioremap(phys_vmembase[m64_num], phys_size[m64_num]);
> -		info->fix.smem_start = (unsigned long)info->screen_base; /* Fake! */
> -		par->ati_regbase = ioremap(phys_guiregbase[m64_num], 0x10000) +
> -						0xFC00ul;
> -		info->fix.mmio_start = (unsigned long)par->ati_regbase; /* Fake! */
> -
> -		aty_st_le32(CLOCK_CNTL, 0x12345678, par);
> -		clock_r = aty_ld_le32(CLOCK_CNTL, par);
> -
> -		switch (clock_r & 0x003F) {
> -		case 0x12:
> -			par->clk_wr_offset = 3; /*  */
> +	/*
> +	 *  Map the video memory (physical address given) to somewhere in the
> +	 *  kernel address space.
> +	 */
> +	size = dev->resource[0].start - dev->resource[0].end;
> +	info->fix.smem_start = dev->resource[0].start;
> +	info->screen_base = ioremap(dev->resource[0], size);
> +
> +	size = 0x10000;
> +	info->fix.mmio_start = dev->resource[1].start + 0xFC00ul;
> +	par->ati_regbase = ioremap(dev->resource[1], size) + 0xFC00ul;
> +
> +	aty_st_le32(CLOCK_CNTL, 0x12345678, par);
> +	clock_r = aty_ld_le32(CLOCK_CNTL, par);
> +
> +	switch (clock_r & 0x003F) {
> +	case 0x12:
> +		par->clk_wr_offset = 3; /*  */
>  			break;
>  		case 0x34:
>  			par->clk_wr_offset = 2; /* Medusa ST-IO ISA Adapter etc. */
> @@ -3595,6 +3580,23 @@
>
>  #endif /* CONFIG_PCI */
>
> +#ifdef	CONFIG_ATARI
> +
> +static struct device_driver atyfb_driver = {
> +	.name		= "atyfb",
> +	.probe		= atyfb_atari_probe,
> +	.remove		= __devexit_p(atyfb_atari_remove),
> +}
> +
> +static void __devexit atyfb_atari_remove(struct device *dev)
> +{
> +	struct fb_info *info = dev_get_drvdata(dev);
> +
> +	atyfb_remove(info);
> +}
> +
> +#endif /* CONFIG_ATARI */
> +
>  #ifndef MODULE
>  static int __init atyfb_setup(char *options)
>  {
> @@ -3650,15 +3652,15 @@
>  		 * Why do we need this silly Mach64 argument?
>  		 * We are already here because of mach64= so its redundant.
>  		 */
> -		else if (MACH_IS_ATARI
> -			 && (!strncmp(this_opt, "Mach64:", 7))) {
> -			static unsigned char m64_num;
> -			static char mach64_str[80];
> +		else if (MACH_IS_ATARI && (!strncmp(this_opt, "Mach64:", 7))) {
> +			struct mach64_device *dev;
> +			unsigned char m64_num = 0;
> +			char mach64_str[80];
> +
>  			strlcpy(mach64_str, this_opt + 7, sizeof(mach64_str));
> -			if (!store_video_par(mach64_str, m64_num)) {
> -				m64_num++;
> -				mach64_count = m64_num;
> -			}
> +			dev = store_video_par(mach64_str, m64_num++);
> +			if (dev != null)
> +				list_add_tail(dev->node, &mach64_list);
>  		}
>  #endif
>  		else
> @@ -3670,6 +3672,8 @@
>
>  static int __init atyfb_init(void)
>  {
> +    int retval = 0;
> +
>  #ifndef MODULE
>      char *option = NULL;
>
> @@ -3679,16 +3683,34 @@
>  #endif
>
>  #ifdef CONFIG_PCI
> -    pci_register_driver(&atyfb_driver);
> +    retval = pci_register_driver(&atyfb_driver);
>  #endif
>  #ifdef CONFIG_ATARI
> -    atyfb_atari_probe();
> +    retval = driver_register(&atyfb_driver);
> +    if (retval < 0) {
> +	struct platform_device *device;
> +
> +	list_for_each_safe(device, &mach64_list, node) {
> +		platform_device_unregister(&device->dev);
> +		kfree(device);
> +	}
> +    }
>  #endif
> -    return 0;
> +    return retval;
>  }
>
>  static void __exit atyfb_exit(void)
>  {
> +#ifdef CONFIG_ATARI
> +	struct platform_device *device;
> +
> +	list_for_each_safe(device, &mach64_list, node) {
> +		platform_device_unregister(&device->dev);
> +		kfree(device);
> +	}
> +	driver_unregister(&atyfb_driver);
> +#endif
> +
>  #ifdef CONFIG_PCI
>  	pci_unregister_driver(&atyfb_driver);
>  #endif
> diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/aty/atyfb.h
> fbdev-2.6/drivers/video/aty/atyfb.h ---
> linus-2.6/drivers/video/aty/atyfb.h	2005-07-11 10:07:21.000000000 -0700 +++
> fbdev-2.6/drivers/video/aty/atyfb.h	2005-07-22 15:36:41.000000000 -0700 @@
> -187,6 +187,13 @@
>  #endif
>  };
>
> +#ifdef CONFIG_ATARI
> +struct mach64_device {
> +	struct list_head node;
> +	struct platform_device *dev;
> +};
> +#endif
> +
>      /*
>       *  ATI Mach64 features
>       */
>
>
> -------------------------------------------------------
> SF.Net email is Sponsored by the Better Software Conference & EXPO
> September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
> Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
> Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

-- 

Best Wishes
Mit freundlichen Grüßen

Alex Kern


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Antonino A. Daplas]

James Simmons wrote:
>>>> I just don't think this patch is as complex as you are making it out
>>>> to be. It is large simply because the same action has been repeated
>>>> for each of the 65 drivers. The action is only a simple deletion of
>>>> two lines.
>>> I'm not arguing over this. What is wrong with breaking this patch up!!!!!
>>> I still have no answer to this!!!! I'm using the hardware flag because
>>> sometimes the driver that does have a hardware cursor wants to turn
>>> support off for it.
>> Set  .fb_cursor = NULL; to turn off the support.
> 
> So forbid hardware drivers that support hware cursors to never turn off 
> there hardware cursor support!!!!!
> 
> Let see what drivers have a flag to control using the hardware cursor.
> 
> au1100fb	int nohwcursor
> intelfb		int hwcursor
> nvidia		int hwcur
> cyberfb
> 
> You need to remove this!!!!
>

Majority of the drivers that does support hardware cursors are written by myself.
If the driver does not want to turn on the hardware cursor, just return -ENODEV.
No need to fudge with pointers.

Tony 


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[James Simmons]

> Please, my patch (based from Jon's) is huge because it's removing > 200 lines
> and it's moving softcursor.c from drivers/video to drivers/video/console where
> it belongs. But it is _not_ intrusive.  In fact, the majority of the patch consists
> of a single logical change which removes the line below from each driver
> 
> - .fb_cursor = softcursor;
> 
> and removes the line below from drivers/video/Kconfig.
> 
> - select FB_SOFT_CURSOR
> 
> Our proposal aims to make complex code simpler and big code smaller. Your patch,
> on the other hand, although small, introduces another flag  FB_HWCURSOR_SOFTCURSOR,
> which is redundant, and adds yet another level of complexity to an already too
> complex code.

I ask you the same question as Jon. Currently several fbdev drivers have a 
optional flag to turn on and off the hardware cursor. Should this functionality
be removed and force all fbdev drivers to always have the hardware cursor 
avaiable. Thus leaving fbcon to control when to use the hardware cursor 
or the software cursor. It all comes down to who controls when the 
hardware driver will use the hardware cursor. As noted several drivers 
have such a flag. You would have to strip that flag out of the drivers as 
well.


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Antonino A. Daplas]

James Simmons wrote:
>> Please, my patch (based from Jon's) is huge because it's removing > 200 lines
>> and it's moving softcursor.c from drivers/video to drivers/video/console where
>> it belongs. But it is _not_ intrusive.  In fact, the majority of the patch consists
>> of a single logical change which removes the line below from each driver
>>
>> - .fb_cursor = softcursor;
>>
>> and removes the line below from drivers/video/Kconfig.
>>
>> - select FB_SOFT_CURSOR
>>
>> Our proposal aims to make complex code simpler and big code smaller. Your patch,
>> on the other hand, although small, introduces another flag  FB_HWCURSOR_SOFTCURSOR,
>> which is redundant, and adds yet another level of complexity to an already too
>> complex code.
>>     
>
> I ask you the same question as Jon. Currently several fbdev drivers have a 
> optional flag to turn on and off the hardware cursor. Should this functionality
> be removed and force all fbdev drivers to always have the hardware cursor 
> avaiable. Thus leaving fbcon to control when to use the hardware cursor 
> or the software cursor. It all comes down to who controls when the 
> hardware driver will use the hardware cursor. As noted several drivers 
> have such a flag. You would have to strip that flag out of the drivers as 
> well.
>
> .
>
>   
BTW, the only drivers that have working hardware cursor support are 
rivafb, intelfb,
nvidiafb, and i810fb. Except for intelfb, I wrote all of them, if not 
the driver, the
hardware cursor support. And intelfb's cursor support was based on i810fb.

To answer your question, drivers can do either of two things:

1. If they don't want to use the hardware cursor, set 
info->fbops->fb_cursor = NULL
in fb_set_par();

2. Or, they can keep info->fbops->fb_cursor() as is, but the function 
will return
-ENODEV instead.

The end result is if info->fbops->fb_cursor == NULL, fbcon will call 
soft_cursor.
Or if the return value of info->fb_ops->fb_cursor is not equal to zero, 
fbcon will
call soft_cursor.

Either way, control remains with the driver, not fbcon.

Tony



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Jon Smirl]

On 8/8/05, James Simmons <jsimmons@infradead.org> wrote:
> 
> > Please, my patch (based from Jon's) is huge because it's removing > 200 lines
> > and it's moving softcursor.c from drivers/video to drivers/video/console where
> > it belongs. But it is _not_ intrusive.  In fact, the majority of the patch consists
> > of a single logical change which removes the line below from each driver
> >
> > - .fb_cursor = softcursor;
> >
> > and removes the line below from drivers/video/Kconfig.
> >
> > - select FB_SOFT_CURSOR
> >
> > Our proposal aims to make complex code simpler and big code smaller. Your patch,
> > on the other hand, although small, introduces another flag  FB_HWCURSOR_SOFTCURSOR,
> > which is redundant, and adds yet another level of complexity to an already too
> > complex code.
> 
> I ask you the same question as Jon. Currently several fbdev drivers have a
> optional flag to turn on and off the hardware cursor. Should this functionality

In the current code I don't view that flag as turning the hardware
cursor on/off. Instead I view it as choosing to use the software or
hardware cursor.  In my view the flag should be eliminated form fbdev.
If a driver has a hardware cursor it is always exposed.

fbconsole is then free to implement the flag if it chooses.

> be removed and force all fbdev drivers to always have the hardware cursor
> avaiable. Thus leaving fbcon to control when to use the hardware cursor
> or the software cursor. It all comes down to who controls when the
> hardware driver will use the hardware cursor. As noted several drivers
> have such a flag. You would have to strip that flag out of the drivers as
> well.


-- 
Jon Smirl
jonsmirl@gmail.com


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Antonino A. Daplas]

Antonino A. Daplas wrote:
> James Simmons wrote:
>>> Please, my patch (based from Jon's) is huge because it's removing > 
>>> 200 lines
>>> and it's moving softcursor.c from drivers/video to 
>>> drivers/video/console where
>>> it belongs. But it is _not_ intrusive. In fact, the majority of the 
>>> patch consists
>>> of a single logical change which removes the line below from each 
>>> driver
>>>
>>> - .fb_cursor = softcursor;
>>>
>>> and removes the line below from drivers/video/Kconfig.
>>>
>>> - select FB_SOFT_CURSOR
>>>
>>> Our proposal aims to make complex code simpler and big code smaller. 
>>> Your patch,
>>> on the other hand, although small, introduces another flag 
>>> FB_HWCURSOR_SOFTCURSOR,
>>> which is redundant, and adds yet another level of complexity to an 
>>> already too
>>> complex code.
>>
>> I ask you the same question as Jon. Currently several fbdev drivers 
>> have a optional flag to turn on and off the hardware cursor. Should 
>> this functionality
>> be removed and force all fbdev drivers to always have the hardware 
>> cursor avaiable. Thus leaving fbcon to control when to use the 
>> hardware cursor or the software cursor. It all comes down to who 
>> controls when the hardware driver will use the hardware cursor. As 
>> noted several drivers have such a flag. You would have to strip that 
>> flag out of the drivers as well.
>>
>> .
>>
> BTW, the only drivers that have working hardware cursor support are 
> rivafb, intelfb,
> nvidiafb, and i810fb. Except for intelfb, I wrote all of them, if not 
> the driver, the
> hardware cursor support. And intelfb's cursor support was based on 
> i810fb.
Erratum: Mach64 also has hardware cursor support, which looks like is 
working. So
I take the above statement back.

Tony


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: Re: [PATCH] mach64 atari patch

[Geert Uytterhoeven]

On Tue, 9 Aug 2005, Antonino A. Daplas wrote:
> James Simmons wrote:
> 1. If they don't want to use the hardware cursor, set info->fbops->fb_cursor =
> NULL
> in fb_set_par();

Hence that fbops cannot be shared between multiple instances of the same
frame buffer device that need different values for fbops->fb_cursor.

> 2. Or, they can keep info->fbops->fb_cursor() as is, but the function will
> return
> -ENODEV instead.

So this one looks better to me, since it doesn't suffer from the limitation of
1.

(I'm thinking of hardware where the availability of a hardware cursor depends
 on the video mode, e.g. the good old Amiga graphics)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] mach64 atari patch

[James Simmons]

Give this patch a try. It includes your fixes.

diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/aty/atyfb_base.c fbdev-2.6/drivers/video/aty/atyfb_base.c
--- linus-2.6/drivers/video/aty/atyfb_base.c	2005-07-11 10:07:21.000000000 -0700
+++ fbdev-2.6/drivers/video/aty/atyfb_base.c	2005-08-12 10:37:08.000000000 -0700
@@ -244,9 +244,6 @@
      */
 
 static int aty_init(struct fb_info *info, const char *name);
-#ifdef CONFIG_ATARI
-static int store_video_par(char *videopar, unsigned char m64_num);
-#endif
 
 static struct crtc saved_crtc;
 static union aty_pll saved_pll;
@@ -321,10 +318,8 @@
 #endif
 
 #ifdef CONFIG_ATARI
-static unsigned int mach64_count __initdata = 0;
-static unsigned long phys_vmembase[FB_MAX] __initdata = { 0, };
-static unsigned long phys_size[FB_MAX] __initdata = { 0, };
-static unsigned long phys_guiregbase[FB_MAX] __initdata = { 0, };
+static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num);
+static LIST_HEAD(mach64_list);
 #endif
 
 /* top -> down is an evolution of mach64 chipset, any corrections? */
@@ -2170,8 +2165,6 @@
      *  Initialisation
      */
 
-static struct fb_info *fb_list = NULL;
-
 static int __init aty_init(struct fb_info *info, const char *name)
 {
 	struct atyfb_par *par = (struct atyfb_par *) info->par;
@@ -2562,8 +2555,6 @@
 	if (register_framebuffer(info) < 0)
 		goto aty_init_exit;
 
-	fb_list = info;
-
 	PRINTKI("fb%d: %s frame buffer device on %s\n",
 	       info->node, info->fix.id, name);
 	return 0;
@@ -2586,37 +2577,6 @@
 	return -1;
 }
 
-#ifdef CONFIG_ATARI
-static int __init store_video_par(char *video_str, unsigned char m64_num)
-{
-	char *p;
-	unsigned long vmembase, size, guiregbase;
-
-	PRINTKI("store_video_par() '%s' \n", video_str);
-
-	if (!(p = strsep(&video_str, ";")) || !*p)
-		goto mach64_invalid;
-	vmembase = simple_strtoul(p, NULL, 0);
-	if (!(p = strsep(&video_str, ";")) || !*p)
-		goto mach64_invalid;
-	size = simple_strtoul(p, NULL, 0);
-	if (!(p = strsep(&video_str, ";")) || !*p)
-		goto mach64_invalid;
-	guiregbase = simple_strtoul(p, NULL, 0);
-
-	phys_vmembase[m64_num] = vmembase;
-	phys_size[m64_num] = size;
-	phys_guiregbase[m64_num] = guiregbase;
-	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size,
-	       guiregbase);
-	return 0;
-
-      mach64_invalid:
-	phys_vmembase[m64_num] = 0;
-	return -1;
-}
-#endif /* CONFIG_ATARI */
-
     /*
      *  Blank the display.
      */
@@ -2771,6 +2731,48 @@
 	return 0;
 }
 
+static void __devexit atyfb_remove(struct fb_info *info)
+{
+	struct atyfb_par *par = (struct atyfb_par *) info->par;
+
+	/* restore video mode */
+	aty_set_crtc(par, &saved_crtc);
+	par->pll_ops->set_pll(info, &saved_pll);
+
+	unregister_framebuffer(info);
+
+#ifdef CONFIG_MTRR
+	if (par->mtrr_reg >= 0) {
+	    mtrr_del(par->mtrr_reg, 0, 0);
+	    par->mtrr_reg = -1;
+	}
+	if (par->mtrr_aper >= 0) {
+	    mtrr_del(par->mtrr_aper, 0, 0);
+	    par->mtrr_aper = -1;
+	}
+#endif
+#ifndef __sparc__
+	if (par->ati_regbase)
+		iounmap(par->ati_regbase);
+	if (info->screen_base)
+		iounmap(info->screen_base);
+#ifdef __BIG_ENDIAN
+	if (info->sprite.addr)
+		iounmap(info->sprite.addr);
+#endif
+#endif
+#ifdef __sparc__
+	kfree(par->mmap_map);
+#endif
+	if (par->aux_start)
+		release_mem_region(par->aux_start, par->aux_size);
+
+	if (par->res_start)
+		release_mem_region(par->res_start, par->res_size);
+
+	framebuffer_release(info);
+}
+
 #ifdef CONFIG_PCI
 
 #ifdef __sparc__
@@ -3452,49 +3454,115 @@
 	return rc;
 }
 
+static void __devexit atyfb_pci_remove(struct pci_dev *pdev)
+{
+	struct fb_info *info = pci_get_drvdata(pdev);
+
+	atyfb_remove(info);
+}
+
+/*
+ * This driver uses its own matching table. That will be more difficult
+ * to fix, so for now, we just match against any ATI ID and let the
+ * probe() function find out what's up. That also mean we don't have
+ * a module ID table though.
+ */
+static struct pci_device_id atyfb_pci_tbl[] = {
+	{ PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
+	  PCI_BASE_CLASS_DISPLAY << 16, 0xff0000, 0 },
+	{ 0, }
+};
+
+static struct pci_driver atyfb_driver = {
+	.name		= "atyfb",
+	.id_table	= atyfb_pci_tbl,
+	.probe		= atyfb_pci_probe,
+	.remove		= __devexit_p(atyfb_pci_remove),
+#ifdef CONFIG_PM
+	.suspend	= atyfb_pci_suspend,
+	.resume		= atyfb_pci_resume,
+#endif /* CONFIG_PM */
+};
+
 #endif /* CONFIG_PCI */
 
 #ifdef CONFIG_ATARI
 
-static int __devinit atyfb_atari_probe(void)
+static struct mach64_device* __init store_video_par(char *video_str, unsigned char m64_num)
 {
-	struct aty_par *par;
+	unsigned long vmembase, size, guiregbase;
+	struct platform_device *atyfb_device;
+	struct mach64_device *device;
+	struct resource *io[2];
+	char *p;
+
+	PRINTKI("store_video_par() '%s' \n", video_str);
+
+	if (!(p = strsep(&video_str, ";")) || !*p)
+		goto mach64_invalid;
+	vmembase = simple_strtoul(p, NULL, 0);
+	if (!(p = strsep(&video_str, ";")) || !*p)
+		goto mach64_invalid;
+	size = simple_strtoul(p, NULL, 0);
+	if (!(p = strsep(&video_str, ";")) || !*p)
+		goto mach64_invalid;
+	guiregbase = simple_strtoul(p, NULL, 0);
+
+	io[0] = request_mem_region(vmembase, size, "atyfb");
+	if (!io[0])
+		return NULL;
+	io[1] = request_mem_region(guiregbase, 0x10000, "atyfb");
+	if (!io[1]) {
+		release_resource(io[0]);
+		return NULL;
+	}
+
+	atyfb_device = platform_device_register_simple("atyfb", m64_num, io, 2);
+	if (IS_ERR(atyfb_device))
+		return NULL;
+
+	device = kmalloc(sizeof(struct mach64_device), GFP_KERNEL);
+	PRINTKI("stored them all: $%08lX $%08lX $%08lX \n", vmembase, size, guiregbase);
+	return device;
+mach64_invalid:
+	return NULL;
+}
+
+static int __devinit atyfb_atari_probe(struct device *device)
+{
+	struct platform_device *dev = to_platform_device(device);
 	struct fb_info *info;
-	int m64_num;
+	struct aty_par *par;
+	int size = 0;
 	u32 clock_r;
 
-	for (m64_num = 0; m64_num < mach64_count; m64_num++) {
-		if (!phys_vmembase[m64_num] || !phys_size[m64_num] ||
-		    !phys_guiregbase[m64_num]) {
-		    PRINTKI("phys_*[%d] parameters not set => returning early. \n", m64_num);
-			continue;
-		}
-
-		info = framebuffer_alloc(sizeof(struct atyfb_par), NULL);
-		if (!info) {
-			PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
-			return -ENOMEM;
-		}
-		par = info->par;
+	info = framebuffer_alloc(sizeof(struct atyfb_par), &dev->dev);
+	if (!info) {
+		PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
+		return -ENOMEM;
+	}
+	par = info->par;
 
-		info->fix = atyfb_fix;
+	info->fix = atyfb_fix;
 
-		par->irq = (unsigned int) -1; /* something invalid */
+	par->irq = (unsigned int) -1; /* something invalid */
 
-		/*
-		 *  Map the video memory (physical address given) to somewhere in the
-		 *  kernel address space.
-		 */
-		info->screen_base = ioremap(phys_vmembase[m64_num], phys_size[m64_num]);
-		info->fix.smem_start = (unsigned long)info->screen_base; /* Fake! */
-		par->ati_regbase = ioremap(phys_guiregbase[m64_num], 0x10000) +
-						0xFC00ul;
-		info->fix.mmio_start = (unsigned long)par->ati_regbase; /* Fake! */
+	/*
+	 *  Map the video memory (physical address given) to somewhere in the
+	 *  kernel address space.
+	 */
+	size = dev->resource[0].start - dev->resource[0].end + 1;
+	info->fix.smem_start = dev->resource[0].start;
+	info->screen_base = ioremap(dev->resource[0].start, size);
+
+	size = 0x10000;
+	info->fix.mmio_start = dev->resource[1].start + 0xFC00ul;
+	par->ati_regbase = ioremap(dev->resource[1].start, size) + 0xFC00ul;
 
-		aty_st_le32(CLOCK_CNTL, 0x12345678, par);
-		clock_r = aty_ld_le32(CLOCK_CNTL, par);
+	aty_st_le32(CLOCK_CNTL, 0x12345678, par);
+	clock_r = aty_ld_le32(CLOCK_CNTL, par);
 
-		switch (clock_r & 0x003F) {
+	switch (clock_r & 0x003F) {
 		case 0x12:
 			par->clk_wr_offset = 3; /*  */
 			break;
@@ -3508,92 +3576,29 @@
 			par->clk_wr_offset = 0; /* Panther 1 ISA Adapter (Gerald) */
 			break;
 		}
-
-		if (aty_init(info, "ISA bus")) {
-			framebuffer_release(info);
-			/* This is insufficient! kernel_map has added two large chunks!! */
-			return -ENXIO;
-		}
 	}
-}
-
-#endif /* CONFIG_ATARI */
-
-static void __devexit atyfb_remove(struct fb_info *info)
-{
-	struct atyfb_par *par = (struct atyfb_par *) info->par;
 
-	/* restore video mode */
-	aty_set_crtc(par, &saved_crtc);
-	par->pll_ops->set_pll(info, &saved_pll);
-
-	unregister_framebuffer(info);
-
-#ifdef CONFIG_MTRR
-	if (par->mtrr_reg >= 0) {
-	    mtrr_del(par->mtrr_reg, 0, 0);
-	    par->mtrr_reg = -1;
-	}
-	if (par->mtrr_aper >= 0) {
-	    mtrr_del(par->mtrr_aper, 0, 0);
-	    par->mtrr_aper = -1;
+	if (aty_init(info, "ISA bus")) {
+		framebuffer_release(info);
+		/* This is insufficient! kernel_map has added two large chunks!! */
+		return -ENXIO;
 	}
-#endif
-#ifndef __sparc__
-	if (par->ati_regbase)
-		iounmap(par->ati_regbase);
-	if (info->screen_base)
-		iounmap(info->screen_base);
-#ifdef __BIG_ENDIAN
-	if (info->sprite.addr)
-		iounmap(info->sprite.addr);
-#endif
-#endif
-#ifdef __sparc__
-	kfree(par->mmap_map);
-#endif
-	if (par->aux_start)
-		release_mem_region(par->aux_start, par->aux_size);
-
-	if (par->res_start)
-		release_mem_region(par->res_start, par->res_size);
-
-	framebuffer_release(info);
 }
 
-#ifdef CONFIG_PCI
-
-static void __devexit atyfb_pci_remove(struct pci_dev *pdev)
+static void __devexit atyfb_atari_remove(struct device *dev)
 {
-	struct fb_info *info = pci_get_drvdata(pdev);
+	struct fb_info *info = dev_get_drvdata(dev);
 
 	atyfb_remove(info);
 }
 
-/*
- * This driver uses its own matching table. That will be more difficult
- * to fix, so for now, we just match against any ATI ID and let the
- * probe() function find out what's up. That also mean we don't have
- * a module ID table though.
- */
-static struct pci_device_id atyfb_pci_tbl[] = {
-	{ PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
-	  PCI_BASE_CLASS_DISPLAY << 16, 0xff0000, 0 },
-	{ 0, }
-};
-
-static struct pci_driver atyfb_driver = {
+static struct device_driver atyfb_driver = {
 	.name		= "atyfb",
-	.id_table	= atyfb_pci_tbl,
-	.probe		= atyfb_pci_probe,
-	.remove		= __devexit_p(atyfb_pci_remove),
-#ifdef CONFIG_PM
-	.suspend	= atyfb_pci_suspend,
-	.resume		= atyfb_pci_resume,
-#endif /* CONFIG_PM */
+	.probe		= atyfb_atari_probe,
+	.remove		= __devexit_p(atyfb_atari_remove),
 };
 
-#endif /* CONFIG_PCI */
+#endif /* CONFIG_ATARI */
 
 #ifndef MODULE
 static int __init atyfb_setup(char *options)
@@ -3650,15 +3655,15 @@
 		 * Why do we need this silly Mach64 argument?
 		 * We are already here because of mach64= so its redundant.
 		 */
-		else if (MACH_IS_ATARI
-			 && (!strncmp(this_opt, "Mach64:", 7))) {
-			static unsigned char m64_num;
-			static char mach64_str[80];
+		else if (MACH_IS_ATARI && (!strncmp(this_opt, "Mach64:", 7))) {
+			struct mach64_device *dev;
+			unsigned char m64_num = 0;
+			char mach64_str[80];
+
 			strlcpy(mach64_str, this_opt + 7, sizeof(mach64_str));
-			if (!store_video_par(mach64_str, m64_num)) {
-				m64_num++;
-				mach64_count = m64_num;
-			}
+			dev = store_video_par(mach64_str, m64_num++); 
+			if (dev != NULL)
+				list_add_tail(&dev->node, &mach64_list);
 		}
 #endif
 		else
@@ -3670,6 +3675,8 @@
 
 static int __init atyfb_init(void)
 {
+    int retval = 0;
+
 #ifndef MODULE
     char *option = NULL;
 
@@ -3679,16 +3686,38 @@
 #endif
 
 #ifdef CONFIG_PCI
-    pci_register_driver(&atyfb_driver);
+    retval = pci_register_driver(&atyfb_driver);
 #endif
 #ifdef CONFIG_ATARI
-    atyfb_atari_probe();
+    retval = driver_register(&atyfb_driver);
+    if (retval < 0) {
+	struct mach64_device *device;
+	struct list_head *p, *q;
+
+	list_for_each_safe(p, q, &mach64_list) {
+		device = list_entry(p, struct mach64_device, node);
+		platform_device_unregister(&device->dev);
+		kfree(device);
+	}
+    }
 #endif
-    return 0;
+    return retval;
 }
 
 static void __exit atyfb_exit(void)
 {
+#ifdef CONFIG_ATARI
+	struct mach64_device *device;
+	struct list_head *p, *q;
+
+	list_for_each_safe(p, q, &mach64_list) {
+		device = list_entry(p, struct mach64_device, node);
+		platform_device_unregister(&device->dev);
+		kfree(device);
+	}
+	driver_unregister(&atyfb_driver);
+#endif
+
 #ifdef CONFIG_PCI
 	pci_unregister_driver(&atyfb_driver);
 #endif
@@ -3718,3 +3747,4 @@
 module_param(nomtrr, bool, 0);
 MODULE_PARM_DESC(nomtrr, "bool: disable use of MTRR registers");
 #endif
+
diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/aty/atyfb.h fbdev-2.6/drivers/video/aty/atyfb.h
--- linus-2.6/drivers/video/aty/atyfb.h	2005-07-11 10:07:21.000000000 -0700
+++ fbdev-2.6/drivers/video/aty/atyfb.h	2005-08-12 10:34:13.000000000 -0700
@@ -187,6 +187,13 @@
 #endif
 };
 
+#ifdef CONFIG_ATARI
+struct mach64_device {
+	struct list_head node;
+	struct platform_device *dev;
+};
+#endif
+
     /*
      *  ATI Mach64 features
      */


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf