mbox series

[00/22] fb: handle remove callbacks in .exit.text and convert to .remove_new

Message ID 20231107091740.3924258-1-u.kleine-koenig@pengutronix.de
Headers show
Series fb: handle remove callbacks in .exit.text and convert to .remove_new | expand

Message

Uwe Kleine-König Nov. 7, 2023, 9:17 a.m. UTC
Hello,

there are currently several platform drivers that have their .remove
callback defined in .exit.text. While this works fine, it comes with a
few downsides: Since commit f177cd0c15fc ("modpost: Don't let "driver"s
reference .exit.*") it triggers a modpost warning unless the driver
struct is marked with __refdata. None of the drivers in
drivers/video/fbdev get that right (which is understandable the warning
was added only recently). While it would be possible to add that marker,
that's also a bit ugly as this bypasses all other section checks that
modpost does. Having the remove callback in .exit.text also means that
the corresponding devices cannot be unbound at runtime which is
sometimes usefull for debugging purposes.

To fix the modpost warning I picked the progressive option and moved the
.remove() callbacks (and for two drivers also .probe()) into .text (i.e.
the default code section) and dropped .suppress_bind_attrs = true (which
is implicitly set for drivers using platform_driver_probe()).  Note even
though these patches fix a warning, it currently only happens with W=1,
so this isn't urgent and there is no need to apply these before v6.7.
The next merge window is fine (although I wouldn't object an earlier
application of course :-) The alternative is to add the __refdata
marker, ideally with a comment describing the need. (See e.g. commit
141626dbc2e6 ("rtc: sh: Mark driver struct with __refdata to prevent
section mismatch warning") .)

As a follow-up I converted the affected drivers to .remove_new(). There
was already a series doing this for the other drivers in
drivers/video/fb, but my coccinelle script missed these drivers as it
didn't handle

	.remove = __exit_p(removefunc),

. See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal. I
considered creating a second series for this conversion, but as the
patches conflict I put all patches in a single series to make it easier
to apply it.

Best regards
Uwe

Uwe Kleine-König (22):
  fb: amifb: Stop using platform_driver_probe()
  fb: atmel_lcdfb: Stop using platform_driver_probe()
  fb: omapfb/analog-tv: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/dpi: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/dsi-cm: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/dvi: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/hdmi: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/opa362: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/sharp-ls037v7dw01: Don't put .remove() in .exit.text and
    drop suppress_bind_attrs
  fb: omapfb/tfp410: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: omapfb/tpd12s015: Don't put .remove() in .exit.text and drop
    suppress_bind_attrs
  fb: amifb: Convert to platform remove callback returning void
  fb: atmel_lcdfb: Convert to platform remove callback returning void
  fb: omapfb/analog-tv: Convert to platform remove callback returning
    void
  fb: omapfb/dpi: Convert to platform remove callback returning void
  fb: omapfb/dsi-cm: Convert to platform remove callback returning void
  fb: omapfb/dvi: Convert to platform remove callback returning void
  fb: omapfb/hdmi: Convert to platform remove callback returning void
  fb: omapfb/opa362: Convert to platform remove callback returning void
  fb: omapfb/sharp-ls037v7dw01: Convert to platform remove callback
    returning void
  fb: omapfb/tfp410: Convert to platform remove callback returning void
  fb: omapfb/tpd12s015: Convert to platform remove callback returning
    void

 drivers/video/fbdev/amifb.c                         | 13 ++++++-------
 drivers/video/fbdev/atmel_lcdfb.c                   | 13 ++++++-------
 .../omap2/omapfb/displays/connector-analog-tv.c     |  7 ++-----
 .../fbdev/omap2/omapfb/displays/connector-dvi.c     |  7 ++-----
 .../fbdev/omap2/omapfb/displays/connector-hdmi.c    |  7 ++-----
 .../fbdev/omap2/omapfb/displays/encoder-opa362.c    |  7 ++-----
 .../fbdev/omap2/omapfb/displays/encoder-tfp410.c    |  7 ++-----
 .../fbdev/omap2/omapfb/displays/encoder-tpd12s015.c |  7 ++-----
 .../video/fbdev/omap2/omapfb/displays/panel-dpi.c   |  7 ++-----
 .../fbdev/omap2/omapfb/displays/panel-dsi-cm.c      |  7 ++-----
 .../omap2/omapfb/displays/panel-sharp-ls037v7dw01.c |  7 ++-----
 11 files changed, 30 insertions(+), 59 deletions(-)


base-commit: 3ff7a5781ceee3befb9224d29cef6e6a4766c5fe

Comments

Helge Deller Nov. 7, 2023, 1:57 p.m. UTC | #1
On 11/7/23 10:17, Uwe Kleine-König wrote:
> there are currently several platform drivers that have their .remove
> callback defined in .exit.text. While this works fine, it comes with a
> few downsides: Since commit f177cd0c15fc ("modpost: Don't let "driver"s
> reference .exit.*") it triggers a modpost warning unless the driver
> struct is marked with __refdata. None of the drivers in
> drivers/video/fbdev get that right (which is understandable the warning
> was added only recently). While it would be possible to add that marker,
> that's also a bit ugly as this bypasses all other section checks that
> modpost does. Having the remove callback in .exit.text also means that
> the corresponding devices cannot be unbound at runtime which is
> sometimes usefull for debugging purposes.
>
> To fix the modpost warning I picked the progressive option and moved the
> .remove() callbacks (and for two drivers also .probe()) into .text (i.e.
> the default code section) and dropped .suppress_bind_attrs = true (which
> is implicitly set for drivers using platform_driver_probe()).  Note even
> though these patches fix a warning, it currently only happens with W=1,
> so this isn't urgent and there is no need to apply these before v6.7.
> The next merge window is fine (although I wouldn't object an earlier
> application of course :-) The alternative is to add the __refdata
> marker, ideally with a comment describing the need. (See e.g. commit
> 141626dbc2e6 ("rtc: sh: Mark driver struct with __refdata to prevent
> section mismatch warning") .)
>
> As a follow-up I converted the affected drivers to .remove_new(). There
> was already a series doing this for the other drivers in
> drivers/video/fb, but my coccinelle script missed these drivers as it
> didn't handle
>
> 	.remove = __exit_p(removefunc),
>
> . See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal. I
> considered creating a second series for this conversion, but as the
> patches conflict I put all patches in a single series to make it easier
> to apply it.

Thanks Uwe!

I've applied the series as-is to the fbdev for-next git tree.
The patches look ok, and if they survive the next few days they will go
upstream soon.

Helge