| Message ID | 20230530081648.2199419-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 22f407278ea43df46f90cece6595e5e8a0d5447c |
| Headers | show |
| Series | spi: mt65xx: Convert to platform remove callback returning void | expand |
Il 30/05/23 10:16, Uwe Kleine-König ha scritto: > Returning an error code in a platform driver's remove function is wrong > most of the time and there is an effort to make the callback return > void. To prepare this rework the function not to exit early. > > There wasn't a real problem because if pm runtime resume failed the only > step missing was pm_runtime_disable() which isn't an issue. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cheers!
Hello, On Tue, May 30, 2023 at 10:16:46AM +0200, Uwe Kleine-König wrote: > Returning an error code in a platform driver's remove function is wrong > most of the time and there is an effort to make the callback return > void. To prepare this rework the function not to exit early. > > There wasn't a real problem because if pm runtime resume failed the only > step missing was pm_runtime_disable() which isn't an issue. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> I just noticed there is a patch in next that conflicts with that one. My merge resolution looks as follows: diff --cc drivers/spi/spi-mt65xx.c index 17162c8661b4,9333a0e8204d..000000000000 --- a/drivers/spi/spi-mt65xx.c +++ b/drivers/spi/spi-mt65xx.c @@@ -1276,18 -1275,21 +1276,24 @@@ static int mtk_spi_remove(struct platfo struct mtk_spi *mdata = spi_master_get_devdata(master); int ret; + if (mdata->use_spimem && !completion_done(&mdata->spimem_done)) + complete(&mdata->spimem_done); + - ret = pm_runtime_resume_and_get(&pdev->dev); - if (ret < 0) - return ret; + ret = pm_runtime_get_sync(&pdev->dev); + if (ret < 0) { + dev_warn(&pdev->dev, "Failed to resume hardware (%pe)\n", ERR_PTR(ret)); + } else { + /* + * If pm runtime resume failed, clks are disabled and + * unprepared. So don't access the hardware and skip clk + * unpreparing. + */ + mtk_spi_reset(mdata); - mtk_spi_reset(mdata); - - if (mdata->dev_comp->no_need_unprepare) { - clk_unprepare(mdata->spi_clk); - clk_unprepare(mdata->spi_hclk); + if (mdata->dev_comp->no_need_unprepare) { + clk_unprepare(mdata->spi_clk); + clk_unprepare(mdata->spi_hclk); + } } pm_runtime_put_noidle(&pdev->dev); If it's too complicated to apply, I can resend a rebased version. After that the other two patches apply just fine. Best regards Uwe
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c index 21c321f43766..9333a0e8204d 100644 --- a/drivers/spi/spi-mt65xx.c +++ b/drivers/spi/spi-mt65xx.c @@ -1275,15 +1275,21 @@ static int mtk_spi_remove(struct platform_device *pdev) struct mtk_spi *mdata = spi_master_get_devdata(master); int ret; - ret = pm_runtime_resume_and_get(&pdev->dev); - if (ret < 0) - return ret; + ret = pm_runtime_get_sync(&pdev->dev); + if (ret < 0) { + dev_warn(&pdev->dev, "Failed to resume hardware (%pe)\n", ERR_PTR(ret)); + } else { + /* + * If pm runtime resume failed, clks are disabled and + * unprepared. So don't access the hardware and skip clk + * unpreparing. + */ + mtk_spi_reset(mdata); - mtk_spi_reset(mdata); - - if (mdata->dev_comp->no_need_unprepare) { - clk_unprepare(mdata->spi_clk); - clk_unprepare(mdata->spi_hclk); + if (mdata->dev_comp->no_need_unprepare) { + clk_unprepare(mdata->spi_clk); + clk_unprepare(mdata->spi_hclk); + } } pm_runtime_put_noidle(&pdev->dev);
Returning an error code in a platform driver's remove function is wrong most of the time and there is an effort to make the callback return void. To prepare this rework the function not to exit early. There wasn't a real problem because if pm runtime resume failed the only step missing was pm_runtime_disable() which isn't an issue. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/spi/spi-mt65xx.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)