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

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

[Uwe Kleine-König]

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
-- 
2.42.0

[PATCH 01/22] fb: amifb: Stop using platform_driver_probe()

[Uwe Kleine-König]

On today's platforms the benefit of platform_driver_probe() isn't that
relevant any more. It allows to drop some code after booting (or module
loading) for .probe() and discard the .remove() function completely if
the driver is built-in. This typically saves a few 100k.

The downside of platform_driver_probe() is that the driver cannot be
bound and unbound at runtime which is ancient and also slightly
complicates testing. There are also thoughts to deprecate
platform_driver_probe() because it adds some complexity in the driver
core for little gain. Also many drivers don't use it correctly. This
driver for example misses to mark the driver struct with __refdata which
is needed to suppress a (W=1) modpost warning:

	WARNING: modpost: drivers/video/fbdev/amifb: section mismatch in reference: amifb_driver+0x4 (section: .data) -> amifb_remove (section: .exit.text)

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/amifb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/amifb.c b/drivers/video/fbdev/amifb.c
index b18c6b4f129a..85da43034166 100644
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -3530,7 +3530,7 @@ static inline void chipfree(void)
 	 * Initialisation
 	 */
 
-static int __init amifb_probe(struct platform_device *pdev)
+static int amifb_probe(struct platform_device *pdev)
 {
 	struct fb_info *info;
 	int tag, i, err = 0;
@@ -3752,7 +3752,7 @@ static int __init amifb_probe(struct platform_device *pdev)
 }
 
 
-static int __exit amifb_remove(struct platform_device *pdev)
+static int amifb_remove(struct platform_device *pdev)
 {
 	struct fb_info *info = platform_get_drvdata(pdev);
 
@@ -3769,13 +3769,13 @@ static int __exit amifb_remove(struct platform_device *pdev)
 }
 
 static struct platform_driver amifb_driver = {
-	.remove = __exit_p(amifb_remove),
-	.driver   = {
+	.probe = amifb_probe,
+	.remove = amifb_remove,
+	.driver = {
 		.name	= "amiga-video",
 	},
 };
-
-module_platform_driver_probe(amifb_driver, amifb_probe);
+module_platform_driver(amifb_driver);
 
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:amiga-video");
-- 
2.42.0

Re: [PATCH 01/22] fb: amifb: Stop using platform_driver_probe()

[Geert Uytterhoeven]

Hi Uwe,

On Tue, Nov 7, 2023 at 10:20 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On today's platforms the benefit of platform_driver_probe() isn't that
> relevant any more. It allows to drop some code after booting (or module
> loading) for .probe() and discard the .remove() function completely if
> the driver is built-in. This typically saves a few 100k.

Which is a lot on platforms with only a few MiBs of RAM...

> The downside of platform_driver_probe() is that the driver cannot be
> bound and unbound at runtime which is ancient and also slightly
> complicates testing. There are also thoughts to deprecate
> platform_driver_probe() because it adds some complexity in the driver
> core for little gain. Also many drivers don't use it correctly. This
> driver for example misses to mark the driver struct with __refdata which
> is needed to suppress a (W=1) modpost warning:
>
>         WARNING: modpost: drivers/video/fbdev/amifb: section mismatch in reference: amifb_driver+0x4 (section: .data) -> amifb_remove (section: .exit.text)
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks for your patch!

> --- a/drivers/video/fbdev/amifb.c
> +++ b/drivers/video/fbdev/amifb.c
> @@ -3530,7 +3530,7 @@ static inline void chipfree(void)
>          * Initialisation
>          */
>
> -static int __init amifb_probe(struct platform_device *pdev)
> +static int amifb_probe(struct platform_device *pdev)

noreply@ellerman.id.au reported the following build failure for
e.g. m68k-defconfig in next-20231108:

WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x15c (section: .text) -> ami_modedb (section: .init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x17a (section: .text) -> amifb_hfmin (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x188 (section: .text) -> amifb_hfmax (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x190 (section: .text) -> amifb_vfmin (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x198 (section: .text) -> amifb_vfmax (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x1ba (section: .text) -> ami_modedb (section: .init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x1c4 (section: .text) -> ami_modedb (section: .init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x1ca (section: .text) -> mode_option (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x1ee (section: .text) -> ami_modedb (section: .init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x398 (section: .text) -> amifb_hfmin (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x39e (section: .text) -> amifb_hfmax (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x3a4 (section: .text) -> amifb_vfmin (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x3aa (section: .text) -> amifb_vfmax (section:
.init.data)
WARNING: modpost: vmlinux: section mismatch in reference:
amifb_probe+0x3f0 (section: .text) -> mode_option (section:
.init.data)
ERROR: modpost: Section mismatches detected.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 01/22] fb: amifb: Stop using platform_driver_probe()

[Helge Deller]

On 11/8/23 22:06, Geert Uytterhoeven wrote:
> On Tue, Nov 7, 2023 at 10:20 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
>> On today's platforms the benefit of platform_driver_probe() isn't that
>> relevant any more. It allows to drop some code after booting (or module
>> loading) for .probe() and discard the .remove() function completely if
>> the driver is built-in. This typically saves a few 100k.
>
> Which is a lot on platforms with only a few MiBs of RAM...

True.
Given the warnings below, what is your suggestion?
Better to drop the amifb patch ?

Helge
...
> WARNING: modpost: vmlinux: section mismatch in reference:
> amifb_probe+0x15c (section: .text) -> ami_modedb (section: .init.data)
> ...

Re: [PATCH 01/22] fb: amifb: Stop using platform_driver_probe()

[Geert Uytterhoeven]

Hi Helge,

On Wed, Nov 8, 2023 at 10:32 PM Helge Deller <deller@gmx.de> wrote:
> On 11/8/23 22:06, Geert Uytterhoeven wrote:
> > On Tue, Nov 7, 2023 at 10:20 AM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> >> On today's platforms the benefit of platform_driver_probe() isn't that
> >> relevant any more. It allows to drop some code after booting (or module
> >> loading) for .probe() and discard the .remove() function completely if
> >> the driver is built-in. This typically saves a few 100k.
> >
> > Which is a lot on platforms with only a few MiBs of RAM...
>
> True.
> Given the warnings below, what is your suggestion?
> Better to drop the amifb patch ?

I think so. There is a reason these drivers use platform_driver_probe().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 01/22] fb: amifb: Stop using platform_driver_probe()

[Helge Deller]

On 11/8/23 22:34, Geert Uytterhoeven wrote:
> Hi Helge,
>
> On Wed, Nov 8, 2023 at 10:32 PM Helge Deller <deller@gmx.de> wrote:
>> On 11/8/23 22:06, Geert Uytterhoeven wrote:
>>> On Tue, Nov 7, 2023 at 10:20 AM Uwe Kleine-König
>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>> On today's platforms the benefit of platform_driver_probe() isn't that
>>>> relevant any more. It allows to drop some code after booting (or module
>>>> loading) for .probe() and discard the .remove() function completely if
>>>> the driver is built-in. This typically saves a few 100k.
>>>
>>> Which is a lot on platforms with only a few MiBs of RAM...
>>
>> True.
>> Given the warnings below, what is your suggestion?
>> Better to drop the amifb patch ?
>
> I think so. There is a reason these drivers use platform_driver_probe().

Ok, I've dropped both amifb patches.

Helge

[PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Uwe Kleine-König]

On today's platforms the benefit of platform_driver_probe() isn't that
relevant any more. It allows to drop some code after booting (or module
loading) for .probe() and discard the .remove() function completely if
the driver is built-in. This typically saves a few 100k.

The downside of platform_driver_probe() is that the driver cannot be
bound and unbound at runtime which is ancient and also slightly
complicates testing. There are also thoughts to deprecate
platform_driver_probe() because it adds some complexity in the driver
core for little gain. Also many drivers don't use it correctly. This
driver for example misses to mark the driver struct with __refdata which
is needed to suppress a (W=1) modpost warning:

	WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove (section: .exit.text)

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/atmel_lcdfb.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
index a908db233409..b218731ef732 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
 	return ret;
 }
 
-static int __init atmel_lcdfb_probe(struct platform_device *pdev)
+static int atmel_lcdfb_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fb_info *info;
@@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
+static int atmel_lcdfb_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fb_info *info = dev_get_drvdata(dev);
@@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
 #endif
 
 static struct platform_driver atmel_lcdfb_driver = {
-	.remove		= __exit_p(atmel_lcdfb_remove),
+	.probe		= atmel_lcdfb_probe,
+	.remove		= atmel_lcdfb_remove,
 	.suspend	= atmel_lcdfb_suspend,
 	.resume		= atmel_lcdfb_resume,
 	.driver		= {
@@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
 	},
 };
 
-module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
+module_platform_driver(atmel_lcdfb_driver, );
 
 MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
 MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>");
-- 
2.42.0

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3553 bytes --]

On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote:
> On today's platforms the benefit of platform_driver_probe() isn't that
> relevant any more. It allows to drop some code after booting (or module
> loading) for .probe() and discard the .remove() function completely if
> the driver is built-in. This typically saves a few 100k.
> 
> The downside of platform_driver_probe() is that the driver cannot be
> bound and unbound at runtime which is ancient and also slightly
> complicates testing. There are also thoughts to deprecate
> platform_driver_probe() because it adds some complexity in the driver
> core for little gain. Also many drivers don't use it correctly. This
> driver for example misses to mark the driver struct with __refdata which
> is needed to suppress a (W=1) modpost warning:
> 
> 	WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove (section: .exit.text)
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/video/fbdev/atmel_lcdfb.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
> index a908db233409..b218731ef732 100644
> --- a/drivers/video/fbdev/atmel_lcdfb.c
> +++ b/drivers/video/fbdev/atmel_lcdfb.c
> @@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
>  	return ret;
>  }
>  
> -static int __init atmel_lcdfb_probe(struct platform_device *pdev)
> +static int atmel_lcdfb_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct fb_info *info;
> @@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
> +static int atmel_lcdfb_remove(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct fb_info *info = dev_get_drvdata(dev);
> @@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
>  #endif
>  
>  static struct platform_driver atmel_lcdfb_driver = {
> -	.remove		= __exit_p(atmel_lcdfb_remove),
> +	.probe		= atmel_lcdfb_probe,
> +	.remove		= atmel_lcdfb_remove,
>  	.suspend	= atmel_lcdfb_suspend,
>  	.resume		= atmel_lcdfb_resume,
>  	.driver		= {
> @@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
>  	},
>  };
>  
> -module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
> +module_platform_driver(atmel_lcdfb_driver, );

Argh, the , must be removed. I had this in my working copy but forgot to
squash it into this commit. Sorry!

Can you squash in the following please?:

diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
index 0531d6f6dcc5..88c75ae7d315 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1308,8 +1308,7 @@ static struct platform_driver atmel_lcdfb_driver = {
 		.of_match_table	= atmel_lcdfb_dt_ids,
 	},
 };
-
-module_platform_driver(atmel_lcdfb_driver, );
+module_platform_driver(atmel_lcdfb_driver);
 
 MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
 MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>");

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Helge Deller]

On 11/7/23 21:01, Uwe Kleine-König wrote:
> On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote:
>> On today's platforms the benefit of platform_driver_probe() isn't that
>> relevant any more. It allows to drop some code after booting (or module
>> loading) for .probe() and discard the .remove() function completely if
>> the driver is built-in. This typically saves a few 100k.
>>
>> The downside of platform_driver_probe() is that the driver cannot be
>> bound and unbound at runtime which is ancient and also slightly
>> complicates testing. There are also thoughts to deprecate
>> platform_driver_probe() because it adds some complexity in the driver
>> core for little gain. Also many drivers don't use it correctly. This
>> driver for example misses to mark the driver struct with __refdata which
>> is needed to suppress a (W=1) modpost warning:
>>
>> 	WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove (section: .exit.text)
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>>   drivers/video/fbdev/atmel_lcdfb.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
>> index a908db233409..b218731ef732 100644
>> --- a/drivers/video/fbdev/atmel_lcdfb.c
>> +++ b/drivers/video/fbdev/atmel_lcdfb.c
>> @@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
>>   	return ret;
>>   }
>>
>> -static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>> +static int atmel_lcdfb_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct fb_info *info;
>> @@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>>   	return ret;
>>   }
>>
>> -static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
>> +static int atmel_lcdfb_remove(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct fb_info *info = dev_get_drvdata(dev);
>> @@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
>>   #endif
>>
>>   static struct platform_driver atmel_lcdfb_driver = {
>> -	.remove		= __exit_p(atmel_lcdfb_remove),
>> +	.probe		= atmel_lcdfb_probe,
>> +	.remove		= atmel_lcdfb_remove,
>>   	.suspend	= atmel_lcdfb_suspend,
>>   	.resume		= atmel_lcdfb_resume,
>>   	.driver		= {
>> @@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
>>   	},
>>   };
>>
>> -module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
>> +module_platform_driver(atmel_lcdfb_driver, );
>
> Argh, the , must be removed. I had this in my working copy but forgot to
> squash it into this commit. Sorry!
>
> Can you squash in the following please?:

Sure.
I fixed it up in the git tree.

Thanks!
Helge

> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
> index 0531d6f6dcc5..88c75ae7d315 100644
> --- a/drivers/video/fbdev/atmel_lcdfb.c
> +++ b/drivers/video/fbdev/atmel_lcdfb.c
> @@ -1308,8 +1308,7 @@ static struct platform_driver atmel_lcdfb_driver = {
>   		.of_match_table	= atmel_lcdfb_dt_ids,
>   	},
>   };
> -
> -module_platform_driver(atmel_lcdfb_driver, );
> +module_platform_driver(atmel_lcdfb_driver);
>
>   MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
>   MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>");
>
> Best regards
> Uwe
>

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Nathan Chancellor]

On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote:
> On today's platforms the benefit of platform_driver_probe() isn't that
> relevant any more. It allows to drop some code after booting (or module
> loading) for .probe() and discard the .remove() function completely if
> the driver is built-in. This typically saves a few 100k.
> 
> The downside of platform_driver_probe() is that the driver cannot be
> bound and unbound at runtime which is ancient and also slightly
> complicates testing. There are also thoughts to deprecate
> platform_driver_probe() because it adds some complexity in the driver
> core for little gain. Also many drivers don't use it correctly. This
> driver for example misses to mark the driver struct with __refdata which
> is needed to suppress a (W=1) modpost warning:
> 
> 	WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove (section: .exit.text)
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/video/fbdev/atmel_lcdfb.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
> index a908db233409..b218731ef732 100644
> --- a/drivers/video/fbdev/atmel_lcdfb.c
> +++ b/drivers/video/fbdev/atmel_lcdfb.c
> @@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
>  	return ret;
>  }
>  
> -static int __init atmel_lcdfb_probe(struct platform_device *pdev)
> +static int atmel_lcdfb_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct fb_info *info;
> @@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
> +static int atmel_lcdfb_remove(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct fb_info *info = dev_get_drvdata(dev);
> @@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
>  #endif
>  
>  static struct platform_driver atmel_lcdfb_driver = {
> -	.remove		= __exit_p(atmel_lcdfb_remove),
> +	.probe		= atmel_lcdfb_probe,
> +	.remove		= atmel_lcdfb_remove,
>  	.suspend	= atmel_lcdfb_suspend,
>  	.resume		= atmel_lcdfb_resume,
>  	.driver		= {
> @@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
>  	},
>  };
>  
> -module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
> +module_platform_driver(atmel_lcdfb_driver, );
>  
>  MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
>  MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>");
> -- 
> 2.42.0
> 

For what it's worth, this introduces a warning when building certain
configurations (such as ARCH=arm multi_v5_defconfig) with clang:

  WARNING: modpost: vmlinux: section mismatch in reference: atmel_lcdfb_probe+0x6c4 (section: .text) -> atmel_lcdfb_init_fbinfo (section: .init.text)
  WARNING: modpost: vmlinux: section mismatch in reference: atmel_lcdfb_probe+0x858 (section: .text) -> atmel_lcdfb_fix (section: .init.rodata)

This appears to be legitimate to me? GCC did not warn but I assume that
is due to differences in inlining. The following clears it up for me,
should I send a standalone patch or should this be squashed in?

Cheers,
Nathan

diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
index 88c75ae7d315..9e391e5eaf9d 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int
 	}
 }
 
-static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
+static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
 	.type		= FB_TYPE_PACKED_PIXELS,
 	.visual		= FB_VISUAL_TRUECOLOR,
 	.xpanstep	= 0,
@@ -841,7 +841,7 @@ static void atmel_lcdfb_task(struct work_struct *work)
 	atmel_lcdfb_reset(sinfo);
 }
 
-static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
+static int atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
 {
 	struct fb_info *info = sinfo->info;
 	int ret = 0;

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Helge Deller]

On 11/8/23 19:48, Nathan Chancellor wrote:
> On Tue, Nov 07, 2023 at 10:17:43AM +0100, Uwe Kleine-König wrote:
>> On today's platforms the benefit of platform_driver_probe() isn't that
>> relevant any more. It allows to drop some code after booting (or module
>> loading) for .probe() and discard the .remove() function completely if
>> the driver is built-in. This typically saves a few 100k.
>>
>> The downside of platform_driver_probe() is that the driver cannot be
>> bound and unbound at runtime which is ancient and also slightly
>> complicates testing. There are also thoughts to deprecate
>> platform_driver_probe() because it adds some complexity in the driver
>> core for little gain. Also many drivers don't use it correctly. This
>> driver for example misses to mark the driver struct with __refdata which
>> is needed to suppress a (W=1) modpost warning:
>>
>> 	WARNING: modpost: drivers/video/fbdev/atmel_lcdfb: section mismatch in reference: atmel_lcdfb_driver+0x4 (section: .data) -> atmel_lcdfb_remove (section: .exit.text)
>>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>>   drivers/video/fbdev/atmel_lcdfb.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
>> index a908db233409..b218731ef732 100644
>> --- a/drivers/video/fbdev/atmel_lcdfb.c
>> +++ b/drivers/video/fbdev/atmel_lcdfb.c
>> @@ -1017,7 +1017,7 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
>>   	return ret;
>>   }
>>
>> -static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>> +static int atmel_lcdfb_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct fb_info *info;
>> @@ -1223,7 +1223,7 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
>>   	return ret;
>>   }
>>
>> -static int __exit atmel_lcdfb_remove(struct platform_device *pdev)
>> +static int atmel_lcdfb_remove(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct fb_info *info = dev_get_drvdata(dev);
>> @@ -1301,7 +1301,8 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
>>   #endif
>>
>>   static struct platform_driver atmel_lcdfb_driver = {
>> -	.remove		= __exit_p(atmel_lcdfb_remove),
>> +	.probe		= atmel_lcdfb_probe,
>> +	.remove		= atmel_lcdfb_remove,
>>   	.suspend	= atmel_lcdfb_suspend,
>>   	.resume		= atmel_lcdfb_resume,
>>   	.driver		= {
>> @@ -1310,7 +1311,7 @@ static struct platform_driver atmel_lcdfb_driver = {
>>   	},
>>   };
>>
>> -module_platform_driver_probe(atmel_lcdfb_driver, atmel_lcdfb_probe);
>> +module_platform_driver(atmel_lcdfb_driver, );
>>
>>   MODULE_DESCRIPTION("AT91 LCD Controller framebuffer driver");
>>   MODULE_AUTHOR("Nicolas Ferre <nicolas.ferre@atmel.com>");
>> --
>> 2.42.0
>>
>
> For what it's worth, this introduces a warning when building certain
> configurations (such as ARCH=arm multi_v5_defconfig) with clang:
>
>    WARNING: modpost: vmlinux: section mismatch in reference: atmel_lcdfb_probe+0x6c4 (section: .text) -> atmel_lcdfb_init_fbinfo (section: .init.text)
>    WARNING: modpost: vmlinux: section mismatch in reference: atmel_lcdfb_probe+0x858 (section: .text) -> atmel_lcdfb_fix (section: .init.rodata)
>
> This appears to be legitimate to me? GCC did not warn but I assume that
> is due to differences in inlining. The following clears it up for me,
> should I send a standalone patch or should this be squashed in?

I've squashed it into the original patch.

Thank you!
Helge


> Cheers,
> Nathan
>
> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
> index 88c75ae7d315..9e391e5eaf9d 100644
> --- a/drivers/video/fbdev/atmel_lcdfb.c
> +++ b/drivers/video/fbdev/atmel_lcdfb.c
> @@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int
>   	}
>   }
>
> -static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
> +static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
>   	.type		= FB_TYPE_PACKED_PIXELS,
>   	.visual		= FB_VISUAL_TRUECOLOR,
>   	.xpanstep	= 0,
> @@ -841,7 +841,7 @@ static void atmel_lcdfb_task(struct work_struct *work)
>   	atmel_lcdfb_reset(sinfo);
>   }
>
> -static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
> +static int atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
>   {
>   	struct fb_info *info = sinfo->info;
>   	int ret = 0;

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

Hello,

On Wed, Nov 08, 2023 at 11:48:05AM -0700, Nathan Chancellor wrote:
> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
> index 88c75ae7d315..9e391e5eaf9d 100644
> --- a/drivers/video/fbdev/atmel_lcdfb.c
> +++ b/drivers/video/fbdev/atmel_lcdfb.c
> @@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int
>  	}
>  }
>  
> -static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
> +static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
>  	.type		= FB_TYPE_PACKED_PIXELS,
>  	.visual		= FB_VISUAL_TRUECOLOR,
>  	.xpanstep	= 0,

