| Message ID | 1446127050-5957-1-git-send-email-balbi@ti.com |
|---|---|
| State | Accepted |
| Commit | e6b5140b706689a38aaeabd9de8fb3e1531cf9cb |
| Headers | show |
Grygorii Strashko <grygorii.strashko@ti.com> writes: > On 10/29/2015 03:57 PM, Felipe Balbi wrote: >> there's no need to call pm_runtime_get_sync() >> followed by pm_runtime_put(). We should, instead, >> just call pm_runtime_put_sync() and pm_runtime_disable(). > > Sry, but why do we need to call pm_runtime_put[_sync]() here? > > My be just pm_runtime_disable() will be ok? and disable with unbalanced pm_runtime_get() ? -- balbi
hi, Grygorii Strashko <grygorii.strashko@ti.com> writes: > On 11/02/2015 05:20 PM, Felipe Balbi wrote: >> Grygorii Strashko <grygorii.strashko@ti.com> writes: >> >>> On 10/29/2015 03:57 PM, Felipe Balbi wrote: >>>> there's no need to call pm_runtime_get_sync() >>>> followed by pm_runtime_put(). We should, instead, >>>> just call pm_runtime_put_sync() and pm_runtime_disable(). >>> >>> Sry, but why do we need to call pm_runtime_put[_sync]() here? >>> >>> My be just pm_runtime_disable() will be ok? >> >> and disable with unbalanced pm_runtime_get() ? >> > > Which one is unbalanced pm_runtime_get()? > There are no pm_runtime_get() in probe, so there you are > going to introduce unbalanced pm_runtime_put_sync() actually :( look at ti_qspi_setup(). I _do_ see, however, that it calls pm_runtime_put_autosuspend() in the same function; what happens if driver is removed after ti_qspi_setup() runs but before put_autosuspend() has time to actually run ? -- balbi
Hi, Grygorii Strashko <grygorii.strashko@ti.com> writes: > On 11/02/2015 06:06 PM, Felipe Balbi wrote: >> >> hi, >> >> Grygorii Strashko <grygorii.strashko@ti.com> writes: >>> On 11/02/2015 05:20 PM, Felipe Balbi wrote: >>>> Grygorii Strashko <grygorii.strashko@ti.com> writes: >>>> >>>>> On 10/29/2015 03:57 PM, Felipe Balbi wrote: >>>>>> there's no need to call pm_runtime_get_sync() >>>>>> followed by pm_runtime_put(). We should, instead, >>>>>> just call pm_runtime_put_sync() and pm_runtime_disable(). >>>>> >>>>> Sry, but why do we need to call pm_runtime_put[_sync]() here? >>>>> >>>>> My be just pm_runtime_disable() will be ok? >>>> >>>> and disable with unbalanced pm_runtime_get() ? >>>> >>> >>> Which one is unbalanced pm_runtime_get()? >>> There are no pm_runtime_get() in probe, so there you are >>> going to introduce unbalanced pm_runtime_put_sync() actually :( >> >> look at ti_qspi_setup(). I _do_ see, however, that it calls >> pm_runtime_put_autosuspend() in the same function; what happens if >> driver is removed after ti_qspi_setup() runs but before >> put_autosuspend() has time to actually run ? >> > > Seems nothing :) If I understand code in __device_release_driver() > right: the .remove() callback will be called after > pm_runtime_put_sync() and device should be disabled at this moment. > > Also, note that ti_qspi_setup() will be called for each SPI device > attached to SPI master and further PM management of SPI master is > performed by SPI core from __spi_pump_messages(). all right, do you want to send a patch fixing it ? -- balbi
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c index 69c1a95b0615..64318fcfacf2 100644 --- a/drivers/spi/spi-ti-qspi.c +++ b/drivers/spi/spi-ti-qspi.c @@ -554,16 +554,7 @@ free_master: static int ti_qspi_remove(struct platform_device *pdev) { - struct ti_qspi *qspi = platform_get_drvdata(pdev); - int ret; - - ret = pm_runtime_get_sync(qspi->dev); - if (ret < 0) { - dev_err(qspi->dev, "pm_runtime_get_sync() failed\n"); - return ret; - } - - pm_runtime_put(qspi->dev); + pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); return 0;
there's no need to call pm_runtime_get_sync() followed by pm_runtime_put(). We should, instead, just call pm_runtime_put_sync() and pm_runtime_disable(). Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/spi/spi-ti-qspi.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html