linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] iio: mxs-lradc: Add support for current source
@ 2014-01-28 13:52 Harald Geyer
  2014-02-08 11:59 ` Jonathan Cameron
  2014-02-08 14:38 ` Marek Vasut
  0 siblings, 2 replies; 6+ messages in thread
From: Harald Geyer @ 2014-01-28 13:52 UTC (permalink / raw)
  To: linux-iio; +Cc: Marek Vasut

We pretend the current source to be an independent 4-bit DAC,
which seems to be a valid use of the device. The channel also
allows reading back the value previously written.

Signed-off-by: Harald Geyer <harald@ccbib.org>
---
The LRADC can drive two of its ADC channels with a defined current
between 0 and 300uA to allow reading thermistors without external
current source. I'm not sure what the right IIO ABI in this case
should be. The way it is done now has the advantage that no new
ABI is needed and somebody might actually use the device as very
low resolution DAC. OTOH the relationship between output and input
channels is a bit non obvious this way.

Also note, that I didn't find any documentation about the expected
current unit in iio. I think uA is a reasonable choice, but this
should be confirmed and documented at least.

Harald

 drivers/staging/iio/adc/mxs-lradc.c |   80 +++++++++++++++++++++++++++++++++++
 1 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index df71669..48b1ed7 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -303,6 +303,11 @@ struct mxs_lradc {
 #define	LRADC_CTRL2				0x20
 #define	LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
 #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
+#define	LRADC_CTRL2_TEMP_SENSOR_IENABLE1	(1 << 9)
+#define	LRADC_CTRL2_TEMP_SENSOR_IENABLE0	(1 << 8)
+#define	LRADC_CTRL2_TEMP_ISRC1_OFFSET		4
+#define	LRADC_CTRL2_TEMP_ISRC0_OFFSET		0
+#define	LRADC_CTRL2_TEMP_ISRC_MASK		(0x0f)
 
 #define	LRADC_STATUS				0x40
 #define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
@@ -891,6 +896,31 @@ static int mxs_lradc_read_temp(struct iio_dev *iio_dev, int *val)
 	return IIO_VAL_INT;
 }
 
+static int mxs_lradc_read_current(struct mxs_lradc *lradc,
+			const struct iio_chan_spec *chan, int *val)
+{
+	*val = 0;
+	if (chan->channel == 0) {
+		if ((readl(lradc->base + LRADC_CTRL2) &
+					LRADC_CTRL2_TEMP_SENSOR_IENABLE0) == 0)
+			return IIO_VAL_INT;
+		*val = (readl(lradc->base + LRADC_CTRL2) >>
+					 LRADC_CTRL2_TEMP_ISRC0_OFFSET) &
+				LRADC_CTRL2_TEMP_ISRC_MASK;
+	} else if (chan->channel == 1) {
+		if ((readl(lradc->base + LRADC_CTRL2) &
+					LRADC_CTRL2_TEMP_SENSOR_IENABLE1) == 0)
+			return IIO_VAL_INT;
+		*val = (readl(lradc->base + LRADC_CTRL2) >>
+					LRADC_CTRL2_TEMP_ISRC1_OFFSET) &
+				LRADC_CTRL2_TEMP_ISRC_MASK;
+	} else {
+		return -EINVAL;
+	}
+
+	return IIO_VAL_INT;
+}
+
 static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 			const struct iio_chan_spec *chan,
 			int *val, int *val2, long m)
@@ -905,6 +935,8 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	case IIO_CHAN_INFO_RAW:
 		if (chan->type == IIO_TEMP)
 			return mxs_lradc_read_temp(iio_dev, val);
+		else if (chan->type == IIO_CURRENT)
+			return mxs_lradc_read_current(lradc, chan, val);
 
 		return mxs_lradc_read_single(iio_dev, chan->channel, val);
 
@@ -916,6 +948,9 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 			*val = 0;
 			*val2 = 253000;
 			return IIO_VAL_INT_PLUS_MICRO;