I wonder if this was broken already before my patch. atmel_lcdfb_probe()
does

	info->fix = atmel_lcdfb_fix;
	
and unless I miss something (this is well possible) that is used e.g. in
atmel_lcdfb_set_par() -> atmel_lcdfb_update_dma(). So atmel_lcdfb_fix
should better not live in .init memory?! Someone with more knowledge
about fbdev might want to take a look and decide if this justifies a
separate fix that should then be backported to stable, too?!

> @@ -841,7 +841,7 @@ static void atmel_lcdfb_task(struct work_struct *work)
>  	atmel_lcdfb_reset(sinfo);
>  }
>  
> -static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
> +static int atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
>  {
>  	struct fb_info *info = sinfo->info;
>  	int ret = 0;

This is only a problem since my patch.

Thanks for your report and patch.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Helge Deller]

On 11/8/23 22:00, Uwe Kleine-König wrote:
> On Wed, Nov 08, 2023 at 11:48:05AM -0700, Nathan Chancellor wrote:
>> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
>> index 88c75ae7d315..9e391e5eaf9d 100644
>> --- a/drivers/video/fbdev/atmel_lcdfb.c
>> +++ b/drivers/video/fbdev/atmel_lcdfb.c
>> @@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int
>>   	}
>>   }
>>
>> -static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
>> +static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
>>   	.type		= FB_TYPE_PACKED_PIXELS,
>>   	.visual		= FB_VISUAL_TRUECOLOR,
>>   	.xpanstep	= 0,
>
> I wonder if this was broken already before my patch. atmel_lcdfb_probe()
> does
>
> 	info->fix = atmel_lcdfb_fix;
>
> and unless I miss something (this is well possible) that is used e.g. in
> atmel_lcdfb_set_par() -> atmel_lcdfb_update_dma(). So atmel_lcdfb_fix
> should better not live in .init memory?! Someone with more knowledge
> about fbdev might want to take a look and decide if this justifies a
> separate fix that should then be backported to stable, too?!

