public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Masahiro Yamada" <masahiroy@kernel.org>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: "Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"David Woodhouse" <David.Woodhouse@intel.com>,
	"Atsushi Nemoto" <anemo@mba.ocn.ne.jp>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	linux-mtd@lists.infradead.org,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	linux-kbuild@vger.kernel.org,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 01/20] mtd: rawnand: txx9ndfmc: Mark driver struct with __refdata to prevent section mismatch warning
Date: Mon, 09 Oct 2023 10:43:46 +0200	[thread overview]
Message-ID: <e38b8a8e-5bd6-41e2-87a1-3b2d23b68bc0@app.fastmail.com> (raw)
In-Reply-To: <CAK7LNASB2HhO6iWNnG-tAzs9wu9mV2PLRf-brnNGkSJj+W23Vw@mail.gmail.com>

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@pengutronix.de> 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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2023-10-09  8:44 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-08 20:01 [PATCH 00/20] mtd: Convert to platform remove callback returning void Uwe Kleine-König
2023-10-08 20:01 ` [PATCH 01/20] mtd: rawnand: txx9ndfmc: Mark driver struct with __refdata to prevent section mismatch warning Uwe Kleine-König
2023-10-09  7:22   ` Masahiro Yamada
2023-10-09  8:43     ` Arnd Bergmann [this message]
2023-10-09 10:30       ` Uwe Kleine-König
2023-10-09 12:46         ` Miquel Raynal
2023-10-09 13:01           ` Arnd Bergmann
2023-10-09 13:37             ` Miquel Raynal
2023-10-16  8:57               ` Miquel Raynal
2023-10-16  9:25         ` Arnd Bergmann
2023-10-16 10:21           ` Uwe Kleine-König
2023-10-17 10:20             ` Masahiro Yamada
2023-10-17 13:20               ` Uwe Kleine-König
2023-10-17 13:45                 ` Greg Kroah-Hartman
2023-10-08 20:01 ` [PATCH 02/20] mtd: rawnand: txx9ndfmc: Drop if block with always false condition Uwe Kleine-König
2023-10-08 20:01 ` [PATCH 03/20] mtd: bcm47xxsflash: Convert to platform remove callback returning void Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 04/20] mtd: docg3: " Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 05/20] mtd: phram: " Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 06/20] mtd: powernv_flash: " Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 07/20] mtd: spear_smi: " Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 08/20] mtd: st_spi_fsm: " Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 09/20] mtd: hyperbus: hbmc-am654: " Uwe Kleine-König
2023-10-16  9:28   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 10/20] mtd: hyperbus: rpc-if: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 11/20] mtd: lpddr2_nvm: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 12/20] mtd: maps: lantiq-flash: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 13/20] mtd: maps: physmap-core: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 14/20] mtd: maps: plat-ram: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 15/20] mtd: maps: pxa2xx-flash: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 16/20] mtd: maps: sa1100-flash: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 17/20] mtd: maps: sun_uflash: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 18/20] mtd: rawnand: txx9ndfmc: " Uwe Kleine-König
2023-10-08 20:01 ` [PATCH 19/20] mtd: spi-nor: hisi-sfc: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-08 20:01 ` [PATCH 20/20] mtd: spi-nor: nxp-spifi: " Uwe Kleine-König
2023-10-16  9:27   ` Miquel Raynal
2023-10-09  6:07 ` [PATCH 00/20] mtd: " Tudor Ambarus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e38b8a8e-5bd6-41e2-87a1-3b2d23b68bc0@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=David.Woodhouse@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=anemo@mba.ocn.ne.jp \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=masahiroy@kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox