[alsa-devel] [PATCH 06/34] don't use __devexit_p to wrap hal2_remove
The function hal2_remove is defined using __exit, so don't use __devexit_p but __exit_p to wrap it.
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de Cc: Takashi Iwai tiwai@suse.de Cc: Jaroslav Kysela perex@perex.cz Cc: alsa-devel@alsa-project.org Cc: linux-kernel@vger.kernel.org --- sound/mips/hal2.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/mips/hal2.c b/sound/mips/hal2.c index c52691c..72523a7 100644 --- a/sound/mips/hal2.c +++ b/sound/mips/hal2.c @@ -926,7 +926,7 @@ static int __exit hal2_remove(struct platform_device *pdev)
static struct platform_driver hal2_driver = { .probe = hal2_probe, - .remove = __devexit_p(hal2_remove), + .remove = __exit_p(hal2_remove), .driver = { .name = "sgihal2", .owner = THIS_MODULE,
At Thu, 1 Oct 2009 10:28:10 +0200, Uwe Kleine-König wrote:
The function hal2_remove is defined using __exit, so don't use __devexit_p but __exit_p to wrap it.
I think it's the other way round. We should replace __exit with __devexit. Ditto for sound/mips/sgio2audio.c.
thanks,
Takashi
Signed-off-by: Uwe Kleine-König u.kleine-koenig@pengutronix.de Cc: Takashi Iwai tiwai@suse.de Cc: Jaroslav Kysela perex@perex.cz Cc: alsa-devel@alsa-project.org Cc: linux-kernel@vger.kernel.org
sound/mips/hal2.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/mips/hal2.c b/sound/mips/hal2.c index c52691c..72523a7 100644 --- a/sound/mips/hal2.c +++ b/sound/mips/hal2.c @@ -926,7 +926,7 @@ static int __exit hal2_remove(struct platform_device *pdev)
static struct platform_driver hal2_driver = { .probe = hal2_probe,
- .remove = __devexit_p(hal2_remove),
- .remove = __exit_p(hal2_remove), .driver = { .name = "sgihal2", .owner = THIS_MODULE,
-- 1.6.4.3
On Thu, Oct 01, 2009 at 10:36:59AM +0200, Takashi Iwai wrote:
At Thu, 1 Oct 2009 10:28:10 +0200, Uwe Kleine-König wrote:
The function hal2_remove is defined using __exit, so don't use __devexit_p but __exit_p to wrap it.
I think it's the other way round. We should replace __exit with __devexit. Ditto for sound/mips/sgio2audio.c.
Actually both ways are possible. I choosed the alternative that doesn't add bloat to the kernel. The cost is that the device isn't hotplugable, but you can still unload the module to unbind the driver.
I don't care much, but prefer slightly my approach as changing the patch is work for me :-)
Best regards Uwe
At Thu, 1 Oct 2009 10:53:55 +0200, Uwe Kleine-König wrote:
On Thu, Oct 01, 2009 at 10:36:59AM +0200, Takashi Iwai wrote:
At Thu, 1 Oct 2009 10:28:10 +0200, Uwe Kleine-König wrote:
The function hal2_remove is defined using __exit, so don't use __devexit_p but __exit_p to wrap it.
I think it's the other way round. We should replace __exit with __devexit. Ditto for sound/mips/sgio2audio.c.
Actually both ways are possible. I choosed the alternative that doesn't add bloat to the kernel. The cost is that the device isn't hotplugable, but you can still unload the module to unbind the driver.
Hm, is it really safe to set remove=NULL although the driver needs some work at unbinding? It looks like that unbind is allowed no matter whether remove is NULL or not. So, it would jus keeps stray resources, and it might conflict at the next bind.
I don't care much, but prefer slightly my approach as changing the patch is work for me :-)
I prefer rather symmetry and safety :) I'm going to change to __devexit.
thanks,
Takashi
Hello,
On Fri, Oct 02, 2009 at 11:02:56AM +0200, Takashi Iwai wrote:
At Thu, 1 Oct 2009 10:53:55 +0200, Uwe Kleine-König wrote:
On Thu, Oct 01, 2009 at 10:36:59AM +0200, Takashi Iwai wrote:
At Thu, 1 Oct 2009 10:28:10 +0200, Uwe Kleine-König wrote:
The function hal2_remove is defined using __exit, so don't use __devexit_p but __exit_p to wrap it.
I think it's the other way round. We should replace __exit with __devexit. Ditto for sound/mips/sgio2audio.c.
Actually both ways are possible. I choosed the alternative that doesn't add bloat to the kernel. The cost is that the device isn't hotplugable, but you can still unload the module to unbind the driver.
Hm, is it really safe to set remove=NULL although the driver needs some work at unbinding? It looks like that unbind is allowed no matter whether remove is NULL or not. So, it would jus keeps stray resources, and it might conflict at the next bind.
I just tried that and you're right. IMHO that's a bug. Greg?
Best regards Uwe
At Fri, 2 Oct 2009 11:20:25 +0200, Uwe Kleine-König wrote:
Hello,
On Fri, Oct 02, 2009 at 11:02:56AM +0200, Takashi Iwai wrote:
At Thu, 1 Oct 2009 10:53:55 +0200, Uwe Kleine-König wrote:
On Thu, Oct 01, 2009 at 10:36:59AM +0200, Takashi Iwai wrote:
At Thu, 1 Oct 2009 10:28:10 +0200, Uwe Kleine-König wrote:
The function hal2_remove is defined using __exit, so don't use __devexit_p but __exit_p to wrap it.
I think it's the other way round. We should replace __exit with __devexit. Ditto for sound/mips/sgio2audio.c.
Actually both ways are possible. I choosed the alternative that doesn't add bloat to the kernel. The cost is that the device isn't hotplugable, but you can still unload the module to unbind the driver.
Hm, is it really safe to set remove=NULL although the driver needs some work at unbinding? It looks like that unbind is allowed no matter whether remove is NULL or not. So, it would jus keeps stray resources, and it might conflict at the next bind.
I just tried that and you're right. IMHO that's a bug. Greg?
I suppose it's a bug of the driver, not the core :) If the driver doesn't need to release resources, it would work fine with remove = NULL. Also, the bus can provide a common remove callback (even it's over the driver's remove callback). So, in theory, it can be NULL.
But, it must be really rare, and non-NULL remove is very likely a bug if the driver is built with CONFIG_HOTPLUG=y...
thanks,
Takashi
participants (2)
-
Takashi Iwai -
Uwe Kleine-König