+		} else if (chan->type == IIO_CURRENT) {
+			*val = 20;
+			return IIO_VAL_INT;
 		}
 
 		*val = lradc->vref_mv[chan->channel];
@@ -977,6 +1012,31 @@ static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
 		}
 
 		break;
+	case IIO_CHAN_INFO_RAW:
+		ret = -EINVAL;
+		if (!chan->output || (val & ~LRADC_CTRL2_TEMP_ISRC_MASK) != 0) {
+			break;
+		} else if (chan->channel == 0) {
+			mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_ISRC_MASK
+					<< LRADC_CTRL2_TEMP_ISRC0_OFFSET,
+				LRADC_CTRL2);
+			mxs_lradc_reg_set(lradc,
+				val << LRADC_CTRL2_TEMP_ISRC0_OFFSET
+					| LRADC_CTRL2_TEMP_SENSOR_IENABLE0,
+				LRADC_CTRL2);
+			ret = 0;
+		} else if (chan->channel == 1) {
+			mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_ISRC_MASK
+					<< LRADC_CTRL2_TEMP_ISRC1_OFFSET,
+				LRADC_CTRL2);
+			mxs_lradc_reg_set(lradc,
+				val << LRADC_CTRL2_TEMP_ISRC1_OFFSET
+					| LRADC_CTRL2_TEMP_SENSOR_IENABLE1,
+				LRADC_CTRL2);
+			ret = 0;
+		}
+
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -1417,6 +1477,24 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
 	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
 	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
 	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
+	{
+		.output = 1,
+		.type = IIO_CURRENT,
+		.indexed = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.channel = 0,
+		.scan_type = {.sign = 'u', .realbits = 4,},
+	},
+	{
+		.output = 1,
+		.type = IIO_CURRENT,
+		.indexed = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.channel = 1,
+		.scan_type = {.sign = 'u', .realbits = 4,},
+	},
 };
 
 static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
@@ -1657,6 +1735,8 @@ static int mxs_lradc_remove(struct platform_device *pdev)
 	struct iio_dev *iio = platform_get_drvdata(pdev);
 	struct mxs_lradc *lradc = iio_priv(iio);
 
+	mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_SENSOR_IENABLE0 |
+				LRADC_CTRL2_TEMP_SENSOR_IENABLE1, LRADC_CTRL2);
 	iio_device_unregister(iio);
 	mxs_lradc_ts_unregister(lradc);
 	mxs_lradc_hw_stop(lradc);
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC] iio: mxs-lradc: Add support for current source
  2014-01-28 13:52 [RFC] iio: mxs-lradc: Add support for current source Harald Geyer
@ 2014-02-08 11:59 ` Jonathan Cameron
  2014-02-12 16:10   ` Mark Brown
  2014-02-08 14:38 ` Marek Vasut
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2014-02-08 11:59 UTC (permalink / raw)
  To: Harald Geyer, linux-iio; +Cc: Marek Vasut, Liam Girdwood, Mark Brown

On 28/01/14 13:52, Harald Geyer wrote:
> We pretend the current source to be an independent 4-bit DAC,
> which seems to be a valid use of the device. The channel also
> allows reading back the value previously written.
>
> Signed-off-by: Harald Geyer <harald@ccbib.org>
> ---
> The LRADC can drive two of its ADC channels with a defined current
> between 0 and 300uA to allow reading thermistors without external
> current source. I'm not sure what the right IIO ABI in this case
> should be. The way it is done now has the advantage that no new
> ABI is needed and somebody might actually use the device as very
> low resolution DAC. OTOH the relationship between output and input
> channels is a bit non obvious this way.
I'd be tempted to do this via the regulator framework instead of IIO.
At somepoint we could have a bridge to allow an iio interface on a regulator
if anyone wants it (you never know)

Have cc'd Mark and Liam for opinions.

I guess the main advantage in doing it in IIO would be to keep stuff in
one logical group.  Still they might just as easily be using an external
regulator to drive thermistors so using the regulator framework would be
more consistent...
>
> Also note, that I didn't find any documentation about the expected
> current unit in iio. I think uA is a reasonable choice, but this
> should be confirmed and documented at least.
Oops, we want to match hwmon for this probably so milli amps rather than micro.
>
> Harald
>
>   drivers/staging/iio/adc/mxs-lradc.c |   80 +++++++++++++++++++++++++++++++++++
>   1 files changed, 80 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index df71669..48b1ed7 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -303,6 +303,11 @@ struct mxs_lradc {
>   #define	LRADC_CTRL2				0x20
>   #define	LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
>   #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
> +#define	LRADC_CTRL2_TEMP_SENSOR_IENABLE1	(1 << 9)
> +#define	LRADC_CTRL2_TEMP_SENSOR_IENABLE0	(1 << 8)
> +#define	LRADC_CTRL2_TEMP_ISRC1_OFFSET		4
> +#define	LRADC_CTRL2_TEMP_ISRC0_OFFSET		0
> +#define	LRADC_CTRL2_TEMP_ISRC_MASK		(0x0f)
>
>   #define	LRADC_STATUS				0x40
>   #define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
> @@ -891,6 +896,31 @@ static int mxs_lradc_read_temp(struct iio_dev *iio_dev, int *val)
>   	return IIO_VAL_INT;
>   }
>
> +static int mxs_lradc_read_current(struct mxs_lradc *lradc,
> +			const struct iio_chan_spec *chan, int *val)
> +{
> +	*val = 0;
> +	if (chan->channel == 0) {
> +		if ((readl(lradc->base + LRADC_CTRL2) &
> +					LRADC_CTRL2_TEMP_SENSOR_IENABLE0) == 0)
> +			return IIO_VAL_INT;
> +		*val = (readl(lradc->base + LRADC_CTRL2) >>
> +					 LRADC_CTRL2_TEMP_ISRC0_OFFSET) &
> +				LRADC_CTRL2_TEMP_ISRC_MASK;
> +	} else if (chan->channel == 1) {
> +		if ((readl(lradc->base + LRADC_CTRL2) &
> +					LRADC_CTRL2_TEMP_SENSOR_IENABLE1) == 0)
> +			return IIO_VAL_INT;
> +		*val = (readl(lradc->base + LRADC_CTRL2) >>
> +					LRADC_CTRL2_TEMP_ISRC1_OFFSET) &
> +				LRADC_CTRL2_TEMP_ISRC_MASK;
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return IIO_VAL_INT;
> +}
> +
>   static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>   			const struct iio_chan_spec *chan,
>   			int *val, int *val2, long m)
> @@ -905,6 +935,8 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>   	case IIO_CHAN_INFO_RAW:
>   		if (chan->type == IIO_TEMP)
>   			return mxs_lradc_read_temp(iio_dev, val);
> +		else if (chan->type == IIO_CURRENT)
> +			return mxs_lradc_read_current(lradc, chan, val);
>
>   		return mxs_lradc_read_single(iio_dev, chan->channel, val);
>
> @@ -916,6 +948,9 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>   			*val = 0;
>   			*val2 = 253000;
>   			return IIO_VAL_INT_PLUS_MICRO;
> +		} else if (chan->type == IIO_CURRENT) {
> +			*val = 20;
> +			return IIO_VAL_INT;
>   		}
>
>   		*val = lradc->vref_mv[chan->channel];
> @@ -977,6 +1012,31 @@ static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
>   		}
>
>   		break;
> +	case IIO_CHAN_INFO_RAW:
> +		ret = -EINVAL;
> +		if (!chan->output || (val & ~LRADC_CTRL2_TEMP_ISRC_MASK) != 0) {
> +			break;
> +		} else if (chan->channel == 0) {
> +			mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_ISRC_MASK
> +					<< LRADC_CTRL2_TEMP_ISRC0_OFFSET,
> +				LRADC_CTRL2);
> +			mxs_lradc_reg_set(lradc,
> +				val << LRADC_CTRL2_TEMP_ISRC0_OFFSET
> +					| LRADC_CTRL2_TEMP_SENSOR_IENABLE0,
> +				LRADC_CTRL2);
> +			ret = 0;
> +		} else if (chan->channel == 1) {
> +			mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_ISRC_MASK
> +					<< LRADC_CTRL2_TEMP_ISRC1_OFFSET,
> +				LRADC_CTRL2);
> +			mxs_lradc_reg_set(lradc,
> +				val << LRADC_CTRL2_TEMP_ISRC1_OFFSET
> +					| LRADC_CTRL2_TEMP_SENSOR_IENABLE1,
> +				LRADC_CTRL2);
> +			ret = 0;
> +		}
> +
> +		break;
>   	default:
>   		ret = -EINVAL;
>   		break;
> @@ -1417,6 +1477,24 @@ static const struct iio_chan_spec mxs_lradc_chan_spec[] = {
>   	MXS_ADC_CHAN(13, IIO_VOLTAGE),	/* VDDD */
>   	MXS_ADC_CHAN(14, IIO_VOLTAGE),	/* VBG */
>   	MXS_ADC_CHAN(15, IIO_VOLTAGE),	/* VDD5V */
> +	{
> +		.output = 1,
> +		.type = IIO_CURRENT,
> +		.indexed = 1,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.channel = 0,
> +		.scan_type = {.sign = 'u', .realbits = 4,},
> +	},
> +	{
> +		.output = 1,
> +		.type = IIO_CURRENT,
> +		.indexed = 1,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.channel = 1,
> +		.scan_type = {.sign = 'u', .realbits = 4,},
> +	},
>   };
>
>   static int mxs_lradc_hw_init(struct mxs_lradc *lradc)
> @@ -1657,6 +1735,8 @@ static int mxs_lradc_remove(struct platform_device *pdev)
>   	struct iio_dev *iio = platform_get_drvdata(pdev);
>   	struct mxs_lradc *lradc = iio_priv(iio);
>
> +	mxs_lradc_reg_clear(lradc, LRADC_CTRL2_TEMP_SENSOR_IENABLE0 |
> +				LRADC_CTRL2_TEMP_SENSOR_IENABLE1, LRADC_CTRL2);
>   	iio_device_unregister(iio);
>   	mxs_lradc_ts_unregister(lradc);
>   	mxs_lradc_hw_stop(lradc);
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] iio: mxs-lradc: Add support for current source
  2014-01-28 13:52 [RFC] iio: mxs-lradc: Add support for current source Harald Geyer
  2014-02-08 11:59 ` Jonathan Cameron
@ 2014-02-08 14:38 ` Marek Vasut
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2014-02-08 14:38 UTC (permalink / raw)
  To: Harald Geyer; +Cc: linux-iio

On Tuesday, January 28, 2014 at 02:52:30 PM, Harald Geyer wrote:
> We pretend the current source to be an independent 4-bit DAC,
> which seems to be a valid use of the device. The channel also
> allows reading back the value previously written.
> 
> Signed-off-by: Harald Geyer <harald@ccbib.org>
> ---
> The LRADC can drive two of its ADC channels with a defined current
> between 0 and 300uA to allow reading thermistors without external
> current source. I'm not sure what the right IIO ABI in this case
> should be. The way it is done now has the advantage that no new
> ABI is needed and somebody might actually use the device as very
> low resolution DAC. OTOH the relationship between output and input
> channels is a bit non obvious this way.
> 
> Also note, that I didn't find any documentation about the expected
> current unit in iio. I think uA is a reasonable choice, but this
> should be confirmed and documented at least.
> 
> Harald
> 
>  drivers/staging/iio/adc/mxs-lradc.c |   80
> +++++++++++++++++++++++++++++++++++ 1 files changed, 80 insertions(+), 0
> deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index df71669..48b1ed7 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -303,6 +303,11 @@ struct mxs_lradc {
>  #define	LRADC_CTRL2				0x20
>  #define	LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET	24
>  #define	LRADC_CTRL2_TEMPSENSE_PWD		(1 << 15)
> +#define	LRADC_CTRL2_TEMP_SENSOR_IENABLE1	(1 << 9)
> +#define	LRADC_CTRL2_TEMP_SENSOR_IENABLE0	(1 << 8)
> +#define	LRADC_CTRL2_TEMP_ISRC1_OFFSET		4
> +#define	LRADC_CTRL2_TEMP_ISRC0_OFFSET		0
> +#define	LRADC_CTRL2_TEMP_ISRC_MASK		(0x0f)
> 
>  #define	LRADC_STATUS				0x40
>  #define	LRADC_STATUS_TOUCH_DETECT_RAW		(1 << 0)
> @@ -891,6 +896,31 @@ static int mxs_lradc_read_temp(struct iio_dev
> *iio_dev, int *val) return IIO_VAL_INT;
>  }
> 
> +static int mxs_lradc_read_current(struct mxs_lradc *lradc,
> +			const struct iio_chan_spec *chan, int *val)
> +{
> +	*val = 0;
> +	if (chan->channel == 0) {
> +		if ((readl(lradc->base + LRADC_CTRL2) &
> +					LRADC_CTRL2_TEMP_SENSOR_IENABLE0) == 0)
> +			return IIO_VAL_INT;
> +		*val = (readl(lradc->base + LRADC_CTRL2) >>
> +					 LRADC_CTRL2_TEMP_ISRC0_OFFSET) &
> +				LRADC_CTRL2_TEMP_ISRC_MASK;
> +	} else if (chan->channel == 1) {
> +		if ((readl(lradc->base + LRADC_CTRL2) &
> +					LRADC_CTRL2_TEMP_SENSOR_IENABLE1) == 0)
> +			return IIO_VAL_INT;
> +		*val = (readl(lradc->base + LRADC_CTRL2) >>
> +					LRADC_CTRL2_TEMP_ISRC1_OFFSET) &
> +				LRADC_CTRL2_TEMP_ISRC_MASK;

You can use parametrized macros here, lik LRADC_CTRL2_TEMP_ISRCn_OFFSET(n) , 
that way you'd trim down the code duplication here nicely. DTTO for the other 
large chunk of code down below.

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] iio: mxs-lradc: Add support for current source
  2014-02-08 11:59 ` Jonathan Cameron
