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

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

[Mugunthan V N]

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

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

[Mugunthan V N]

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

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

[Mugunthan V N]

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

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

[John Syne]

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

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

[John Syne]

> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm@ti.com> =
wrote:
>=20
> 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.
>=20
> 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.
>=20
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>=20
> This patch depends on ADC DMA patch series [1]
>=20
> 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].
>=20
> Measured the performance with the iio_generic_buffer with the
> patch [3] applied
>=20
> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
> [2] - http://pastebin.ubuntu.com/23357935/
> [3] - http://pastebin.ubuntu.com/23357939/
>=20
> ---
> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>=20
> 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=E2=80=99t this be better to add this to the devicetree?

	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 0x500 0x500 =
0x500>;
	ti,chan-step-sampledelay =3D <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
>=20
> -#define ADC_CLK			3000000
> +#define ADC_CLK			24000000
> #define TOTAL_STEPS		16
> #define TOTAL_CHANNELS		8
> #define FIFO1_THRESHOLD		19
> --=20
> 2.10.1.502.g6598894
>=20

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

[John Syne]

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

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

[Mugunthan V N]

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

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

[Mugunthan V N]

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

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

[Mugunthan V N]

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

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

[John Syne]

> 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

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

[John Syne]

> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> =
wrote:
>=20
> 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:
>>>>=20
>>>> 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.
>>>>=20
>>>> 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.
>>>>=20
>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>> ---
>>>>=20
>>>> This patch depends on ADC DMA patch series [1]
>>>>=20
>>>> 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].
>>>>=20
>>>> Measured the performance with the iio_generic_buffer with the
>>>> patch [3] applied
>>>>=20
>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>=20
>>>> ---
>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>=20
>>>> 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=E2=80=99t this be better to add this to the devicetree?
>>=20
>> 	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>> 	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 0x500 0x500 =
0x500>;
>> 	ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>=20
> 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=E2=80=99m doing this =
by connecting sensors to the ADC inputs. I=E2=80=99m not using this =
driver for a touchscreen.=20

Regards,
John
>=20
> Regards
> Mugunthan V N

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

[John Syne]

> 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

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

[John Syne]

> 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

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

[John Syne]

> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>=20
>>=20
>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> =
wrote:
>>=20
>> 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:
>>>>>=20
>>>>> 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.
>>>>>=20
>>>>> 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.
>>>>>=20
>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>> ---
>>>>>=20
>>>>> This patch depends on ADC DMA patch series [1]
>>>>>=20
>>>>> 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].
>>>>>=20
>>>>> Measured the performance with the iio_generic_buffer with the
>>>>> patch [3] applied
>>>>>=20
>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>>=20
>>>>> ---
>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>=20
>>>>> 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=E2=80=99t this be better to add this to the devicetree?
>>>=20
>>> 	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>> 	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 0x500 0x500 =
0x500>;
>>> 	ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>=20
>> 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=E2=80=99m doing =
this by connecting sensors to the ADC inputs. I=E2=80=99m not using this =
driver for a touchscreen.=20

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

=
https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-AD=
C-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

>=20
> Regards,
> John
>>=20
>> Regards
>> Mugunthan V N

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

[John Syne]

> 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

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

[Vignesh R]

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

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

[Vignesh R]

On Tuesday 25 October 2016 11:46 AM, John Syne wrote:
> 
>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>>
>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> 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

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

[Vignesh R]

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

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

[Lee Jones]

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

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

[Lee Jones]

On Mon, 24 Oct 2016, John Syne wrote:
> > On Oct 24, 2016, at 11:01 PM, John Syne <john3909-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> wrote:
> >> 
> >> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
> >>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org>
> >>>>> ---
> >>>>> 
> >>>>> 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

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

[Lee Jones]

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

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

[John Syne]

> 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

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

[John Syne]

> On Oct 24, 2016, at 11:37 PM, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> wrote:
> 
> 
> 
> On Tuesday 25 October 2016 11:46 AM, John Syne wrote:
>> 
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 
>>>> 
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> 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

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

[John Syne]

