lkml.org 
[lkml]   [2017]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2 1/4] spi: rockchip: fix error handling when probe
On Tue, Jun 13, 2017 at 01:25:40PM +0800, Jeffy Chen wrote:
> After failed to request dma tx chain, we need to disable pm_runtime.
> Also cleanup error labels for better readability.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v2: None

Looks good to me. I guess the original code was sort of trying to do a
different style of error handling, where you name the labels after the
point where the failure comes from (i.e., if ioremap fails, you 'goto
ioremap_fail' or something like that). But since we have enough devm_*
usage here that doesn't need unwound explicitly, and we didn't add
extra labels for all them, then that "style" doesn't really make much
sense.

Reviewed-by: Brian Norris <briannorris@chromium.org>

> drivers/spi/spi-rockchip.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
> index acf31f3..bab9b13 100644
> --- a/drivers/spi/spi-rockchip.c
> +++ b/drivers/spi/spi-rockchip.c
> @@ -684,33 +684,33 @@ static int rockchip_spi_probe(struct platform_device *pdev)
> rs->regs = devm_ioremap_resource(&pdev->dev, mem);
> if (IS_ERR(rs->regs)) {
> ret = PTR_ERR(rs->regs);
> - goto err_ioremap_resource;
> + goto err_put_master;
> }
>
> rs->apb_pclk = devm_clk_get(&pdev->dev, "apb_pclk");
> if (IS_ERR(rs->apb_pclk)) {
> dev_err(&pdev->dev, "Failed to get apb_pclk\n");
> ret = PTR_ERR(rs->apb_pclk);
> - goto err_ioremap_resource;
> + goto err_put_master;
> }
>
> rs->spiclk = devm_clk_get(&pdev->dev, "spiclk");
> if (IS_ERR(rs->spiclk)) {
> dev_err(&pdev->dev, "Failed to get spi_pclk\n");
> ret = PTR_ERR(rs->spiclk);
> - goto err_ioremap_resource;
> + goto err_put_master;
> }
>
> ret = clk_prepare_enable(rs->apb_pclk);
> if (ret) {
> dev_err(&pdev->dev, "Failed to enable apb_pclk\n");
> - goto err_ioremap_resource;
> + goto err_put_master;
> }
>
> ret = clk_prepare_enable(rs->spiclk);
> if (ret) {
> dev_err(&pdev->dev, "Failed to enable spi_clk\n");
> - goto err_spiclk_enable;
> + goto err_disable_apbclk;
> }
>
> spi_enable_chip(rs, 0);
> @@ -728,7 +728,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
> if (!rs->fifo_len) {
> dev_err(&pdev->dev, "Failed to get fifo length\n");
> ret = -EINVAL;
> - goto err_get_fifo_len;
> + goto err_disable_spiclk;
> }
>
> spin_lock_init(&rs->lock);
> @@ -755,7 +755,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
> /* Check tx to see if we need defer probing driver */
> if (PTR_ERR(rs->dma_tx.ch) == -EPROBE_DEFER) {
> ret = -EPROBE_DEFER;
> - goto err_get_fifo_len;
> + goto err_disable_pm_runtime;
> }
> dev_warn(rs->dev, "Failed to request TX DMA channel\n");
> rs->dma_tx.ch = NULL;
> @@ -786,23 +786,24 @@ static int rockchip_spi_probe(struct platform_device *pdev)
> ret = devm_spi_register_master(&pdev->dev, master);
> if (ret) {
> dev_err(&pdev->dev, "Failed to register master\n");
> - goto err_register_master;
> + goto err_free_dma_rx;
> }
>
> return 0;
>
> -err_register_master:
> - pm_runtime_disable(&pdev->dev);
> +err_free_dma_rx:
> if (rs->dma_rx.ch)
> dma_release_channel(rs->dma_rx.ch);
> err_free_dma_tx:
> if (rs->dma_tx.ch)
> dma_release_channel(rs->dma_tx.ch);
> -err_get_fifo_len:
> +err_disable_pm_runtime:
> + pm_runtime_disable(&pdev->dev);
> +err_disable_spiclk:
> clk_disable_unprepare(rs->spiclk);
> -err_spiclk_enable:
> +err_disable_apbclk:
> clk_disable_unprepare(rs->apb_pclk);
> -err_ioremap_resource:
> +err_put_master:
> spi_master_put(master);
>
> return ret;
> --
> 2.1.4
>
>

\
 
 \ /
  Last update: 2017-06-13 19:30    [W:0.125 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site