On Mon, Jun 13, 2022 at 10:13:16AM +0300, Adrian Hunter wrote:
> On 11/06/22 00:12, Uwe Kleine-König wrote:
> > sdhci_pltfm_unregister() returns 0 unconditionally and returning an
> > error in a platform remove callback isn't very sensible. (The only
> > effect of the latter is that the device core emits a generic warning and
> > then removes the device anyhow.)
> >
> > So return 0 unconditionally to make it obvious there is no error
> > forwarded to the upper layers.
> >
> > This is a preparation for making platform remove callbacks return void.
>
> This preparation seems a bit unnatural.
IMHO it's not. I have to adapt all ~4800 platform drivers together in a
single commit to change the prototype of the return callback.
Now assume you want to review that commit and make sure there are no
relevant changes in behaviour. Without the preparation in the commit
under discussion, the change to sdhci-of-arasan.c would look as follows:
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1733,7 +1733,4 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
static int sdhci_arasan_remove(struct platform_device *pdev)
{
- int ret;
struct sdhci_host *host = platform_get_drvdata(pdev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
@@ -1747,11 +1746,11 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
sdhci_arasan_unregister_sdclk(&pdev->dev);
- ret = sdhci_pltfm_unregister(pdev);
+ sdhci_pltfm_unregister(pdev);
clk_disable_unprepare(clk_ahb);
-
- return ret;
}
static struct platform_driver sdhci_arasan_driver = {
So you have to look up then what sdhci_pltfm_unregister() does and if
it's ok to ignore the return value. Should I mention that in the commit
log? What about the other drivers?
If however the change to the sdhci-arasan driver is only
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 3997cad1f793..84c949bd99c8 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1749,8 +1749,6 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
sdhci_pltfm_unregister(pdev);
clk_disable_unprepare(clk_ahb);
-
- return 0;
}
static struct platform_driver sdhci_arasan_driver = {
, it's trivial to see that there is no relevant driver specific change.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
