[PATCH] ads7846: Add possibility to use external vref on ads7846

[PATCH] ads7846: Add possibility to use external vref on ads7846

[Alexander Stein]

Just set vref_mv in your platform config to use external vref. Otherwise
the internal one is used.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
This slight semantic change of vref_mv should not introduce a regression.
All boards using the 7846 model didn't set vref_mv which results in the
expected behavior.

Alexander
 drivers/input/touchscreen/ads7846.c |    8 +++-----
 include/linux/spi/ads7846.h         |    3 ++-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 1de1c19..a19e340 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -109,6 +109,7 @@ struct ads7846 {
 	u16			pressure_max;
 
 	bool			swap_xy;
+	bool			use_internal;
 
 	struct ads7846_packet	*packet;
 
@@ -307,7 +308,6 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
 	struct ads7846 *ts = dev_get_drvdata(dev);
 	struct ser_req *req;
 	int status;
-	int use_internal;
 
 	req = kzalloc(sizeof *req, GFP_KERNEL);
 	if (!req)
@@ -315,11 +315,8 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
 
 	spi_message_init(&req->msg);
 
-	/* FIXME boards with ads7846 might use external vref instead ... */
-	use_internal = (ts->model == 7846);
-
 	/* maybe turn on internal vREF, and let it settle */
-	if (use_internal) {
+	if (ts->use_internal) {
 		req->ref_on = REF_ON;
 		req->xfer[0].tx_buf = &req->ref_on;
 		req->xfer[0].len = 1;
@@ -509,6 +506,7 @@ static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
 		if (!ts->vref_mv) {
 			dev_dbg(&spi->dev, "assuming 2.5V internal vREF\n");
 			ts->vref_mv = 2500;
+			ts->use_internal = true;
 		}
 		break;
 	case 7845:
diff --git a/include/linux/spi/ads7846.h b/include/linux/spi/ads7846.h
index 92bd083..c64de9d 100644
--- a/include/linux/spi/ads7846.h
+++ b/include/linux/spi/ads7846.h
@@ -14,7 +14,8 @@ enum ads7846_filter {
 struct ads7846_platform_data {
 	u16	model;			/* 7843, 7845, 7846, 7873. */
 	u16	vref_delay_usecs;	/* 0 for external vref; etc */
-	u16	vref_mv;		/* external vref value, milliVolts */
+	u16	vref_mv;		/* external vref value, milliVolts
+					 * ads7846: if 0, use internal vref */
 	bool	keep_vref_on;		/* set to keep vref on for differential
 					 * measurements as well */
 	bool	swap_xy;		/* swap x and y axes */
-- 
1.7.3.4

Re: [PATCH] ads7846: Add possibility to use external vref on ads7846

[Jonathan Cameron]

On 05/05/11 09:32, Alexander Stein wrote:
> Just set vref_mv in your platform config to use external vref. Otherwise
> the internal one is used.
Would be nicer to see this done with the regulator framework. But then if you
have a use case on a board where that isn't available, then fair enough.
> 
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> ---
> This slight semantic change of vref_mv should not introduce a regression.
> All boards using the 7846 model didn't set vref_mv which results in the
> expected behavior.
> 
> Alexander
>  drivers/input/touchscreen/ads7846.c |    8 +++-----
>  include/linux/spi/ads7846.h         |    3 ++-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index 1de1c19..a19e340 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -109,6 +109,7 @@ struct ads7846 {
>  	u16			pressure_max;
>  
>  	bool			swap_xy;
> +	bool			use_internal;
>  
>  	struct ads7846_packet	*packet;
>  
> @@ -307,7 +308,6 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
>  	struct ads7846 *ts = dev_get_drvdata(dev);
>  	struct ser_req *req;
>  	int status;
> -	int use_internal;
>  
>  	req = kzalloc(sizeof *req, GFP_KERNEL);
>  	if (!req)
> @@ -315,11 +315,8 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
>  
>  	spi_message_init(&req->msg);
>  
> -	/* FIXME boards with ads7846 might use external vref instead ... */
> -	use_internal = (ts->model == 7846);
> -
>  	/* maybe turn on internal vREF, and let it settle */
> -	if (use_internal) {
> +	if (ts->use_internal) {
>  		req->ref_on = REF_ON;
>  		req->xfer[0].tx_buf = &req->ref_on;
>  		req->xfer[0].len = 1;
> @@ -509,6 +506,7 @@ static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
>  		if (!ts->vref_mv) {
>  			dev_dbg(&spi->dev, "assuming 2.5V internal vREF\n");
>  			ts->vref_mv = 2500;
> +			ts->use_internal = true;
>  		}
>  		break;
>  	case 7845:
> diff --git a/include/linux/spi/ads7846.h b/include/linux/spi/ads7846.h
> index 92bd083..c64de9d 100644
> --- a/include/linux/spi/ads7846.h
> +++ b/include/linux/spi/ads7846.h
> @@ -14,7 +14,8 @@ enum ads7846_filter {
>  struct ads7846_platform_data {
>  	u16	model;			/* 7843, 7845, 7846, 7873. */
>  	u16	vref_delay_usecs;	/* 0 for external vref; etc */
> -	u16	vref_mv;		/* external vref value, milliVolts */
> +	u16	vref_mv;		/* external vref value, milliVolts
> +					 * ads7846: if 0, use internal vref */
>  	bool	keep_vref_on;		/* set to keep vref on for differential
>  					 * measurements as well */
>  	bool	swap_xy;		/* swap x and y axes */

Re: [PATCH] ads7846: Add possibility to use external vref on ads7846

[Alexander Stein]

Am Donnerstag, 5. Mai 2011, 11:08:18 schrieb Jonathan Cameron:
> On 05/05/11 09:32, Alexander Stein wrote:
> > Just set vref_mv in your platform config to use external vref. Otherwise
> > the internal one is used.
> 
> Would be nicer to see this done with the regulator framework. But then if
> you have a use case on a board where that isn't available, then fair
> enough.

Well, for the regulator framework I would redesign the driver a bit. Currently 
the analogue inputs are displayed as mV in sysfs. If the current analogue 
input voltage is shown via the regulator framework the ads7846 driver itself 
can shows the actual ADC values. The latter ones don't need to know the 
reference voltage.
But I think this is a different problem than choosing the reference voltage 
for an ADC for what this patch is for.

Alexander

Re: [PATCH] ads7846: Add possibility to use external vref on ads7846

[Jonathan Cameron]

On 05/05/11 10:44, Alexander Stein wrote:
> Am Donnerstag, 5. Mai 2011, 11:08:18 schrieb Jonathan Cameron:
>> On 05/05/11 09:32, Alexander Stein wrote:
>>> Just set vref_mv in your platform config to use external vref. Otherwise
>>> the internal one is used.
>>
>> Would be nicer to see this done with the regulator framework. But then if
>> you have a use case on a board where that isn't available, then fair
>> enough.
> 
> Well, for the regulator framework I would redesign the driver a bit. Currently 
> the analogue inputs are displayed as mV in sysfs. If the current analogue 
> input voltage is shown via the regulator framework the ads7846 driver itself 
> can shows the actual ADC values. The latter ones don't need to know the 
> reference voltage.
> But I think this is a different problem than choosing the reference voltage 
> for an ADC for what this patch is for.
I'm somewhat confused by what you mean here.  The regulator framework is all
about power supplies.  This device would merely be a consumer that asks for
what (if anything) is being uspplied to the external vref line. Basically
it's a more general alternative to supplying the value via platform data.

Re: [PATCH] ads7846: Add possibility to use external vref on ads7846

[Alexander Stein]

Am Donnerstag, 5. Mai 2011, 12:01:25 schrieb Jonathan Cameron:
> On 05/05/11 10:44, Alexander Stein wrote:
> > Am Donnerstag, 5. Mai 2011, 11:08:18 schrieb Jonathan Cameron:
> >> On 05/05/11 09:32, Alexander Stein wrote:
> >>> Just set vref_mv in your platform config to use external vref.
> >>> Otherwise the internal one is used.
> >> 
> >> Would be nicer to see this done with the regulator framework. But then
> >> if you have a use case on a board where that isn't available, then fair
> >> enough.
> > 
> > Well, for the regulator framework I would redesign the driver a bit.
> > Currently the analogue inputs are displayed as mV in sysfs. If the
> > current analogue input voltage is shown via the regulator framework the
> > ads7846 driver itself can shows the actual ADC values. The latter ones
> > don't need to know the reference voltage.
> > But I think this is a different problem than choosing the reference
> > voltage for an ADC for what this patch is for.
> 
> I'm somewhat confused by what you mean here.  The regulator framework is
> all about power supplies.  This device would merely be a consumer that
> asks for what (if anything) is being uspplied to the external vref line.
> Basically it's a more general alternative to supplying the value via
> platform data.

Maybe I did get you wrong. I thought you meant retrieving the voltage at the 
analogue input somehow using the regulator framework.
I don't know much about this framework, so how can you handle this: The 
external vref is board-specific, e.g. hard-wired to 3,3V? There is no 
regulator which could adjust the voltage attached to ext vref.

Alexander

Re: [PATCH] ads7846: Add possibility to use external vref on ads7846

[Jonathan Cameron]

On 05/05/11 11:16, Alexander Stein wrote:
> Am Donnerstag, 5. Mai 2011, 12:01:25 schrieb Jonathan Cameron:
>> On 05/05/11 10:44, Alexander Stein wrote:
>>> Am Donnerstag, 5. Mai 2011, 11:08:18 schrieb Jonathan Cameron:
>>>> On 05/05/11 09:32, Alexander Stein wrote:
>>>>> Just set vref_mv in your platform config to use external vref.
>>>>> Otherwise the internal one is used.
>>>>
>>>> Would be nicer to see this done with the regulator framework. But then
>>>> if you have a use case on a board where that isn't available, then fair
>>>> enough.
>>>
>>> Well, for the regulator framework I would redesign the driver a bit.
>>> Currently the analogue inputs are displayed as mV in sysfs. If the
>>> current analogue input voltage is shown via the regulator framework the
>>> ads7846 driver itself can shows the actual ADC values. The latter ones
>>> don't need to know the reference voltage.
>>> But I think this is a different problem than choosing the reference
>>> voltage for an ADC for what this patch is for.
>>
>> I'm somewhat confused by what you mean here.  The regulator framework is
>> all about power supplies.  This device would merely be a consumer that
>> asks for what (if anything) is being uspplied to the external vref line.
>> Basically it's a more general alternative to supplying the value via
>> platform data.
> 
> Maybe I did get you wrong. I thought you meant retrieving the voltage at the 
> analogue input somehow using the regulator framework.
> I don't know much about this framework, so how can you handle this: The 
> external vref is board-specific, e.g. hard-wired to 3,3V? There is no 
> regulator which could adjust the voltage attached to ext vref.
Yup, you specify it as a fixed voltage regulator.  The reason to do this
is that others may well wire it up to a variable regulator (typical
embedded pmic chip for example).

Re: [PATCH] ads7846: Add possibility to use external vref on ads7846

[Alexander Stein]

Am Donnerstag, 5. Mai 2011, 12:33:53 schrieb Jonathan Cameron:
> On 05/05/11 11:16, Alexander Stein wrote:
> > Am Donnerstag, 5. Mai 2011, 12:01:25 schrieb Jonathan Cameron:
> >> On 05/05/11 10:44, Alexander Stein wrote:
> >>> Am Donnerstag, 5. Mai 2011, 11:08:18 schrieb Jonathan Cameron:
> >>>> On 05/05/11 09:32, Alexander Stein wrote:
> >>>>> Just set vref_mv in your platform config to use external vref.
> >>>>> Otherwise the internal one is used.
> >>>> 
> >>>> Would be nicer to see this done with the regulator framework. But then
> >>>> if you have a use case on a board where that isn't available, then
> >>>> fair enough.
> >>> 
> >>> Well, for the regulator framework I would redesign the driver a bit.
> >>> Currently the analogue inputs are displayed as mV in sysfs. If the
> >>> current analogue input voltage is shown via the regulator framework the
> >>> ads7846 driver itself can shows the actual ADC values. The latter ones
> >>> don't need to know the reference voltage.
> >>> But I think this is a different problem than choosing the reference
> >>> voltage for an ADC for what this patch is for.
> >> 
> >> I'm somewhat confused by what you mean here.  The regulator framework is
> >> all about power supplies.  This device would merely be a consumer that
> >> asks for what (if anything) is being uspplied to the external vref line.
> >> Basically it's a more general alternative to supplying the value via
> >> platform data.
> > 
> > Maybe I did get you wrong. I thought you meant retrieving the voltage at
> > the analogue input somehow using the regulator framework.
> > I don't know much about this framework, so how can you handle this: The
> > external vref is board-specific, e.g. hard-wired to 3,3V? There is no
> > regulator which could adjust the voltage attached to ext vref.
> 
> Yup, you specify it as a fixed voltage regulator.  The reason to do this
> is that others may well wire it up to a variable regulator (typical
> embedded pmic chip for example).

Ok, sounds ok to me. But you will then require the regulator framework to be 
setup for each board using the ads7846 driver.
Nevertheless I don't have an elegant idea to switch between internal vref 
(which is 2,5V fixed) and the (possible variable) ext vref in board-setup.

Alexander