I don't think a backport this is necessary.
The "__initconst" atmel_lcdfb_fix struct was only copied in the
"__init" atmel_lcdfb_probe() function.
So, both were dropped at the same time in older kernels.

Since your patch dropped the "__init" from atmel_lcdfb_probe(),
the __initconst from atmel_lcdfb_fix has to be removed too.

So, I believe folding in Nathan's patch is OK and we don't need
a seperate (or backport) patch.

Helge

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On Wed, Nov 08, 2023 at 10:24:09PM +0100, Helge Deller wrote:
> On 11/8/23 22:00, Uwe Kleine-König wrote:
> > On Wed, Nov 08, 2023 at 11:48:05AM -0700, Nathan Chancellor wrote:
> > > diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
> > > index 88c75ae7d315..9e391e5eaf9d 100644
> > > --- a/drivers/video/fbdev/atmel_lcdfb.c
> > > +++ b/drivers/video/fbdev/atmel_lcdfb.c
> > > @@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int
> > >   	}
> > >   }
> > > 
> > > -static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
> > > +static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
> > >   	.type		= FB_TYPE_PACKED_PIXELS,
> > >   	.visual		= FB_VISUAL_TRUECOLOR,
> > >   	.xpanstep	= 0,
> > 
> > I wonder if this was broken already before my patch. atmel_lcdfb_probe()
> > does
> > 
> > 	info->fix = atmel_lcdfb_fix;
> > 
> > and unless I miss something (this is well possible) that is used e.g. in
> > atmel_lcdfb_set_par() -> atmel_lcdfb_update_dma(). So atmel_lcdfb_fix
> > should better not live in .init memory?! Someone with more knowledge
> > about fbdev might want to take a look and decide if this justifies a
> > separate fix that should then be backported to stable, too?!
> 
> I don't think a backport this is necessary.
> The "__initconst" atmel_lcdfb_fix struct was only copied in the
> "__init" atmel_lcdfb_probe() function.
> So, both were dropped at the same time in older kernels.

