| Message ID | bd442556c0094be2c240f070d15ce2061b376c09.1659288898.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: dw_mmc-rockchip: Fix the dw_mci_rockchip_remove() function | expand |
On Sun, 31 Jul 2022 at 19:35, Christophe JAILLET <christophe.jaillet@wanadoo.fr> 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@wanadoo.fr> > --- > /!\ 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
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);
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@wanadoo.fr> --- /!\ 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(-)