On Sun, 31 Jul 2022 at 19:35, Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> wrote: > > Having a something_get() function call in a remove function is unusual. > A something_put() is more likely. > > More over the remove() function does not match the error handling of the > probe(). > > Fix the remove() function to match the error handling path of the probe. > > Fixes: f90142683f04 ("mmc: dw_mmc-rockchip: add runtime PM support") > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> > --- > /!\ SPECULATIVE /!\ > > I have a limited knowledge of the pm_ API. > However, as said, the error handling path of the probe looks more logical > to me. > > Moreover, some more or less similar code can be found in > drivers/mmc/host/dw_mmc-exynos.c. This patch also align this rockchip > driver to the exynos's one. > > So review with care. > --- > drivers/mmc/host/dw_mmc-rockchip.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c > index 2a99f15f527f..b5893c738b4a 100644 > --- a/drivers/mmc/host/dw_mmc-rockchip.c > +++ b/drivers/mmc/host/dw_mmc-rockchip.c > @@ -373,8 +373,8 @@ static int dw_mci_rockchip_probe(struct platform_device *pdev) > > static int dw_mci_rockchip_remove(struct platform_device *pdev) > { > - pm_runtime_get_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > + pm_runtime_set_suspended(&pdev->dev); > pm_runtime_put_noidle(&pdev->dev); > > dw_mci_pltfm_remove(pdev); dw_mci_pltfm_remove() needs to be called with an active/powered host device. That's why the call to pm_runtime_get_sync() is done, so I don't think there is anything wrong with the existing code. Kind regards Uffe