[0/4] ata: Drop platform_driver_probe() and convert to platform remove callback returning void (part II)

Message

Uwe Kleine-König Nov. 5, 2023, 3 p.m. UTC

Hello,

there are two different types of patches here that would justify to
different series. But as the patches are not independent I chose to put
them in a single series.

The first two 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
drivers here got it nearly right. The alternative to these patches is to
add __refdata 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").
Note that the warning only happens starting with commit f177cd0c15fc
("modpost: Don't let "driver"s reference .exit.*") that is expected to
be part of v6.7-rc1.

The remaining two 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 are trivial as the remove functions return zero
unconditionally. The other ata drivers were already converted earlier,
my coccinelle script just missed these two drivers (because of the
__exit_p macro).

Best regards
Uwe

Uwe Kleine-König (4):
  ata: pata_falcon: Stop using module_platform_driver_probe()
  ata: pata_gayle: Stop using module_platform_driver_probe()
  ata: pata_falcon: Convert to platform remove callback returning void
  ata: pata_gayle: Convert to platform remove callback returning void

 drivers/ata/pata_falcon.c | 11 +++++------
 drivers/ata/pata_gayle.c  | 11 +++++------
 2 files changed, 10 insertions(+), 12 deletions(-)


base-commit: e27090b1413ff236ca1aec26d6b022149115de2c

Comments

Niklas Cassel Nov. 7, 2023, 10:32 a.m. UTC | #1

On Sun, Nov 05, 2023 at 04:00:35PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> there are two different types of patches here that would justify to
> different series. But as the patches are not independent I chose to put
> them in a single series.
> 
> The first two 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
> drivers here got it nearly right. The alternative to these patches is to
> add __refdata 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").
> Note that the warning only happens starting with commit f177cd0c15fc
> ("modpost: Don't let "driver"s reference .exit.*") that is expected to
> be part of v6.7-rc1.

Hello Uwe,

Considering that these patches are for e.g. amiga, which might not have
that much spare RAM to start with, perhaps these 100K extra that you get
from using module_platform_driver_probe() is actually sigificant on these
platforms?

Although these two drivers seem to be the only ATA drivers left using this:
$ git grep platform_driver_probe drivers/ata | wc -l
2

Thus, for the series:
Reviewed-by: Niklas Cassel <[e-mail]>

Damien Le Moal Nov. 8, 2023, 1:10 a.m. UTC | #2

On 11/6/23 00:00, Uwe Kleine-König wrote:
> Hello,
> 
> there are two different types of patches here that would justify to
> different series. But as the patches are not independent I chose to put
> them in a single series.
> 
> The first two 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
> drivers here got it nearly right. The alternative to these patches is to
> add __refdata 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").
> Note that the warning only happens starting with commit f177cd0c15fc
> ("modpost: Don't let "driver"s reference .exit.*") that is expected to
> be part of v6.7-rc1.
> 
> The remaining two 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 are trivial as the remove functions return zero
> unconditionally. The other ata drivers were already converted earlier,
> my coccinelle script just missed these two drivers (because of the
> __exit_p macro).

Applied to for-6.7-fixes. Thanks !

> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (4):
>   ata: pata_falcon: Stop using module_platform_driver_probe()
>   ata: pata_gayle: Stop using module_platform_driver_probe()
>   ata: pata_falcon: Convert to platform remove callback returning void
>   ata: pata_gayle: Convert to platform remove callback returning void
> 
>  drivers/ata/pata_falcon.c | 11 +++++------
>  drivers/ata/pata_gayle.c  | 11 +++++------
>  2 files changed, 10 insertions(+), 12 deletions(-)
> 
> 
> base-commit: e27090b1413ff236ca1aec26d6b022149115de2c