> On Oct 24, 2016, at 11:37 PM, Vignesh R <vigneshr@ti.com> wrote:
>=20
>=20
>=20
> On Tuesday 25 October 2016 11:46 AM, John Syne wrote:
>>=20
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909@gmail.com> wrote:
>>>=20
>>>>=20
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com> =
wrote:
>>>>=20
> [...]
>>>>>>>=20
>>>>>>> ---
>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>=20
>>>>>>> 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=E2=80=99t this be better to add this to the devicetree?
>>>>>=20
>>>>> 	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>>> 	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 0x500 0x500 =
0x500>;
>>>>> 	ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>>=20
>>>> 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=E2=80=99m doing =
this by connecting sensors to the ADC inputs. I=E2=80=99m not using this =
driver for a touchscreen.
>>=20
>=20
> ti_am335x_adc driver already supports above DT parameters and its upto
> the user to adjust these parameters as required.
>=20
>> Here is a DT overlay were this gets using on the BeagleBoneBlack. =20
>>=20
>> =
https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-AD=
C-00A0.dts
>>=20
>> 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.
>>=20
>=20
> 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_a=
m335x_adc.c#L447

Regards,
John
>=20
> --=20
> Regards
> Vignesh

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

[John Syne]

> 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

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

[John Syne]

> 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

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

[John Syne]

> On Oct 24, 2016, at 11:38 PM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> 
> On Mon, 24 Oct 2016, John Syne wrote:
>>> On Oct 24, 2016, at 11:01 PM, John Syne <john3909-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> wrote:
>>>> 
>>>> On Tuesday 25 October 2016 02:28 AM, John Syne wrote:
>>>>>>> On Oct 23, 2016, at 11:02 PM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org>
>>>>>>> ---
>>>>>>> 
>>>>>>> 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

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

[John Syne]

> On Oct 24, 2016, at 11:38 PM, Lee Jones <lee.jones@linaro.org> wrote:
>=20
> 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:
>>>>=20
>>>> 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:
>>>>>>>=20
>>>>>>> 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.
>>>>>>>=20
>>>>>>> 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.
>>>>>>>=20
>>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>>>> ---
>>>>>>>=20
>>>>>>> This patch depends on ADC DMA patch series [1]
>>>>>>>=20
>>>>>>> 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].
>>>>>>>=20
>>>>>>> Measured the performance with the iio_generic_buffer with the
>>>>>>> patch [3] applied
>>>>>>>=20
>>>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>>>>=20
>>>>>>> ---
>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>=20
>>>>>>> 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=E2=80=99t this be better to add this to the devicetree?
>>>>>=20
>>>>> 	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>>>>> 	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 0x500 0x500 =
0x500>;
>>>>> 	ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>>>=20
>>>> 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=E2=80=99m doing =
this by connecting sensors to the ADC inputs. I=E2=80=99m not using this =
driver for a touchscreen.=20
>>=20
>> Here is a DT overlay were this gets using on the BeagleBoneBlack. =20
>>=20
>> =
https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-AD=
C-00A0.dts
>>=20
>> 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.
>=20
> This looks like configuration, no?
>=20
> 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=E2=80=99m 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
>=20
> --=20
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org =E2=94=82 Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

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

[John Syne]

> 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

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

[Lee Jones]

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

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

[Lee Jones]

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

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

[John Syne]

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


> On Oct 26, 2016, at 1:48 AM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> 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.
Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. I defer to the domain experts to decide the best way forward. 

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


[-- Attachment #2: Type: text/html, Size: 14941 bytes --]

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

[Mugunthan V N]

On Monday 24 October 2016 11:32 AM, Mugunthan V N 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>

Found issues with touchscreen usage with GUI on my AM335x GP EVM. Fixed
with change the "ti,charge-delay" DT property based on change in ADC
clocks. Will be sending v2 patch soon.

Regards
Mugunthan V N

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

[Mugunthan V N]

On Monday 24 October 2016 11:32 AM, Mugunthan V N 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>

Found issues with touchscreen usage with GUI on my AM335x GP EVM. Fixed
with change the "ti,charge-delay" DT property based on change in ADC
clocks. Will be sending v2 patch soon.

Regards
Mugunthan V N

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

[Mugunthan V N]

On Monday 24 October 2016 11:32 AM, Mugunthan V N 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>

Found issues with touchscreen usage with GUI on my AM335x GP EVM. Fixed
with change the "ti,charge-delay" DT property based on change in ADC
clocks. Will be sending v2 patch soon.

Regards
Mugunthan V N

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

[John Syne]

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

> 
> On Oct 26, 2016, at 1:48 AM, Lee Jones <lee.jones@linaro.org <mailto:lee.jones@linaro.org>> wrote:
> 
> On Tue, 25 Oct 2016, John Syne wrote:
>>> On Oct 24, 2016, at 11:38 PM, Lee Jones <lee.jones@linaro.org <mailto: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 <mailto:john3909@gmail.com>> wrote:
>>>>>> On Oct 24, 2016, at 10:52 PM, Mugunthan V N <mugunthanvnm@ti.com <mailto: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 <mailto: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 <mailto: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 <http://www.spinics.net/lists/devicetree/msg145045.html>
>>>>>>>>> [2] - http://pastebin.ubuntu.com/23357935/ <http://pastebin.ubuntu.com/23357935/>
>>>>>>>>> [3] - http://pastebin.ubuntu.com/23357939/ <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 <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.
Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn’t desirable. Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. I guess the DT code in ti_am335x_adc.c should be removed. 

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

[-- Attachment #2: Type: text/html, Size: 16667 bytes --]

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

[John Syne]

> 
> On Oct 26, 2016, at 1:48 AM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> 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.
Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn’t desirable. Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. I guess the DT code in ti_am335x_adc.c should be removed. 

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

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

[John Syne]

>=20
> On Oct 26, 2016, at 1:48 AM, Lee Jones <lee.jones@linaro.org> wrote:
>=20
> 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:
>>>>>>=20
>>>>>> 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:
>>>>>>>>>=20
>>>>>>>>> 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.
>>>>>>>>>=20
>>>>>>>>> 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.
>>>>>>>>>=20
>>>>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>>>>>>> ---
>>>>>>>>>=20
>>>>>>>>> This patch depends on ADC DMA patch series [1]
>>>>>>>>>=20
>>>>>>>>> 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].
>>>>>>>>>=20
>>>>>>>>> Measured the performance with the iio_generic_buffer with the
>>>>>>>>> patch [3] applied
>>>>>>>>>=20
>>>>>>>>> [1] - http://www.spinics.net/lists/devicetree/msg145045.html
>>>>>>>>> [2] - http://pastebin.ubuntu.com/23357935/
>>>>>>>>> [3] - http://pastebin.ubuntu.com/23357939/
>>>>>>>>>=20
>>>>>>>>> ---
>>>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>=20
>>>>>>>>> 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=E2=80=99t this be better to add this to the devicetree?
>>>>>>>=20
>>>>>>> 	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 =
0x16>;
>>>>>>> 	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 =
0x500 0x500 0x500>;
>>>>>>> 	ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 =
0x0>;
>>>>>>=20
>>>>>> 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=E2=80=99m =
doing this by connecting sensors to the ADC inputs. I=E2=80=99m not =
using this driver for a touchscreen.=20
>>>>=20
>>>> Here is a DT overlay were this gets using on the BeagleBoneBlack. =20=

>>>>=20
>>>> =
https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-AD=
C-00A0.dts
>>>>=20
>>>> 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.
>>>=20
>>> This looks like configuration, no?
>>>=20
>>> 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=E2=80=99m not sure =
what is the correct way to handle this. Surely it is better to handle =
this in DT vs hard coding these settings?
>=20
> I think setting the UART baud rate is also an invalid DT entry.
>=20
> 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.
Yeah, this has been inconsistent for a long time. My only point was that =
these DT parameters had already been implemented in the ti_am335x_adc KM =
and I thought that this was better than hard coding these settings. =
Implementing this in Kconfig means rebuilding the KM, which isn=E2=80=99t =
desirable. Perhaps this should be done via sysfs attributes so as you =
say, a userspace app can configure this driver. I guess the DT code in =
ti_am335x_adc.c should be removed.=20

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

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

[John Syne]

> 
> On Oct 26, 2016, at 1:48 AM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> 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.
Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn?t desirable. Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. I guess the DT code in ti_am335x_adc.c should be removed. 

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

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

[Vignesh R]

On Friday 28 October 2016 02:47 AM, John Syne 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. 
>>>>>
>>>>> 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.
> Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn’t desirable. 

>Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. 

This was discussed when DT properties were added.  Patches are welcome
to add sysfs entries. There is nothing wrong with specifying an initial
value in the DT.

>I guess the DT code in ti_am335x_adc.c should be removed. 
> 

Removing DT properties is not an option as it will break DT backward
compatibility.

-- 
Regards
Vignesh

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

[Vignesh R]

On Friday 28 October 2016 02:47 AM, John Syne 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. 
>>>>>
>>>>> 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.
> Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn’t desirable. 

>Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. 

This was discussed when DT properties were added.  Patches are welcome
to add sysfs entries. There is nothing wrong with specifying an initial
value in the DT.

>I guess the DT code in ti_am335x_adc.c should be removed. 
> 

Removing DT properties is not an option as it will break DT backward
compatibility.

-- 
Regards
Vignesh

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

[Vignesh R]

On Friday 28 October 2016 02:47 AM, John Syne 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. 
>>>>>
>>>>> 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.
> Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn?t desirable. 

>Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. 

This was discussed when DT properties were added.  Patches are welcome
to add sysfs entries. There is nothing wrong with specifying an initial
value in the DT.

>I guess the DT code in ti_am335x_adc.c should be removed. 
> 

Removing DT properties is not an option as it will break DT backward
compatibility.

-- 
Regards
Vignesh

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

[John Syne]

> On Oct 31, 2016, at 4:39 AM, Vignesh R <vigneshr@ti.com> wrote:
> 
> 
> 
> On Friday 28 October 2016 02:47 AM, John Syne 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. 
>>>>>> 
>>>>>> 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.
>> Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn’t desirable. 
> 
>> Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. 
> 
> This was discussed when DT properties were added.  Patches are welcome
> to add sysfs entries. There is nothing wrong with specifying an initial
> value in the DT.
> 
>> I guess the DT code in ti_am335x_adc.c should be removed. 
>> 
> 
> Removing DT properties is not an option as it will break DT backward
> compatibility.
Hi Vignesh,

OK, then back to my original question. Given that these DT properties are supported in the driver, shouldn’t the following be added to am33xx.dtsi and am4372.dtsi?

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
> 
> -- 
> Regards
> Vignesh

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

[John Syne]

> On Oct 31, 2016, at 4:39 AM, Vignesh R <vigneshr@ti.com> wrote:
>=20
>=20
>=20
> On Friday 28 October 2016 02:47 AM, John Syne wrote:
>=20
>>>>>>>>>>>=20
>>>>>>>>>>> ---
>>>>>>>>>>> include/linux/mfd/ti_am335x_tscadc.h | 4 ++--
>>>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>>=20
>>>>>>>>>>> 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=E2=80=99t this be better to add this to the devicetree?
>>>>>>>>>=20
>>>>>>>>> 	ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 =
0x16>;
>>>>>>>>> 	ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 =
0x500 0x500 0x500>;
>>>>>>>>> 	ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 =
0x0>;
>>>>>>>>=20
>>>>>>>> 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=E2=80=99m =
doing this by connecting sensors to the ADC inputs. I=E2=80=99m not =
using this driver for a touchscreen.=20
>>>>>>=20
>>>>>> Here is a DT overlay were this gets using on the BeagleBoneBlack. =
=20
>>>>>>=20
>>>>>> =
https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-AD=
C-00A0.dts
>>>>>>=20
>>>>>> 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.
>>>>>=20
>>>>> This looks like configuration, no?
>>>>>=20
>>>>> 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=E2=80=99m =
not sure what is the correct way to handle this. Surely it is better to =
handle this in DT vs hard coding these settings?
>>>=20
>>> I think setting the UART baud rate is also an invalid DT entry.
>>>=20
>>> 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.
>> Yeah, this has been inconsistent for a long time. My only point was =
that these DT parameters had already been implemented in the =
ti_am335x_adc KM and I thought that this was better than hard coding =
these settings. Implementing this in Kconfig means rebuilding the KM, =
which isn=E2=80=99t desirable.=20
>=20
>> Perhaps this should be done via sysfs attributes so as you say, a =
userspace app can configure this driver.=20
>=20
> This was discussed when DT properties were added.  Patches are welcome
> to add sysfs entries. There is nothing wrong with specifying an =
initial
> value in the DT.
>=20
>> I guess the DT code in ti_am335x_adc.c should be removed.=20
>>=20
>=20
> Removing DT properties is not an option as it will break DT backward
> compatibility.
Hi Vignesh,

OK, then back to my original question. Given that these DT properties =
are supported in the driver, shouldn=E2=80=99t the following be added to =
am33xx.dtsi and am4372.dtsi?

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

Regards,
John
>=20
> --=20
> Regards
> Vignesh

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

[John Syne]

> On Oct 31, 2016, at 4:39 AM, Vignesh R <vigneshr@ti.com> wrote:
> 
> 
> 
> On Friday 28 October 2016 02:47 AM, John Syne 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. 
>>>>>> 
>>>>>> 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.
>> Yeah, this has been inconsistent for a long time. My only point was that these DT parameters had already been implemented in the ti_am335x_adc KM and I thought that this was better than hard coding these settings. Implementing this in Kconfig means rebuilding the KM, which isn?t desirable. 
> 
>> Perhaps this should be done via sysfs attributes so as you say, a userspace app can configure this driver. 
> 
> This was discussed when DT properties were added.  Patches are welcome
> to add sysfs entries. There is nothing wrong with specifying an initial
> value in the DT.
> 
>> I guess the DT code in ti_am335x_adc.c should be removed. 
>> 
> 
> Removing DT properties is not an option as it will break DT backward
> compatibility.
Hi Vignesh,

OK, then back to my original question. Given that these DT properties are supported in the driver, shouldn?t the following be added to am33xx.dtsi and am4372.dtsi?

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
> 
> -- 
> Regards
> Vignesh

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

[Vignesh R]

Hi,

On Thursday 10 November 2016 05:23 AM, John Syne wrote:
> OK, then back to my original question. Given that these DT properties are supported in the driver
> 

Below properties are supported by only by ti_am3335x_adc driver and not
ti_am335x_tsc driver. As author of this patch pointed out in another
reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
see a use case where these values needs to be tweaked for tsc channels,
therefore it does not make sense to be DT properties.

> shouldn’t the following be added to am33xx.dtsi and am4372.dtsi?

Its totally upto board dts files to allocate channels for tsc and adc.
So, how could these be added to dtsi files?

> 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
>>
>> -- 
>> Regards
>> Vignesh
> 

-- 
Regards
Vignesh

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

[Vignesh R]

Hi,

On Thursday 10 November 2016 05:23 AM, John Syne wrote:
> OK, then back to my original question. Given that these DT properties are supported in the driver
> 

Below properties are supported by only by ti_am3335x_adc driver and not
ti_am335x_tsc driver. As author of this patch pointed out in another
reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
see a use case where these values needs to be tweaked for tsc channels,
therefore it does not make sense to be DT properties.

> shouldn’t the following be added to am33xx.dtsi and am4372.dtsi?

Its totally upto board dts files to allocate channels for tsc and adc.
So, how could these be added to dtsi files?

> 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
>>
>> -- 
>> Regards
>> Vignesh
> 

-- 
Regards
Vignesh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Vignesh R]

