On Mon, Oct 9, 2023, at 09:22, Masahiro Yamada wrote:
> On Mon, Oct 9, 2023 at 5:02 AM Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
>>
>> As described in the added code comment, a reference to .exit.text is ok
>> for drivers registered via module_platform_driver_probe(). Make this
>> explicit to prevent a section mismatch warning with
>
> We have thousands of module_platform_drivers.
> I would be scared if they started to add __refdata.
>
> I am not sure if this is the right direction.
For a normal module_platform_driver(), this would indeed be
wrong, but as Uwe said above there is a special case for
module_platform_driver_probe(), which implicitly sets the
drv->driver.suppress_bind_attrs=true flag.
> In my understanding of the current DT overlay,
> there is no way to create/remove a platform device dynamically.
> I do not know if that will happen in the future.
For drivers without suppress_bind_attrs, you can manually
unbind the device from a driver, which in case of a loadable
module ends up calling the .remove callback (this is fine),
but in a built-in driver this would use a NULL pointer for
.remove and cause unexpected behavior.
This is the list of module_platform_driver_probe() instances
I found that have a .remove callback:
drivers/ata/pata_falcon.c- .remove = __exit_p(pata_falcon_remove_one),
drivers/ata/pata_gayle.c- .remove = __exit_p(pata_gayle_remove_one),
drivers/char/hw_random/mxc-rnga.c- .remove = __exit_p(mxc_rnga_remove),
drivers/hwmon/mc13783-adc.c- .remove_new = mc13783_adc_remove,
drivers/input/mouse/amimouse.c- .remove = __exit_p(amimouse_remove),
drivers/input/serio/q40kbd.c- .remove_new = q40kbd_remove,
drivers/input/touchscreen/mc13783_ts.c- .remove_new = mc13783_ts_remove,
drivers/leds/leds-mc13783.c- .remove_new = mc13xxx_led_remove,
drivers/media/platform/renesas/sh_vou.c: .remove_new = sh_vou_remove,
drivers/media/rc/meson-ir-tx.c- .remove_new = meson_irtx_remove,
drivers/memory/emif.c- .remove = __exit_p(emif_remove),
drivers/memory/ti-aemif.c- .remove = aemif_remove,
drivers/memory/ti-emif-pm.c- .remove = ti_emif_remove,
drivers/mtd/devices/docg3.c- .remove = docg3_release,
drivers/net/ethernet/broadcom/tg3.c- .remove = tg3_remove_one,
drivers/rapidio/switches/idt_gen3.c- .remove = idtg3_remove,
drivers/video/fbdev/cg3.c- .remove_new = cg3_remove,
drivers/mtd/nand/raw/fsmc_nand.c- .remove_new = fsmc_nand_remove,
drivers/mtd/nand/raw/orion_nand.c- .remove_new = orion_nand_remove,
drivers/mtd/nand/raw/sh_flctl.c- .remove_new = flctl_remove,
drivers/mtd/nand/raw/txx9ndfmc.c- .remove = __exit_p(txx9ndfmc_remove),
drivers/net/ethernet/cirrus/cs89x0.c- .remove_new = cs89x0_platform_remove,
drivers/parport/parport_amiga.c- .remove = __exit_p(amiga_parallel_remove),
drivers/power/reset/at91-poweroff.c- .remove = __exit_p(at91_poweroff_remove),
drivers/power/reset/at91-reset.c- .remove = __exit_p(at91_reset_remove),
drivers/power/reset/at91-sama5d2_shdwc.c- .remove = __exit_p(at91_shdwc_remove),
drivers/rtc/rtc-at91rm9200.c- .remove = __exit_p(at91_rtc_remove),
drivers/rtc/rtc-at91sam9.c- .remove_new = at91_rtc_remove,
drivers/rtc/rtc-ftrtc010.c- .remove_new = ftrtc010_rtc_remove,
drivers/rtc/rtc-imxdi.c- .remove = __exit_p(dryice_rtc_remove),
drivers/rtc/rtc-mc13xxx.c- .remove_new = mc13xxx_rtc_remove,
drivers/rtc/rtc-mv.c- .remove = __exit_p(mv_rtc_remove),
drivers/rtc/rtc-pcap.c- .remove = __exit_p(pcap_rtc_remove),
drivers/rtc/rtc-pxa.c- .remove = __exit_p(pxa_rtc_remove),
drivers/rtc/rtc-sh.c- .remove = __exit_p(sh_rtc_remove),
drivers/scsi/a3000.c- .remove = __exit_p(amiga_a3000_scsi_remove),
drivers/scsi/a4000t.c- .remove = __exit_p(amiga_a4000t_scsi_remove),
drivers/scsi/atari_scsi.c- .remove = __exit_p(atari_scsi_remove),
drivers/scsi/mac_scsi.c- .remove = __exit_p(mac_scsi_remove),
drivers/scsi/sun3_scsi.c- .remove = __exit_p(sun3_scsi_remove),
drivers/tty/amiserial.c- .remove = __exit_p(amiga_serial_remove),
drivers/usb/gadget/udc/at91_udc.c- .remove = at91udc_remove,
drivers/staging/emxx_udc/emxx_udc.c- .remove_new = nbu2ss_drv_remove,
drivers/usb/gadget/udc/aspeed_udc.c- .remove = ast_udc_remove,
drivers/usb/gadget/udc/at91_udc.c- .remove = at91udc_remove,
drivers/usb/gadget/udc/atmel_usba_udc.c- .remove_new = usba_udc_remove,
drivers/usb/gadget/udc/bcm63xx_udc.c- .remove_new = bcm63xx_udc_remove,
drivers/usb/gadget/udc/dummy_hcd.c- .remove_new = dummy_udc_remove,
drivers/usb/gadget/udc/fsl_qe_udc.c- .remove_new = qe_udc_remove,
drivers/usb/gadget/udc/fsl_udc_core.c- .remove = fsl_udc_remove,
drivers/usb/gadget/udc/lpc32xx_udc.c- .remove = lpc32xx_udc_remove,
drivers/usb/gadget/udc/mv_udc_core.c- .remove_new = mv_udc_remove,
drivers/usb/gadget/udc/omap_udc.c- .remove_new = omap_udc_remove,
drivers/usb/gadget/udc/pch_udc.c- .remove = pch_udc_remove,
drivers/usb/gadget/udc/pxa25x_udc.c- .remove = pxa25x_udc_remove,
drivers/usb/gadget/udc/pxa27x_udc.c- .remove_new = pxa_udc_remove,
drivers/usb/gadget/udc/renesas_usbf.c- .remove_new = usbf_remove,
drivers/usb/gadget/udc/tegra-xudc.c- .remove_new = tegra_xudc_remove,
drivers/usb/gadget/udc/udc-xilinx.c- .remove_new = xudc_remove,
drivers/usb/usbip/vudc_main.c- .remove = vudc_remove,
drivers/usb/gadget/udc/fusb300_udc.c- .remove_new = fusb300_remove,
drivers/usb/gadget/udc/lpc32xx_udc.c- .remove = lpc32xx_udc_remove,
drivers/usb/gadget/udc/m66592-udc.c- .remove_new = m66592_remove,
drivers/usb/gadget/udc/r8a66597-udc.c- .remove_new = r8a66597_remove,
drivers/usb/host/r8a66597-hcd.c- .remove_new = r8a66597_remove,
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/vrfb.c- .remove = __exit_p(vrfb_remove),
drivers/virt/coco/sev-guest/sev-guest.c- .remove = __exit_p(sev_guest_remove),
drivers/watchdog/at91rm9200_wdt.c- .remove_new = at91wdt_remove,
drivers/watchdog/at91sam9_wdt.c- .remove = __exit_p(at91wdt_remove),
drivers/watchdog/txx9wdt.c- .remove = __exit_p(txx9wdt_remove),
Out of these, only 29 have the .remove callback in the .exit.text section,
and they all use __exit_p:
drivers/ata/pata_falcon.c:static int __exit pata_falcon_remove_one(struct platform_device *pdev)
drivers/ata/pata_gayle.c:static int __exit pata_gayle_remove_one(struct platform_device *pdev)
drivers/char/hw_random/mxc-rnga.c:static int __exit mxc_rnga_remove(struct platform_device *pdev)
drivers/input/mouse/amimouse.c:static int __exit amimouse_remove(struct platform_device *pdev)
drivers/memory/emif.c:static int __exit emif_remove(struct platform_device *pdev)
drivers/mtd/nand/raw/txx9ndfmc.c:static int __exit txx9ndfmc_remove(struct platform_device *dev)
drivers/parport/parport_amiga.c:static int __exit amiga_parallel_remove(struct platform_device *pdev)
drivers/power/reset/at91-poweroff.c:static int __exit at91_poweroff_remove(struct platform_device *pdev)
drivers/power/reset/at91-reset.c:static int __exit at91_reset_remove(struct platform_device *pdev)
drivers/power/reset/at91-sama5d2_shdwc.c:static int __exit at91_shdwc_remove(struct platform_device *pdev)
drivers/rtc/rtc-at91rm9200.c:static int __exit at91_rtc_remove(struct platform_device *pdev)
drivers/rtc/rtc-imxdi.c:static int __exit dryice_rtc_remove(struct platform_device *pdev)
drivers/rtc/rtc-mv.c:static int __exit mv_rtc_remove(struct platform_device *pdev)
drivers/rtc/rtc-pcap.c:static int __exit pcap_rtc_remove(struct platform_device *pdev)
drivers/rtc/rtc-pxa.c:static int __exit pxa_rtc_remove(struct platform_device *pdev)
drivers/rtc/rtc-sh.c:static int __exit sh_rtc_remove(struct platform_device *pdev)
drivers/scsi/a3000.c:static int __exit amiga_a3000_scsi_remove(struct platform_device *pdev)
drivers/scsi/a4000t.c:static int __exit amiga_a4000t_scsi_remove(struct platform_device *pdev)
drivers/scsi/atari_scsi.c:static int __exit atari_scsi_remove(struct platform_device *pdev)
drivers/scsi/mac_scsi.c:static int __exit mac_scsi_remove(struct platform_device *pdev)
drivers/scsi/sun3_scsi.c:static int __exit sun3_scsi_remove(struct platform_device *pdev)
drivers/tty/amiserial.c:static int __exit amiga_serial_remove(struct platform_device *pdev)
drivers/video/fbdev/amifb.c:static int __exit amifb_remove(struct platform_device *pdev)
drivers/video/fbdev/atmel_lcdfb.c:static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
drivers/video/fbdev/omap2/omapfb/vrfb.c:static void __exit vrfb_remove(struct platform_device *pdev)
drivers/virt/coco/sev-guest/sev-guest.c:static int __exit sev_guest_remove(struct platform_device *pdev)
drivers/watchdog/at91sam9_wdt.c:static int __exit at91wdt_remove(struct platform_device *pdev)
drivers/watchdog/at91sam9_wdt.c:static int __exit at91wdt_remove(struct platform_device *pdev)
drivers/watchdog/txx9wdt.c:static int __exit txx9wdt_remove(struct platform_device *dev)
These are mostly ancient drivers, and half of them don't even have
DT support, so I think annotating them with __refdata is probably
fine, but removing the __exit_p() and __exit annotations would
also work, depending on the individual maintainer's preferences.
There are three more drivers that set .suppress_bind_addrs=true
and have a __exit_p .remove callback (coresight-etm4x-core.c,
pcie-kirin.c, omapfb), they would also need to make the same
decision to shut up the warning.
All other drivers that have an __exit annotated .remove callback
are in probably broken and need a proper fix.
Arnd
