| Message ID | 55D39EB9.2090001@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 19.08.2015 06:08, Heiner Kallweit wrote: > The runtime PM handlers take care of enabling / disabling the clock. > Therefore use them to simplify the suspend / resume handlers. > > pm_runtime_suspend/resume internally check whether suspending/resuming > is possible and return 1 in case PM state is suspended/active already. > > Currently there also seems to be a bug in the resume handler > as suspend as well as resume check for !pm_runtime_disabled(dev). Actually currently there is a bug because PM runtime is not disabled and concurrent runtime suspends/resumes can happen. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/spi/spi-s3c64xx.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 15a6de1..78a6f04 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -1242,10 +1242,9 @@ static int s3c64xx_spi_suspend(struct device *dev) > if (ret) > return ret; > > - if (!pm_runtime_suspended(dev)) { > - clk_disable_unprepare(sdd->clk); > - clk_disable_unprepare(sdd->src_clk); > - } > + ret = pm_runtime_suspend(dev); I think you need pm_runtime_force_suspend() instead. Same for resume. Best regards, Krzysztof > + if (ret < 0) > + return ret; > > sdd->cur_speed = 0; /* Output Clock is stopped */ > > @@ -1257,14 +1256,14 @@ static int s3c64xx_spi_resume(struct device *dev) > struct spi_master *master = dev_get_drvdata(dev); > struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); > struct s3c64xx_spi_info *sci = sdd->cntrlr_info; > + int ret; > > if (sci->cfg_gpio) > sci->cfg_gpio(); > > - if (!pm_runtime_suspended(dev)) { > - clk_prepare_enable(sdd->src_clk); > - clk_prepare_enable(sdd->clk); > - } > + ret = pm_runtime_resume(dev); > + if (ret < 0) > + return ret; > > s3c64xx_spi_hwinit(sdd, sdd->port_id); > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 19.08.2015 um 03:11 schrieb Krzysztof Kozlowski: > On 19.08.2015 06:08, Heiner Kallweit wrote: >> The runtime PM handlers take care of enabling / disabling the clock. >> Therefore use them to simplify the suspend / resume handlers. >> >> pm_runtime_suspend/resume internally check whether suspending/resuming >> is possible and return 1 in case PM state is suspended/active already. >> >> Currently there also seems to be a bug in the resume handler >> as suspend as well as resume check for !pm_runtime_disabled(dev). > > Actually currently there is a bug because PM runtime is not disabled and > concurrent runtime suspends/resumes can happen. > >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/spi/spi-s3c64xx.c | 15 +++++++-------- >> 1 file changed, 7 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 15a6de1..78a6f04 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -1242,10 +1242,9 @@ static int s3c64xx_spi_suspend(struct device *dev) >> if (ret) >> return ret; >> >> - if (!pm_runtime_suspended(dev)) { >> - clk_disable_unprepare(sdd->clk); >> - clk_disable_unprepare(sdd->src_clk); >> - } >> + ret = pm_runtime_suspend(dev); > > I think you need pm_runtime_force_suspend() instead. Same for resume. You're right. Will update the patch. Regards, Heiner > > Best regards, > Krzysztof > >> + if (ret < 0) >> + return ret; >> >> sdd->cur_speed = 0; /* Output Clock is stopped */ >> >> @@ -1257,14 +1256,14 @@ static int s3c64xx_spi_resume(struct device *dev) >> struct spi_master *master = dev_get_drvdata(dev); >> struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); >> struct s3c64xx_spi_info *sci = sdd->cntrlr_info; >> + int ret; >> >> if (sci->cfg_gpio) >> sci->cfg_gpio(); >> >> - if (!pm_runtime_suspended(dev)) { >> - clk_prepare_enable(sdd->src_clk); >> - clk_prepare_enable(sdd->clk); >> - } >> + ret = pm_runtime_resume(dev); >> + if (ret < 0) >> + return ret; >> >> s3c64xx_spi_hwinit(sdd, sdd->port_id); >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 15a6de1..78a6f04 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -1242,10 +1242,9 @@ static int s3c64xx_spi_suspend(struct device *dev) if (ret) return ret; - if (!pm_runtime_suspended(dev)) { - clk_disable_unprepare(sdd->clk); - clk_disable_unprepare(sdd->src_clk); - } + ret = pm_runtime_suspend(dev); + if (ret < 0) + return ret; sdd->cur_speed = 0; /* Output Clock is stopped */ @@ -1257,14 +1256,14 @@ static int s3c64xx_spi_resume(struct device *dev) struct spi_master *master = dev_get_drvdata(dev); struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); struct s3c64xx_spi_info *sci = sdd->cntrlr_info; + int ret; if (sci->cfg_gpio) sci->cfg_gpio(); - if (!pm_runtime_suspended(dev)) { - clk_prepare_enable(sdd->src_clk); - clk_prepare_enable(sdd->clk); - } + ret = pm_runtime_resume(dev); + if (ret < 0) + return ret; s3c64xx_spi_hwinit(sdd, sdd->port_id);
The runtime PM handlers take care of enabling / disabling the clock. Therefore use them to simplify the suspend / resume handlers. pm_runtime_suspend/resume internally check whether suspending/resuming is possible and return 1 in case PM state is suspended/active already. Currently there also seems to be a bug in the resume handler as suspend as well as resume check for !pm_runtime_disabled(dev). Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/spi/spi-s3c64xx.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)