Hi,

On Thursday 10 November 2016 05:23 AM, John Syne wrote:
> OK, then back to my original question. Given that these DT properties are supported in the driver
> 

Below properties are supported by only by ti_am3335x_adc driver and not
ti_am335x_tsc driver. As author of this patch pointed out in another
reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
see a use case where these values needs to be tweaked for tsc channels,
therefore it does not make sense to be DT properties.

> shouldn’t the following be added to am33xx.dtsi and am4372.dtsi?

Its totally upto board dts files to allocate channels for tsc and adc.
So, how could these be added to dtsi files?

> 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
>>
>> -- 
>> Regards
>> Vignesh
> 

-- 
Regards
Vignesh

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

[Vignesh R]

Hi,

On Thursday 10 November 2016 05:23 AM, John Syne wrote:
> OK, then back to my original question. Given that these DT properties are supported in the driver
> 

Below properties are supported by only by ti_am3335x_adc driver and not
ti_am335x_tsc driver. As author of this patch pointed out in another
reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
see a use case where these values needs to be tweaked for tsc channels,
therefore it does not make sense to be DT properties.

> shouldn?t the following be added to am33xx.dtsi and am4372.dtsi?

Its totally upto board dts files to allocate channels for tsc and adc.
So, how could these be added to dtsi files?

