All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Artur Rojek <contact@artur-rojek.eu>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Paul Cercueil <paul@crapouillou.net>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Heiko Stuebner <heiko@sntech.de>,
	"Ezequiel Garcia" <ezequiel@vanguardiasur.com.ar>,
	<linux-input@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 2/7] IIO: Ingenic JZ47xx: Error check clk_enable calls.
Date: Tue, 19 May 2020 19:15:11 +0100	[thread overview]
Message-ID: <20200519191511.00007c78@Huawei.com> (raw)
In-Reply-To: <20200517194904.34758-2-contact@artur-rojek.eu>

On Sun, 17 May 2020 21:48:59 +0200
Artur Rojek <contact@artur-rojek.eu> wrote:

> Introduce error checks for the clk_enable calls used in this driver.
> As part of the changes, move clk_enable/clk_disable calls out of
> ingenic_adc_set_config and into respective logic of its callers.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> Tested-by: Paul Cercueil <paul@crapouillou.net>

One trivial thing inline.

> ---
> 
>  Changes:
> 
>  v6: new patch
> 
>  v7: no change
> 
>  drivers/iio/adc/ingenic-adc.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
> index 39c0a609fc94..6c3bbba7c44b 100644
> --- a/drivers/iio/adc/ingenic-adc.c
> +++ b/drivers/iio/adc/ingenic-adc.c
> @@ -73,7 +73,6 @@ static void ingenic_adc_set_config(struct ingenic_adc *adc,
>  {
>  	uint32_t cfg;
>  
> -	clk_enable(adc->clk);
>  	mutex_lock(&adc->lock);
>  
>  	cfg = readl(adc->base + JZ_ADC_REG_CFG) & ~mask;
> @@ -81,7 +80,6 @@ static void ingenic_adc_set_config(struct ingenic_adc *adc,
>  	writel(cfg, adc->base + JZ_ADC_REG_CFG);
>  
>  	mutex_unlock(&adc->lock);
> -	clk_disable(adc->clk);
>  }
>  
>  static void ingenic_adc_enable(struct ingenic_adc *adc,
> @@ -124,6 +122,8 @@ static int ingenic_adc_write_raw(struct iio_dev *iio_dev,
>  				 long m)
>  {
>  	struct ingenic_adc *adc = iio_priv(iio_dev);
> +	struct device *dev = iio_dev->dev.parent;
> +	int ret;
>  
>  	switch (m) {
>  	case IIO_CHAN_INFO_SCALE:
> @@ -131,6 +131,14 @@ static int ingenic_adc_write_raw(struct iio_dev *iio_dev,
>  		case INGENIC_ADC_BATTERY:
>  			if (!adc->soc_data->battery_vref_mode)
>  				return -EINVAL;
> +
> +			ret = clk_enable(adc->clk);
> +			if (ret) {
> +				dev_err(dev, "Failed to enable clock: %d\n",
> +					ret);
> +				return ret;
> +			}
> +
>  			if (val > JZ_ADC_BATTERY_LOW_VREF) {
>  				ingenic_adc_set_config(adc,
>  						       JZ_ADC_REG_CFG_BAT_MD,
> @@ -142,6 +150,9 @@ static int ingenic_adc_write_raw(struct iio_dev *iio_dev,
>  						       JZ_ADC_REG_CFG_BAT_MD);
>  				adc->low_vref_mode = true;
>  			}
> +
> +			clk_disable(adc->clk);
> +
>  			return 0;
>  		default:
>  			return -EINVAL;
> @@ -317,6 +328,13 @@ static int ingenic_adc_read_chan_info_raw(struct ingenic_adc *adc,
>  					  int *val)
>  {
>  	int bit, ret, engine = (chan->channel == INGENIC_ADC_BATTERY);
> +	struct device *dev = iio_priv_to_dev(adc)->dev.parent;
> +
> +	ret = clk_enable(adc->clk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable clock: %d\n", ret);
> +		return ret;
> +	}

It almost certainly doesn't matter, but if we are going to move the clk enable
outside the lock, we should do the same with the disable.

>  
>  	/* We cannot sample AUX/AUX2 in parallel. */
>  	mutex_lock(&adc->aux_lock);
> @@ -325,7 +343,6 @@ static int ingenic_adc_read_chan_info_raw(struct ingenic_adc *adc,
>  		ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_AUX_MD, bit);
>  	}
>  
> -	clk_enable(adc->clk);
>  	ret = ingenic_adc_capture(adc, engine);
>  	if (ret)
>  		goto out;



  reply	other threads:[~2020-05-19 18:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-17 19:48 [PATCH v7 1/7] dt-bindings: iio/adc: Convert ingenic-adc docs to YAML Artur Rojek
2020-05-17 19:48 ` [PATCH v7 2/7] IIO: Ingenic JZ47xx: Error check clk_enable calls Artur Rojek
2020-05-19 18:15   ` Jonathan Cameron [this message]
2020-05-17 19:49 ` [PATCH v7 3/7] IIO: Ingenic JZ47xx: Add xlate cb to retrieve correct channel idx Artur Rojek
2020-05-17 19:49 ` [PATCH v7 4/7] dt-bindings: iio/adc: Add touchscreen idx for JZ47xx SoC ADC Artur Rojek
2020-05-17 19:49 ` [PATCH v7 5/7] IIO: Ingenic JZ47xx: Add touchscreen mode Artur Rojek
2020-05-17 19:49 ` [PATCH v7 6/7] dt-bindings: input: Add docs for ADC driven joystick Artur Rojek
2020-05-18 14:22   ` Rob Herring
2020-05-17 19:49 ` [PATCH v7 7/7] input: joystick: Add ADC attached joystick driver Artur Rojek
2020-05-17 23:19   ` kbuild test robot
2020-05-17 23:19     ` kbuild test robot
2020-05-19 18:25   ` Jonathan Cameron
2020-05-19 20:43   ` Andy Shevchenko
2020-05-19 21:02     ` Paul Cercueil
2020-05-26 21:34 ` [PATCH v7 1/7] dt-bindings: iio/adc: Convert ingenic-adc docs to YAML Rob Herring

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=20200519191511.00007c78@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=contact@artur-rojek.eu \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=heiko@sntech.de \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=paul@crapouillou.net \
    --cc=robh+dt@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 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.