@ 2014-02-12 16:10   ` Mark Brown
  2014-02-12 17:20     ` Jonathan Cameron
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2014-02-12 16:10 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Harald Geyer, linux-iio, Marek Vasut, Liam Girdwood

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On Sat, Feb 08, 2014 at 11:59:38AM +0000, Jonathan Cameron wrote:
> On 28/01/14 13:52, Harald Geyer wrote:

> >The LRADC can drive two of its ADC channels with a defined current
> >between 0 and 300uA to allow reading thermistors without external
> >current source. I'm not sure what the right IIO ABI in this case

> I'd be tempted to do this via the regulator framework instead of IIO.
> At somepoint we could have a bridge to allow an iio interface on a regulator
> if anyone wants it (you never know)

I'm very worried about the idea of an ABI - if IIO wants to offer
something that is implemented using the regulator API that's fine but
doing it directly from the regulator API sounds worrying.

> I guess the main advantage in doing it in IIO would be to keep stuff in
> one logical group.  Still they might just as easily be using an external
> regulator to drive thermistors so using the regulator framework would be
> more consistent...

The current that's driven seems more a property of the thermistor than
of the regulator if it is a generic regulator.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] iio: mxs-lradc: Add support for current source
  2014-02-12 16:10   ` Mark Brown
@ 2014-02-12 17:20     ` Jonathan Cameron
  2014-02-12 17:28       ` Lars-Peter Clausen
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2014-02-12 17:20 UTC (permalink / raw)
  To: Mark Brown; +Cc: Harald Geyer, linux-iio, Marek Vasut, Liam Girdwood



On February 12, 2014 4:10:46 PM GMT+00:00, Mark Brown <broonie@kernel.org> wrote:
>On Sat, Feb 08, 2014 at 11:59:38AM +0000, Jonathan Cameron wrote:
>> On 28/01/14 13:52, Harald Geyer wrote:
>
>> >The LRADC can drive two of its ADC channels with a defined current
>> >between 0 and 300uA to allow reading thermistors without external
>> >current source. I'm not sure what the right IIO ABI in this case
>
>> I'd be tempted to do this via the regulator framework instead of IIO.
>> At somepoint we could have a bridge to allow an iio interface on a
>regulator
>> if anyone wants it (you never know)
>
>I'm very worried about the idea of an ABI - if IIO wants to offer
>something that is implemented using the regulator API that's fine but
>doing it directly from the regulator API sounds worrying.

Would definitely be implemented using the regulator ABI. Would effectively pipe DAC writes from user space to regulator voltage requests.

Not sure I really like the idea anyway but some DACs look awfully like regulators so it might become an issue...

Definitely leave it until someone cares though!
>
>> I guess the main advantage in doing it in IIO would be to keep stuff
>in
>> one logical group.  Still they might just as easily be using an
>external
>> regulator to drive thermistors so using the regulator framework would
>be
>> more consistent...
>
>The current that's driven seems more a property of the thermistor than
>of the regulator if it is a generic regulator.

True. So do we give the thermistor a driver of its own?  Might make sense with it using a regulator and an iio channel. Output probably as hwmon?

Would be a small bit of glue code really.  There are quite a few similar cases with analog sensors that we don't currently represent at all.  Trick would be writing something generic and lightweight.

J



-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] iio: mxs-lradc: Add support for current source
  2014-02-12 17:20     ` Jonathan Cameron
