All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: Sowjanya Komatineni <skomatineni@nvidia.com>,
	thierry.reding@gmail.com, jonathanh@nvidia.com,
	frankc@nvidia.com, hverkuil@xs4all.nl, sakari.ailus@iki.fi,
	robh+dt@kernel.org, helen.koike@collabora.com
Cc: sboyd@kernel.org, gregkh@linuxfoundation.org,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org
Subject: Re: [RFC PATCH v2 04/18] i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
Date: Wed, 17 Jun 2020 07:52:17 +0300	[thread overview]
Message-ID: <f7997a48-eaec-839e-e0cd-cde718bd2e72@gmail.com> (raw)
In-Reply-To: <1592358094-23459-5-git-send-email-skomatineni@nvidia.com>

17.06.2020 04:41, Sowjanya Komatineni пишет:
> tegra_i2c_runtime_resume does not disable prior enabled clocks
> properly.
> 
> This patch fixes it.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 3be1018..1b459ca 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -668,7 +668,7 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
>  		ret = clk_enable(i2c_dev->slow_clk);
>  		if (ret < 0) {
>  			dev_err(dev, "failed to enable slow clock: %d\n", ret);
> -			return ret;
> +			goto disable_fast_clk;
>  		}
>  	}
>  
> @@ -676,11 +676,16 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
>  	if (ret < 0) {
>  		dev_err(i2c_dev->dev,
>  			"Enabling div clk failed, err %d\n", ret);
> -		clk_disable(i2c_dev->fast_clk);
> -		return ret;
> +		goto disable_slow_clk;
>  	}
>  
>  	return 0;
> +
> +disable_slow_clk:
> +	clk_disable(i2c_dev->slow_clk);
> +disable_fast_clk:
> +	clk_disable(i2c_dev->fast_clk);
> +	return ret;
>  }
>  
>  static int __maybe_unused tegra_i2c_runtime_suspend(struct device *dev)
> 

This looks good to me. Could you please add an additional patch to
remove all the other conditions of the clk enable/disable? The current
code was already inconsistent because in most cases there are
conditions, but not in all cases.

WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Sowjanya Komatineni
	<skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	frankc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org,
	sakari.ailus-X3B1VOXEql0@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	helen.koike-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org
Cc: sboyd-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [RFC PATCH v2 04/18] i2c: tegra: Fix the error path in tegra_i2c_runtime_resume
Date: Wed, 17 Jun 2020 07:52:17 +0300	[thread overview]
Message-ID: <f7997a48-eaec-839e-e0cd-cde718bd2e72@gmail.com> (raw)
In-Reply-To: <1592358094-23459-5-git-send-email-skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

17.06.2020 04:41, Sowjanya Komatineni пишет:
> tegra_i2c_runtime_resume does not disable prior enabled clocks
> properly.
> 
> This patch fixes it.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 3be1018..1b459ca 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -668,7 +668,7 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
>  		ret = clk_enable(i2c_dev->slow_clk);
>  		if (ret < 0) {
>  			dev_err(dev, "failed to enable slow clock: %d\n", ret);
> -			return ret;
> +			goto disable_fast_clk;
>  		}
>  	}
>  
> @@ -676,11 +676,16 @@ static int __maybe_unused tegra_i2c_runtime_resume(struct device *dev)
>  	if (ret < 0) {
>  		dev_err(i2c_dev->dev,
>  			"Enabling div clk failed, err %d\n", ret);
> -		clk_disable(i2c_dev->fast_clk);
> -		return ret;
> +		goto disable_slow_clk;
>  	}
>  
>  	return 0;
> +
> +disable_slow_clk:
> +	clk_disable(i2c_dev->slow_clk);
> +disable_fast_clk:
> +	clk_disable(i2c_dev->fast_clk);
> +	return ret;
>  }
>  
>  static int __maybe_unused tegra_i2c_runtime_suspend(struct device *dev)
> 