> 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
>>
>> -- 
>> Regards
>> Vignesh
> 

-- 
Regards
Vignesh

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

[John Syne]

> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
> 
> Hi,
> 
> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>> OK, then back to my original question. Given that these DT properties are supported in the driver
>> 
> 
> Below properties are supported by only by ti_am3335x_adc driver and not
> ti_am335x_tsc driver. As author of this patch pointed out in another
> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
> see a use case where these values needs to be tweaked for tsc channels,
> therefore it does not make sense to be DT properties.
Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

tscadc: tscadc@44e0d000 {
			compatible = "ti,am3359-tscadc";
			reg = <0x44e0d000 0x1000>;
			interrupt-parent = <&intc>;
			interrupts = <16>;
			ti,hwmods = "adc_tsc";
			status = "disabled";
			dmas = <&edma 53 0>, <&edma 57 0>;
			dma-names = "fifo0", "fifo1”;

			tsc {
				compatible = "ti,am3359-tsc";
			};
			am335x_adc: adc {
				#io-channel-cells = <1>;
				ti,adc-channels = <0 1 2 3 4 5 6>;
				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
				compatible = "ti,am3359-adc";
			};
};

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

Regards,
John
> 
>> shouldn’t the following be added to am33xx.dtsi and am4372.dtsi?
> 
> Its totally upto board dts files to allocate channels for tsc and adc.
> So, how could these be added to dtsi files?
> 
>> 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
>>> 
>>> -- 
>>> Regards
>>> Vignesh
>> 
> 
> -- 
> Regards
> Vignesh

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

[John Syne]

> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> wrote:
> 
> Hi,
> 
> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>> OK, then back to my original question. Given that these DT properties are supported in the driver
>> 
> 
> Below properties are supported by only by ti_am3335x_adc driver and not
> ti_am335x_tsc driver. As author of this patch pointed out in another
> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
> see a use case where these values needs to be tweaked for tsc channels,
> therefore it does not make sense to be DT properties.
Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

tscadc: tscadc@44e0d000 {
			compatible = "ti,am3359-tscadc";
			reg = <0x44e0d000 0x1000>;
			interrupt-parent = <&intc>;
			interrupts = <16>;
			ti,hwmods = "adc_tsc";
			status = "disabled";
			dmas = <&edma 53 0>, <&edma 57 0>;
			dma-names = "fifo0", "fifo1”;

			tsc {
				compatible = "ti,am3359-tsc";
			};
			am335x_adc: adc {
				#io-channel-cells = <1>;
				ti,adc-channels = <0 1 2 3 4 5 6>;
				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
				compatible = "ti,am3359-adc";
			};
};

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

Regards,
John
> 
>> shouldn’t the following be added to am33xx.dtsi and am4372.dtsi?
> 
> Its totally upto board dts files to allocate channels for tsc and adc.
> So, how could these be added to dtsi files?
> 
>> 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
>>> 
>>> -- 
>>> Regards
>>> Vignesh
>> 
> 
> -- 
> Regards
> Vignesh

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

[John Syne]

> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
>=20
> Hi,
>=20
> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>> OK, then back to my original question. Given that these DT properties =
are supported in the driver
>>=20
>=20
> Below properties are supported by only by ti_am3335x_adc driver and =
not
> ti_am335x_tsc driver. As author of this patch pointed out in another
> reply, there is no need to change step-opendelay for tsc. AFAIK, I =
don't
> see a use case where these values needs to be tweaked for tsc =
channels,
> therefore it does not make sense to be DT properties.
Yeah, the confusion was mine because the author of this patch series was =
proposing to hard code these settings while DT properties already =
existed in the ti_am335x_adc driver. I use the ADC for sensor =
measurement and do not use the touchscreen features. I already pointed =
out where these DT parameters should be added by referencing how this =
was done in one of the BBB DT overlay files [1]. I am just proposing the =
same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here =
is what I was proposing. Granted, the adc-channels should be restricted =
to the subset of channels not used by tsc, but you get the idea.

tscadc: tscadc@44e0d000 {
			compatible =3D "ti,am3359-tscadc";
			reg =3D <0x44e0d000 0x1000>;
			interrupt-parent =3D <&intc>;
			interrupts =3D <16>;
			ti,hwmods =3D "adc_tsc";
			status =3D "disabled";
			dmas =3D <&edma 53 0>, <&edma 57 0>;
			dma-names =3D "fifo0", "fifo1=E2=80=9D;

			tsc {
				compatible =3D "ti,am3359-tsc";
			};
			am335x_adc: adc {
				#io-channel-cells =3D <1>;
				ti,adc-channels =3D <0 1 2 3 4 5 6>;
				ti,chan-step-avg =3D <0x16 0x16 0x16 =
0x16 0x16 0x16 0x16>;
				ti,chan-step-opendelay =3D <0x98 0x98 =
0x98 0x98 0x98 0x98 0x98>;
				ti,chan-step-sampledelay =3D <0x0 0x0 =
0x0 0x0 0x0 0x0 0x0>;
				compatible =3D "ti,am3359-adc";
			};
};

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

Regards,
John
>=20
>> shouldn=E2=80=99t the following be added to am33xx.dtsi and =
am4372.dtsi?
>=20
> Its totally upto board dts files to allocate channels for tsc and adc.
> So, how could these be added to dtsi files?
>=20
>> ti,chan-step-avg =3D <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
>> ti,chan-step-opendelay =3D <0x500 0x500 0x500 0x500 0x500 0x500 =
0x500>;
>> ti,chan-step-sampledelay =3D <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
>>=20
>> Regards,
>> John
>>>=20
>>> --=20
>>> Regards
>>> Vignesh
>>=20
>=20
> --=20
> Regards
> Vignesh

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

[John Syne]

> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
> 
> Hi,
> 
> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>> OK, then back to my original question. Given that these DT properties are supported in the driver
>> 
> 
> Below properties are supported by only by ti_am3335x_adc driver and not
> ti_am335x_tsc driver. As author of this patch pointed out in another
> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
> see a use case where these values needs to be tweaked for tsc channels,
> therefore it does not make sense to be DT properties.
Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

tscadc: tscadc at 44e0d000 {
			compatible = "ti,am3359-tscadc";
			reg = <0x44e0d000 0x1000>;
			interrupt-parent = <&intc>;
			interrupts = <16>;
			ti,hwmods = "adc_tsc";
			status = "disabled";
			dmas = <&edma 53 0>, <&edma 57 0>;
			dma-names = "fifo0", "fifo1?;

			tsc {
				compatible = "ti,am3359-tsc";
			};
			am335x_adc: adc {
				#io-channel-cells = <1>;
				ti,adc-channels = <0 1 2 3 4 5 6>;
				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
				compatible = "ti,am3359-adc";
			};
};

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

Regards,
John
> 
>> shouldn?t the following be added to am33xx.dtsi and am4372.dtsi?
> 
> Its totally upto board dts files to allocate channels for tsc and adc.
> So, how could these be added to dtsi files?
> 
>> 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
>>> 
>>> -- 
>>> Regards
>>> Vignesh
>> 
> 
> -- 
> Regards
> Vignesh

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

[Vignesh R]

On Friday 11 November 2016 09:00 AM, John Syne wrote:
> 
>> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>> Hi,
>>
>> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>>> OK, then back to my original question. Given that these DT properties are supported in the driver
>>>
>>
>> Below properties are supported by only by ti_am3335x_adc driver and not
>> ti_am335x_tsc driver. As author of this patch pointed out in another
>> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
>> see a use case where these values needs to be tweaked for tsc channels,
>> therefore it does not make sense to be DT properties.
> Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. 

> Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

There are 4 wire, 5 wire and 8 wire touchscreens. Therefore there is no
way to know in advance, how many channels are available for adc. IMO,
this is not a SoC specific configuration but board dependent. Hence, all
channel related configurations need to be in board specific dts files
and not in top level dtsi files.

> 
> tscadc: tscadc@44e0d000 {
> 			compatible = "ti,am3359-tscadc";
> 			reg = <0x44e0d000 0x1000>;
> 			interrupt-parent = <&intc>;
> 			interrupts = <16>;
> 			ti,hwmods = "adc_tsc";
> 			status = "disabled";
> 			dmas = <&edma 53 0>, <&edma 57 0>;
> 			dma-names = "fifo0", "fifo1”;
> 
> 			tsc {
> 				compatible = "ti,am3359-tsc";
> 			};
> 			am335x_adc: adc {
> 				#io-channel-cells = <1>;
> 				ti,adc-channels = <0 1 2 3 4 5 6>;
> 				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> 				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
> 				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> 				compatible = "ti,am3359-adc";
> 			};
> };
> 
> [1]https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> 

-- 
Regards
Vignesh

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

[Vignesh R]

On Friday 11 November 2016 09:00 AM, John Syne wrote:
> 
>> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> wrote:
>>
>> Hi,
>>
>> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>>> OK, then back to my original question. Given that these DT properties are supported in the driver
>>>
>>
>> Below properties are supported by only by ti_am3335x_adc driver and not
>> ti_am335x_tsc driver. As author of this patch pointed out in another
>> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
>> see a use case where these values needs to be tweaked for tsc channels,
>> therefore it does not make sense to be DT properties.
> Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. 

> Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

There are 4 wire, 5 wire and 8 wire touchscreens. Therefore there is no
way to know in advance, how many channels are available for adc. IMO,
this is not a SoC specific configuration but board dependent. Hence, all
channel related configurations need to be in board specific dts files
and not in top level dtsi files.

> 
> tscadc: tscadc@44e0d000 {
> 			compatible = "ti,am3359-tscadc";
> 			reg = <0x44e0d000 0x1000>;
> 			interrupt-parent = <&intc>;
> 			interrupts = <16>;
> 			ti,hwmods = "adc_tsc";
> 			status = "disabled";
> 			dmas = <&edma 53 0>, <&edma 57 0>;
> 			dma-names = "fifo0", "fifo1”;
> 
> 			tsc {
> 				compatible = "ti,am3359-tsc";
> 			};
> 			am335x_adc: adc {
> 				#io-channel-cells = <1>;
> 				ti,adc-channels = <0 1 2 3 4 5 6>;
> 				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> 				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
> 				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> 				compatible = "ti,am3359-adc";
> 			};
> };
> 
> [1]https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> 

