Skip to contentSkip to conversation

video: fbdev: i810: remove incorrect __exit markups

PATCH In review Dmitry Torokhov 2 participants 4 messages Mar 2, 2017 – Mar 20, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 video: fbdev: i810: remove incorrect __exit markups

Even if bus is not hot-pluggable, devices can be unbound from the driver via sysfs, so we should not be using __exit annotations on remove() methods. The only exception is drivers registered with platform_driver_probe() which specifically disables sysfs bind/unbind attributes.

---
 drivers/video/fbdev/i810/i810_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/video/fbdev/i810/i810_main.c +3 -3
index 483ab2592d0c..2488baab7c89 100644
@@ -81,7 +81,7 @@ static u32 voffset;
 static int i810fb_cursor(struct fb_info *info, struct fb_cursor *cursor);
 static int i810fb_init_pci(struct pci_dev *dev,
 			   const struct pci_device_id *entry);
-static void __exit i810fb_remove_pci(struct pci_dev *dev);
+static void i810fb_remove_pci(struct pci_dev *dev);
 static int i810fb_resume(struct pci_dev *dev);
 static int i810fb_suspend(struct pci_dev *dev, pm_message_t state);
 
@@ -128,7 +128,7 @@ static struct pci_driver i810fb_driver = {
 	.name     =	"i810fb",
 	.id_table =	i810fb_pci_tbl,
 	.probe    =	i810fb_init_pci,
-	.remove   =	__exit_p(i810fb_remove_pci),
+	.remove   =	i810fb_remove_pci,
 	.suspend  =     i810fb_suspend,
 	.resume   =     i810fb_resume,
 };
@@ -2123,7 +2123,7 @@ static void i810fb_release_resource(struct fb_info *info,
 
 }
 
-static void __exit i810fb_remove_pci(struct pci_dev *dev)
+static void i810fb_remove_pci(struct pci_dev *dev)
 {
 	struct fb_info *info = pci_get_drvdata(dev);
 	struct i810fb_par *par = info->par;
Signature · 5 lines
2.12.0.rc1.440.g5b76565f74-goog


-- 
Dmitry
#1 lore raw
Quoted 7 lines · Dmitry Torokhov

Even if bus is not hot-pluggable, devices can be unbound from the driver via sysfs, so we should not be using __exit annotations on remove() methods. The only exception is drivers registered with platform_driver_probe() which specifically disables sysfs bind/unbind attributes.

Patch queued for 4.12, thanks.

It seems that there are still some fbdev drivers with this issue:

$ git grep __exit_p drivers/video/fbdev/
drivers/video/fbdev/amifb.c:    .remove = __exit_p(amifb_remove),
drivers/video/fbdev/atmel_lcdfb.c:      .remove         = __exit_p(atmel_lcdfb_remove),
drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c:        .remove = __exit_p(tvc_remove),
drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c:      .remove = __exit_p(dvic_remove),
drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c:     .remove = __exit_p(hdmic_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c:     .remove = __exit_p(opa362_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c:     .remove = __exit_p(tfp410_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c:  .remove = __exit_p(tpd_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c:  .remove = __exit_p(panel_dpi_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c:       .remove = __exit_p(dsicm_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c:    .remove = __exit_p(sharp_ls_remove),
drivers/video/fbdev/omap2/omapfb/vrfb.c:        .remove         = __exit_p(vrfb_remove),

(these drivers don't disable sysfs bind/unbind attributes)

Could you please also take care of fixing them?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

#2 lore raw

Hi Bartlomiej,

Quoted 7 lines · Bartlomiej Zolnierkiewicz
Quoted 6 lines · Dmitry Torokhov

Even if bus is not hot-pluggable, devices can be unbound from the driver via sysfs, so we should not be using __exit annotations on remove() methods. The only exception is drivers registered with platform_driver_probe() which specifically disables sysfs bind/unbind attributes.

Quoted 5 lines
Quoted 1 line

Patch queued for 4.12, thanks.

It seems that there are still some fbdev drivers with this issue:

I do not think so:

Quoted 3 lines
$ git grep __exit_p drivers/video/fbdev/
drivers/video/fbdev/amifb.c:    .remove = __exit_p(amifb_remove),

Registered as module_platform_driver_probe(), which ends up suppressing sysfs bind/unbind attributes.

Quoted 1 line

drivers/video/fbdev/atmel_lcdfb.c: .remove = __exit_p(atmel_lcdfb_remove),

Registered as module_platform_driver_probe(), which ends up suppressing sysfs bind/unbind attributes.

Quoted 9 lines

drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c: .remove = __exit_p(tvc_remove),

drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c:      .remove = __exit_p(dvic_remove),
drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c:     .remove = __exit_p(hdmic_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c:     .remove = __exit_p(opa362_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c:     .remove = __exit_p(tfp410_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c:  .remove = __exit_p(tpd_remove),

drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c: .remove = __exit_p(panel_dpi_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c: .remove = __exit_p(dsicm_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c: .remove = __exit_p(sharp_ls_remove),

As far as I can see all omapfb/displays/ drivers explicitly disable bind/unbind attributes by setting:

.suppress_bind_attrs = true,

Quoted 1 line

drivers/video/fbdev/omap2/omapfb/vrfb.c: .remove = __exit_p(vrfb_remove),

Registered as module_platform_driver_probe(), which ends up suppressing sysfs bind/unbind attributes.

Thanks.

Signature · 1 line
Dmitry
#3 lore raw
Quoted 43 lines · Dmitry Torokhov

Hi Bartlomiej,

Quoted 7 lines · Bartlomiej Zolnierkiewicz
Quoted 6 lines · Dmitry Torokhov

Even if bus is not hot-pluggable, devices can be unbound from the driver via sysfs, so we should not be using __exit annotations on remove() methods. The only exception is drivers registered with platform_driver_probe() which specifically disables sysfs bind/unbind attributes.

Quoted 5 lines
Quoted 1 line

Patch queued for 4.12, thanks.

It seems that there are still some fbdev drivers with this issue:

I do not think so:

Quoted 3 lines
$ git grep __exit_p drivers/video/fbdev/
drivers/video/fbdev/amifb.c:    .remove = __exit_p(amifb_remove),

Registered as module_platform_driver_probe(), which ends up suppressing sysfs bind/unbind attributes.

Quoted 1 line

drivers/video/fbdev/atmel_lcdfb.c: .remove = __exit_p(atmel_lcdfb_remove),

Registered as module_platform_driver_probe(), which ends up suppressing sysfs bind/unbind attributes.

Quoted 9 lines

drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c: .remove = __exit_p(tvc_remove),

drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c:      .remove = __exit_p(dvic_remove),
drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c:     .remove = __exit_p(hdmic_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c:     .remove = __exit_p(opa362_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c:     .remove = __exit_p(tfp410_remove),
drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c:  .remove = __exit_p(tpd_remove),

drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c: .remove = __exit_p(panel_dpi_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c: .remove = __exit_p(dsicm_remove),
drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c: .remove = __exit_p(sharp_ls_remove),

As far as I can see all omapfb/displays/ drivers explicitly disable bind/unbind attributes by setting:

Quoted 7 lines

.suppress_bind_attrs = true,

Quoted 1 line

drivers/video/fbdev/omap2/omapfb/vrfb.c: .remove = __exit_p(vrfb_remove),

Registered as module_platform_driver_probe(), which ends up suppressing sysfs bind/unbind attributes.

You're right of course, thanks for explaining this and auditing all fbdev drivers.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics