| Message ID | 4e16c0aba047b068aaa41c1d180d98ccb441afd0.1496668108.git.arvind.yadav.cs@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 6, 2017 at 12:44 PM, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: > clk_prepare_enable() can fail here and we must check its return value. > @@ -1676,7 +1676,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) > - clk_prepare_enable(ssp->clk); > + status = clk_prepare_enable(ssp->clk); This one looks fine. > @@ -1855,8 +1859,13 @@ static int pxa2xx_spi_resume(struct device *dev) > /* Enable the SSP clock */ > - if (!pm_runtime_suspended(dev)) > - clk_prepare_enable(ssp->clk); > + if (!pm_runtime_suspended(dev)) { > + status = clk_prepare_enable(ssp->clk); > + if (status) { > + dev_err(dev, "Failed to prepare clock\n"); > + return status; > + } This... > @@ -1886,8 +1895,7 @@ static int pxa2xx_spi_runtime_resume(struct device *dev) > { > struct driver_data *drv_data = dev_get_drvdata(dev); > > - clk_prepare_enable(drv_data->ssp->clk); > - return 0; > + return clk_prepare_enable(drv_data->ssp->clk); ...and especially this should be carefully checked since there are differences in behaviour how system or driver will be resumed. So, the question is how did you test it?
On Tuesday 06 June 2017 04:24 PM, Andy Shevchenko wrote: > On Tue, Jun 6, 2017 at 12:44 PM, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: >> clk_prepare_enable() can fail here and we must check its return value. >> @@ -1676,7 +1676,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) >> - clk_prepare_enable(ssp->clk); >> + status = clk_prepare_enable(ssp->clk); > This one looks fine. > >> @@ -1855,8 +1859,13 @@ static int pxa2xx_spi_resume(struct device *dev) >> /* Enable the SSP clock */ >> - if (!pm_runtime_suspended(dev)) >> - clk_prepare_enable(ssp->clk); >> + if (!pm_runtime_suspended(dev)) { >> + status = clk_prepare_enable(ssp->clk); >> + if (status) { >> + dev_err(dev, "Failed to prepare clock\n"); >> + return status; >> + } > This... > >> @@ -1886,8 +1895,7 @@ static int pxa2xx_spi_runtime_resume(struct device *dev) >> { >> struct driver_data *drv_data = dev_get_drvdata(dev); >> >> - clk_prepare_enable(drv_data->ssp->clk); >> - return 0; >> + return clk_prepare_enable(drv_data->ssp->clk); > ...and especially this should be carefully checked since there are > differences in behaviour how system or driver will be resumed. yes true, here clk_prepare_enable will return 0 on successful attempt. what do you suggest here, we should not return like this.? > > So, the question is how did you test it? It can fail, I am not able to produce clock failure issue. If you have any suggestion. please let me know. > -arvind -- 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
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index 47b65d7..e9c0fe5 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1676,7 +1676,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) } /* Enable SOC clock */ - clk_prepare_enable(ssp->clk); + status = clk_prepare_enable(ssp->clk); + if (status) { + dev_err(&pdev->dev, "Failed to prepare clock\n"); + goto out_error_master_alloc; + } master->max_speed_hz = clk_get_rate(ssp->clk); @@ -1855,8 +1859,13 @@ static int pxa2xx_spi_resume(struct device *dev) int status; /* Enable the SSP clock */ - if (!pm_runtime_suspended(dev)) - clk_prepare_enable(ssp->clk); + if (!pm_runtime_suspended(dev)) { + status = clk_prepare_enable(ssp->clk); + if (status) { + dev_err(dev, "Failed to prepare clock\n"); + return status; + } + } /* Restore LPSS private register bits */ if (is_lpss_ssp(drv_data)) @@ -1886,8 +1895,7 @@ static int pxa2xx_spi_runtime_resume(struct device *dev) { struct driver_data *drv_data = dev_get_drvdata(dev); - clk_prepare_enable(drv_data->ssp->clk); - return 0; + return clk_prepare_enable(drv_data->ssp->clk); } #endif
clk_prepare_enable() can fail here and we must check its return value. Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> --- drivers/spi/spi-pxa2xx.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)