Re: radeonfb fix, fixed

[Jeff Garzik]

Dave Jones wrote:
> 
> On Sun, 13 Jan 2002, Erik Andersen wrote:
> 
> > > This patch is needed to make radeonfb compile and work.
> > > It is based on an earlier patch on the list attributed to
> > > Ani Joshi, plus adds the needed devinit fix.
> > Oops.  That patch had some crap in it.  Lets try that again.
> 
> Uncompressed, that patch was just 4kb. When not too big, it's
> considered acceptable (and preferred) to send them as plaintext
> to the list for ease of quoting.
> 
> Patch looks ok, but this bit..
> 
> diff -urN linux/drivers/video.virgin/radeonfb.c
> linux/drivers/video/radeonfb.c
> --- linux/drivers/video.virgin/radeonfb.c   Sun Jan 13 19:09:54 2002
> +++ linux/drivers/video/radeonfb.c  Sun Jan 13 19:41:00 2002
> @@ -686,7 +686,7 @@
>     name:       "radeonfb",
>     id_table:   radeonfb_pci_table,
>     probe:      radeonfb_pci_register,
> -   remove:     radeonfb_pci_unregister,
> +   remove:     __devexit_p(radeonfb_pci_unregister),
>  };
> 
> Is that really needed ?  Hotplugable radeons ?

To rant on a general topic, this wholesale converting drivers to
__devexit without much thought, to fix a simple compile error, may wind
up biting users in the ass later on.  Sometimes, like in the case of
radeonfb and many other fbdev drivers, the driver is -not- able to deal
with all hotplug issues without further fixes.  Fixing radeonfb as with
the above simply hides those problems...

	Jeff

-- 
Jeff Garzik      | Alternate titles for LOTR:
Building 1024    | Fast Times at Uruk-Hai
MandrakeSoft     | The Took, the Elf, His Daughter and Her Lover
                 | Samwise Gamgee: International Hobbit of Mystery