From: Brian Norris <briannorris@chromium.org>
To: Jeffy Chen <jeffy.chen@rock-chips.com>
Cc: linux-kernel@vger.kernel.org, broonie@kernel.org,
dianders@chromium.org, heiko@sntech.de,
linux-spi@vger.kernel.org, linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/4] spi: rockchip: fix error handling when probe
Date: Tue, 13 Jun 2017 10:29:32 -0700 [thread overview]
Message-ID: <20170613172931.GA9026@google.com> (raw)
In-Reply-To: <1497331543-8565-1-git-send-email-jeffy.chen@rock-chips.com>
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
>
>
next prev parent reply other threads:[~2017-06-13 17:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-13 5:25 [PATCH v2 1/4] spi: rockchip: fix error handling when probe Jeffy Chen
[not found] ` <1497331543-8565-1-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2017-06-13 5:25 ` [PATCH v2 2/4] spi: rockchip: add support for "cs-gpios" dts property Jeffy Chen
2017-06-13 17:24 ` kbuild test robot
2017-06-13 17:33 ` Brian Norris
[not found] ` <20170613173346.GB9026-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-06-14 1:27 ` jeffy
2017-06-13 5:25 ` [PATCH v2 3/4] dt-bindings: spi/rockchip: add "cs-gpios" optional property Jeffy Chen
2017-06-13 17:29 ` Brian Norris [this message]
[not found] ` <1497331543-8565-4-git-send-email-jeffy.chen@rock-chips.com>
[not found] ` <20170613175043.GC9026@google.com>
[not found] ` <20170613182225.smahsf3jzvbc7w7z@sirena.org.uk>
[not found] ` <20170620004739.GA67314@google.com>
[not found] ` <CAD=FV=UKe5M-M=gXwdjp25vpR0CHtk1APRF0_b4PXQrGOc40+A@mail.gmail.com>
[not found] ` <594C905E.1040208@rock-chips.com>
[not found] ` <CAD=FV=VWjbGpY4nj2L0_6xaYY2_tp0pJ5q=BgppL9GWefBUGog@mail.gmail.com>
[not found] ` <594D0723.7010108@rock-chips.com>
[not found] ` <CAD=FV=XjW4a9mqH0UtUAmHkj-aAO75bpSXuyy__jBB7YC8PBVg@mail.gmail.com>
[not found] ` <CAD=FV=XjW4a9mqH0UtUAmHkj-aAO75bpSXuyy__jBB7YC8PBVg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-06-26 3:26 ` [PATCH v2 4/4] arm64: dts: rockchip: use cs-gpios for cros_ec_spi jeffy
2017-06-26 3:27 ` jeffy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170613172931.GA9026@google.com \
--to=briannorris@chromium.org \
--cc=broonie@kernel.org \
--cc=dianders@chromium.org \
--cc=heiko@sntech.de \
--cc=jeffy.chen@rock-chips.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).