-- 
Regards
Vignesh

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

[Vignesh R]

On Friday 11 November 2016 09:00 AM, John Syne wrote:
> 
>> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>> Hi,
>>
>> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>>> OK, then back to my original question. Given that these DT properties are supported in the driver
>>>
>>
>> Below properties are supported by only by ti_am3335x_adc driver and not
>> ti_am335x_tsc driver. As author of this patch pointed out in another
>> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
>> see a use case where these values needs to be tweaked for tsc channels,
>> therefore it does not make sense to be DT properties.
> Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. 

> Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

There are 4 wire, 5 wire and 8 wire touchscreens. Therefore there is no
way to know in advance, how many channels are available for adc. IMO,
this is not a SoC specific configuration but board dependent. Hence, all
channel related configurations need to be in board specific dts files
and not in top level dtsi files.

> 
> tscadc: tscadc@44e0d000 {
> 			compatible = "ti,am3359-tscadc";
> 			reg = <0x44e0d000 0x1000>;
> 			interrupt-parent = <&intc>;
> 			interrupts = <16>;
> 			ti,hwmods = "adc_tsc";
> 			status = "disabled";
> 			dmas = <&edma 53 0>, <&edma 57 0>;
> 			dma-names = "fifo0", "fifo1”;
> 
> 			tsc {
> 				compatible = "ti,am3359-tsc";
> 			};
> 			am335x_adc: adc {
> 				#io-channel-cells = <1>;
> 				ti,adc-channels = <0 1 2 3 4 5 6>;
> 				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> 				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
> 				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> 				compatible = "ti,am3359-adc";
> 			};
> };
> 
> [1]https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> 

-- 
Regards
Vignesh

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

[Vignesh R]

On Friday 11 November 2016 09:00 AM, John Syne wrote:
> 
>> On Nov 9, 2016, at 9:07 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>> Hi,
>>
>> On Thursday 10 November 2016 05:23 AM, John Syne wrote:
>>> OK, then back to my original question. Given that these DT properties are supported in the driver
>>>
>>
>> Below properties are supported by only by ti_am3335x_adc driver and not
>> ti_am335x_tsc driver. As author of this patch pointed out in another
>> reply, there is no need to change step-opendelay for tsc. AFAIK, I don't
>> see a use case where these values needs to be tweaked for tsc channels,
>> therefore it does not make sense to be DT properties.
> Yeah, the confusion was mine because the author of this patch series was proposing to hard code these settings while DT properties already existed in the ti_am335x_adc driver. I use the ADC for sensor measurement and do not use the touchscreen features. I already pointed out where these DT parameters should be added by referencing how this was done in one of the BBB DT overlay files [1]. I am just proposing the same is done as a default in the AM33xx.dtsi and AM4372.dtsi files. Here is what I was proposing. 

> Granted, the adc-channels should be restricted to the subset of channels not used by tsc, but you get the idea.

There are 4 wire, 5 wire and 8 wire touchscreens. Therefore there is no
way to know in advance, how many channels are available for adc. IMO,
this is not a SoC specific configuration but board dependent. Hence, all
channel related configurations need to be in board specific dts files
and not in top level dtsi files.

> 
> tscadc: tscadc at 44e0d000 {
> 			compatible = "ti,am3359-tscadc";
> 			reg = <0x44e0d000 0x1000>;
> 			interrupt-parent = <&intc>;
> 			interrupts = <16>;
> 			ti,hwmods = "adc_tsc";
> 			status = "disabled";
> 			dmas = <&edma 53 0>, <&edma 57 0>;
> 			dma-names = "fifo0", "fifo1?;
> 
> 			tsc {
> 				compatible = "ti,am3359-tsc";
> 			};
> 			am335x_adc: adc {
> 				#io-channel-cells = <1>;
> 				ti,adc-channels = <0 1 2 3 4 5 6>;
> 				ti,chan-step-avg = <0x16 0x16 0x16 0x16 0x16 0x16 0x16>;
> 				ti,chan-step-opendelay = <0x98 0x98 0x98 0x98 0x98 0x98 0x98>;
> 				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
> 				compatible = "ti,am3359-adc";
> 			};
> };
> 
> [1]https://github.com/RobertCNelson/bb.org-overlays/blob/master/src/arm/BB-ADC-00A0.dts
> 

-- 
Regards
Vignesh