This looks good to me. Could you please add an additional patch to
remove all the other conditions of the clk enable/disable? The current
code was already inconsistent because in most cases there are
conditions, but not in all cases.

  reply	other threads:[~2020-06-17  4:52 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-17  1:41 [RFC PATCH v2 00/18] Support for Tegra video capture from external sensor Sowjanya Komatineni
2020-06-17  1:41 ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 01/18] dt-bindings: i2c: tegra: Document Tegra210 VI I2C clocks and power-domains Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-07-13 23:49   ` Rob Herring
2020-07-13 23:49     ` Rob Herring
2020-06-17  1:41 ` [RFC PATCH v2 02/18] arm64: tegra: Add missing clocks and power-domains to Tegra210 VI I2C Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 03/18] i2c: tegra: Don't mark VI I2C as IRQ safe runtime PM Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 04/18] i2c: tegra: Fix the error path in tegra_i2c_runtime_resume Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  4:52   ` Dmitry Osipenko [this message]
2020-06-17  4:52     ` Dmitry Osipenko
2020-06-17  1:41 ` [RFC PATCH v2 05/18] i2c: tegra: Fix runtime resume to re-init VI I2C Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 06/18] i2c: tegra: Avoid tegra_i2c_init_dma() for Tegra210 vi i2c Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 07/18] media: tegra-video: Fix channel format alignment Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 08/18] media: tegra-video: Enable TPG based on kernel config Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-29  9:28   ` Hans Verkuil
2020-06-29 14:48     ` Sowjanya Komatineni
2020-06-29 14:48       ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 09/18] media: tegra-video: Update format lookup to offset based Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 10/18] dt-bindings: tegra: Update VI and CSI bindings with port info Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 11/18] media: tegra-video: Add support for external sensor capture Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-07-06  9:10   ` Hans Verkuil
2020-07-06  9:10     ` Hans Verkuil
2020-07-06 16:53     ` Sowjanya Komatineni
2020-07-06 16:53       ` Sowjanya Komatineni
     [not found]     ` <6e09f5d3-85ca-5bf9-8617-b9c8bec36615@nvidia.com>
2020-07-07  9:51       ` Hans Verkuil
2020-07-07  9:51         ` Hans Verkuil
2020-07-07 10:32         ` Sowjanya Komatineni
2020-07-07 10:32           ` Sowjanya Komatineni
     [not found]     ` <6ee18b4d-b63b-8053-1b7e-c3ec7c1d4956@nvidia.com>
     [not found]       ` <6846e5bb-db1d-c2ff-c52c-70a2094c5b50@nvidia.com>
2020-07-07 19:35         ` Hans Verkuil
2020-07-07 20:29           ` Sowjanya Komatineni
2020-07-07 20:29             ` Sowjanya Komatineni
2020-07-07 20:41             ` Sowjanya Komatineni
2020-07-07 20:41               ` Sowjanya Komatineni
2020-07-07 21:15               ` Sowjanya Komatineni
2020-07-07 21:15                 ` Sowjanya Komatineni
2020-07-06 11:49   ` Hans Verkuil
2020-07-06 11:49     ` Hans Verkuil
2020-06-17  1:41 ` [RFC PATCH v2 12/18] media: tegra-video: Add support for selection ioctl ops Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-07-02 13:54   ` Hans Verkuil
2020-07-02 13:54     ` Hans Verkuil
2020-07-02 21:20     ` Sowjanya Komatineni
2020-07-02 21:20       ` Sowjanya Komatineni
2020-07-03  8:06       ` Hans Verkuil
2020-07-03 17:12         ` Sowjanya Komatineni
2020-07-03 17:12           ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 13/18] gpu: host1x: mipi: Update tegra_mipi_request() to be node based Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-18  0:27   ` Dmitry Osipenko
2020-06-18  0:27     ` Dmitry Osipenko
2020-06-17  1:41 ` [RFC PATCH v2 14/18] gpu: host1x: mipi: Split tegra_mipi_calibrate and tegra_mipi_wait Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-18  0:35   ` Dmitry Osipenko
2020-06-18  0:35     ` Dmitry Osipenko
2020-06-17  1:41 ` [RFC PATCH v2 15/18] media: tegra-video: Add CSI MIPI pads calibration Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 16/18] media: tegra-video: Compute settle times based on the clock rate Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 17/18] arm64: tegra: jetson-tx1: Add camera supplies Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-17  1:41 ` [RFC PATCH v2 18/18] arm64: tegra: Enable Tegra VI CSI support for Jetson Nano Sowjanya Komatineni
2020-06-17  1:41   ` Sowjanya Komatineni
2020-06-30  9:21 ` [RFC PATCH v2 00/18] Support for Tegra video capture from external sensor Hans Verkuil
2020-06-30  9:21   ` Hans Verkuil
2020-06-30 14:58   ` Sowjanya Komatineni
2020-06-30 14:58     ` Sowjanya Komatineni
2020-06-30 15:13     ` Hans Verkuil
2020-06-30 15:44       ` Sowjanya Komatineni
2020-06-30 15:44         ` Sowjanya Komatineni
2020-06-30 16:17         ` Sowjanya Komatineni
2020-06-30 16:17           ` Sowjanya Komatineni
2020-06-30 16:34           ` Sowjanya Komatineni
2020-06-30 16:34             ` Sowjanya Komatineni
2020-07-01 16:54             ` Hans Verkuil
2020-07-01 16:54               ` Hans Verkuil
2020-07-01 17:07               ` Sowjanya Komatineni
2020-07-01 17:07                 ` Sowjanya Komatineni
2020-07-02 10:49               ` Hans Verkuil
2020-07-02 14:08 ` Hans Verkuil
2020-07-02 14:08   ` Hans Verkuil
2020-07-02 21:21   ` Sowjanya Komatineni
2020-07-02 21:21     ` Sowjanya Komatineni

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=f7997a48-eaec-839e-e0cd-cde718bd2e72@gmail.com \
    --to=digetx@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=frankc@nvidia.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=helen.koike@collabora.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jonathanh@nvidia.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=sakari.ailus@iki.fi \
    --cc=sboyd@kernel.org \
    --cc=skomatineni@nvidia.com \
    --cc=thierry.reding@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.