Hi,
This short series has 4 patches that apply on the mfd-next tree that fix/cleanup the am335x_tsadc driver.
1- Fix. ADC driver would read data from fifo before sampling completes. Fixed by waiting for sequencer to complete and then read data
Acked by IIO.
2- Fix. TSC registers would be configured even in ADC only mode. Fixed so that they are configured only when TSC is enabled.
3- Cleanup. Removes a check on the ADC clock which isn't needed anymore.
4- Typo. Coordinate is spelled Coordiante. Corrected in all files in one patch.
Note: The typo patch has been sent before on all relevant lists too. No response..
Patil, Rachna (3):
iio: ti_am335x_adc: Fix wrong samples received on 1st read
MFD: ti_tscadc: disable TSC config registers in adc mode
MFD: ti_tscadc: ADC Clock check not required
Zubair Lutfullah (1):
mfd: input: arm: dts: doc: ti_am335x: typo fixes (coordiante ->
coordinate)
.../bindings/input/touchscreen/ti-tsc-adc.txt | 4 +--
arch/arm/boot/dts/am335x-evm.dts | 2 +-
drivers/iio/adc/ti_am335x_adc.c | 30 ++++++++++++++------
drivers/input/touchscreen/ti_am335x_tsc.c | 2 +-
drivers/mfd/ti_am335x_tscadc.c | 26 ++++++++---------
include/linux/mfd/ti_am335x_tscadc.h | 17 ++++++++++-
6 files changed, 55 insertions(+), 26 deletions(-)
--
1.7.9.5
From: "Patil, Rachna" <[email protected]>
Previously we tried to read data form ADC even before ADC sequencer
finished sampling. This led to wrong samples.
We now wait on ADC status register idle bit to be set.
Signed-off-by: Patil, Rachna <[email protected]>
Signed-off-by: Zubair Lutfullah <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/iio/adc/ti_am335x_adc.c | 30 ++++++++++++++++++++++--------
include/linux/mfd/ti_am335x_tscadc.h | 16 ++++++++++++++++
2 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 4427e8e..f73fee1 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -60,7 +60,6 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
{
unsigned int stepconfig;
int i, steps;
- u32 step_en;
/*
* There are 16 configurable steps and 8 analog input
@@ -86,8 +85,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
adc_dev->channel_step[i] = steps;
steps++;
}
- step_en = get_adc_step_mask(adc_dev);
- am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
+
}
static const char * const chan_name_ain[] = {
@@ -142,10 +140,22 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
int *val, int *val2, long mask)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
- int i;
- unsigned int fifo1count, read;
+ int i, map_val;
+ unsigned int fifo1count, read, stepid;
u32 step = UINT_MAX;
bool found = false;
+ u32 step_en;
+ unsigned long timeout = jiffies + usecs_to_jiffies
+ (IDLE_TIMEOUT * adc_dev->channels);
+ step_en = get_adc_step_mask(adc_dev);
+ am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
+
+ /* Wait for ADC sequencer to complete sampling */
+ while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) {
+ if (time_after(jiffies, timeout))
+ return -EAGAIN;
+ }
+ map_val = chan->channel + TOTAL_CHANNELS;
/*
* When the sub-system is first enabled,
@@ -170,12 +180,16 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
for (i = 0; i < fifo1count; i++) {
read = tiadc_readl(adc_dev, REG_FIFO1);
- if (read >> 16 == step) {
- *val = read & 0xfff;
+ stepid = read & FIFOREAD_CHNLID_MASK;
+ stepid = stepid >> 0x10;
+
+ if (stepid == map_val) {
+ read = read & FIFOREAD_DATA_MASK;
found = true;
+ *val = read;
}
}
- am335x_tsc_se_update(adc_dev->mfd_tscadc);
+
if (found == false)
return -EBUSY;
return IIO_VAL_INT;
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 8d73fe2..db1791b 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -113,11 +113,27 @@
#define CNTRLREG_8WIRE CNTRLREG_AFE_CTRL(3)
#define CNTRLREG_TSCENB BIT(7)
+/* FIFO READ Register */
+#define FIFOREAD_DATA_MASK (0xfff << 0)
+#define FIFOREAD_CHNLID_MASK (0xf << 16)
+
+/* Sequencer Status */
+#define SEQ_STATUS BIT(5)
+
#define ADC_CLK 3000000
#define MAX_CLK_DIV 7
#define TOTAL_STEPS 16
#define TOTAL_CHANNELS 8
+/*
+* ADC runs at 3MHz, and it takes
+* 15 cycles to latch one data output.
+* Hence the idle time for ADC to
+* process one sample data would be
+* around 5 micro seconds.
+*/
+#define IDLE_TIMEOUT 5 /* microsec */
+
#define TSCADC_CELLS 2
struct ti_tscadc_dev {
--
1.7.9.5
Did a grep for 'coordiante' and replaced them all with 'coordinate'
---
.../bindings/input/touchscreen/ti-tsc-adc.txt | 4 ++--
arch/arm/boot/dts/am335x-evm.dts | 2 +-
drivers/input/touchscreen/ti_am335x_tsc.c | 2 +-
drivers/mfd/ti_am335x_tscadc.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
index 491c97b..3e22aec 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
@@ -6,7 +6,7 @@ Required properties:
ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
support on the platform.
ti,x-plate-resistance: X plate resistance
- ti,coordiante-readouts: The sequencer supports a total of 16
+ ti,coordinate-readouts: The sequencer supports a total of 16
programmable steps each step is used to
read a single coordinate. A single
readout is enough but multiple reads can
@@ -34,7 +34,7 @@ Example:
tsc {
ti,wires = <4>;
ti,x-plate-resistance = <200>;
- ti,coordiante-readouts = <5>;
+ ti,coordinate-readouts = <5>;
ti,wire-config = <0x00 0x11 0x22 0x33>;
};
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 0fa4c7f..e50b781 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -250,7 +250,7 @@
tsc {
ti,wires = <4>;
ti,x-plate-resistance = <200>;
- ti,coordiante-readouts = <5>;
+ ti,coordinate-readouts = <5>;
ti,wire-config = <0x00 0x11 0x22 0x33>;
};
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 0e9f02a..6422f65 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -348,7 +348,7 @@ static int titsc_parse_dt(struct platform_device *pdev,
if (err < 0)
return err;
- err = of_property_read_u32(node, "ti,coordiante-readouts",
+ err = of_property_read_u32(node, "ti,coordinate-readouts",
&ts_dev->coordinate_readouts);
if (err < 0)
return err;
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index cd74d59..1686ae6 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -106,7 +106,7 @@ static int ti_tscadc_probe(struct platform_device *pdev)
node = of_get_child_by_name(pdev->dev.of_node, "tsc");
of_property_read_u32(node, "ti,wires", &tsc_wires);
- of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
+ of_property_read_u32(node, "ti,coordinate-readouts", &readouts);
node = of_get_child_by_name(pdev->dev.of_node, "adc");
of_property_for_each_u32(node, "ti,adc-channels", prop, cur, val) {
--
1.7.9.5
From: "Patil, Rachna" <[email protected]>
ADC is ideally expected to work at a frequency of 3MHz.
The present code had a check, which returned error if the frequency
went below the threshold value. But since AM335x supports various
working frequencies, this check is not required.
Now the code just uses the internal ADC clock divider to set the ADC
frequency w.r.t the sys clock.
Signed-off-by: Patil, Rachna <[email protected]>
Signed-off-by: Zubair Lutfullah <[email protected]>
---
drivers/mfd/ti_am335x_tscadc.c | 6 +-----
include/linux/mfd/ti_am335x_tscadc.h | 1 -
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index d72001c..cd74d59 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -197,11 +197,7 @@ static int ti_tscadc_probe(struct platform_device *pdev)
clock_rate = clk_get_rate(clk);
clk_put(clk);
clk_value = clock_rate / ADC_CLK;
- if (clk_value < MAX_CLK_DIV) {
- dev_err(&pdev->dev, "clock input less than min clock requirement\n");
- err = -EINVAL;
- goto err_disable_clk;
- }
+
/* TSCADC_CLKDIV needs to be configured to the value minus 1 */
clk_value = clk_value - 1;
tscadc_writel(tscadc, REG_CLKDIV, clk_value);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index db1791b..25f2c61 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -121,7 +121,6 @@
#define SEQ_STATUS BIT(5)
#define ADC_CLK 3000000
-#define MAX_CLK_DIV 7
#define TOTAL_STEPS 16
#define TOTAL_CHANNELS 8
--
1.7.9.5
From: "Patil, Rachna" <[email protected]>
AFE Pen Ctrl and TouchScreen transistors enabling is not
required when only ADC mode is being used, so check for availability of
TSC driver before accessing control register.
Signed-off-by: Patil, Rachna <[email protected]>
Acked-by: Vaibhav Hiremath <[email protected]>
Signed-off-by: Zubair Lutfullah <[email protected]>
---
drivers/mfd/ti_am335x_tscadc.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index b003a16..d72001c 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -208,13 +208,14 @@ static int ti_tscadc_probe(struct platform_device *pdev)
/* Set the control register bits */
ctrl = CNTRLREG_STEPCONFIGWRT |
- CNTRLREG_TSCENB |
- CNTRLREG_STEPID |
- CNTRLREG_4WIRE;
+ CNTRLREG_STEPID;
+ if (tsc_wires > 0)
+ ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
tscadc_writel(tscadc, REG_CTRL, ctrl);
/* Set register bits for Idle Config Mode */
- tscadc_idle_config(tscadc);
+ if (tsc_wires > 0)
+ tscadc_idle_config(tscadc);
/* Enable the TSC module enable bit */
ctrl = tscadc_readl(tscadc, REG_CTRL);
@@ -294,10 +295,13 @@ static int tscadc_resume(struct device *dev)
pm_runtime_get_sync(dev);
/* context restore */
- ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_TSCENB |
- CNTRLREG_STEPID | CNTRLREG_4WIRE;
+ ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_STEPID;
+ if (tscadc_dev->tsc_cell != -1)
+ ctrl |= CNTRLREG_TSCENB | CNTRLREG_4WIRE;
tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
- tscadc_idle_config(tscadc_dev);
+
+ if (tscadc_dev->tsc_cell != -1)
+ tscadc_idle_config(tscadc_dev);
am335x_tsc_se_update(tscadc_dev);
restore = tscadc_readl(tscadc_dev, REG_CTRL);
tscadc_writel(tscadc_dev, REG_CTRL,
--
1.7.9.5
On 07/20/2013 05:27 PM, Zubair Lutfullah wrote:
> From: "Patil, Rachna" <[email protected]>
>
> Previously we tried to read data form ADC even before ADC sequencer
> finished sampling. This led to wrong samples.
> We now wait on ADC status register idle bit to be set.
>
> Signed-off-by: Patil, Rachna <[email protected]>
> Signed-off-by: Zubair Lutfullah <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>
Applied this one to the fixes-togreg branch of iio.git.
I'd imagine the others will go through mfd?
> ---
> drivers/iio/adc/ti_am335x_adc.c | 30 ++++++++++++++++++++++--------
> include/linux/mfd/ti_am335x_tscadc.h | 16 ++++++++++++++++
> 2 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index 4427e8e..f73fee1 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -60,7 +60,6 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
> {
> unsigned int stepconfig;
> int i, steps;
> - u32 step_en;
>
> /*
> * There are 16 configurable steps and 8 analog input
> @@ -86,8 +85,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
> adc_dev->channel_step[i] = steps;
> steps++;
> }
> - step_en = get_adc_step_mask(adc_dev);
> - am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
> +
> }
>
> static const char * const chan_name_ain[] = {
> @@ -142,10 +140,22 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
> int *val, int *val2, long mask)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> - int i;
> - unsigned int fifo1count, read;
> + int i, map_val;
> + unsigned int fifo1count, read, stepid;
> u32 step = UINT_MAX;
> bool found = false;
> + u32 step_en;
> + unsigned long timeout = jiffies + usecs_to_jiffies
> + (IDLE_TIMEOUT * adc_dev->channels);
> + step_en = get_adc_step_mask(adc_dev);
> + am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
> +
> + /* Wait for ADC sequencer to complete sampling */
> + while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) {
> + if (time_after(jiffies, timeout))
> + return -EAGAIN;
> + }
> + map_val = chan->channel + TOTAL_CHANNELS;
>
> /*
> * When the sub-system is first enabled,
> @@ -170,12 +180,16 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> for (i = 0; i < fifo1count; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> - if (read >> 16 == step) {
> - *val = read & 0xfff;
> + stepid = read & FIFOREAD_CHNLID_MASK;
> + stepid = stepid >> 0x10;
> +
> + if (stepid == map_val) {
> + read = read & FIFOREAD_DATA_MASK;
> found = true;
> + *val = read;
> }
> }
> - am335x_tsc_se_update(adc_dev->mfd_tscadc);
> +
> if (found == false)
> return -EBUSY;
> return IIO_VAL_INT;
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index 8d73fe2..db1791b 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -113,11 +113,27 @@
> #define CNTRLREG_8WIRE CNTRLREG_AFE_CTRL(3)
> #define CNTRLREG_TSCENB BIT(7)
>
> +/* FIFO READ Register */
> +#define FIFOREAD_DATA_MASK (0xfff << 0)
> +#define FIFOREAD_CHNLID_MASK (0xf << 16)
> +
> +/* Sequencer Status */
> +#define SEQ_STATUS BIT(5)
> +
> #define ADC_CLK 3000000
> #define MAX_CLK_DIV 7
> #define TOTAL_STEPS 16
> #define TOTAL_CHANNELS 8
>
> +/*
> +* ADC runs at 3MHz, and it takes
> +* 15 cycles to latch one data output.
> +* Hence the idle time for ADC to
> +* process one sample data would be
> +* around 5 micro seconds.
> +*/
> +#define IDLE_TIMEOUT 5 /* microsec */
> +
> #define TSCADC_CELLS 2
>
> struct ti_tscadc_dev {
>
On Sat, 20 Jul 2013, Zubair Lutfullah wrote:
> Did a grep for 'coordiante' and replaced them all with 'coordinate'
> ---
> .../bindings/input/touchscreen/ti-tsc-adc.txt | 4 ++--
> arch/arm/boot/dts/am335x-evm.dts | 2 +-
> drivers/input/touchscreen/ti_am335x_tsc.c | 2 +-
> drivers/mfd/ti_am335x_tscadc.c | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
Once these DT bindings move from a development state to something more
concrete, I guess misspellings such as these will just have to remain
incorrect. However, as we are still in a state of development you can
have my:
Acked-by: Lee Jones <[email protected]>
... for the MFD part.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 20 Jul 2013, Zubair Lutfullah wrote:
> From: "Patil, Rachna" <[email protected]>
>
> ADC is ideally expected to work at a frequency of 3MHz.
> The present code had a check, which returned error if the frequency
> went below the threshold value. But since AM335x supports various
> working frequencies, this check is not required.
> Now the code just uses the internal ADC clock divider to set the ADC
> frequency w.r.t the sys clock.
>
> Signed-off-by: Patil, Rachna <[email protected]>
> Signed-off-by: Zubair Lutfullah <[email protected]>
> ---
> drivers/mfd/ti_am335x_tscadc.c | 6 +-----
> include/linux/mfd/ti_am335x_tscadc.h | 1 -
> 2 files changed, 1 insertion(+), 6 deletions(-)
Applied, thanks.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 20 Jul 2013, Zubair Lutfullah wrote:
> From: "Patil, Rachna" <[email protected]>
>
> AFE Pen Ctrl and TouchScreen transistors enabling is not
> required when only ADC mode is being used, so check for availability of
> TSC driver before accessing control register.
>
> Signed-off-by: Patil, Rachna <[email protected]>
> Acked-by: Vaibhav Hiremath <[email protected]>
> Signed-off-by: Zubair Lutfullah <[email protected]>
> ---
> drivers/mfd/ti_am335x_tscadc.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
Applied, thanks.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog