Re: radeonfb fix, fixed

Jeff Garzik (jgarzik@mandrakesoft.com)
Mon, 14 Jan 2002 18:55:51 -0500


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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/