All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-iio@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	Naveen Krishna Chatradhi <ch.naveen@samsung.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Doug Anderson <dianders@chromium.org>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Rob Herring <rob.herring@calxeda.com>
Subject: Re: [PATCH v2 2/4] iio/adc: (max1363) Add support for external reference voltage
Date: Sun, 03 Feb 2013 12:12:45 +0000	[thread overview]
Message-ID: <510E543D.50201@kernel.org> (raw)
In-Reply-To: <1359853180-5664-3-git-send-email-linux@roeck-us.net>

On 02/03/2013 12:59 AM, Guenter Roeck wrote:
> Implement external reference voltage as regulator named "vref".
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Applied to togreg branch of iio.git.
Thanks,

The remaining two patches will have sit a while.
Patch 3 effects stuff outside IIO so I'll wait to see if anyone
screams that it is going to cause merge nightmares!

Patch 4 is clearly still under discussion though a consensus seems
to have more or less been reached.

Thanks again for this work Guenter. I like the way one of my oldest
drivers is getting cleaned up along the way without me having to
do anything much ;)

> ---
> v2: Use regulator API to specify vref instead of creating new devicetree
>     bindings.
>     Keep reference voltage internally in uV, as this is the scale provided
>     by the regulator subsystem. We need the value in uV anyway, so that does
>     make some sense.
> 
>  drivers/iio/adc/max1363.c |   52 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index 1353fda..cac73d7 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -163,6 +163,8 @@ struct max1363_chip_info {
>   * @mask_low:		bitmask for enabled low thresholds
>   * @thresh_high:	high threshold values
>   * @thresh_low:		low threshold values
> + * @vref:		Reference voltage regulator
> + * @vref_uv:		Actual (external or internal) reference voltage
>   */
>  struct max1363_state {
>  	struct i2c_client		*client;
> @@ -182,6 +184,8 @@ struct max1363_state {
>  	/* 4x unipolar first then the fours bipolar ones */
>  	s16				thresh_high[8];
>  	s16				thresh_low[8];
> +	struct regulator		*vref;
> +	u32				vref_uv;
>  };
>  
>  #define MAX1363_MODE_SINGLE(_num, _mask) {				\
> @@ -393,6 +397,8 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct max1363_state *st = iio_priv(indio_dev);
>  	int ret;
> +	unsigned long scale_uv;
> +
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
>  		ret = max1363_read_single_chan(indio_dev, chan, val, m);
> @@ -400,16 +406,10 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
>  			return ret;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
> -		if ((1 << (st->chip_info->bits + 1)) >
> -		    st->chip_info->int_vref_mv) {
> -			*val = 0;
> -			*val2 = 500000;
> -			return IIO_VAL_INT_PLUS_MICRO;
> -		} else {
> -			*val = (st->chip_info->int_vref_mv)
> -				>> st->chip_info->bits;
> -			return IIO_VAL_INT;
> -		}
> +		scale_uv = st->vref_uv >> st->chip_info->bits;
> +		*val = scale_uv / 1000;
> +		*val2 = (scale_uv % 1000) * 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1390,12 +1390,16 @@ static const struct max1363_chip_info max1363_chip_info_tbl[] = {
>  
>  static int max1363_initial_setup(struct max1363_state *st)
>  {
> -	st->setupbyte = MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_VDD
> -		| MAX1363_SETUP_POWER_UP_INT_REF
> -		| MAX1363_SETUP_INT_CLOCK
> +	st->setupbyte = MAX1363_SETUP_INT_CLOCK
>  		| MAX1363_SETUP_UNIPOLAR
>  		| MAX1363_SETUP_NORESET;
>  
> +	if (st->vref)
> +		st->setupbyte |= MAX1363_SETUP_AIN3_IS_REF_EXT_TO_REF;
> +	else
> +		st->setupbyte |= MAX1363_SETUP_POWER_UP_INT_REF
> +		  | MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_INT;
> +
>  	/* Set scan mode writes the config anyway so wait until then */
>  	st->setupbyte = MAX1363_SETUP_BYTE(st->setupbyte);
>  	st->current_mode = &max1363_mode_table[st->chip_info->default_mode];
> @@ -1533,6 +1537,7 @@ static int max1363_probe(struct i2c_client *client,
>  	int ret;
>  	struct max1363_state *st;
>  	struct iio_dev *indio_dev;
> +	struct regulator *vref;
>  
>  	indio_dev = iio_device_alloc(sizeof(struct max1363_state));
>  	if (indio_dev == NULL) {
> @@ -1563,6 +1568,23 @@ static int max1363_probe(struct i2c_client *client,
>  	st->chip_info = &max1363_chip_info_tbl[id->driver_data];
>  	st->client = client;
>  
> +	st->vref_uv = st->chip_info->int_vref_mv * 1000;
> +	vref = devm_regulator_get(&client->dev, "vref");
> +	if (!IS_ERR(vref)) {
> +		int vref_uv;
> +
> +		ret = regulator_enable(vref);
> +		if (ret)
> +			goto error_disable_reg;
> +		st->vref = vref;
> +		vref_uv = regulator_get_voltage(vref);
> +		if (vref_uv <= 0) {
> +			ret = -EINVAL;
> +			goto error_disable_reg;
> +		}
> +		st->vref_uv = vref_uv;
> +	}
> +
>  	ret = max1363_alloc_scan_masks(indio_dev);
>  	if (ret)
>  		goto error_disable_reg;
> @@ -1604,6 +1626,8 @@ static int max1363_probe(struct i2c_client *client,
>  error_uninit_buffer:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  error_disable_reg:
> +	if (st->vref)
> +		regulator_disable(st->vref);
>  	regulator_disable(st->reg);
>  error_unregister_map:
>  	iio_map_array_unregister(indio_dev);
> @@ -1620,6 +1644,8 @@ static int max1363_remove(struct i2c_client *client)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> +	if (st->vref)
> +		regulator_disable(st->vref);
>  	regulator_disable(st->reg);
>  	iio_map_array_unregister(indio_dev);
>  	iio_device_free(indio_dev);
> 

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	Naveen Krishna Chatradhi
	<ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>,
	Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Subject: Re: [PATCH v2 2/4] iio/adc: (max1363) Add support for external reference voltage
Date: Sun, 03 Feb 2013 12:12:45 +0000	[thread overview]
Message-ID: <510E543D.50201@kernel.org> (raw)
In-Reply-To: <1359853180-5664-3-git-send-email-linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

On 02/03/2013 12:59 AM, Guenter Roeck wrote:
> Implement external reference voltage as regulator named "vref".
> 
> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
Applied to togreg branch of iio.git.
Thanks,

The remaining two patches will have sit a while.
Patch 3 effects stuff outside IIO so I'll wait to see if anyone
screams that it is going to cause merge nightmares!

Patch 4 is clearly still under discussion though a consensus seems
to have more or less been reached.

Thanks again for this work Guenter. I like the way one of my oldest
drivers is getting cleaned up along the way without me having to
do anything much ;)

> ---
> v2: Use regulator API to specify vref instead of creating new devicetree
>     bindings.
>     Keep reference voltage internally in uV, as this is the scale provided
>     by the regulator subsystem. We need the value in uV anyway, so that does
>     make some sense.
> 
>  drivers/iio/adc/max1363.c |   52 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index 1353fda..cac73d7 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -163,6 +163,8 @@ struct max1363_chip_info {
>   * @mask_low:		bitmask for enabled low thresholds
>   * @thresh_high:	high threshold values
>   * @thresh_low:		low threshold values
> + * @vref:		Reference voltage regulator
> + * @vref_uv:		Actual (external or internal) reference voltage
>   */
>  struct max1363_state {
>  	struct i2c_client		*client;
> @@ -182,6 +184,8 @@ struct max1363_state {
>  	/* 4x unipolar first then the fours bipolar ones */
>  	s16				thresh_high[8];
>  	s16				thresh_low[8];
> +	struct regulator		*vref;
> +	u32				vref_uv;
>  };
>  
>  #define MAX1363_MODE_SINGLE(_num, _mask) {				\
> @@ -393,6 +397,8 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
>  {
>  	struct max1363_state *st = iio_priv(indio_dev);
>  	int ret;
> +	unsigned long scale_uv;
> +
>  	switch (m) {
>  	case IIO_CHAN_INFO_RAW:
>  		ret = max1363_read_single_chan(indio_dev, chan, val, m);
> @@ -400,16 +406,10 @@ static int max1363_read_raw(struct iio_dev *indio_dev,
>  			return ret;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
> -		if ((1 << (st->chip_info->bits + 1)) >
> -		    st->chip_info->int_vref_mv) {
> -			*val = 0;
> -			*val2 = 500000;
> -			return IIO_VAL_INT_PLUS_MICRO;
> -		} else {
> -			*val = (st->chip_info->int_vref_mv)
> -				>> st->chip_info->bits;
> -			return IIO_VAL_INT;
> -		}
> +		scale_uv = st->vref_uv >> st->chip_info->bits;
> +		*val = scale_uv / 1000;
> +		*val2 = (scale_uv % 1000) * 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1390,12 +1390,16 @@ static const struct max1363_chip_info max1363_chip_info_tbl[] = {
>  
>  static int max1363_initial_setup(struct max1363_state *st)
>  {
> -	st->setupbyte = MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_VDD
> -		| MAX1363_SETUP_POWER_UP_INT_REF
> -		| MAX1363_SETUP_INT_CLOCK
> +	st->setupbyte = MAX1363_SETUP_INT_CLOCK
>  		| MAX1363_SETUP_UNIPOLAR
>  		| MAX1363_SETUP_NORESET;
>  
> +	if (st->vref)
> +		st->setupbyte |= MAX1363_SETUP_AIN3_IS_REF_EXT_TO_REF;
> +	else
> +		st->setupbyte |= MAX1363_SETUP_POWER_UP_INT_REF
> +		  | MAX1363_SETUP_AIN3_IS_AIN3_REF_IS_INT;
> +
>  	/* Set scan mode writes the config anyway so wait until then */
>  	st->setupbyte = MAX1363_SETUP_BYTE(st->setupbyte);
>  	st->current_mode = &max1363_mode_table[st->chip_info->default_mode];
> @@ -1533,6 +1537,7 @@ static int max1363_probe(struct i2c_client *client,
>  	int ret;
>  	struct max1363_state *st;
>  	struct iio_dev *indio_dev;
> +	struct regulator *vref;
>  
>  	indio_dev = iio_device_alloc(sizeof(struct max1363_state));
>  	if (indio_dev == NULL) {
> @@ -1563,6 +1568,23 @@ static int max1363_probe(struct i2c_client *client,
>  	st->chip_info = &max1363_chip_info_tbl[id->driver_data];
>  	st->client = client;
>  
> +	st->vref_uv = st->chip_info->int_vref_mv * 1000;
> +	vref = devm_regulator_get(&client->dev, "vref");
> +	if (!IS_ERR(vref)) {
> +		int vref_uv;
> +
> +		ret = regulator_enable(vref);
> +		if (ret)
> +			goto error_disable_reg;
> +		st->vref = vref;
> +		vref_uv = regulator_get_voltage(vref);
> +		if (vref_uv <= 0) {
> +			ret = -EINVAL;
> +			goto error_disable_reg;
> +		}
> +		st->vref_uv = vref_uv;
> +	}
> +
>  	ret = max1363_alloc_scan_masks(indio_dev);
>  	if (ret)
>  		goto error_disable_reg;
> @@ -1604,6 +1626,8 @@ static int max1363_probe(struct i2c_client *client,
>  error_uninit_buffer:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  error_disable_reg:
> +	if (st->vref)
> +		regulator_disable(st->vref);
>  	regulator_disable(st->reg);
>  error_unregister_map:
>  	iio_map_array_unregister(indio_dev);
> @@ -1620,6 +1644,8 @@ static int max1363_remove(struct i2c_client *client)
>  
>  	iio_device_unregister(indio_dev);
>  	iio_triggered_buffer_cleanup(indio_dev);
> +	if (st->vref)
> +		regulator_disable(st->vref);
>  	regulator_disable(st->reg);
>  	iio_map_array_unregister(indio_dev);
>  	iio_device_free(indio_dev);
> 

  reply	other threads:[~2013-02-03 12:12 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-03  0:59 [RFC v2 0/4] iio: Devicetree support Guenter Roeck
2013-02-03  0:59 ` Guenter Roeck
2013-02-03  0:59 ` [PATCH 1/4] iio: max1363: Use devm_ functions whereever possible to allocate resources Guenter Roeck
2013-02-03  0:59   ` Guenter Roeck
2013-02-03 12:10   ` Jonathan Cameron
2013-02-03 12:10     ` Jonathan Cameron
2013-02-03 17:18     ` Guenter Roeck
2013-02-03 17:18       ` Guenter Roeck
2013-02-03 18:02       ` Jonathan Cameron
2013-02-03 18:02         ` Jonathan Cameron
2013-02-03 18:16         ` Guenter Roeck
2013-02-03 18:16           ` Guenter Roeck
2013-02-03  0:59 ` [PATCH v2 2/4] iio/adc: (max1363) Add support for external reference voltage Guenter Roeck
2013-02-03  0:59   ` Guenter Roeck
2013-02-03 12:12   ` Jonathan Cameron [this message]
2013-02-03 12:12     ` Jonathan Cameron
2013-02-03  0:59 ` [PATCH RFC 3/4] iio: Update iio_channel_get API to use consumer device pointer as argument Guenter Roeck
2013-02-03  0:59   ` Guenter Roeck
2013-02-03  0:59 ` [PATCH v2 4/4] iio: Add OF support Guenter Roeck
2013-02-03  0:59   ` Guenter Roeck
2013-02-03  1:30   ` Tomasz Figa
2013-02-03  1:30     ` Tomasz Figa
2013-02-03  2:06     ` Guenter Roeck
2013-02-03  2:06       ` Guenter Roeck
2013-02-03 11:29       ` Lars-Peter Clausen
2013-02-03 11:29         ` Lars-Peter Clausen
2013-02-03 11:52         ` Tomasz Figa
2013-02-03 11:52           ` Tomasz Figa
2013-02-03 12:22           ` Lars-Peter Clausen
2013-02-03 12:22             ` Lars-Peter Clausen
2013-02-03 17:01           ` Guenter Roeck
2013-02-03 17:01             ` Guenter Roeck
2013-02-03 17:30             ` Tomasz Figa
2013-02-03 17:30               ` Tomasz Figa
2013-02-03 18:55               ` Lars-Peter Clausen
2013-02-03 18:55                 ` Lars-Peter Clausen
2013-02-03 20:58                 ` Jonathan Cameron
2013-02-03 20:58                   ` Jonathan Cameron
2013-02-03 22:44                   ` Lars-Peter Clausen
2013-02-03 22:44                     ` Lars-Peter Clausen
2013-02-03 23:14                 ` Tomasz Figa
2013-02-03 23:14                   ` Tomasz Figa
2013-02-04 17:12                   ` Guenter Roeck
2013-02-04 17:12                     ` Guenter Roeck
2013-02-04 17:41                     ` Lars-Peter Clausen
2013-02-04 17:41                       ` Lars-Peter Clausen
2013-02-04 17:51                     ` Guenter Roeck
2013-02-04 17:51                       ` Guenter Roeck
2013-02-04 18:00                       ` Tomasz Figa
2013-02-04 18:00                         ` Tomasz Figa
2013-02-04 18:09                         ` Guenter Roeck
2013-02-04 18:09                           ` Guenter Roeck
2013-02-03 16:31         ` Guenter Roeck
2013-02-03 16:31           ` Guenter Roeck
2013-02-03 19:00           ` Lars-Peter Clausen
2013-02-03 19:00             ` Lars-Peter Clausen
2013-02-03 14:17   ` Lars-Peter Clausen
2013-02-03 14:17     ` Lars-Peter Clausen
2013-02-03 16:22     ` Guenter Roeck
2013-02-03 16:22       ` Guenter Roeck
2013-02-04 16:37       ` Guenter Roeck
2013-02-04 16:37         ` Guenter Roeck

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=510E543D.50201@kernel.org \
    --to=jic23@kernel.org \
    --cc=ch.naveen@samsung.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=dianders@chromium.org \
    --cc=grant.likely@secretlab.ca \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=rob.herring@calxeda.com \
    --cc=tomasz.figa@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.