But info and so info->fix live longer than the probe function, don't
they? So a call to atmel_lcdfb_update_dma() should better not happen
when .init is already discarded, right?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Helge Deller]

On 11/8/23 22:52, Uwe Kleine-König wrote:
> On Wed, Nov 08, 2023 at 10:24:09PM +0100, Helge Deller wrote:
>> On 11/8/23 22:00, Uwe Kleine-König wrote:
>>> On Wed, Nov 08, 2023 at 11:48:05AM -0700, Nathan Chancellor wrote:
>>>> diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
>>>> index 88c75ae7d315..9e391e5eaf9d 100644
>>>> --- a/drivers/video/fbdev/atmel_lcdfb.c
>>>> +++ b/drivers/video/fbdev/atmel_lcdfb.c
>>>> @@ -220,7 +220,7 @@ static inline void atmel_lcdfb_power_control(struct atmel_lcdfb_info *sinfo, int
>>>>    	}
>>>>    }
>>>>
>>>> -static const struct fb_fix_screeninfo atmel_lcdfb_fix __initconst = {
>>>> +static const struct fb_fix_screeninfo atmel_lcdfb_fix = {
>>>>    	.type		= FB_TYPE_PACKED_PIXELS,
>>>>    	.visual		= FB_VISUAL_TRUECOLOR,
>>>>    	.xpanstep	= 0,
>>>
>>> I wonder if this was broken already before my patch. atmel_lcdfb_probe()
>>> does
>>>
>>> 	info->fix = atmel_lcdfb_fix;
>>>
>>> and unless I miss something (this is well possible) that is used e.g. in
>>> atmel_lcdfb_set_par() -> atmel_lcdfb_update_dma(). So atmel_lcdfb_fix
>>> should better not live in .init memory?! Someone with more knowledge
>>> about fbdev might want to take a look and decide if this justifies a
>>> separate fix that should then be backported to stable, too?!
>>
>> I don't think a backport this is necessary.
>> The "__initconst" atmel_lcdfb_fix struct was only copied in the
>> "__init" atmel_lcdfb_probe() function.
>> So, both were dropped at the same time in older kernels.
>
> But info and so info->fix live longer than the probe function, don't
> they?

Yes, they do.
But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
(and not a pointer to it). So that should be ok.

Helge

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 689 bytes --]

Hello,

On Wed, Nov 08, 2023 at 10:57:00PM +0100, Helge Deller wrote:
> On 11/8/23 22:52, Uwe Kleine-König wrote:
> > But info and so info->fix live longer than the probe function, don't
> > they?
> 
> Yes, they do.
> But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
> (and not a pointer to it). So that should be ok.

If you say so that's good. I grepped a bit around and didn't find a
place where a copy is made. But that's probably me and I'll consider the
case closed.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Helge Deller]

On 11/9/23 07:24, Uwe Kleine-König wrote:
> Hello,
>
> On Wed, Nov 08, 2023 at 10:57:00PM +0100, Helge Deller wrote:
>> On 11/8/23 22:52, Uwe Kleine-König wrote:
>>> But info and so info->fix live longer than the probe function, don't
>>> they?
>>
>> Yes, they do.
>> But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
>> (and not a pointer to it). So that should be ok.
>
> If you say so that's good. I grepped a bit around and didn't find a
> place where a copy is made. But that's probably me and I'll consider the
> case closed.

It's not directly obvious, but the copy happens in the line you pointed
out previously.

In include/linux/fb.h:

