[PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[Vignesh R]

Hi,

This patch adds optional DT properties for tscadc to set open delay,
sample delay and number of averages per sample for each adc channel.
Open delay, sample delay and averaging are some of the parameters that
affect the sampling rate and accuracy of the tscadc. Decreasing delays
and averaging helps to achieve higher sampling rates, while increasing
this parameters provides greater accuracy. Hence, this patch provides DT
properties to set the initial values for delays and number of averages
per sample. User space control via sysfs can be added later.


Vignesh R (2):
  iio: adc: ti_am335x_adc: refactor DT parsing into a function
  iio: adc: ti_am335x_adc: make sample delay, open delay, averaging DT
    parameters

 .../bindings/input/touchscreen/ti-tsc-adc.txt      | 24 +++++++
 drivers/iio/adc/ti_am335x_adc.c                    | 83 +++++++++++++++++-----
 2 files changed, 91 insertions(+), 16 deletions(-)

-- 
1.9.1

Fwd: Fwd: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[Hannes Petermaier]

Hi Vignesh,

did you recognize my last email ? whats your opinion around that =

best regards,
Hannes
----- Forwarded by Hannes Petermaier/Eggelsberg/AT/B&R on 14.04.2015 13:14 
-----

Hannes Petermaier/Eggelsberg/AT/B&R schrieb am 07.04.2015 14:30:19:

> From: Hannes Petermaier/Eggelsberg/AT/B&R
> To: vigneshr-l0cyMroinI0@public.gmane.org
> Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Date: 07.04.2015 14:30
> Subject: WG: Fwd: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT 
properties for tscadc
> 
> Hi Vignesh,
> 
> would it be possible to add some more channel-specific settings ?
> 
> It would be nice to have allmost full control to the STEPCONFIGx 
register.
> 
> At least we need to write the bits
> 
> SEL_RFM_SWC_1_0
> SEL_INM_SWC_3_0 
> SEL_RFP_SWC_2_0 
> 
> In the current mainline version only (SEL_INP_SWC_3_0) is written.
> So for the other bits "0" is value is used, for my point of view this is 
not correct.
> 
> For example if we want to read a value from AIN5 the negative pin from 
adc is 
> muxed allways to AIN0.
> In fact i can readout heavy jitter even if AIN5 is connected to ground - 
after
> setting up negative adc pin within code (to use REFN)  the readout value 
is 0 
> as expected without nameable jitter.
> If i short AIN0 also to ground, jitter is also eliminated.
> 
> Maybe this is also some fault of TI SoC ... in normal case somebody 
could 
> expect, that negative adc pin is equal even the Diff_CNTRL bit isn't set 
- but
> in practice it isn't.
> 
> Also actually it isn't possible to make some accurate measurement due to 
the 
> fact that allways VDDA_ADC is used as positive reference.
> 
> So it would be nice to have control around this bits.
> Whats your opinion around that?
> 
> best regards,
> Hannes
> ----- Forwarded by Hannes Petermaier/Eggelsberg/AT/B&R on 07.04.2015 
14:17 -----
> 
> From: Johannes Pointner <johannes.pointner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> To: hannes.petermaier-5O2GiQo/Ci2aMPzRcYMCawC/G2K4zDHf@public.gmane.org
> Date: 07.04.2015 13:58
> Subject: Fwd: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties 
for tscadc
> 
> ---------- Forwarded message ----------
> From: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
> Date: 2015-03-31 13:12 GMT+02:00
> Subject: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for 
tscadc
> To: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, Ian Campbell
> <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>, Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
> Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Hartmut Knaack <knaack.h-Mmb7MZpHnFY@public.gmane.org>, Lars-Peter Clausen
> <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>, Peter Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>, Dmitry Torokhov
> <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Karol Wrona <k.wrona-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, Jan
> Kardell <jan.kardell-KSZdJiTw9mzQT0dZR+AlfA@public.gmane.org>, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
> fcooper-l0cyMroinI0@public.gmane.org, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
> 
> 
> 
> Hi,
> 
> This patch adds optional DT properties for tscadc to set open delay,
> sample delay and number of averages per sample for each adc channel.
> Open delay, sample delay and averaging are some of the parameters that
> affect the sampling rate and accuracy of the tscadc. Decreasing delays
> and averaging helps to achieve higher sampling rates, while increasing
> this parameters provides greater accuracy. Hence, this patch provides DT
> properties to set the initial values for delays and number of averages
> per sample. User space control via sysfs can be added later.
> 
> 
> Vignesh R (2):
>   iio: adc: ti_am335x_adc: refactor DT parsing into a function
>   iio: adc: ti_am335x_adc: make sample delay, open delay, averaging DT
>     parameters
> 
>  .../bindings/input/touchscreen/ti-tsc-adc.txt      | 24 +++++++
>  drivers/iio/adc/ti_am335x_adc.c                    | 83 
+++++++++++++++++-----
>  2 files changed, 91 insertions(+), 16 deletions(-)
> 
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[Vignesh R]

Hi Hannes,

On Tuesday 14 April 2015 04:46 PM, Hannes Petermaier wrote:
> Hi Vignesh,
> 
> did you recognize my last email ? whats your opinion around that =

Apologies...

> 
> best regards,
> Hannes
> ----- Forwarded by Hannes Petermaier/Eggelsberg/AT/B&R on 14.04.2015 13:14 
> -----
> 
> Hannes Petermaier/Eggelsberg/AT/B&R schrieb am 07.04.2015 14:30:19:
> 
>> From: Hannes Petermaier/Eggelsberg/AT/B&R
>> To: vigneshr-l0cyMroinI0@public.gmane.org
>> Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Date: 07.04.2015 14:30
>> Subject: WG: Fwd: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT 
> properties for tscadc
>>
>> Hi Vignesh,
>>
>> would it be possible to add some more channel-specific settings ?
>>
>> It would be nice to have allmost full control to the STEPCONFIGx 
> register.
>>
>> At least we need to write the bits
>>
>> SEL_RFM_SWC_1_0
>> SEL_INM_SWC_3_0 
>> SEL_RFP_SWC_2_0 
>>
>> In the current mainline version only (SEL_INP_SWC_3_0) is written.
>> So for the other bits "0" is value is used, for my point of view this is 
> not correct.
>>
>> For example if we want to read a value from AIN5 the negative pin from 
> adc is 
>> muxed allways to AIN0.

Sorry... I didn't understand what you meant by"AIN5 is muxed always with
AIN0"?

>> In fact i can readout heavy jitter even if AIN5 is connected to ground - 
> after
>> setting up negative adc pin within code (to use REFN)  the readout value 
> is 0 
>> as expected without nameable jitter.
>> If i short AIN0 also to ground, jitter is also eliminated.

Hmmm... nobody has reported such behavior before. ADC support for
am335x-evm/beaglebone has been there for quite long time, but nobody
reported any jitter on AIN5 line. I think this may be specific to
your setup. Can you provide more info with regard to your setup?
Which kernel? Is it am335x-evm or beaglebone or a custom board?

>>
>> Maybe this is also some fault of TI SoC ... in normal case somebody 
> could 
>> expect, that negative adc pin is equal even the Diff_CNTRL bit isn't set 
> - but
>> in practice it isn't.
>>
>> Also actually it isn't possible to make some accurate measurement due to 
> the 
>> fact that allways VDDA_ADC is used as positive reference.
>>
>> So it would be nice to have control around this bits.
>> Whats your opinion around that?

Sorry, I am not yet clear on your bug/use-case.

Please comment inline while replying on mailing list

Regards
Vignesh

Re: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[Vignesh R]

Hi Hannes,

On Tuesday 14 April 2015 04:46 PM, Hannes Petermaier wrote:
> Hi Vignesh,
> 
> did you recognize my last email ? whats your opinion around that =
> 
> best regards,
> Hannes
> ----- Forwarded by Hannes Petermaier/Eggelsberg/AT/B&R on 14.04.2015 13:14 
> -----
> 
> Hannes Petermaier/Eggelsberg/AT/B&R schrieb am 07.04.2015 14:30:19:
> 
>> From: Hannes Petermaier/Eggelsberg/AT/B&R
>> To: vigneshr-l0cyMroinI0@public.gmane.org
>> Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Date: 07.04.2015 14:30
>> Subject: WG: Fwd: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT 
> properties for tscadc
>>
>> Hi Vignesh,
>>
>> would it be possible to add some more channel-specific settings ?
>>
>> It would be nice to have allmost full control to the STEPCONFIGx 
> register.
>>
>> At least we need to write the bits
>>
>> SEL_RFM_SWC_1_0
>> SEL_INM_SWC_3_0 
>> SEL_RFP_SWC_2_0 
>>
>> In the current mainline version only (SEL_INP_SWC_3_0) is written.
>> So for the other bits "0" is value is used, for my point of view this is 
> not correct.
>>
>> For example if we want to read a value from AIN5 the negative pin from 
> adc is 
>> muxed allways to AIN0.

Sorry... I didn't understand what you meant by"AIN5 is muxed always with
AIN0"?

>> In fact i can readout heavy jitter even if AIN5 is connected to ground - 
> after
>> setting up negative adc pin within code (to use REFN)  the readout value 
> is 0 
>> as expected without nameable jitter.
>> If i short AIN0 also to ground, jitter is also eliminated.

Hmmm... nobody has reported such behavior before. ADC support for
am335x-evm/beaglebone has been there for quite long time, but nobody
reported any jitter on AIN5 line. I think this may be specific to
your setup. Can you provide more info with regard to your setup?
Which kernel? Is it am335x-evm or beaglebone or a custom board?

>>
>> Maybe this is also some fault of TI SoC ... in normal case somebody 
> could 
>> expect, that negative adc pin is equal even the Diff_CNTRL bit isn't set 
> - but
>> in practice it isn't.
>>
>> Also actually it isn't possible to make some accurate measurement due to 
> the 
>> fact that allways VDDA_ADC is used as positive reference.
>>
>> So it would be nice to have control around this bits.
>> Whats your opinion around that?

Sorry, I am not yet clear on your bug/use-case.

Please comment inline while replying on mailing list

Regards
Vignesh

Re: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[Hannes Petermaier]

> Hi Hannes,
Hi Vignesh,
thanks for answer.

> >>
> >> would it be possible to add some more channel-specific settings ?
> >>
> >> It would be nice to have allmost full control to the STEPCONFIGx 
> > register.
> >>
> >> At least we need to write the bits
> >>
> >> SEL_RFM_SWC_1_0
> >> SEL_INM_SWC_3_0 
> >> SEL_RFP_SWC_2_0 
> >>
> >> In the current mainline version only (SEL_INP_SWC_3_0) is written.
> >> So for the other bits "0" is value is used, for my point of view this 
is 
> > not correct.
> >>
> >> For example if we want to read a value from AIN5 the negative pin 
from 
> > adc is 
> >> muxed allways to AIN0.
> 
> Sorry... I didn't understand what you meant by"AIN5 is muxed always with
> AIN0"?
Have a look to the TRM (spruh73k.pdf) Page 1740 / Figure 12-2. Functional 
Block Diagram.
There you can see that the ADC-cell which has two inputs, one positive and 
one negative.
Also there are two reference inputs, one positive - one negative.

All this "pins" are muxed, because only one channel per time can be 
sampled.
This muxes are controlled through the STEPCONFIGx registers.

If you want for example take some measurement from AIN5 the driver muxes 
the positive input from the ADC to AIN5 by setting the bits for 
SEL_INP<3:0> - this is ok.
But the bits for SEL_INM<3:0> are still 'zero'. 
In summary this results in following mux-setting (regarding page 1771 in 
TRM):

positive-reference muxed to   VDDA
negative-reference muxed to   VSSA
positive-input muxed to       AIN5
negative-input muxed to       AIN0

>From this setup we run into 2 problems:
- the negative input terminal is muxed maybe to wrong potential
In much cases we have a single-ended signal so this setup looks good at 
first, because the "Diff_CNTRL" bit is also false. 
In fact there is an influence to the reading if the negative 
input-terminal isn't setup correctly (to VSSA or REFN). 
Maybe i interpret the "Diff_CNTRL" not correctly, there is no detailed 
description within the TRM - maybe some of your workmates can explain you 
the functionality of this bit.

- reference is allways taken from VDDA/VSSA
For a precision measurement you dont't use in normal case the 
analog-supply.
This rail brings noise, drift - all things whicht we don't need for 
accurate measurement.

> 
> >> In fact i can readout heavy jitter even if AIN5 is connected to 
ground - 
> > after
> >> setting up negative adc pin within code (to use REFN)  the readout 
value 
> > is 0 
> >> as expected without nameable jitter.
> >> If i short AIN0 also to ground, jitter is also eliminated.
> 
> Hmmm... nobody has reported such behavior before. ADC support for
> am335x-evm/beaglebone has been there for quite long time, but nobody
> reported any jitter on AIN5 line. I think this may be specific to
> your setup. Can you provide more info with regard to your setup?
> Which kernel? Is it am335x-evm or beaglebone or a custom board?
Maybe nobody does some precision measurement with beaglebone.
For operating some touchscreen or readout a potentiometer for evaluation 
purpose it is still good enough.

Kernel is current mainline, 4.0
Board is some custom board of my company.
But all this parameters shouldn't have some influence to the case.

> >>
> >> Maybe this is also some fault of TI SoC ... in normal case somebody 
> > could 
> >> expect, that negative adc pin is equal even the Diff_CNTRL bit isn't 
set 
> > - but
> >> in practice it isn't.
> >>
> >> Also actually it isn't possible to make some accurate measurement due 
to 
> > the 
> >> fact that allways VDDA_ADC is used as positive reference.
> >>
> >> So it would be nice to have control around this bits.
> >> Whats your opinion around that?
> 
> Sorry, I am not yet clear on your bug/use-case.
> 
> Please comment inline while replying on mailing list
okay so ?
 
> Regards
> Vignesh
best regards,
Hannes



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties for tscadc

[Vignesh R]

Hi Hannes,

On Wednesday 29 April 2015 10:36 AM, Hannes Petermaier wrote:
> Hi Vignesh,
> 
> any comments on this ?
> I didn't hear anything last 2 weeks from you.

Apologies...  For some reason my mail client classified your reply mails
as junk, hence I never look into it.

I agree that making SEL_INM_SWC_3_0, SEL_RFM_SWC_1_0 and SEL_RFP_SWC_2_0
configurable is good to have. But I don't think I will be able to work
on it anytime sooner.

Regards
Vignesh

> 
> best regards,
> Hannes
> 
> ----- Forwarded by Hannes Petermaier/Eggelsberg/AT/B&R on 29.04.2015 07:03 
> -----
> 
>> From: Hannes Petermaier/Eggelsberg/AT/B&R
>> To: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dmitry Torokhov 
> <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, 
>> fcooper-l0cyMroinI0@public.gmane.org, Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, Ian Campbell<ijc
>> +devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>, Jan Kardell <jan.kardell-KSZdJiTw9mzQT0dZR+AlfA@public.gmane.org>, 
> Johannes 
>> Pointner <Johannes.Pointner-5O2GiQo/Ci2aMPzRcYMCawC/G2K4zDHf@public.gmane.org>, Hartmut Knaack 
>> <knaack.h-Mmb7MZpHnFY@public.gmane.org>, Karol Wrona <k.wrona-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, Lars-Peter Clausen 
> 
>> <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, 
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, 
>> Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>, 
> Peter 
>> Meerwald <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Date: 15.04.2015 07:33
>> Subject: Re: [PATCH 0/2] iio: ti_am335x_adc: Add optional DT properties 
> for tscadc
>>
>>> Hi Hannes,
>> Hi Vignesh,
>> thanks for answer.
>>
>>>>>
>>>>> would it be possible to add some more channel-specific settings ?
>>>>>
>>>>> It would be nice to have allmost full control to the STEPCONFIGx 
>>>> register.
>>>>>
>>>>> At least we need to write the bits
>>>>>
>>>>> SEL_RFM_SWC_1_0
>>>>> SEL_INM_SWC_3_0 
>>>>> SEL_RFP_SWC_2_0 
>>>>>
>>>>> In the current mainline version only (SEL_INP_SWC_3_0) is written.
>>>>> So for the other bits "0" is value is used, for my point of view 
> this is 
>>>> not correct.
>>>>>
>>>>> For example if we want to read a value from AIN5 the negative pin 
> from 
>>>> adc is 
>>>>> muxed allways to AIN0.
>>>
>>> Sorry... I didn't understand what you meant by"AIN5 is muxed always 
> with
>>> AIN0"?
>> Have a look to the TRM (spruh73k.pdf) Page 1740 / Figure 12-2. 
> Functional Block Diagram.
>> There you can see that the ADC-cell which has two inputs, one positive 
> and onenegative.
>> Also there are two reference inputs, one positive - one negative.
>>
>> All this "pins" are muxed, because only one channel per time can be 
> sampled.
>> This muxes are controlled through the STEPCONFIGx registers.
>>
>> If you want for example take some measurement from AIN5 the driver muxes 
> the 
>> positive input from the ADC to AIN5 by setting the bits for SEL_INP<3:0> 
> - this is ok.
>> But the bits for SEL_INM<3:0> are still 'zero'. 
>> In summary this results in following mux-setting (regarding page 1771 in 
> TRM):
>>
>> positive-reference muxed to   VDDA
>> negative-reference muxed to   VSSA
>> positive-input muxed to       AIN5
>> negative-input muxed to       AIN0
>>
>> From this setup we run into 2 problems:
>> - the negative input terminal is muxed maybe to wrong potential
>> In much cases we have a single-ended signal so this setup looks good at 
> first,
>> because the "Diff_CNTRL" bit is also false. 
>> In fact there is an influence to the reading if the negative 
> input-terminal 
>> isn't setup correctly (to VSSA or REFN). 
>> Maybe i interpret the "Diff_CNTRL" not correctly, there is no detailed 
>> description within the TRM - maybe some of your workmates can explain 
> you the 
>> functionality of this bit.
>>
>> - reference is allways taken from VDDA/VSSA
>> For a precision measurement you dont't use in normal case the 
> analog-supply.
>> This rail brings noise, drift - all things whicht we don't need for 
> accurate 
>> measurement.
>>
>>>
>>>>> In fact i can readout heavy jitter even if AIN5 is connected to 
> ground - 
>>>> after
>>>>> setting up negative adc pin within code (to use REFN)  the readout 
> value 
>>>> is 0 
>>>>> as expected without nameable jitter.
>>>>> If i short AIN0 also to ground, jitter is also eliminated.
>>>
>>> Hmmm... nobody has reported such behavior before. ADC support for
>>> am335x-evm/beaglebone has been there for quite long time, but nobody
>>> reported any jitter on AIN5 line. I think this may be specific to
>>> your setup. Can you provide more info with regard to your setup?
>>> Which kernel? Is it am335x-evm or beaglebone or a custom board?
>> Maybe nobody does some precision measurement with beaglebone.
>> For operating some touchscreen or readout a potentiometer for evaluation 
> 
>> purpose it is still good enough.
>>
>> Kernel is current mainline, 4.0
>> Board is some custom board of my company.
>> But all this parameters shouldn't have some influence to the case.
>>
>>>>>
>>>>> Maybe this is also some fault of TI SoC ... in normal case somebody 
> 
>>>> could 
>>>>> expect, that negative adc pin is equal even the Diff_CNTRL bit 
> isn't set 
>>>> - but
>>>>> in practice it isn't.
>>>>>
>>>>> Also actually it isn't possible to make some accurate measurement 
> due to 
>>>> the 
>>>>> fact that allways VDDA_ADC is used as positive reference.
>>>>>
>>>>> So it would be nice to have control around this bits.
>>>>> Whats your opinion around that?
>>>
>>> Sorry, I am not yet clear on your bug/use-case.
>>>
>>> Please comment inline while replying on mailing list
>> okay so ?
>>
>>> Regards
>>> Vignesh
>> best regards,
>> Hannes
>>
>>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html