Hello,
there are two different types of patches here that would justify to
different series. But as the patches are not independant I chose to put
them in a single series.
The first three patches drop usage of platform_driver_probe(). This is a
concept that isn't so relevant any more today. I didn't check, but it
saves typically only a few 100k and there are thoughts to deprecate it
to simplify the core. Getting the usage right is not trivial though the
at91 drivers got it nearly right. The alternative to these patches is to
add __ref to the driver struct ideally with a comment describing the
need like is e.g. done in commit 5b44abbc39ca ("platform/x86: hp-wmi::
Mark driver struct with __refdata to prevent section mismatch warning").
The remaining patches convert the platform drivers to .remove_new(), see
commit 5c5a7680e67b ("platform: Provide a remove callback that returns
no value") for an extended explanation and the eventual goal. All
conversions but one are trivial as the remove functions return zero
unconditionally. The only exception is the tps65086-restart driver.
Best regards
Uwe
Uwe Kleine-König (14):
power: reset: at91-poweroff: Stop using module_platform_driver_probe()
power: reset: at91-reset:: Stop using module_platform_driver_probe()
power: reset: at91-sama5d2_shdwc: Stop using
module_platform_driver_probe()
power: reset: as3722-poweroff: Convert to platform remove callback
returning void
power: reset: at91-poweroff: Convert to platform remove callback
returning void
power: reset: atc260x-poweroff: Convert to platform remove callback
returning void
power: reset: ltc2952-poweroff: Convert to platform remove callback
returning void
power: reset: mt6323-poweroff: Convert to platform remove callback
returning void
power: reset: qnap-poweroff: Convert to platform remove callback
returning void
power: reset: regulator-poweroff: Convert to platform remove callback
returning void
power: reset: restart-poweroff: Convert to platform remove callback
returning void
power: reset: rmobile-reset: Convert to platform remove callback
returning void
power: reset: syscon-poweroff: Convert to platform remove callback
returning void
power: reset: tps65086-restart: Convert to platform remove callback
returning void
drivers/power/reset/as3722-poweroff.c | 6 ++----
drivers/power/reset/at91-poweroff.c | 11 +++++------
drivers/power/reset/at91-reset.c | 9 +++++----
drivers/power/reset/at91-sama5d2_shdwc.c | 9 +++++----
drivers/power/reset/atc260x-poweroff.c | 6 ++----
drivers/power/reset/ltc2952-poweroff.c | 5 ++---
drivers/power/reset/mt6323-poweroff.c | 6 ++----
drivers/power/reset/qnap-poweroff.c | 5 ++---
drivers/power/reset/regulator-poweroff.c | 6 ++----
drivers/power/reset/restart-poweroff.c | 6 ++----
drivers/power/reset/rmobile-reset.c | 5 ++---
drivers/power/reset/syscon-poweroff.c | 6 ++----
drivers/power/reset/tps65086-restart.c | 12 +++++++-----
13 files changed, 40 insertions(+), 52 deletions(-)
base-commit: e27090b1413ff236ca1aec26d6b022149115de2c
--
2.42.0
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 <redacted>
---
drivers/power/reset/qnap-poweroff.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
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 <redacted>
---
drivers/power/reset/as3722-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/ltc2952-poweroff.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
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 <redacted>
---
drivers/power/reset/regulator-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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().
Returning an error if unregister_restart_handler() failed has no effect
but triggering another error message. So converting this driver to
.remove_new() has no effect but to suppress the duplicated error message.
Signed-off-by: Uwe Kleine-König <redacted>
---
drivers/power/reset/tps65086-restart.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
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 <redacted>
---
drivers/power/reset/syscon-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/rmobile-reset.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
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 <redacted>
---
drivers/power/reset/restart-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/atc260x-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 so 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 __ref which is
needed to suppress a (W=1) modpost warning.
Signed-off-by: Uwe Kleine-König <redacted>
---
drivers/power/reset/at91-poweroff.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/mt6323-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/at91-poweroff.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 so 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 __ref which is
needed to suppress a (W=1) modpost warning.
Signed-off-by: Uwe Kleine-König <redacted>
---
drivers/power/reset/at91-sama5d2_shdwc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
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 so 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 __ref which is
needed to suppress a (W=1) modpost warning.
Signed-off-by: Uwe Kleine-König <redacted>
---
drivers/power/reset/at91-reset.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/at91-reset.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <redacted>
---
drivers/power/reset/at91-sama5d2_shdwc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
Hello Sebastian,
On Sat, Nov 04, 2023 at 10:15:02PM +0100, Uwe Kleine-König wrote:
there are two different types of patches here that would justify to
different series. But as the patches are not independant I chose to put
them in a single series.
The first three patches drop usage of platform_driver_probe(). This is a
concept that isn't so relevant any more today. I didn't check, but it
saves typically only a few 100k and there are thoughts to deprecate it
to simplify the core. Getting the usage right is not trivial though the
at91 drivers got it nearly right. The alternative to these patches is to
add __ref to the driver struct ideally with a comment describing the
need like is e.g. done in commit 5b44abbc39ca ("platform/x86: hp-wmi::
Mark driver struct with __refdata to prevent section mismatch warning").
The remaining patches convert the platform drivers to .remove_new(), see
commit 5c5a7680e67b ("platform: Provide a remove callback that returns
no value") for an extended explanation and the eventual goal. All
conversions but one are trivial as the remove functions return zero
unconditionally. The only exception is the tps65086-restart driver.
I forgot to convert two of the drivers that used platform_driver_probe()
before. I added them as patches 15 and 16 to this series in the hope
that b4 (or whatever tool you use to apply patches) gets that right.
Uwe Kleine-König (14):
power: reset: at91-poweroff: Stop using module_platform_driver_probe()
power: reset: at91-reset:: Stop using module_platform_driver_probe()
If you could do s/::/:/ on that patch description that would be great.
(Or should I resend for that?)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Date: 2023-11-06 08:21:47
Il 04/11/23 22:15, Uwe Kleine-König ha scritto:
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 <redacted>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
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 so 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 __ref which is
needed to suppress a (W=1) modpost warning.
Signed-off-by: Uwe Kleine-König <redacted>
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 so 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 __ref which is
needed to suppress a (W=1) modpost warning.
Signed-off-by: Uwe Kleine-König <redacted>
Other than "::" in title:
Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
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 so 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 __ref which is
needed to suppress a (W=1) modpost warning.
Signed-off-by: Uwe Kleine-König <redacted>
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 <redacted>
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 <redacted>
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 <redacted>
From: Sebastian Reichel <hidden> Date: 2023-11-16 00:39:22
On Sat, 04 Nov 2023 22:15:02 +0100, Uwe Kleine-König wrote:
there are two different types of patches here that would justify to
different series. But as the patches are not independant I chose to put
them in a single series.
The first three patches drop usage of platform_driver_probe(). This is a
concept that isn't so relevant any more today. I didn't check, but it
saves typically only a few 100k and there are thoughts to deprecate it
to simplify the core. Getting the usage right is not trivial though the
at91 drivers got it nearly right. The alternative to these patches is to
add __ref to the driver struct ideally with a comment describing the
need like is e.g. done in commit 5b44abbc39ca ("platform/x86: hp-wmi::
Mark driver struct with __refdata to prevent section mismatch warning").
[...]