struct fb_info {
...
         struct fb_var_screeninfo var;   /* Current var */
         struct fb_fix_screeninfo fix;   /* Current fix */

so, "fb_info.fix" is a struct, and not a pointer.

In drivers/video/fbdev/atmel_lcdfb.c:
static int atmel_lcdfb_probe(struct platform_device *pdev)
{
...
         info->fix = atmel_lcdfb_fix;  //  (line 1065)

this becomes effectively a:
	memcpy(&info->fix, &atmel_lcdfb_fix, sizeof(struct fb_fix_screeninfo));

so, the compiler copies the "__initconst" data over to the info->fix struct.

Helge

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Nicolas Ferre]

On 09/11/2023 at 10:55, Helge Deller wrote:
> On 11/9/23 07:24, Uwe Kleine-König wrote:
>> Hello,
>>
>> On Wed, Nov 08, 2023 at 10:57:00PM +0100, Helge Deller wrote:
>>> On 11/8/23 22:52, Uwe Kleine-König wrote:
>>>> But info and so info->fix live longer than the probe function, don't
>>>> they?
>>>
>>> Yes, they do.
>>> But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
>>> (and not a pointer to it). So that should be ok.
>>
>> If you say so that's good. I grepped a bit around and didn't find a
>> place where a copy is made. But that's probably me and I'll consider the
>> case closed.
> 
> It's not directly obvious, but the copy happens in the line you pointed
> out previously.
> 
> In include/linux/fb.h:
> 
> struct fb_info {
> ...
>           struct fb_var_screeninfo var;   /* Current var */
>           struct fb_fix_screeninfo fix;   /* Current fix */
> 
> so, "fb_info.fix" is a struct, and not a pointer.
> 
> In drivers/video/fbdev/atmel_lcdfb.c:
> static int atmel_lcdfb_probe(struct platform_device *pdev)
> {
> ...
>           info->fix = atmel_lcdfb_fix;  //  (line 1065)
> 
> this becomes effectively a:
>          memcpy(&info->fix, &atmel_lcdfb_fix, sizeof(struct fb_fix_screeninfo));
> 
> so, the compiler copies the "__initconst" data over to the info->fix struct.

Helge, Uwe and Nathan,

Thanks a lot for making this move, patch and detailed explanation.

Best regards,
   Nicolas

Re: [PATCH 02/22] fb: atmel_lcdfb: Stop using platform_driver_probe()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]

Hello Helge,

On Thu, Nov 09, 2023 at 10:55:41AM +0100, Helge Deller wrote:
> On 11/9/23 07:24, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Wed, Nov 08, 2023 at 10:57:00PM +0100, Helge Deller wrote:
> > > On 11/8/23 22:52, Uwe Kleine-König wrote:
> > > > But info and so info->fix live longer than the probe function, don't
> > > > they?
> > > 
> > > Yes, they do.
> > > But AFAICS info->fix contains a *copy* of the initial atmel_lcdfb_fix struct
> > > (and not a pointer to it). So that should be ok.
> > 
> > If you say so that's good. I grepped a bit around and didn't find a
> > place where a copy is made. But that's probably me and I'll consider the
> > case closed.
> 
> It's not directly obvious, but the copy happens in the line you pointed
> out previously.
> 
> In include/linux/fb.h:
> 
> struct fb_info {
> ...
>         struct fb_var_screeninfo var;   /* Current var */
>         struct fb_fix_screeninfo fix;   /* Current fix */
> 
> so, "fb_info.fix" is a struct, and not a pointer.
> 
> In drivers/video/fbdev/atmel_lcdfb.c:
> static int atmel_lcdfb_probe(struct platform_device *pdev)
> {
> ...
>         info->fix = atmel_lcdfb_fix;  //  (line 1065)
> 
> this becomes effectively a:
> 	memcpy(&info->fix, &atmel_lcdfb_fix, sizeof(struct fb_fix_screeninfo));

Ah right. Thanks for that hint. I didn't spot this and grepped for
memcpy and memdup.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 03/22] fb: omapfb/analog-tv: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv: section mismatch in reference: tvc_connector_driver+0x4 (section: .data) -> tvc_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../video/fbdev/omap2/omapfb/displays/connector-analog-tv.c  | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c b/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c
index 0daaf9f89bab..85fa58f48a81 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c
@@ -221,7 +221,7 @@ static int tvc_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit tvc_remove(struct platform_device *pdev)
+static int tvc_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -247,11 +247,10 @@ MODULE_DEVICE_TABLE(of, tvc_of_match);
 
 static struct platform_driver tvc_connector_driver = {
 	.probe	= tvc_probe,
-	.remove	= __exit_p(tvc_remove),
+	.remove	= tvc_remove,
 	.driver	= {
 		.name	= "connector-analog-tv",
 		.of_match_table = tvc_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 04/22] fb: omapfb/dpi: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/panel-dpi: section mismatch in reference: panel_dpi_driver+0x4 (section: .data) -> panel_dpi_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c
index 9790053c5877..aa6faa5ba158 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c
@@ -211,7 +211,7 @@ static int panel_dpi_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit panel_dpi_remove(struct platform_device *pdev)
+static int panel_dpi_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -236,11 +236,10 @@ MODULE_DEVICE_TABLE(of, panel_dpi_of_match);
 
 static struct platform_driver panel_dpi_driver = {
 	.probe = panel_dpi_probe,
-	.remove = __exit_p(panel_dpi_remove),
+	.remove = panel_dpi_remove,
 	.driver = {
 		.name = "panel-dpi",
 		.of_match_table = panel_dpi_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 05/22] fb: omapfb/dsi-cm: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm: section mismatch in reference: dsicm_driver+0x4 (section: .data) -> dsicm_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
index 77fce1223a64..3d0978167144 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
@@ -1241,7 +1241,7 @@ static int dsicm_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit dsicm_remove(struct platform_device *pdev)
+static int dsicm_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -1282,11 +1282,10 @@ MODULE_DEVICE_TABLE(of, dsicm_of_match);
 
 static struct platform_driver dsicm_driver = {
 	.probe = dsicm_probe,
-	.remove = __exit_p(dsicm_remove),
+	.remove = dsicm_remove,
 	.driver = {
 		.name = "panel-dsi-cm",
 		.of_match_table = dsicm_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 06/22] fb: omapfb/dvi: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/connector-dvi: section mismatch in reference: dvi_connector_driver+0x4 (section: .data) -> dvic_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c b/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c
index c8ad3ef42bd3..2a5824fe8ea0 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c
@@ -303,7 +303,7 @@ static int dvic_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit dvic_remove(struct platform_device *pdev)
+static int dvic_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -330,11 +330,10 @@ MODULE_DEVICE_TABLE(of, dvic_of_match);
 
 static struct platform_driver dvi_connector_driver = {
 	.probe	= dvic_probe,
-	.remove	= __exit_p(dvic_remove),
+	.remove	= dvic_remove,
 	.driver	= {
 		.name	= "connector-dvi",
 		.of_match_table = dvic_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 07/22] fb: omapfb/hdmi: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi: section mismatch in reference: hdmi_connector_driver+0x4 (section: .data) -> hdmic_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c b/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c
index 8f9ff9fb4ca4..f76664c69481 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c
@@ -249,7 +249,7 @@ static int hdmic_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit hdmic_remove(struct platform_device *pdev)
+static int hdmic_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -274,11 +274,10 @@ MODULE_DEVICE_TABLE(of, hdmic_of_match);
 
 static struct platform_driver hdmi_connector_driver = {
 	.probe	= hdmic_probe,
-	.remove	= __exit_p(hdmic_remove),
+	.remove	= hdmic_remove,
 	.driver	= {
 		.name	= "connector-hdmi",
 		.of_match_table = hdmic_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 08/22] fb: omapfb/opa362: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410: section mismatch in reference: tfp410_driver+0x4 (section: .data) -> tfp410_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c b/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c
index dd29dc5c77ec..866d71489358 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c
@@ -231,7 +231,7 @@ static int opa362_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit opa362_remove(struct platform_device *pdev)
+static int opa362_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -260,11 +260,10 @@ MODULE_DEVICE_TABLE(of, opa362_of_match);
 
 static struct platform_driver opa362_driver = {
 	.probe	= opa362_probe,
-	.remove	= __exit_p(opa362_remove),
+	.remove	= opa362_remove,
 	.driver	= {
 		.name	= "amplifier-opa362",
 		.of_match_table = opa362_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 09/22] fb: omapfb/sharp-ls037v7dw01: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01: section mismatch in reference: sharp_ls_driver+0x4 (section: .data) -> sharp_ls_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c    | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c
index cc30758300e2..d228d74f3bd5 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c
@@ -292,7 +292,7 @@ static int sharp_ls_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit sharp_ls_remove(struct platform_device *pdev)
+static int sharp_ls_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -317,11 +317,10 @@ MODULE_DEVICE_TABLE(of, sharp_ls_of_match);
 
 static struct platform_driver sharp_ls_driver = {
 	.probe = sharp_ls_probe,
-	.remove = __exit_p(sharp_ls_remove),
+	.remove = sharp_ls_remove,
 	.driver = {
 		.name = "panel-sharp-ls037v7dw01",
 		.of_match_table = sharp_ls_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 10/22] fb: omapfb/tfp410: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410: section mismatch in reference: tfp410_driver+0x4 (section: .data) -> tfp410_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c
index 7bac420169a6..6aa21afc8b21 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c
@@ -217,7 +217,7 @@ static int tfp410_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit tfp410_remove(struct platform_device *pdev)
+static int tfp410_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -247,11 +247,10 @@ MODULE_DEVICE_TABLE(of, tfp410_of_match);
 
 static struct platform_driver tfp410_driver = {
 	.probe	= tfp410_probe,
-	.remove	= __exit_p(tfp410_remove),
+	.remove	= tfp410_remove,
 	.driver	= {
 		.name	= "tfp410",
 		.of_match_table = tfp410_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 11/22] fb: omapfb/tpd12s015: Don't put .remove() in .exit.text and drop suppress_bind_attrs

[Uwe Kleine-König]

On today's platforms the memory savings of putting the remove function
in .exit isn't that relevant any more. It only matters for built-in
drivers and typically saves a few 100k.

The downside is that the driver cannot be unbound at runtime which is
ancient and also slightly complicates testing. Also it requires to mark
the driver struct with __refdata which is needed to suppress a (W=1)
modpost warning:

	WARNING: modpost: drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015: section mismatch in reference: tpd_driver+0x4 (section: .data) -> tpd_remove (section: .exit.text)

To simplify matters, move the remove callback to .text and drop
.suppress_bind_attrs = true.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c    | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c
index 67f0c9250e9e..0bdedc0f6527 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c
@@ -283,7 +283,7 @@ static int tpd_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int __exit tpd_remove(struct platform_device *pdev)
+static int tpd_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -313,11 +313,10 @@ MODULE_DEVICE_TABLE(of, tpd_of_match);
 
 static struct platform_driver tpd_driver = {
 	.probe	= tpd_probe,
-	.remove	= __exit_p(tpd_remove),
+	.remove	= tpd_remove,
 	.driver	= {
 		.name	= "tpd12s015",
 		.of_match_table = tpd_of_match,
-		.suppress_bind_attrs = true,
 	},
 };
 
-- 
2.42.0

[PATCH 12/22] fb: amifb: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/amifb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/amifb.c b/drivers/video/fbdev/amifb.c
index 85da43034166..8ff5d0e0b6e9 100644
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -3752,7 +3752,7 @@ static int amifb_probe(struct platform_device *pdev)
 }
 
 
-static int amifb_remove(struct platform_device *pdev)
+static void amifb_remove(struct platform_device *pdev)
 {
 	struct fb_info *info = platform_get_drvdata(pdev);
 
@@ -3765,12 +3765,11 @@ static int amifb_remove(struct platform_device *pdev)
 	chipfree();
 	framebuffer_release(info);
 	amifb_video_off();
-	return 0;
 }
 
 static struct platform_driver amifb_driver = {
 	.probe = amifb_probe,
-	.remove = amifb_remove,
+	.remove_new = amifb_remove,
 	.driver = {
 		.name	= "amiga-video",
 	},
-- 
2.42.0

[PATCH 13/22] fb: atmel_lcdfb: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/atmel_lcdfb.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
index b218731ef732..0531d6f6dcc5 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1223,14 +1223,14 @@ static int atmel_lcdfb_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int atmel_lcdfb_remove(struct platform_device *pdev)
+static void atmel_lcdfb_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fb_info *info = dev_get_drvdata(dev);
 	struct atmel_lcdfb_info *sinfo;
 
 	if (!info || !info->par)
-		return 0;
+		return;
 	sinfo = info->par;
 
 	cancel_work_sync(&sinfo->task);
@@ -1252,8 +1252,6 @@ static int atmel_lcdfb_remove(struct platform_device *pdev)
 	}
 
 	framebuffer_release(info);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM
@@ -1302,7 +1300,7 @@ static int atmel_lcdfb_resume(struct platform_device *pdev)
 
 static struct platform_driver atmel_lcdfb_driver = {
 	.probe		= atmel_lcdfb_probe,
-	.remove		= atmel_lcdfb_remove,
+	.remove_new	= atmel_lcdfb_remove,
 	.suspend	= atmel_lcdfb_suspend,
 	.resume		= atmel_lcdfb_resume,
 	.driver		= {
-- 
2.42.0

[PATCH 14/22] fb: omapfb/analog-tv: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../video/fbdev/omap2/omapfb/displays/connector-analog-tv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c b/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c
index 85fa58f48a81..c6786726a1af 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/connector-analog-tv.c
@@ -221,7 +221,7 @@ static int tvc_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int tvc_remove(struct platform_device *pdev)
+static void tvc_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -233,8 +233,6 @@ static int tvc_remove(struct platform_device *pdev)
 	tvc_disconnect(dssdev);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id tvc_of_match[] = {
@@ -247,7 +245,7 @@ MODULE_DEVICE_TABLE(of, tvc_of_match);
 
 static struct platform_driver tvc_connector_driver = {
 	.probe	= tvc_probe,
-	.remove	= tvc_remove,
+	.remove_new = tvc_remove,
 	.driver	= {
 		.name	= "connector-analog-tv",
 		.of_match_table = tvc_of_match,
-- 
2.42.0

[PATCH 15/22] fb: omapfb/dpi: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c
index aa6faa5ba158..937f9091274f 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dpi.c
@@ -211,7 +211,7 @@ static int panel_dpi_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int panel_dpi_remove(struct platform_device *pdev)
+static void panel_dpi_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -223,8 +223,6 @@ static int panel_dpi_remove(struct platform_device *pdev)
 	panel_dpi_disconnect(dssdev);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id panel_dpi_of_match[] = {
@@ -236,7 +234,7 @@ MODULE_DEVICE_TABLE(of, panel_dpi_of_match);
 
 static struct platform_driver panel_dpi_driver = {
 	.probe = panel_dpi_probe,
-	.remove = panel_dpi_remove,
+	.remove_new = panel_dpi_remove,
 	.driver = {
 		.name = "panel-dpi",
 		.of_match_table = panel_dpi_of_match,
-- 
2.42.0

[PATCH 16/22] fb: omapfb/dsi-cm: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
index 3d0978167144..adb8881bac28 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-dsi-cm.c
@@ -1241,7 +1241,7 @@ static int dsicm_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int dsicm_remove(struct platform_device *pdev)
+static void dsicm_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -1269,8 +1269,6 @@ static int dsicm_remove(struct platform_device *pdev)
 
 	/* reset, to be sure that the panel is in a valid state */
 	dsicm_hw_reset(ddata);
-
-	return 0;
 }
 
 static const struct of_device_id dsicm_of_match[] = {
@@ -1282,7 +1280,7 @@ MODULE_DEVICE_TABLE(of, dsicm_of_match);
 
 static struct platform_driver dsicm_driver = {
 	.probe = dsicm_probe,
-	.remove = dsicm_remove,
+	.remove_new = dsicm_remove,
 	.driver = {
 		.name = "panel-dsi-cm",
 		.of_match_table = dsicm_of_match,
-- 
2.42.0

[PATCH 17/22] fb: omapfb/dvi: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c b/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c
index 2a5824fe8ea0..0cc9294f89b4 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/connector-dvi.c
@@ -303,7 +303,7 @@ static int dvic_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int dvic_remove(struct platform_device *pdev)
+static void dvic_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -317,8 +317,6 @@ static int dvic_remove(struct platform_device *pdev)
 	omap_dss_put_device(in);
 
 	i2c_put_adapter(ddata->i2c_adapter);
-
-	return 0;
 }
 
 static const struct of_device_id dvic_of_match[] = {
@@ -330,7 +328,7 @@ MODULE_DEVICE_TABLE(of, dvic_of_match);
 
 static struct platform_driver dvi_connector_driver = {
 	.probe	= dvic_probe,
-	.remove	= dvic_remove,
+	.remove_new = dvic_remove,
 	.driver	= {
 		.name	= "connector-dvi",
 		.of_match_table = dvic_of_match,
-- 
2.42.0

[PATCH 18/22] fb: omapfb/hdmi: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c b/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c
index f76664c69481..b862a32670ae 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/connector-hdmi.c
@@ -249,7 +249,7 @@ static int hdmic_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int hdmic_remove(struct platform_device *pdev)
+static void hdmic_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -261,8 +261,6 @@ static int hdmic_remove(struct platform_device *pdev)
 	hdmic_disconnect(dssdev);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id hdmic_of_match[] = {
@@ -274,7 +272,7 @@ MODULE_DEVICE_TABLE(of, hdmic_of_match);
 
 static struct platform_driver hdmi_connector_driver = {
 	.probe	= hdmic_probe,
-	.remove	= hdmic_remove,
+	.remove_new = hdmic_remove,
 	.driver	= {
 		.name	= "connector-hdmi",
 		.of_match_table = hdmic_of_match,
-- 
2.42.0

[PATCH 19/22] fb: omapfb/opa362: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c b/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c
index 866d71489358..f0d3eb581166 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/encoder-opa362.c
@@ -231,7 +231,7 @@ static int opa362_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int opa362_remove(struct platform_device *pdev)
+static void opa362_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -248,8 +248,6 @@ static int opa362_remove(struct platform_device *pdev)
 		opa362_disconnect(dssdev, dssdev->dst);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id opa362_of_match[] = {
@@ -260,7 +258,7 @@ MODULE_DEVICE_TABLE(of, opa362_of_match);
 
 static struct platform_driver opa362_driver = {
 	.probe	= opa362_probe,
-	.remove	= opa362_remove,
+	.remove_new = opa362_remove,
 	.driver	= {
 		.name	= "amplifier-opa362",
 		.of_match_table = opa362_of_match,
-- 
2.42.0

[PATCH 20/22] fb: omapfb/sharp-ls037v7dw01: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c   | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c b/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c
index d228d74f3bd5..e37268cf8dca 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/panel-sharp-ls037v7dw01.c
@@ -292,7 +292,7 @@ static int sharp_ls_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int sharp_ls_remove(struct platform_device *pdev)
+static void sharp_ls_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -304,8 +304,6 @@ static int sharp_ls_remove(struct platform_device *pdev)
 	sharp_ls_disconnect(dssdev);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id sharp_ls_of_match[] = {
@@ -317,7 +315,7 @@ MODULE_DEVICE_TABLE(of, sharp_ls_of_match);
 
 static struct platform_driver sharp_ls_driver = {
 	.probe = sharp_ls_probe,
-	.remove = sharp_ls_remove,
+	.remove_new = sharp_ls_remove,
 	.driver = {
 		.name = "panel-sharp-ls037v7dw01",
 		.of_match_table = sharp_ls_of_match,
-- 
2.42.0

[PATCH 21/22] fb: omapfb/tfp410: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c
index 6aa21afc8b21..c8aca4592949 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tfp410.c
@@ -217,7 +217,7 @@ static int tfp410_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int tfp410_remove(struct platform_device *pdev)
+static void tfp410_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -234,8 +234,6 @@ static int tfp410_remove(struct platform_device *pdev)
 		tfp410_disconnect(dssdev, dssdev->dst);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id tfp410_of_match[] = {
@@ -247,7 +245,7 @@ MODULE_DEVICE_TABLE(of, tfp410_of_match);
 
 static struct platform_driver tfp410_driver = {
 	.probe	= tfp410_probe,
-	.remove	= tfp410_remove,
+	.remove_new = tfp410_remove,
 	.driver	= {
 		.name	= "tfp410",
 		.of_match_table = tfp410_of_match,
-- 
2.42.0

[PATCH 22/22] fb: omapfb/tpd12s015: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c   | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c
index 0bdedc0f6527..eb3926d0361b 100644
--- a/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c
+++ b/drivers/video/fbdev/omap2/omapfb/displays/encoder-tpd12s015.c
@@ -283,7 +283,7 @@ static int tpd_probe(struct platform_device *pdev)
 	return r;
 }
 
-static int tpd_remove(struct platform_device *pdev)
+static void tpd_remove(struct platform_device *pdev)
 {
 	struct panel_drv_data *ddata = platform_get_drvdata(pdev);
 	struct omap_dss_device *dssdev = &ddata->dssdev;
@@ -300,8 +300,6 @@ static int tpd_remove(struct platform_device *pdev)
 		tpd_disconnect(dssdev, dssdev->dst);
 
 	omap_dss_put_device(in);
-
-	return 0;
 }
 
 static const struct of_device_id tpd_of_match[] = {
@@ -313,7 +311,7 @@ MODULE_DEVICE_TABLE(of, tpd_of_match);
 
 static struct platform_driver tpd_driver = {
 	.probe	= tpd_probe,
-	.remove	= tpd_remove,
+	.remove_new = tpd_remove,
 	.driver	= {
 		.name	= "tpd12s015",
 		.of_match_table = tpd_of_match,
-- 
2.42.0

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

[Helge Deller]

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