Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1506917 > unrolled thread

[PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz

Started byMugunthan V N <mugunthanvnm@ti.com>
First post2016-10-24 08:10 +0200
Last post2016-10-26 10:50 +0200
Articles 10 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz Mugunthan V N <mugunthanvnm@ti.com> - 2016-10-24 08:10 +0200
    Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz John Syne <john3909@gmail.com> - 2016-10-24 23:00 +0200
      Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to  24MHz Mugunthan V N <mugunthanvnm@ti.com> - 2016-10-25 08:00 +0200
        Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz John Syne <john3909@gmail.com> - 2016-10-25 08:10 +0200
          Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz John Syne <john3909@gmail.com> - 2016-10-25 08:20 +0200
            Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to  24MHz Vignesh R <vigneshr@ti.com> - 2016-10-25 08:40 +0200
              Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz John Syne <john3909@gmail.com> - 2016-10-25 17:40 +0200
            Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock  to 24MHz Lee Jones <lee.jones@linaro.org> - 2016-10-25 08:40 +0200
              Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz John Syne <john3909@gmail.com> - 2016-10-25 17:50 +0200
                Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock  to 24MHz Lee Jones <lee.jones@linaro.org> - 2016-10-26 10:50 +0200

#1506917 — [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz

FromMugunthan V N <mugunthanvnm@ti.com>
Date2016-10-24 08:10 +0200
Subject[PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
Message-ID<svGCm-3GK-15@gated-at.bofh.it>
Increase ADC reference clock from 3MHz to 24MHz so that the
sampling rates goes up from 100K samples per second to 800K
samples per second on AM335x and AM437x SoC.

Also increase opendelay for touchscreen configuration to
equalize the increase in ADC reference clock frequency,
which results in the same amount touch events reported via
evtest on AM335x GP EVM.

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---

This patch depends on ADC DMA patch series [1]

Without DMA support, when ADC ref clock is set at 24MHz, I am
seeing fifo overflow as CPU is not able to pull the ADC samples.
This answers that DMA support is must for ADC to consume the
samples generated at 24MHz with no open, step delay or
averaging with patch [2].

Measured the performance with the iio_generic_buffer with the
patch [3] applied

[1] - http://www.spinics.net/lists/devicetree/msg145045.html
[2] - http://pastebin.ubuntu.com/23357935/
[3] - http://pastebin.ubuntu.com/23357939/

---
 include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index b9a53e0..96c4207 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -90,7 +90,7 @@
 /* Delay register */
 #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
 #define STEPDELAY_OPEN(val)	((val) << 0)
-#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
+#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x500)
 #define STEPDELAY_SAMPLE_MASK	(0xFF << 24)
 #define STEPDELAY_SAMPLE(val)	((val) << 24)
 #define STEPCONFIG_SAMPLEDLY	STEPDELAY_SAMPLE(0)
@@ -137,7 +137,7 @@
 #define SEQ_STATUS BIT(5)
 #define CHARGE_STEP		0x11
 
-#define ADC_CLK			3000000
+#define ADC_CLK			24000000
 #define TOTAL_STEPS		16
 #define TOTAL_CHANNELS		8
 #define FIFO1_THRESHOLD		19
-- 
2.10.1.502.g6598894

[toc] | [next] | [standalone]

#1507729

FromJohn Syne <john3909@gmail.com>
Date2016-10-24 23:00 +0200
Message-ID<svUvE-4ir-33@gated-at.bofh.it>
In reply to#1506917
> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> 
> Increase ADC reference clock from 3MHz to 24MHz so that the
> sampling rates goes up from 100K samples per second to 800K
> samples per second on AM335x and AM437x SoC.
> 
> Also increase opendelay for touchscreen configuration to
> equalize the increase in ADC reference clock frequency,
> which results in the same amount touch events reported via
> evtest on AM335x GP EVM.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> 
> This patch depends on ADC DMA patch series [1]
> 
> Without DMA support, when ADC ref clock is set at 24MHz, I am
> seeing fifo overflow as CPU is not able to pull the ADC samples.
> This answers that DMA support is must for ADC to consume the
> samples generated at 24MHz with no open, step delay or
> averaging with patch [2].
> 
> Measured the performance with the iio_generic_buffer with the
> patch [3] applied
> 
> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
> [2] - http://pastebin.ubuntu.com/23357935/
> [3] - http://pastebin.ubuntu.com/23357939/
> 
> ---
> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index b9a53e0..96c4207 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -90,7 +90,7 @@
> /* Delay register */
> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
> #define STEPDELAY_OPEN(val)	((val) << 0)
> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
Wouldn’t this be better to add this to the devicetree?

	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;

Regards,
John
> +#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x500)
> #define STEPDELAY_SAMPLE_MASK	(0xFF << 24)
> #define STEPDELAY_SAMPLE(val)	((val) << 24)
> #define STEPCONFIG_SAMPLEDLY	STEPDELAY_SAMPLE(0)
> @@ -137,7 +137,7 @@
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP		0x11
> 
> -#define ADC_CLK			3000000
> +#define ADC_CLK			24000000
> #define TOTAL_STEPS		16
> #define TOTAL_CHANNELS		8
> #define FIFO1_THRESHOLD		19
> -- 
> 2.10.1.502.g6598894
>

[toc] | [prev] | [next] | [standalone]

#1507992 — Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz

FromMugunthan V N <mugunthanvnm@ti.com>
Date2016-10-25 08:00 +0200
SubjectRe: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
Message-ID<sw2Wd-1r5-5@gated-at.bofh.it>
In reply to#1507729
On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
>> > On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> > 
>> > Increase ADC reference clock from 3MHz to 24MHz so that the
>> > sampling rates goes up from 100K samples per second to 800K
>> > samples per second on AM335x and AM437x SoC.
>> > 
>> > Also increase opendelay for touchscreen configuration to
>> > equalize the increase in ADC reference clock frequency,
>> > which results in the same amount touch events reported via
>> > evtest on AM335x GP EVM.
>> > 
>> > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> > ---
>> > 
>> > This patch depends on ADC DMA patch series [1]
>> > 
>> > Without DMA support, when ADC ref clock is set at 24MHz, I am
>> > seeing fifo overflow as CPU is not able to pull the ADC samples.
>> > This answers that DMA support is must for ADC to consume the
>> > samples generated at 24MHz with no open, step delay or
>> > averaging with patch [2].
>> > 
>> > Measured the performance with the iio_generic_buffer with the
>> > patch [3] applied
>> > 
>> > [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>> > [2] - http://pastebin.ubuntu.com/23357935/
>> > [3] - http://pastebin.ubuntu.com/23357939/
>> > 
>> > ---
>> > include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> > index b9a53e0..96c4207 100644
>> > --- a/include/linux/mfd/ti_am335x_tscadc.h
>> > +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> > @@ -90,7 +90,7 @@
>> > /* Delay register */
>> > #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>> > #define STEPDELAY_OPEN(val)	((val) << 0)
>> > -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
> Wouldn’t this be better to add this to the devicetree?
> 
> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;

For a touch screen, there is not need to change in these parameter
settings, so my opinion is to keep it as is. Or am I missing something?

Regards
Mugunthan V N

[toc] | [prev] | [next] | [standalone]

#1507994

FromJohn Syne <john3909@gmail.com>
Date2016-10-25 08:10 +0200
Message-ID<sw35T-1Ka-3@gated-at.bofh.it>
In reply to#1507992
> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> 
> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> 
>>>> Increase ADC reference clock from 3MHz to 24MHz so that the
>>>> sampling rates goes up from 100K samples per second to 800K
>>>> samples per second on AM335x and AM437x SoC.
>>>> 
>>>> Also increase opendelay for touchscreen configuration to
>>>> equalize the increase in ADC reference clock frequency,
>>>> which results in the same amount touch events reported via
>>>> evtest on AM335x GP EVM.
>>>> 
>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>> ---
>>>> 
>>>> This patch depends on ADC DMA patch series [1]
>>>> 
>>>> Without DMA support, when ADC ref clock is set at 24MHz, I am
>>>> seeing fifo overflow as CPU is not able to pull the ADC samples.
>>>> This answers that DMA support is must for ADC to consume the
>>>> samples generated at 24MHz with no open, step delay or
>>>> averaging with patch [2].
>>>> 
>>>> Measured the performance with the iio_generic_buffer with the
>>>> patch [3] applied
>>>> 
>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>> 
>>>> ---
>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>> index b9a53e0..96c4207 100644
>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>> @@ -90,7 +90,7 @@
>>>> /* Delay register */
>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>> Wouldn’t this be better to add this to the devicetree?
>> 
>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> 
> For a touch screen, there is not need to change in these parameter
> settings, so my opinion is to keep it as is. Or am I missing something?
I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen. 

Regards,
John
> 
> Regards
> Mugunthan V N

[toc] | [prev] | [next] | [standalone]

#1507999

FromJohn Syne <john3909@gmail.com>
Date2016-10-25 08:20 +0200
Message-ID<sw3fz-1Qe-7@gated-at.bofh.it>
In reply to#1507994
> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
> 
>> 
>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> 
>> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
>>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>>> 
>>>>> Increase ADC reference clock from 3MHz to 24MHz so that the
>>>>> sampling rates goes up from 100K samples per second to 800K
>>>>> samples per second on AM335x and AM437x SoC.
>>>>> 
>>>>> Also increase opendelay for touchscreen configuration to
>>>>> equalize the increase in ADC reference clock frequency,
>>>>> which results in the same amount touch events reported via
>>>>> evtest on AM335x GP EVM.
>>>>> 
>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>> ---
>>>>> 
>>>>> This patch depends on ADC DMA patch series [1]
>>>>> 
>>>>> Without DMA support, when ADC ref clock is set at 24MHz, I am
>>>>> seeing fifo overflow as CPU is not able to pull the ADC samples.
>>>>> This answers that DMA support is must for ADC to consume the
>>>>> samples generated at 24MHz with no open, step delay or
>>>>> averaging with patch [2].
>>>>> 
>>>>> Measured the performance with the iio_generic_buffer with the
>>>>> patch [3] applied
>>>>> 
>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>> 
>>>>> ---
>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>> 
>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>>> index b9a53e0..96c4207 100644
>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>>> @@ -90,7 +90,7 @@
>>>>> /* Delay register */
>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>>> Wouldn’t this be better to add this to the devicetree?
>>> 
>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>> 
>> For a touch screen, there is not need to change in these parameter
>> settings, so my opinion is to keep it as is. Or am I missing something?
> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen. 

Here is a DT overlay were this gets using on the BeagleBoneBlack.  

https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts

Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.

Regards,
John

> 
> Regards,
> John
>> 
>> Regards
>> Mugunthan V N

[toc] | [prev] | [next] | [standalone]

#1508001 — Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz

FromVignesh R <vigneshr@ti.com>
Date2016-10-25 08:40 +0200
SubjectRe: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
Message-ID<sw3yV-1WB-7@gated-at.bofh.it>
In reply to#1507999

On Tuesday 25 October 2016 11:46 AM, John Syne wrote:
> 
>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>>
>>>
>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>
[...]
>>>>>>
>>>>>> ---
>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>> index b9a53e0..96c4207 100644
>>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>> @@ -90,7 +90,7 @@
>>>>>> /* Delay register */
>>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>>>> Wouldn’t this be better to add this to the devicetree?
>>>>
>>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>
>>> For a touch screen, there is not need to change in these parameter
>>> settings, so my opinion is to keep it as is. Or am I missing something?
>> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen.
> 

ti_am335x_adc driver already supports above DT parameters and its upto
the user to adjust these parameters as required.

> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
> 
> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> 
> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.
> 

Touchscreen driver (ti_am335x_tsc.c) does not support above DT parameters.

-- 
Regards
Vignesh

[toc] | [prev] | [next] | [standalone]

#1508382

FromJohn Syne <john3909@gmail.com>
Date2016-10-25 17:40 +0200
Message-ID<swbZw-7sf-3@gated-at.bofh.it>
In reply to#1508001
> On Oct 24, 2016, at 11:37 PM, Vignesh R <vigneshr@ti.com> wrote:
> 
> 
> 
> On Tuesday 25 October 2016 11:46 AM, John Syne wrote:
>> 
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>>> 
>>>> 
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> 
> [...]
>>>>>>> 
>>>>>>> ---
>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> index b9a53e0..96c4207 100644
>>>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> @@ -90,7 +90,7 @@
>>>>>>> /* Delay register */
>>>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>>>>> Wouldn’t this be better to add this to the devicetree?
>>>>> 
>>>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>>>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>> 
>>>> For a touch screen, there is not need to change in these parameter
>>>> settings, so my opinion is to keep it as is. Or am I missing something?
>>> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen.
>> 
> 
> ti_am335x_adc driver already supports above DT parameters and its upto
> the user to adjust these parameters as required.
> 
>> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
>> 
>> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
>> 
>> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.
>> 
> 
> Touchscreen driver (ti_am335x_tsc.c) does not support above DT parameters.
This patch series also modifies ti_am335x_adc.c

https://github.com/analogdevicesinc/linux/blob/master/drivers/iio/adc/ti_am335x_adc.c#L447

Regards,
John
> 
> -- 
> Regards
> Vignesh

[toc] | [prev] | [next] | [standalone]

#1508005 — Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz

FromLee Jones <lee.jones@linaro.org>
Date2016-10-25 08:40 +0200
SubjectRe: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
Message-ID<sw3yV-1WB-13@gated-at.bofh.it>
In reply to#1507999
On Mon, 24 Oct 2016, John Syne wrote:
> > On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
> >> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> >> 
> >> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
> >>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> >>>>> 
> >>>>> Increase ADC reference clock from 3MHz to 24MHz so that the
> >>>>> sampling rates goes up from 100K samples per second to 800K
> >>>>> samples per second on AM335x and AM437x SoC.
> >>>>> 
> >>>>> Also increase opendelay for touchscreen configuration to
> >>>>> equalize the increase in ADC reference clock frequency,
> >>>>> which results in the same amount touch events reported via
> >>>>> evtest on AM335x GP EVM.
> >>>>> 
> >>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> >>>>> ---
> >>>>> 
> >>>>> This patch depends on ADC DMA patch series [1]
> >>>>> 
> >>>>> Without DMA support, when ADC ref clock is set at 24MHz, I am
> >>>>> seeing fifo overflow as CPU is not able to pull the ADC samples.
> >>>>> This answers that DMA support is must for ADC to consume the
> >>>>> samples generated at 24MHz with no open, step delay or
> >>>>> averaging with patch [2].
> >>>>> 
> >>>>> Measured the performance with the iio_generic_buffer with the
> >>>>> patch [3] applied
> >>>>> 
> >>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
> >>>>> [2] - http://pastebin.ubuntu.com/23357935/
> >>>>> [3] - http://pastebin.ubuntu.com/23357939/
> >>>>> 
> >>>>> ---
> >>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
> >>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>> 
> >>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> >>>>> index b9a53e0..96c4207 100644
> >>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
> >>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> >>>>> @@ -90,7 +90,7 @@
> >>>>> /* Delay register */
> >>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
> >>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
> >>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
> >>> Wouldn’t this be better to add this to the devicetree?
> >>> 
> >>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> >>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
> >>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> >> 
> >> For a touch screen, there is not need to change in these parameter
> >> settings, so my opinion is to keep it as is. Or am I missing something?
> > I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen. 
> 
> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
> 
> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> 
> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.

This looks like configuration, no?

DT should be used to describe the hardware.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1508390

FromJohn Syne <john3909@gmail.com>
Date2016-10-25 17:50 +0200
Message-ID<swc9c-7vM-13@gated-at.bofh.it>
In reply to#1508005
> On Oct 24, 2016, at 11:38 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> On Mon, 24 Oct 2016, John Syne wrote:
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>> 
>>>> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
>>>>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>>>>>>> 
>>>>>>> Increase ADC reference clock from 3MHz to 24MHz so that the
>>>>>>> sampling rates goes up from 100K samples per second to 800K
>>>>>>> samples per second on AM335x and AM437x SoC.
>>>>>>> 
>>>>>>> Also increase opendelay for touchscreen configuration to
>>>>>>> equalize the increase in ADC reference clock frequency,
>>>>>>> which results in the same amount touch events reported via
>>>>>>> evtest on AM335x GP EVM.
>>>>>>> 
>>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>>>> ---
>>>>>>> 
>>>>>>> This patch depends on ADC DMA patch series [1]
>>>>>>> 
>>>>>>> Without DMA support, when ADC ref clock is set at 24MHz, I am
>>>>>>> seeing fifo overflow as CPU is not able to pull the ADC samples.
>>>>>>> This answers that DMA support is must for ADC to consume the
>>>>>>> samples generated at 24MHz with no open, step delay or
>>>>>>> averaging with patch [2].
>>>>>>> 
>>>>>>> Measured the performance with the iio_generic_buffer with the
>>>>>>> patch [3] applied
>>>>>>> 
>>>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>>>> 
>>>>>>> ---
>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> index b9a53e0..96c4207 100644
>>>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>>>>>>> @@ -90,7 +90,7 @@
>>>>>>> /* Delay register */
>>>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
>>>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
>>>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
>>>>> Wouldn’t this be better to add this to the devicetree?
>>>>> 
>>>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
>>>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>> 
>>>> For a touch screen, there is not need to change in these parameter
>>>> settings, so my opinion is to keep it as is. Or am I missing something?
>>> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen. 
>> 
>> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
>> 
>> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
>> 
>> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.
> 
> This looks like configuration, no?
> 
> DT should be used to describe the hardware.
You may be right, but how is this different to setting the baud rate on a serial channel or sampling rate on a audio channel? Looking through the DT, there are many configuration settings, so I’m not sure what is the correct way to handle this. Surely it is better to handle this in DT vs hard coding these settings?

Regards,
John
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1508940 — Re: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz

FromLee Jones <lee.jones@linaro.org>
Date2016-10-26 10:50 +0200
SubjectRe: [PATCH] drivers: mfd: ti_am335x_tscadc: increase ADC ref clock to 24MHz
Message-ID<sws4i-1cs-25@gated-at.bofh.it>
In reply to#1508390
On Tue, 25 Oct 2016, John Syne wrote:
> > On Oct 24, 2016, at 11:38 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 24 Oct 2016, John Syne wrote:
> >>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
> >>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> >>>> 
> >>>> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
> >>>>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> >>>>>>> 
> >>>>>>> Increase ADC reference clock from 3MHz to 24MHz so that the
> >>>>>>> sampling rates goes up from 100K samples per second to 800K
> >>>>>>> samples per second on AM335x and AM437x SoC.
> >>>>>>> 
> >>>>>>> Also increase opendelay for touchscreen configuration to
> >>>>>>> equalize the increase in ADC reference clock frequency,
> >>>>>>> which results in the same amount touch events reported via
> >>>>>>> evtest on AM335x GP EVM.
> >>>>>>> 
> >>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> >>>>>>> ---
> >>>>>>> 
> >>>>>>> This patch depends on ADC DMA patch series [1]
> >>>>>>> 
> >>>>>>> Without DMA support, when ADC ref clock is set at 24MHz, I am
> >>>>>>> seeing fifo overflow as CPU is not able to pull the ADC samples.
> >>>>>>> This answers that DMA support is must for ADC to consume the
> >>>>>>> samples generated at 24MHz with no open, step delay or
> >>>>>>> averaging with patch [2].
> >>>>>>> 
> >>>>>>> Measured the performance with the iio_generic_buffer with the
> >>>>>>> patch [3] applied
> >>>>>>> 
> >>>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
> >>>>>>> [2] - http://pastebin.ubuntu.com/23357935/
> >>>>>>> [3] - http://pastebin.ubuntu.com/23357939/
> >>>>>>> 
> >>>>>>> ---
> >>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
> >>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>>> 
> >>>>>>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> >>>>>>> index b9a53e0..96c4207 100644
> >>>>>>> --- a/include/linux/mfd/ti_am335x_tscadc.h
> >>>>>>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> >>>>>>> @@ -90,7 +90,7 @@
> >>>>>>> /* Delay register */
> >>>>>>> #define STEPDELAY_OPEN_MASK	(0x3FFFF << 0)
> >>>>>>> #define STEPDELAY_OPEN(val)	((val) << 0)
> >>>>>>> -#define STEPCONFIG_OPENDLY	STEPDELAY_OPEN(0x098)
> >>>>> Wouldn’t this be better to add this to the devicetree?
> >>>>> 
> >>>>> 	ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> >>>>> 	ti,chan-step-opendelay = <0x500 0x500 0x500 0x500 0x500 0x500 0x500>;
> >>>>> 	ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> >>>> 
> >>>> For a touch screen, there is not need to change in these parameter
> >>>> settings, so my opinion is to keep it as is. Or am I missing something?
> >>> I was thinking that if you are using this driver as an ADC, you may want the flexibility to make these changes in the DT. I’m doing this by connecting sensors to the ADC inputs. I’m not using this driver for a touchscreen. 
> >> 
> >> Here is a DT overlay were this gets using on the BeagleBoneBlack.  
> >> 
> >> https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> >> 
> >> Besides, these DT features are already implemented in the driver so it is just a matter of adding these entries to the am33xx.dtsi & am4372.dtsi, which you modified in this patch series.
> > 
> > This looks like configuration, no?
> > 
> > DT should be used to describe the hardware.
> You may be right, but how is this different to setting the baud rate on a serial channel or sampling rate on a audio channel? Looking through the DT, there are many configuration settings, so I’m not sure what is the correct way to handle this. Surely it is better to handle this in DT vs hard coding these settings?

I think setting the UART baud rate is also an invalid DT entry.

It's okay to list all of the options in DT, but to actually select
one, that should be done either in userspace or as a kernel option.
Perhaps as a Kconfig selection.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web