@ 2014-02-12 17:28       ` Lars-Peter Clausen
  0 siblings, 0 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2014-02-12 17:28 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mark Brown, Harald Geyer, linux-iio, Marek Vasut, Liam Girdwood

On 02/12/2014 06:20 PM, Jonathan Cameron wrote:
>
>
> On February 12, 2014 4:10:46 PM GMT+00:00, Mark Brown <broonie@kernel.org> wrote:
>> On Sat, Feb 08, 2014 at 11:59:38AM +0000, Jonathan Cameron wrote:
>>> On 28/01/14 13:52, Harald Geyer wrote:
>>
>>>> The LRADC can drive two of its ADC channels with a defined current
>>>> between 0 and 300uA to allow reading thermistors without external
>>>> current source. I'm not sure what the right IIO ABI in this case
>>
>>> I'd be tempted to do this via the regulator framework instead of IIO.
>>> At somepoint we could have a bridge to allow an iio interface on a
>> regulator
>>> if anyone wants it (you never know)
>>
>> I'm very worried about the idea of an ABI - if IIO wants to offer
>> something that is implemented using the regulator API that's fine but
>> doing it directly from the regulator API sounds worrying.
>
> Would definitely be implemented using the regulator ABI. Would effectively pipe DAC writes from user space to regulator voltage requests.
>
> Not sure I really like the idea anyway but some DACs look awfully like regulators so it might become an issue...

Yep, e.g. see drivers/regulator/ad5398.c

- Lars

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-02-12 17:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-28 13:52 [RFC] iio: mxs-lradc: Add support for current source Harald Geyer
2014-02-08 11:59 ` Jonathan Cameron
2014-02-12 16:10   ` Mark Brown
2014-02-12 17:20     ` Jonathan Cameron
2014-02-12 17:28       ` Lars-Peter Clausen
2014-02-08 14:38 ` Marek Vasut

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).