[PATCH v2 0/4] iio: at91: Add touch screen support in at91 adc

[PATCH v2 0/4] iio: at91: Add touch screen support in at91 adc

[Josh Wu]

This patch series will add touch screen support for at91 adc driver.
To enable touch screen you need add the touch screen related dt properties.

v1 --> v2:
  1. use multiple compatible string for different ADC IP.
  2. drop the patch that add the adc-clock in dt since we will move to clock
     common framework.
  3. don't expose the driver configuration to dt. Such like sample period,
     pen detect debounce time and etc.
  4. add a new dt parameter: ts-pressure-threshold.

Josh Wu (4):
  iio: at91: fix adc_clk overflow
  iio: at91: Use different prescal, startup mask in MR for different IP
  iio: at91: ADC start-up time calculation changed since at91sam9x5
  iio: at91: introduce touch screen support in iio adc driver

 .../devicetree/bindings/arm/atmel-adc.txt          |   10 +-
 arch/arm/mach-at91/include/mach/at91_adc.h         |   41 +-
 drivers/iio/adc/at91_adc.c                         |  468 ++++++++++++++++++--
 3 files changed, 479 insertions(+), 40 deletions(-)

-- 
1.7.10

[PATCH v2 1/4] iio: at91: fix adc_clk overflow

[Josh Wu]

The adc_clk variable is currently defined using a 32-bits unsigned integer,
which will overflow under some very valid range of operations.

Such overflow will occur if, for example, the parent clock is set to a
20MHz frequency and the ADC startup time is larger than 215ns.

To fix this, introduce an intermediate variable holding the clock rate
in kHz.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
v1 --> v2:
  refined the commit message.

 drivers/iio/adc/at91_adc.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index b6db6a0..9cf8ab5 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -582,7 +582,7 @@ static const struct iio_info at91_adc_info = {
 
 static int at91_adc_probe(struct platform_device *pdev)
 {
-	unsigned int prsc, mstrclk, ticks, adc_clk, shtim;
+	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
 	int ret;
 	struct iio_dev *idev;
 	struct at91_adc_state *st;
@@ -680,6 +680,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 	 */
 	mstrclk = clk_get_rate(st->clk);
 	adc_clk = clk_get_rate(st->adc_clk);
+	adc_clk_khz = adc_clk / 1000;
 	prsc = (mstrclk / (2 * adc_clk)) - 1;
 
 	if (!st->startup_time) {
@@ -693,15 +694,15 @@ static int at91_adc_probe(struct platform_device *pdev)
 	 * defined in the electrical characteristics of the board, divided by 8.
 	 * The formula thus is : Startup Time = (ticks + 1) * 8 / ADC Clock
 	 */
-	ticks = round_up((st->startup_time * adc_clk /
-			  1000000) - 1, 8) / 8;
+	ticks = round_up((st->startup_time * adc_clk_khz /
+			  1000) - 1, 8) / 8;
 	/*
 	 * a minimal Sample and Hold Time is necessary for the ADC to guarantee
 	 * the best converted final value between two channels selection
 	 * The formula thus is : Sample and Hold Time = (shtim + 1) / ADCClock
 	 */
-	shtim = round_up((st->sample_hold_time * adc_clk /
-			  1000000) - 1, 1);
+	shtim = round_up((st->sample_hold_time * adc_clk_khz /
+			  1000) - 1, 1);
 
 	reg = AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL;
 	reg |= AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP;
-- 
1.7.10

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Josh Wu]

For at91 boards, there are different IPs for adc. Different IPs has different
STARTUP & PRESCAL mask in ADC_MR.

This patch introduce the multiple compatible string for those different IPs.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
v1 --> v2:
  1. use multiple compatible string for different IPs instead of use run-time
     IP check of hardware register.
  2. rebase on top of linus' master tree.

 .../devicetree/bindings/arm/atmel-adc.txt          |    3 +-
 arch/arm/mach-at91/include/mach/at91_adc.h         |    7 ++--
 drivers/iio/adc/at91_adc.c                         |   34 ++++++++++++++++++--
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/atmel-adc.txt b/Documentation/devicetree/bindings/arm/atmel-adc.txt
index 16769d9..6e60781 100644
--- a/Documentation/devicetree/bindings/arm/atmel-adc.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-adc.txt
@@ -1,7 +1,8 @@
 * AT91's Analog to Digital Converter (ADC)
 
 Required properties:
-  - compatible: Should be "atmel,at91sam9260-adc"
+  - compatible: Should be "atmel,<chip>-adc"
+    <chip> can be "at91sam9260", "at91sam9g45" or "at91sam9x5"
   - reg: Should contain ADC registers location and length
   - interrupts: Should contain the IRQ line for the ADC
   - atmel,adc-channel-base: Offset of the first channel data register
diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
index 8e7ed5c..f00926a 100644
--- a/arch/arm/mach-at91/include/mach/at91_adc.h
+++ b/arch/arm/mach-at91/include/mach/at91_adc.h
@@ -28,9 +28,12 @@
 #define			AT91_ADC_TRGSEL_EXTERNAL	(6 << 1)
 #define		AT91_ADC_LOWRES		(1 << 4)	/* Low Resolution */
 #define		AT91_ADC_SLEEP		(1 << 5)	/* Sleep Mode */
-#define		AT91_ADC_PRESCAL	(0x3f << 8)	/* Prescalar Rate Selection */
+#define		AT91_ADC_PRESCAL_9260	(0x3f << 8)	/* Prescalar Rate Selection */
+#define		AT91_ADC_PRESCAL_9G45	(0xff << 8)
 #define			AT91_ADC_PRESCAL_(x)	((x) << 8)
-#define		AT91_ADC_STARTUP	(0x1f << 16)	/* Startup Up Time */
+#define		AT91_ADC_STARTUP_9260	(0x1f << 16)	/* Startup Up Time */
+#define		AT91_ADC_STARTUP_9G45	(0x7f << 16)
+#define		AT91_ADC_STARTUP_9X5	(0xf << 16)
 #define			AT91_ADC_STARTUP_(x)	((x) << 16)
 #define		AT91_ADC_SHTIM		(0xf  << 24)	/* Sample & Hold Time */
 #define			AT91_ADC_SHTIM_(x)	((x) << 24)
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 9cf8ab5..b41eb60 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -39,6 +39,11 @@
 #define at91_adc_writel(st, reg, val) \
 	(writel_relaxed(val, st->reg_base + reg))
 
+struct at91_adc_caps {
+	u32	mr_prescal_mask;
+	u32	mr_startup_mask;
+};
+
 struct at91_adc_state {
 	struct clk		*adc_clk;
 	u16			*buffer;
@@ -62,6 +67,7 @@ struct at91_adc_state {
 	u32			res;		/* resolution used for convertions */
 	bool			low_res;	/* the resolution corresponds to the lowest one */
 	wait_queue_head_t	wq_data_avail;
+	struct at91_adc_caps	*caps;
 };
 
 static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
@@ -429,6 +435,8 @@ ret:
 	return ret;
 }
 
+static const struct of_device_id at91_adc_dt_ids[];
+
 static int at91_adc_probe_dt(struct at91_adc_state *st,
 			     struct platform_device *pdev)
 {
@@ -441,6 +449,9 @@ static int at91_adc_probe_dt(struct at91_adc_state *st,
 	if (!node)
 		return -EINVAL;
 
+	st->caps = (struct at91_adc_caps *)
+		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
+
 	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
 
 	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
@@ -704,8 +715,8 @@ static int at91_adc_probe(struct platform_device *pdev)
 	shtim = round_up((st->sample_hold_time * adc_clk_khz /
 			  1000) - 1, 1);
 
-	reg = AT91_ADC_PRESCAL_(prsc) & AT91_ADC_PRESCAL;
-	reg |= AT91_ADC_STARTUP_(ticks) & AT91_ADC_STARTUP;
+	reg = AT91_ADC_PRESCAL_(prsc) & st->caps->mr_prescal_mask;
+	reg |= AT91_ADC_STARTUP_(ticks) & st->caps->mr_startup_mask;
 	if (st->low_res)
 		reg |= AT91_ADC_LOWRES;
 	if (st->sleep_mode)
@@ -776,8 +787,25 @@ static int at91_adc_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_OF
+static struct at91_adc_caps at91sam9260_caps = {
+	.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
+	.mr_startup_mask = AT91_ADC_STARTUP_9260,
+};
+
+static struct at91_adc_caps at91sam9g45_caps = {
+	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
+	.mr_startup_mask = AT91_ADC_STARTUP_9G45,
+};
+
+static struct at91_adc_caps at91sam9x5_caps = {
+	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,	/* same as 9G45 */
+	.mr_startup_mask = AT91_ADC_STARTUP_9X5,
+};
+
 static const struct of_device_id at91_adc_dt_ids[] = {
-	{ .compatible = "atmel,at91sam9260-adc" },
+	{ .compatible = "atmel,at91sam9260-adc", .data = &at91sam9260_caps },
+	{ .compatible = "atmel,at91sam9g45-adc", .data = &at91sam9g45_caps },
+	{ .compatible = "atmel,at91sam9x5-adc", .data = &at91sam9x5_caps },
 	{},
 };
 MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);
-- 
1.7.10

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Maxime Ripard]

Hi Josh,

On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> For at91 boards, there are different IPs for adc. Different IPs has
> different STARTUP & PRESCAL mask in ADC_MR.
> 
> This patch introduce the multiple compatible string for those
> different IPs.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>

Overall it looks like the right ways, but I think we can take it a step
further.

I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
atmel,adc-status-register, atmel,adc-trigger-register properties (and
probably the triggers as well description as well).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130815/5a870082/attachment.sig>

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Josh Wu]

Hi, Maxime

On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> Hi Josh,
>
> On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
>> For at91 boards, there are different IPs for adc. Different IPs has
>> different STARTUP & PRESCAL mask in ADC_MR.
>>
>> This patch introduce the multiple compatible string for those
>> different IPs.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> Overall it looks like the right ways, but I think we can take it a step
> further.
>
> I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> atmel,adc-status-register, atmel,adc-trigger-register properties (and
> probably the triggers as well description as well).

yeah, right. Currently I want to drop following:

atmel,adc-drdy-mask, atmel,adc-status-register, atmel,adc-trigger-register, atmel,adc-channel-base

For the adc-num-channels, I'd like to leave it in dt parameters. It is a 
description for an adc capablity.

For the triggers, I am not decided. An obvious benifit to remove trigger 
in dt will save many lines of code.

>
> Maxime
>

Best Regards,
Josh Wu

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Josh Wu]

Add Ludovic in the loop.

On 8/22/2013 5:51 PM, Josh Wu wrote:
> Hi, Maxime
>
> On 8/16/2013 3:20 AM, Maxime Ripard wrote:
>> Hi Josh,
>>
>> On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
>>> For at91 boards, there are different IPs for adc. Different IPs has
>>> different STARTUP & PRESCAL mask in ADC_MR.
>>>
>>> This patch introduce the multiple compatible string for those
>>> different IPs.
>>>
>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Overall it looks like the right ways, but I think we can take it a step
>> further.
>>
>> I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
>> atmel,adc-status-register, atmel,adc-trigger-register properties (and
>> probably the triggers as well description as well).
>
> yeah, right. Currently I want to drop following:
>
> atmel,adc-drdy-mask, atmel,adc-status-register, 
> atmel,adc-trigger-register, atmel,adc-channel-base
>
> For the adc-num-channels, I'd like to leave it in dt parameters. It is 
> a description for an adc capablity.
>
> For the triggers, I am not decided. An obvious benifit to remove 
> trigger in dt will save many lines of code.
>
>>
>> Maxime
>>
>
> Best Regards,
> Josh Wu

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Desroches, Ludovic]

Hi,

On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
> Add Ludovic in the loop.
> 
> On 8/22/2013 5:51 PM, Josh Wu wrote:
> >Hi, Maxime
> >
> >On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> >>Hi Josh,
> >>
> >>On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> >>>For at91 boards, there are different IPs for adc. Different IPs has
> >>>different STARTUP & PRESCAL mask in ADC_MR.
> >>>
> >>>This patch introduce the multiple compatible string for those
> >>>different IPs.
> >>>
> >>>Signed-off-by: Josh Wu <josh.wu@atmel.com>
> >>Overall it looks like the right ways, but I think we can take it a step
> >>further.
> >>
> >>I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> >>atmel,adc-status-register, atmel,adc-trigger-register properties (and
> >>probably the triggers as well description as well).
> >
> >yeah, right. Currently I want to drop following:
> >
> >atmel,adc-drdy-mask, atmel,adc-status-register,
> >atmel,adc-trigger-register, atmel,adc-channel-base
> >
> >For the adc-num-channels, I'd like to leave it in dt parameters.
> >It is a description for an adc capablity.

About this parameter, I'll remove it too from the dt bindings. To set it you
need to have a look to the datasheet and to copy a constant value into the
dt. From my point of view, it means than this parameter should be managed by
the driver and by the dt.

On the other side since there are some dynamic allocation depending on this
parameter maybe it makes sense to keep it in the dt. If the user wants to use
only 2 channels why doing allocation for max channel number. By the way, this
case is only valid if he uses the two first channels.


> >
> >For the triggers, I am not decided. An obvious benifit to remove
> >trigger in dt will save many lines of code.
> >
> >>
> >>Maxime
> >>
> >
> >Best Regards,
> >Josh Wu
>

Since we are talking about reworking this binding I was thinking about
resolution stuff. Filling atmel,adc-res is also copying constant value from
the device datasheet, so if I was consistent I would say it has to be removed
too. But I am not consistent! I mean by doing this the only thing the user
will have to fill is the resolution value. He can't set the value he wants,
there are only two choices. By keeping it into the dt then he will immediately
see the choices he has.


Regards

Ludovic 

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Maxime Ripard]

Hi Ludovic, Josh,

On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
> On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
> > On 8/22/2013 5:51 PM, Josh Wu wrote:
> > >Hi, Maxime
> > >
> > >On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> > >>Hi Josh,
> > >>
> > >>On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> > >>>For at91 boards, there are different IPs for adc. Different IPs has
> > >>>different STARTUP & PRESCAL mask in ADC_MR.
> > >>>
> > >>>This patch introduce the multiple compatible string for those
> > >>>different IPs.
> > >>>
> > >>>Signed-off-by: Josh Wu <josh.wu@atmel.com>
> > >>Overall it looks like the right ways, but I think we can take it a step
> > >>further.
> > >>
> > >>I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> > >>atmel,adc-status-register, atmel,adc-trigger-register properties (and
> > >>probably the triggers as well description as well).
> > >
> > >yeah, right. Currently I want to drop following:
> > >
> > >atmel,adc-drdy-mask, atmel,adc-status-register,
> > >atmel,adc-trigger-register, atmel,adc-channel-base
> > >
> > >For the adc-num-channels, I'd like to leave it in dt parameters.
> > >It is a description for an adc capablity.
> 
> About this parameter, I'll remove it too from the dt bindings. To set it you
> need to have a look to the datasheet and to copy a constant value into the
> dt. From my point of view, it means than this parameter should be managed by
> the driver and by the dt.
> 
> On the other side since there are some dynamic allocation depending on this
> parameter maybe it makes sense to keep it in the dt. If the user wants to use
> only 2 channels why doing allocation for max channel number. By the way, this
> case is only valid if he uses the two first channels.

I don't recall it very well, is there any reason to not have it in the
DT? Can the ADC channels be used for something else? Or is it just some
IP-specific number of channels?

> > >
> > >For the triggers, I am not decided. An obvious benifit to remove
> > >trigger in dt will save many lines of code.
> > >
> > >>
> > >>Maxime
> > >>
> > >
> > >Best Regards,
> > >Josh Wu
> >
> 
> Since we are talking about reworking this binding I was thinking about
> resolution stuff. Filling atmel,adc-res is also copying constant value from
> the device datasheet, so if I was consistent I would say it has to be removed
> too. But I am not consistent! I mean by doing this the only thing the user
> will have to fill is the resolution value. He can't set the value he wants,
> there are only two choices. By keeping it into the dt then he will immediately
> see the choices he has.

But the resolution should probably be somehow user-defined, probably not
really related to the DT has well. I think some other IIO ADC drivers
are using sysfs files for this purpose, maybe that would be a better
fit?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130823/36edda68/attachment.sig>

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Ludovic Desroches]

On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:
> Hi Ludovic, Josh,
> 
> On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
> > On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
> > > On 8/22/2013 5:51 PM, Josh Wu wrote:
> > > >Hi, Maxime
> > > >
> > > >On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> > > >>Hi Josh,
> > > >>
> > > >>On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> > > >>>For at91 boards, there are different IPs for adc. Different IPs has
> > > >>>different STARTUP & PRESCAL mask in ADC_MR.
> > > >>>
> > > >>>This patch introduce the multiple compatible string for those
> > > >>>different IPs.
> > > >>>
> > > >>>Signed-off-by: Josh Wu <josh.wu@atmel.com>
> > > >>Overall it looks like the right ways, but I think we can take it a step
> > > >>further.
> > > >>
> > > >>I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> > > >>atmel,adc-status-register, atmel,adc-trigger-register properties (and
> > > >>probably the triggers as well description as well).
> > > >
> > > >yeah, right. Currently I want to drop following:
> > > >
> > > >atmel,adc-drdy-mask, atmel,adc-status-register,
> > > >atmel,adc-trigger-register, atmel,adc-channel-base
> > > >
> > > >For the adc-num-channels, I'd like to leave it in dt parameters.
> > > >It is a description for an adc capablity.
> > 
> > About this parameter, I'll remove it too from the dt bindings. To set it you
> > need to have a look to the datasheet and to copy a constant value into the
> > dt. From my point of view, it means than this parameter should be managed by
> > the driver and by the dt.
> > 
> > On the other side since there are some dynamic allocation depending on this
> > parameter maybe it makes sense to keep it in the dt. If the user wants to use
> > only 2 channels why doing allocation for max channel number. By the way, this
> > case is only valid if he uses the two first channels.
> 
> I don't recall it very well, is there any reason to not have it in the
> DT? Can the ADC channels be used for something else? Or is it just some
> IP-specific number of channels?
> 

It is IP-specific. I don't see what benefit we could have to keep it in the DT?
But Josh seems to have arguments to keep it.

> > > >
> > > >For the triggers, I am not decided. An obvious benifit to remove
> > > >trigger in dt will save many lines of code.
> > > >
> > > >>
> > > >>Maxime
> > > >>
> > > >
> > > >Best Regards,
> > > >Josh Wu
> > >
> > 
> > Since we are talking about reworking this binding I was thinking about
> > resolution stuff. Filling atmel,adc-res is also copying constant value from
> > the device datasheet, so if I was consistent I would say it has to be removed
> > too. But I am not consistent! I mean by doing this the only thing the user
> > will have to fill is the resolution value. He can't set the value he wants,
> > there are only two choices. By keeping it into the dt then he will immediately
> > see the choices he has.
> 
> But the resolution should probably be somehow user-defined, probably not
> really related to the DT has well. I think some other IIO ADC drivers
> are using sysfs files for this purpose, maybe that would be a better
> fit?

It sounds to be a good solution.


Ludovic

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Josh Wu]

Hi, Ludovic and Maxime

On 8/26/2013 4:32 PM, Ludovic Desroches wrote:
> On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:
>> Hi Ludovic, Josh,
>>
>> On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
>>> On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
>>>> On 8/22/2013 5:51 PM, Josh Wu wrote:
>>>>> Hi, Maxime
>>>>>
>>>>> On 8/16/2013 3:20 AM, Maxime Ripard wrote:
>>>>>> Hi Josh,
>>>>>>
>>>>>> On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
>>>>>>> For at91 boards, there are different IPs for adc. Different IPs has
>>>>>>> different STARTUP & PRESCAL mask in ADC_MR.
>>>>>>>
>>>>>>> This patch introduce the multiple compatible string for those
>>>>>>> different IPs.
>>>>>>>
>>>>>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>>>>> Overall it looks like the right ways, but I think we can take it a step
>>>>>> further.
>>>>>>
>>>>>> I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
>>>>>> atmel,adc-status-register, atmel,adc-trigger-register properties (and
>>>>>> probably the triggers as well description as well).
>>>>> yeah, right. Currently I want to drop following:
>>>>>
>>>>> atmel,adc-drdy-mask, atmel,adc-status-register,
>>>>> atmel,adc-trigger-register, atmel,adc-channel-base
>>>>>
>>>>> For the adc-num-channels, I'd like to leave it in dt parameters.
>>>>> It is a description for an adc capablity.
>>> About this parameter, I'll remove it too from the dt bindings. To set it you
>>> need to have a look to the datasheet and to copy a constant value into the
>>> dt. From my point of view, it means than this parameter should be managed by
>>> the driver and by the dt.
>>>
>>> On the other side since there are some dynamic allocation depending on this
>>> parameter maybe it makes sense to keep it in the dt. If the user wants to use
>>> only 2 channels why doing allocation for max channel number. By the way, this
>>> case is only valid if he uses the two first channels.
>> I don't recall it very well, is there any reason to not have it in the
>> DT? Can the ADC channels be used for something else? Or is it just some
>> IP-specific number of channels?
>>
> It is IP-specific. I don't see what benefit we could have to keep it in the DT?
> But Josh seems to have arguments to keep it.

I'm ok to remove the channel number. What I worried is there also has a 
channel-used mask in DT.
This mask should be removed too if channel number is removed.
So maybe we can also use the sysfs to set the mask.

>
>>>>> For the triggers, I am not decided. An obvious benifit to remove
>>>>> trigger in dt will save many lines of code.
>>>>>
>>>>>> Maxime
>>>>>>
>>>>> Best Regards,
>>>>> Josh Wu
>>> Since we are talking about reworking this binding I was thinking about
>>> resolution stuff. Filling atmel,adc-res is also copying constant value from
>>> the device datasheet, so if I was consistent I would say it has to be removed
>>> too. But I am not consistent! I mean by doing this the only thing the user
>>> will have to fill is the resolution value. He can't set the value he wants,
>>> there are only two choices. By keeping it into the dt then he will immediately
>>> see the choices he has.
>> But the resolution should probably be somehow user-defined, probably not
>> really related to the DT has well. I think some other IIO ADC drivers
>> are using sysfs files for this purpose, maybe that would be a better
>> fit?
> It sounds to be a good solution.

ok, I will check the other IIO ADC driver about this point.
Maybe this sysfs replacement need a bit more time. I prefer to send out 
the patches first without the sysfs implement in v3.
And the sysfs replacement patch will be send out serperately. What do 
you think? Maxime.

>
>
> Ludovic

Best Regards,
Josh Wu

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Maxime Ripard]

On Mon, Aug 26, 2013 at 06:03:31PM +0800, Josh Wu wrote:
> Hi, Ludovic and Maxime
> 
> On 8/26/2013 4:32 PM, Ludovic Desroches wrote:
> >On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:
> >>Hi Ludovic, Josh,
> >>
> >>On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
> >>>On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
> >>>>On 8/22/2013 5:51 PM, Josh Wu wrote:
> >>>>>Hi, Maxime
> >>>>>
> >>>>>On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> >>>>>>Hi Josh,
> >>>>>>
> >>>>>>On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> >>>>>>>For at91 boards, there are different IPs for adc. Different IPs has
> >>>>>>>different STARTUP & PRESCAL mask in ADC_MR.
> >>>>>>>
> >>>>>>>This patch introduce the multiple compatible string for those
> >>>>>>>different IPs.
> >>>>>>>
> >>>>>>>Signed-off-by: Josh Wu <josh.wu@atmel.com>
> >>>>>>Overall it looks like the right ways, but I think we can take it a step
> >>>>>>further.
> >>>>>>
> >>>>>>I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> >>>>>>atmel,adc-status-register, atmel,adc-trigger-register properties (and
> >>>>>>probably the triggers as well description as well).
> >>>>>yeah, right. Currently I want to drop following:
> >>>>>
> >>>>>atmel,adc-drdy-mask, atmel,adc-status-register,
> >>>>>atmel,adc-trigger-register, atmel,adc-channel-base
> >>>>>
> >>>>>For the adc-num-channels, I'd like to leave it in dt parameters.
> >>>>>It is a description for an adc capablity.
> >>>About this parameter, I'll remove it too from the dt bindings. To set it you
> >>>need to have a look to the datasheet and to copy a constant value into the
> >>>dt. From my point of view, it means than this parameter should be managed by
> >>>the driver and by the dt.
> >>>
> >>>On the other side since there are some dynamic allocation depending on this
> >>>parameter maybe it makes sense to keep it in the dt. If the user wants to use
> >>>only 2 channels why doing allocation for max channel number. By the way, this
> >>>case is only valid if he uses the two first channels.
> >>I don't recall it very well, is there any reason to not have it in the
> >>DT? Can the ADC channels be used for something else? Or is it just some
> >>IP-specific number of channels?
> >>
> >It is IP-specific. I don't see what benefit we could have to keep it in the DT?
> >But Josh seems to have arguments to keep it.
> 
> I'm ok to remove the channel number. What I worried is there also
> has a channel-used mask in DT.
> This mask should be removed too if channel number is removed.
> So maybe we can also use the sysfs to set the mask.

Indeed, that would make adc-channel-used irrelevant. But I'm not sure
the mask is useful at all. Just enable all the channels and that's it?

> >>>>>For the triggers, I am not decided. An obvious benifit to remove
> >>>>>trigger in dt will save many lines of code.
> >>>>>
> >>>>>>Maxime
> >>>>>>
> >>>>>Best Regards,
> >>>>>Josh Wu
> >>>Since we are talking about reworking this binding I was thinking about
> >>>resolution stuff. Filling atmel,adc-res is also copying constant value from
> >>>the device datasheet, so if I was consistent I would say it has to be removed
> >>>too. But I am not consistent! I mean by doing this the only thing the user
> >>>will have to fill is the resolution value. He can't set the value he wants,
> >>>there are only two choices. By keeping it into the dt then he will immediately
> >>>see the choices he has.
> >>But the resolution should probably be somehow user-defined, probably not
> >>really related to the DT has well. I think some other IIO ADC drivers
> >>are using sysfs files for this purpose, maybe that would be a better
> >>fit?
> >It sounds to be a good solution.
> 
> ok, I will check the other IIO ADC driver about this point.
> Maybe this sysfs replacement need a bit more time. I prefer to send
> out the patches first without the sysfs implement in v3.
> And the sysfs replacement patch will be send out serperately. What
> do you think? Maxime.

Yes, of course. The resolution rework can definitely be done later.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130827/09e64a41/attachment.sig>

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Nicolas Ferre]

On 27/08/2013 10:15, Maxime Ripard :
> On Mon, Aug 26, 2013 at 06:03:31PM +0800, Josh Wu wrote:
>> Hi, Ludovic and Maxime
>>
>> On 8/26/2013 4:32 PM, Ludovic Desroches wrote:
>>> On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:
>>>> Hi Ludovic, Josh,
>>>>
>>>> On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
>>>>> On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
>>>>>> On 8/22/2013 5:51 PM, Josh Wu wrote:
>>>>>>> Hi, Maxime
>>>>>>>
>>>>>>> On 8/16/2013 3:20 AM, Maxime Ripard wrote:
>>>>>>>> Hi Josh,
>>>>>>>>
>>>>>>>> On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
>>>>>>>>> For at91 boards, there are different IPs for adc. Different IPs has
>>>>>>>>> different STARTUP & PRESCAL mask in ADC_MR.
>>>>>>>>>
>>>>>>>>> This patch introduce the multiple compatible string for those
>>>>>>>>> different IPs.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>>>>>>> Overall it looks like the right ways, but I think we can take it a step
>>>>>>>> further.
>>>>>>>>
>>>>>>>> I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
>>>>>>>> atmel,adc-status-register, atmel,adc-trigger-register properties (and
>>>>>>>> probably the triggers as well description as well).
>>>>>>> yeah, right. Currently I want to drop following:
>>>>>>>
>>>>>>> atmel,adc-drdy-mask, atmel,adc-status-register,
>>>>>>> atmel,adc-trigger-register, atmel,adc-channel-base
>>>>>>>
>>>>>>> For the adc-num-channels, I'd like to leave it in dt parameters.
>>>>>>> It is a description for an adc capablity.
>>>>> About this parameter, I'll remove it too from the dt bindings. To set it you
>>>>> need to have a look to the datasheet and to copy a constant value into the
>>>>> dt. From my point of view, it means than this parameter should be managed by
>>>>> the driver and by the dt.
>>>>>
>>>>> On the other side since there are some dynamic allocation depending on this
>>>>> parameter maybe it makes sense to keep it in the dt. If the user wants to use
>>>>> only 2 channels why doing allocation for max channel number. By the way, this
>>>>> case is only valid if he uses the two first channels.
>>>> I don't recall it very well, is there any reason to not have it in the
>>>> DT? Can the ADC channels be used for something else? Or is it just some
>>>> IP-specific number of channels?
>>>>
>>> It is IP-specific. I don't see what benefit we could have to keep it in the DT?
>>> But Josh seems to have arguments to keep it.
>>
>> I'm ok to remove the channel number. What I worried is there also
>> has a channel-used mask in DT.
>> This mask should be removed too if channel number is removed.
>> So maybe we can also use the sysfs to set the mask.
>
> Indeed, that would make adc-channel-used irrelevant. But I'm not sure
> the mask is useful at all. Just enable all the channels and that's it?

On the top of my head: keeping all channels enabled, won't it have an 
impact on:
1/ power consumption?
2/ minimum period of sampling for a particular channel in case of
    continuous ADC trigger?

And do not forget that sysfs is an interface that is:
- needing a userspace tool to be configured properly (even just an
   echo xx)
- hard to design
- a strict ABI that can't be changed once deployed

But yes, it is true that if the user has to configure ADC at runtime, it 
is certainly an interface worth considering...

>>>>>>> For the triggers, I am not decided. An obvious benifit to remove
>>>>>>> trigger in dt will save many lines of code.
>>>>>>>
>>>>>>>> Maxime
>>>>>>>>
>>>>>>> Best Regards,
>>>>>>> Josh Wu
>>>>> Since we are talking about reworking this binding I was thinking about
>>>>> resolution stuff. Filling atmel,adc-res is also copying constant value from
>>>>> the device datasheet, so if I was consistent I would say it has to be removed
>>>>> too. But I am not consistent! I mean by doing this the only thing the user
>>>>> will have to fill is the resolution value. He can't set the value he wants,
>>>>> there are only two choices. By keeping it into the dt then he will immediately
>>>>> see the choices he has.
>>>> But the resolution should probably be somehow user-defined, probably not
>>>> really related to the DT has well. I think some other IIO ADC drivers
>>>> are using sysfs files for this purpose, maybe that would be a better
>>>> fit?
>>> It sounds to be a good solution.
>>
>> ok, I will check the other IIO ADC driver about this point.
>> Maybe this sysfs replacement need a bit more time. I prefer to send
>> out the patches first without the sysfs implement in v3.
>> And the sysfs replacement patch will be send out serperately. What
>> do you think? Maxime.
>
> Yes, of course. The resolution rework can definitely be done later.
>
> Maxime
>


-- 
Nicolas Ferre

[PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Maxime Ripard]

Hi Nicolas,

On Tue, Aug 27, 2013 at 11:05:22AM +0200, Nicolas Ferre wrote:
> On 27/08/2013 10:15, Maxime Ripard :
> >On Mon, Aug 26, 2013 at 06:03:31PM +0800, Josh Wu wrote:
> >>Hi, Ludovic and Maxime
> >>
> >>On 8/26/2013 4:32 PM, Ludovic Desroches wrote:
> >>>On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:
> >>>>Hi Ludovic, Josh,
> >>>>
> >>>>On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
> >>>>>On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
> >>>>>>On 8/22/2013 5:51 PM, Josh Wu wrote:
> >>>>>>>Hi, Maxime
> >>>>>>>
> >>>>>>>On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> >>>>>>>>Hi Josh,
> >>>>>>>>
> >>>>>>>>On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> >>>>>>>>>For at91 boards, there are different IPs for adc. Different IPs has
> >>>>>>>>>different STARTUP & PRESCAL mask in ADC_MR.
> >>>>>>>>>
> >>>>>>>>>This patch introduce the multiple compatible string for those
> >>>>>>>>>different IPs.
> >>>>>>>>>
> >>>>>>>>>Signed-off-by: Josh Wu <josh.wu@atmel.com>
> >>>>>>>>Overall it looks like the right ways, but I think we can take it a step
> >>>>>>>>further.
> >>>>>>>>
> >>>>>>>>I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> >>>>>>>>atmel,adc-status-register, atmel,adc-trigger-register properties (and
> >>>>>>>>probably the triggers as well description as well).
> >>>>>>>yeah, right. Currently I want to drop following:
> >>>>>>>
> >>>>>>>atmel,adc-drdy-mask, atmel,adc-status-register,
> >>>>>>>atmel,adc-trigger-register, atmel,adc-channel-base
> >>>>>>>
> >>>>>>>For the adc-num-channels, I'd like to leave it in dt parameters.
> >>>>>>>It is a description for an adc capablity.
> >>>>>About this parameter, I'll remove it too from the dt bindings. To set it you
> >>>>>need to have a look to the datasheet and to copy a constant value into the
> >>>>>dt. From my point of view, it means than this parameter should be managed by
> >>>>>the driver and by the dt.
> >>>>>
> >>>>>On the other side since there are some dynamic allocation depending on this
> >>>>>parameter maybe it makes sense to keep it in the dt. If the user wants to use
> >>>>>only 2 channels why doing allocation for max channel number. By the way, this
> >>>>>case is only valid if he uses the two first channels.
> >>>>I don't recall it very well, is there any reason to not have it in the
> >>>>DT? Can the ADC channels be used for something else? Or is it just some
> >>>>IP-specific number of channels?
> >>>>
> >>>It is IP-specific. I don't see what benefit we could have to keep it in the DT?
> >>>But Josh seems to have arguments to keep it.
> >>
> >>I'm ok to remove the channel number. What I worried is there also
> >>has a channel-used mask in DT.
> >>This mask should be removed too if channel number is removed.
> >>So maybe we can also use the sysfs to set the mask.
> >
> >Indeed, that would make adc-channel-used irrelevant. But I'm not sure
> >the mask is useful at all. Just enable all the channels and that's it?
> 
> On the top of my head: keeping all channels enabled, won't it have
> an impact on:
> 1/ power consumption?
> 2/ minimum period of sampling for a particular channel in case of
>    continuous ADC trigger?

I don't know, you tell me ;)

This is exactly why I was asking.

> And do not forget that sysfs is an interface that is:
> - needing a userspace tool to be configured properly (even just an
>   echo xx)
> - hard to design
> - a strict ABI that can't be changed once deployed
> 
> But yes, it is true that if the user has to configure ADC at
> runtime, it is certainly an interface worth considering...

Yes, of course. But the resolution is something I'd expect to be
user-configurable, probably at runtime, and the DT doesn't make it very
convenient to achieve.

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130827/155d6cfd/attachment-0001.sig>

[PATCH v2 3/4] iio: at91: ADC start-up time calculation changed since at91sam9x5

[Josh Wu]

Since in at91sam9x5, sama5d3x chip. the start up time calucation is changed.
This patch can choose different start up time calculation formula for different
chips.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
v1 --> v2:
  add a new variable for different start_time calculation.

 drivers/iio/adc/at91_adc.c |   42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index b41eb60..9b0ffff 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -40,6 +40,7 @@
 	(writel_relaxed(val, st->reg_base + reg))
 
 struct at91_adc_caps {
+	bool	start_time_use_lookup_tab;
 	u32	mr_prescal_mask;
 	u32	mr_startup_mask;
 };
@@ -700,13 +701,39 @@ static int at91_adc_probe(struct platform_device *pdev)
 		goto error_disable_adc_clk;
 	}
 
-	/*
-	 * Number of ticks needed to cover the startup time of the ADC as
-	 * defined in the electrical characteristics of the board, divided by 8.
-	 * The formula thus is : Startup Time = (ticks + 1) * 8 / ADC Clock
-	 */
-	ticks = round_up((st->startup_time * adc_clk_khz /
-			  1000) - 1, 8) / 8;
+	if (!st->caps->start_time_use_lookup_tab) {
+		/*
+		 * Number of ticks needed to cover the startup time of the ADC
+		 * as defined in the electrical characteristics of the board,
+		 * divided by 8. The formula thus is :
+		 *   Startup Time = (ticks + 1) * 8 / ADC Clock
+		 */
+		ticks = round_up((st->startup_time * adc_clk_khz /
+				  1000) - 1, 8) / 8;
+	} else {
+		/*
+		 * For sama5d3x and at91sam9x5, the formula changes to:
+		 * Startup Time = <lookup_table_value> / ADC Clock
+		 */
+		const int startup_lookup[] = {
+			0  , 8  , 16 , 24 ,
+			64 , 80 , 96 , 112,
+			512, 576, 640, 704,
+			768, 832, 896, 960
+			};
+
+		int i, size = ARRAY_SIZE(startup_lookup);
+		ticks = st->startup_time * adc_clk_khz / 1000;
+		for (i = 0; i < size; i++)
+			if (ticks < startup_lookup[i])
+				break;
+
+		ticks = i;
+		if (ticks == size)
+			/* Reach the end of lookup table */
+			ticks = size - 1;
+	}
+
 	/*
 	 * a minimal Sample and Hold Time is necessary for the ADC to guarantee
 	 * the best converted final value between two channels selection
@@ -798,6 +825,7 @@ static struct at91_adc_caps at91sam9g45_caps = {
 };
 
 static struct at91_adc_caps at91sam9x5_caps = {
+	.start_time_use_lookup_tab = true,
 	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,	/* same as 9G45 */
 	.mr_startup_mask = AT91_ADC_STARTUP_9X5,
 };
-- 
1.7.10

[PATCH v2 3/4] iio: at91: ADC start-up time calculation changed since at91sam9x5

[Maxime Ripard]

Hi Josh,

On Sun, Aug 11, 2013 at 07:04:30PM +0800, Josh Wu wrote:
> Since in at91sam9x5, sama5d3x chip. the start up time calucation is
> changed.  This patch can choose different start up time calculation
> formula for different chips.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
> v1 --> v2:
>   add a new variable for different start_time calculation.
> 
>  drivers/iio/adc/at91_adc.c |   42 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index b41eb60..9b0ffff 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -40,6 +40,7 @@
>  	(writel_relaxed(val, st->reg_base + reg))
>  
>  struct at91_adc_caps {
> +	bool	start_time_use_lookup_tab;
>  	u32	mr_prescal_mask;
>  	u32	mr_startup_mask;
>  };
> @@ -700,13 +701,39 @@ static int at91_adc_probe(struct platform_device *pdev)
>  		goto error_disable_adc_clk;
>  	}
>  
> -	/*
> -	 * Number of ticks needed to cover the startup time of the ADC as
> -	 * defined in the electrical characteristics of the board, divided by 8.
> -	 * The formula thus is : Startup Time = (ticks + 1) * 8 / ADC Clock
> -	 */
> -	ticks = round_up((st->startup_time * adc_clk_khz /
> -			  1000) - 1, 8) / 8;
> +	if (!st->caps->start_time_use_lookup_tab) {
> +		/*
> +		 * Number of ticks needed to cover the startup time of the ADC
> +		 * as defined in the electrical characteristics of the board,
> +		 * divided by 8. The formula thus is :
> +		 *   Startup Time = (ticks + 1) * 8 / ADC Clock
> +		 */
> +		ticks = round_up((st->startup_time * adc_clk_khz /
> +				  1000) - 1, 8) / 8;
> +	} else {
> +		/*
> +		 * For sama5d3x and at91sam9x5, the formula changes to:
> +		 * Startup Time = <lookup_table_value> / ADC Clock
> +		 */
> +		const int startup_lookup[] = {
> +			0  , 8  , 16 , 24 ,
> +			64 , 80 , 96 , 112,
> +			512, 576, 640, 704,
> +			768, 832, 896, 960
> +			};
> +
> +		int i, size = ARRAY_SIZE(startup_lookup);
> +		ticks = st->startup_time * adc_clk_khz / 1000;
> +		for (i = 0; i < size; i++)
> +			if (ticks < startup_lookup[i])
> +				break;
> +
> +		ticks = i;
> +		if (ticks == size)
> +			/* Reach the end of lookup table */
> +			ticks = size - 1;
> +	}
> +

Maybe just declaring two different functions here to calculate the
number of ticks, and just holding the function pointer in your structure
would make things more readable and future-proof?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130815/6deb8364/attachment-0001.sig>

[PATCH v2 3/4] iio: at91: ADC start-up time calculation changed since at91sam9x5

[Josh Wu]

Hi, Maxime

On 8/16/2013 3:27 AM, Maxime Ripard wrote:
> Hi Josh,
>
> On Sun, Aug 11, 2013 at 07:04:30PM +0800, Josh Wu wrote:
>> Since in at91sam9x5, sama5d3x chip. the start up time calucation is
>> changed.  This patch can choose different start up time calculation
>> formula for different chips.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>> v1 --> v2:
>>    add a new variable for different start_time calculation.
>>
>>   drivers/iio/adc/at91_adc.c |   42 +++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 35 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>> index b41eb60..9b0ffff 100644
>> --- a/drivers/iio/adc/at91_adc.c
>> +++ b/drivers/iio/adc/at91_adc.c
>> @@ -40,6 +40,7 @@
>>   	(writel_relaxed(val, st->reg_base + reg))
>>   
>>   struct at91_adc_caps {
>> +	bool	start_time_use_lookup_tab;
>>   	u32	mr_prescal_mask;
>>   	u32	mr_startup_mask;
>>   };
>> @@ -700,13 +701,39 @@ static int at91_adc_probe(struct platform_device *pdev)
>>   		goto error_disable_adc_clk;
>>   	}
>>   
>> -	/*
>> -	 * Number of ticks needed to cover the startup time of the ADC as
>> -	 * defined in the electrical characteristics of the board, divided by 8.
>> -	 * The formula thus is : Startup Time = (ticks + 1) * 8 / ADC Clock
>> -	 */
>> -	ticks = round_up((st->startup_time * adc_clk_khz /
>> -			  1000) - 1, 8) / 8;
>> +	if (!st->caps->start_time_use_lookup_tab) {
>> +		/*
>> +		 * Number of ticks needed to cover the startup time of the ADC
>> +		 * as defined in the electrical characteristics of the board,
>> +		 * divided by 8. The formula thus is :
>> +		 *   Startup Time = (ticks + 1) * 8 / ADC Clock
>> +		 */
>> +		ticks = round_up((st->startup_time * adc_clk_khz /
>> +				  1000) - 1, 8) / 8;
>> +	} else {
>> +		/*
>> +		 * For sama5d3x and at91sam9x5, the formula changes to:
>> +		 * Startup Time = <lookup_table_value> / ADC Clock
>> +		 */
>> +		const int startup_lookup[] = {
>> +			0  , 8  , 16 , 24 ,
>> +			64 , 80 , 96 , 112,
>> +			512, 576, 640, 704,
>> +			768, 832, 896, 960
>> +			};
>> +
>> +		int i, size = ARRAY_SIZE(startup_lookup);
>> +		ticks = st->startup_time * adc_clk_khz / 1000;
>> +		for (i = 0; i < size; i++)
>> +			if (ticks < startup_lookup[i])
>> +				break;
>> +
>> +		ticks = i;
>> +		if (ticks == size)
>> +			/* Reach the end of lookup table */
>> +			ticks = size - 1;
>> +	}
>> +
> Maybe just declaring two different functions here to calculate the
> number of ticks, and just holding the function pointer in your structure
> would make things more readable and future-proof?

I'll do this in next version. thanks.

>
> Maxime
>
Best Regards,
Josh Wu

[PATCH v2 4/4] iio: at91: introduce touch screen support in iio adc driver

[Josh Wu]

AT91 ADC hardware integrate touch screen support. So this patch add touch
screen support for at91 adc iio driver.
To enable touch screen support in adc, you need to add the dt parameters:
  1. which type of touch are used? (4 or 5 wires), sample period time.
  2. correct pressure detect threshold value.

In the meantime, since touch screen will use a interal period trigger of adc,
so it is conflict to other hardware triggers. Driver will disable the hardware
trigger support if touch screen is enabled.

This driver has been tested in AT91SAM9X5-EK and SAMA5D3x-EK.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
v1 --> v2:
  1. use the multiple compatible string for different IPs.
  2. hide the driver configuration in driver code instead of exposing them in dt.
     Such like sample period, pen detect debounce time and etc.
  3. add a new dt parameter: ts-pressure-threshold. this value is much related
     with different chips and boards.

 .../devicetree/bindings/arm/atmel-adc.txt          |    7 +
 arch/arm/mach-at91/include/mach/at91_adc.h         |   34 ++
 drivers/iio/adc/at91_adc.c                         |  385 ++++++++++++++++++--
 3 files changed, 402 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/atmel-adc.txt b/Documentation/devicetree/bindings/arm/atmel-adc.txt
index 6e60781..c01eaca 100644
--- a/Documentation/devicetree/bindings/arm/atmel-adc.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-adc.txt
@@ -28,6 +28,13 @@ Optional properties:
 		       resolution will be used.
   - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
   - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
+  - atmel,adc-ts-wires: Number of touch screen wires. Should be 4 or 5. If this
+                        value is set, then adc driver will enable touch screen
+                        support.
+    NOTE: when adc touch screen enabled, the adc hardware trigger will be
+          disabled. Since touch screen will occupied the trigger register.
+  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
+                                     make touch detect more precision.
  
 Optional trigger Nodes:
   - Required properties:
diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
index f00926a..86f42a1 100644
--- a/arch/arm/mach-at91/include/mach/at91_adc.h
+++ b/arch/arm/mach-at91/include/mach/at91_adc.h
@@ -57,8 +57,42 @@
 #define AT91_ADC_IER		0x24		/* Interrupt Enable Register */
 #define AT91_ADC_IDR		0x28		/* Interrupt Disable Register */
 #define AT91_ADC_IMR		0x2C		/* Interrupt Mask Register */
+#define		AT91_ADC_IER_PEN	(1 << 29)
+#define		AT91_ADC_IER_NOPEN	(1 << 30)
+#define		AT91_ADC_IER_XRDY	(1 << 20)
+#define		AT91_ADC_IER_YRDY	(1 << 21)
+#define		AT91_ADC_IER_PRDY	(1 << 22)
+#define		AT91_ADC_ISR_PENS	(1 << 31)
 
 #define AT91_ADC_CHR(n)		(0x30 + ((n) * 4))	/* Channel Data Register N */
 #define		AT91_ADC_DATA		(0x3ff)
 
+#define AT91_ADC_ACR		0x94	/* Analog Control Register */
+#define		AT91_ADC_ACR_PENDETSENS	(0x3 << 0)	/* pull-up resistor */
+
+#define AT91_ADC_TSMR		0xB0
+#define		AT91_ADC_TSMR_TSMODE	(3 << 0)	/* Touch Screen Mode */
+#define			AT91_ADC_TSMR_TSMODE_NONE		(0 << 0)
+#define			AT91_ADC_TSMR_TSMODE_4WIRE_NO_PRESS	(1 << 0)
+#define			AT91_ADC_TSMR_TSMODE_4WIRE_PRESS	(2 << 0)
+#define			AT91_ADC_TSMR_TSMODE_5WIRE		(3 << 0)
+#define		AT91_ADC_TSMR_TSAV	(3 << 4)	/* Averages samples */
+#define			AT91_ADC_TSMR_TSAV_(x)		((x) << 4)
+#define		AT91_ADC_TSMR_SCTIM	(0x0f << 16)	/* Switch closure time */
+#define		AT91_ADC_TSMR_PENDBC	(0x0f << 28)	/* Pen Debounce time */
+#define			AT91_ADC_TSMR_PENDBC_(x)	((x) << 28)
+#define		AT91_ADC_TSMR_NOTSDMA	(1 << 22)	/* No Touchscreen DMA */
+#define		AT91_ADC_TSMR_PENDET_DIS	(0 << 24)	/* Pen contact detection disable */
+#define		AT91_ADC_TSMR_PENDET_ENA	(1 << 24)	/* Pen contact detection enable */
+
+#define AT91_ADC_TSXPOSR	0xB4
+#define AT91_ADC_TSYPOSR	0xB8
+#define AT91_ADC_TSPRESSR	0xBC
+
+/* Trigger Register bit field */
+#define		AT91_ADC_TRGR_TRGPER	(0xffff << 16)
+#define			AT91_ADC_TRGR_TRGPER_(x)	((x) << 16)
+#define		AT91_ADC_TRGR_TRGMOD	(0x7 << 0)
+#define			AT91_ADC_TRGR_MOD_PERIOD_TRIG	(5 << 0)
+
 #endif
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 9b0ffff..b2495cb 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -11,6 +11,7 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
@@ -39,12 +40,34 @@
 #define at91_adc_writel(st, reg, val) \
 	(writel_relaxed(val, st->reg_base + reg))
 
+#define DRIVER_NAME		"at91_adc"
+#define MAX_POS_BITS		12
+
+#define TOUCH_SAMPLE_PERIOD_US		2000	/* 2ms */
+#define TOUCH_PEN_DETECT_DEBOUNCE_US	200
+
 struct at91_adc_caps {
+	bool	has_ts;		/* Support touch screen */
+	bool	has_tsmr;	/* only at91sam9x5, sama5d3 have TSMR reg */
+	/*
+	 * Numbers of sampling data will be averaged. Can be 0~3.
+	 * Hardware can average (2 ^ ts_filter_average) sample data.
+	 */
+	u8	ts_filter_average;
+	/* Pen Detection input pull-up resistor, can be 0~3 */
+	u8	ts_pen_detect_sensitivity;
+
 	bool	start_time_use_lookup_tab;
 	u32	mr_prescal_mask;
 	u32	mr_startup_mask;
 };
 
+enum atmel_adc_ts_type {
+	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
+	ATMEL_ADC_TOUCHSCREEN_4WIRE,
+	ATMEL_ADC_TOUCHSCREEN_5WIRE,
+};
+
 struct at91_adc_state {
 	struct clk		*adc_clk;
 	u16			*buffer;
@@ -69,6 +92,26 @@ struct at91_adc_state {
 	bool			low_res;	/* the resolution corresponds to the lowest one */
 	wait_queue_head_t	wq_data_avail;
 	struct at91_adc_caps	*caps;
+
+	/*
+	 * Following ADC channels are shared by touchscreen:
+	 *
+	 * CH0 -- Touch screen XP/UL
+	 * CH1 -- Touch screen XM/UR
+	 * CH2 -- Touch screen YP/LL
+	 * CH3 -- Touch screen YM/Sense
+	 * CH4 -- Touch screen LR(5-wire only)
+	 *
+	 * The bitfields below represents the reserved channel in the
+	 * touchscreen mode.
+	 */
+#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 0)
+#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 0)
+	enum atmel_adc_ts_type	touchscreen_type;
+	struct input_dev	*ts_input;
+
+	u16			ts_sample_period_val;
+	u32			ts_pressure_threshold;
 };
 
 static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
@@ -103,14 +146,10 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
+/* Handler for classic adc channel eoc trigger */
+void handle_adc_eoc_trigger(int irq, struct iio_dev *idev)
 {
-	struct iio_dev *idev = private;
 	struct at91_adc_state *st = iio_priv(idev);
-	u32 status = at91_adc_readl(st, st->registers->status_register);
-
-	if (!(status & st->registers->drdy_mask))
-		return IRQ_HANDLED;
 
 	if (iio_buffer_enabled(idev)) {
 		disable_irq_nosync(irq);
@@ -120,6 +159,114 @@ static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
 		st->done = true;
 		wake_up_interruptible(&st->wq_data_avail);
 	}
+}
+
+static int at91_ts_sample(struct at91_adc_state *st)
+{
+	unsigned int xscale, yscale, reg, z1, z2;
+	unsigned int x, y, pres, xpos, ypos;
+	unsigned int rxp = 1;
+	unsigned int factor = 1000;
+	struct iio_dev *idev = iio_priv_to_dev(st);
+
+	unsigned int xyz_mask_bits = st->res;
+	unsigned int xyz_mask = (1 << xyz_mask_bits) - 1;
+
+	/* calculate position */
+	/* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */
+	reg = at91_adc_readl(st, AT91_ADC_TSXPOSR);
+	xpos = reg & xyz_mask;
+	x = (xpos << MAX_POS_BITS) - xpos;
+	xscale = (reg >> 16) & xyz_mask;
+	if (xscale != 0)
+		x /= xscale;
+	else
+		dev_err(&idev->dev, "xscale == 0!!!\n");
+
+	/* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */
+	reg = at91_adc_readl(st, AT91_ADC_TSYPOSR);
+	ypos = reg & xyz_mask;
+	y = (ypos << MAX_POS_BITS) - ypos;
+	yscale = (reg >> 16) & xyz_mask;
+	if (yscale != 0)
+		y /= yscale;
+	else
+		dev_err(&idev->dev, "yscale == 0!!!\n");
+
+	/* calculate the pressure */
+	reg = at91_adc_readl(st, AT91_ADC_TSPRESSR);
+	z1 = reg & xyz_mask;
+	z2 = (reg >> 16) & xyz_mask;
+
+	if (z1 != 0)
+		pres = rxp * (x * factor / 1024) * (z2 * factor / z1 - factor)
+			/ factor;
+	else
+		pres = st->ts_pressure_threshold;	/* no pen contacted */
+
+	dev_dbg(&idev->dev, "xpos = %d, xscale = %d, ypos = %d, yscale = %d, z1 = %d, z2 = %d, press = %d\n",
+				xpos, xscale, ypos, yscale, z1, z2, pres);
+
+	if (pres < st->ts_pressure_threshold) {
+		dev_dbg(&idev->dev, "x = %d, y = %d, pressure = %d\n",
+					x, y, pres / factor);
+		input_report_abs(st->ts_input, ABS_X, x);
+		input_report_abs(st->ts_input, ABS_Y, y);
+		input_report_abs(st->ts_input, ABS_PRESSURE, pres);
+		input_report_key(st->ts_input, BTN_TOUCH, 1);
+		input_sync(st->ts_input);
+	} else {
+		dev_dbg(&idev->dev, "pressure too low: not reporting\n");
+	}
+
+	return 0;
+}
+
+
+static irqreturn_t at91_adc_interrupt(int irq, void *private)
+{
+	struct iio_dev *idev = private;
+	struct at91_adc_state *st = iio_priv(idev);
+	u32 status = at91_adc_readl(st, st->registers->status_register);
+	const uint32_t ts_data_irq_mask =
+		AT91_ADC_IER_XRDY |
+		AT91_ADC_IER_YRDY |
+		AT91_ADC_IER_PRDY;
+
+	if (status & st->registers->drdy_mask)
+		handle_adc_eoc_trigger(irq, idev);
+
+	if (status & AT91_ADC_IER_PEN) {
+		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
+		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_NOPEN |
+			ts_data_irq_mask);
+		/* Set up period trigger for sampling */
+		at91_adc_writel(st, st->registers->trigger_register,
+			AT91_ADC_TRGR_MOD_PERIOD_TRIG |
+			AT91_ADC_TRGR_TRGPER_(st->ts_sample_period_val));
+	} else if (status & AT91_ADC_IER_NOPEN) {
+		at91_adc_writel(st, st->registers->trigger_register, 0);
+		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_NOPEN |
+			ts_data_irq_mask);
+		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
+
+		input_report_key(st->ts_input, BTN_TOUCH, 0);
+		input_sync(st->ts_input);
+	} else if ((status & ts_data_irq_mask) == ts_data_irq_mask) {
+		/* Now all touchscreen data is ready */
+
+		if (status & AT91_ADC_ISR_PENS) {
+			/* validate data by pen contact */
+			at91_ts_sample(st);
+		} else {
+			/* triggered by event that is no pen contact, just read
+			 * them to clean the interrupt and discard all.
+			 */
+			at91_adc_readl(st, AT91_ADC_TSXPOSR);
+			at91_adc_readl(st, AT91_ADC_TSYPOSR);
+			at91_adc_readl(st, AT91_ADC_TSPRESSR);
+		}
+	}
 
 	return IRQ_HANDLED;
 }
@@ -129,6 +276,16 @@ static int at91_adc_channel_init(struct iio_dev *idev)
 	struct at91_adc_state *st = iio_priv(idev);
 	struct iio_chan_spec *chan_array, *timestamp;
 	int bit, idx = 0;
+	unsigned long rsvd_mask = 0;
+
+	/* If touchscreen is enable, then reserve the adc channels */
+	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
+		rsvd_mask = CHAN_MASK_TOUCHSCREEN_4WIRE;
+	else if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_5WIRE)
+		rsvd_mask = CHAN_MASK_TOUCHSCREEN_5WIRE;
+
+	/* set up the channel mask to reserve touchscreen channels */
+	st->channels_mask &= ~rsvd_mask;
 
 	idev->num_channels = bitmap_weight(&st->channels_mask,
 					   st->num_channels) + 1;
@@ -438,6 +595,37 @@ ret:
 
 static const struct of_device_id at91_adc_dt_ids[];
 
+static int at91_adc_probe_dt_ts(struct device_node *node,
+	struct at91_adc_state *st, struct device *dev)
+{
+	int ret;
+	u32 prop;
+
+	ret = of_property_read_u32(node, "atmel,adc-ts-wires", &prop);
+	if (ret)
+		dev_info(dev, "Touchscreen not enabled.\n");
+	else if (prop == 4)
+		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_4WIRE;
+	else if (prop == 5)
+		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_5WIRE;
+	else
+		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
+				prop);
+
+	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_NONE)
+		return 0;
+
+	prop = 0;
+	of_property_read_u32(node, "atmel,adc-ts-pressure-threshold", &prop);
+	st->ts_pressure_threshold = prop;
+	if (st->ts_pressure_threshold) {
+		return 0;
+	} else {
+		dev_err(dev, "Invalid pressure threshold for the touchscreen\n");
+		return -EINVAL;
+	}
+}
+
 static int at91_adc_probe_dt(struct at91_adc_state *st,
 			     struct platform_device *pdev)
 {
@@ -561,6 +749,12 @@ static int at91_adc_probe_dt(struct at91_adc_state *st,
 		i++;
 	}
 
+	/* Check if touchscreen is supported. */
+	if (st->caps->has_ts)
+		return at91_adc_probe_dt_ts(node, st, &idev->dev);
+	else
+		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
+
 	return 0;
 
 error_ret:
@@ -592,6 +786,114 @@ static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 };
 
+/* Touchscreen related functions */
+static int atmel_ts_open(struct input_dev *dev)
+{
+	struct at91_adc_state *st = input_get_drvdata(dev);
+
+	at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
+	return 0;
+}
+
+static void atmel_ts_close(struct input_dev *dev)
+{
+	struct at91_adc_state *st = input_get_drvdata(dev);
+
+	at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
+}
+
+static int at91_ts_hw_init(struct at91_adc_state *st, u32 adc_clk_khz)
+{
+	u32 reg = 0, pendbc;
+	int i = 0;
+
+	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
+		reg = AT91_ADC_TSMR_TSMODE_4WIRE_PRESS;
+	else
+		reg = AT91_ADC_TSMR_TSMODE_5WIRE;
+
+	/* a Pen Detect Debounce Time is necessary for the ADC Touch to avoid
+	 * pen detect noise.
+	 * The formula is : Pen Detect Debounce Time = (2 ^ pendbc) / ADCClock
+	 */
+	pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * adc_clk_khz / 1000, 1);
+
+	while (pendbc >> ++i)
+		;	/* Empty! Find the shift offset */
+	if (abs(pendbc - (1 << i)) < abs(pendbc - (1 << (i - 1))))
+		pendbc = i;
+	else
+		pendbc = i - 1;
+
+	if (st->caps->has_tsmr) {
+		reg |= AT91_ADC_TSMR_TSAV_(st->caps->ts_filter_average)
+				& AT91_ADC_TSMR_TSAV;
+		reg |= AT91_ADC_TSMR_PENDBC_(pendbc) & AT91_ADC_TSMR_PENDBC;
+		reg |= AT91_ADC_TSMR_NOTSDMA;
+		reg |= AT91_ADC_TSMR_PENDET_ENA;
+		reg |= 0x03 << 8;	/* TSFREQ, need bigger than TSAV */
+
+		at91_adc_writel(st, AT91_ADC_TSMR, reg);
+	} else {
+		/* TODO: for 9g45 which has no TSMR */
+	}
+
+	/* Change adc internal resistor value for better pen detection,
+	 * default value is 100 kOhm.
+	 * 0 = 200 kOhm, 1 = 150 kOhm, 2 = 100 kOhm, 3 = 50 kOhm
+	 * option only available on ES2 and higher
+	 */
+	at91_adc_writel(st, AT91_ADC_ACR, st->caps->ts_pen_detect_sensitivity
+			& AT91_ADC_ACR_PENDETSENS);
+
+	/* Sample Peroid Time = (TRGPER + 1) / ADCClock */
+	st->ts_sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
+			adc_clk_khz / 1000) - 1, 1);
+
+	return 0;
+}
+
+static int at91_ts_register(struct at91_adc_state *st)
+{
+	struct input_dev *input;
+	struct iio_dev *idev = iio_priv_to_dev(st);
+	int ret;
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_err(&idev->dev, "Failed to allocate TS device!\n");
+		return -ENOMEM;
+	}
+
+	input->name = DRIVER_NAME;
+	input->id.bustype = BUS_HOST;
+	input->dev.parent = idev->dev.parent;
+	input->open = atmel_ts_open;
+	input->close = atmel_ts_close;
+
+	__set_bit(EV_ABS, input->evbit);
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(BTN_TOUCH, input->keybit);
+	input_set_abs_params(input, ABS_X, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
+
+	st->ts_input = input;
+	input_set_drvdata(input, st);
+
+	ret = input_register_device(input);
+	if (ret)
+		input_free_device(st->ts_input);
+
+	return ret;
+}
+
+static void at91_ts_unregister(struct at91_adc_state *st)
+{
+	input_unregister_device(st->ts_input);
+	input_free_device(st->ts_input);
+}
+
 static int at91_adc_probe(struct platform_device *pdev)
 {
 	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
@@ -648,7 +950,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 	at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST);
 	at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF);
 	ret = request_irq(st->irq,
-			  at91_adc_eoc_trigger,
+			  at91_adc_interrupt,
 			  0,
 			  pdev->dev.driver->name,
 			  idev);
@@ -693,6 +995,10 @@ static int at91_adc_probe(struct platform_device *pdev)
 	mstrclk = clk_get_rate(st->clk);
 	adc_clk = clk_get_rate(st->adc_clk);
 	adc_clk_khz = adc_clk / 1000;
+
+	dev_dbg(&pdev->dev, "Master clock is set as: %d Hz, adc_clk should set as: %d Hz\n",
+		mstrclk, adc_clk);
+
 	prsc = (mstrclk / (2 * adc_clk)) - 1;
 
 	if (!st->startup_time) {
@@ -761,30 +1067,52 @@ static int at91_adc_probe(struct platform_device *pdev)
 	init_waitqueue_head(&st->wq_data_avail);
 	mutex_init(&st->lock);
 
-	ret = at91_adc_buffer_init(idev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
-		goto error_disable_adc_clk;
-	}
+	/*
+	 * Since touch screen will set trigger register as period trigger. So
+	 * when touch screen is enabled, then we have to disable hardware
+	 * trigger for classic adc.
+	 */
+	if (!st->touchscreen_type) {
+		ret = at91_adc_buffer_init(idev);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
+			goto error_disable_adc_clk;
+		}
 
-	ret = at91_adc_trigger_init(idev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
-		goto error_unregister_buffer;
+		ret = at91_adc_trigger_init(idev);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
+			at91_adc_buffer_remove(idev);
+			goto error_disable_adc_clk;
+		}
+	} else {
+		if (!st->caps->has_tsmr) {
+			dev_err(&pdev->dev, "We don't support non-TSMR adc\n");
+			goto error_disable_adc_clk;
+		}
+
+		ret = at91_ts_register(st);
+		if (ret)
+			goto error_disable_adc_clk;
+
+		at91_ts_hw_init(st, adc_clk_khz);
 	}
 
 	ret = iio_device_register(idev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Couldn't register the device.\n");
-		goto error_remove_triggers;
+		goto error_iio_device_register;
 	}
 
 	return 0;
 
-error_remove_triggers:
-	at91_adc_trigger_remove(idev);
-error_unregister_buffer:
-	at91_adc_buffer_remove(idev);
+error_iio_device_register:
+	if (!st->touchscreen_type) {
+		at91_adc_trigger_remove(idev);
+		at91_adc_buffer_remove(idev);
+	} else {
+		at91_ts_unregister(st);
+	}
 error_disable_adc_clk:
 	clk_disable_unprepare(st->adc_clk);
 error_disable_clk:
@@ -803,8 +1131,12 @@ static int at91_adc_remove(struct platform_device *pdev)
 	struct at91_adc_state *st = iio_priv(idev);
 
 	iio_device_unregister(idev);
-	at91_adc_trigger_remove(idev);
-	at91_adc_buffer_remove(idev);
+	if (!st->touchscreen_type) {
+		at91_adc_trigger_remove(idev);
+		at91_adc_buffer_remove(idev);
+	} else {
+		at91_ts_unregister(st);
+	}
 	clk_disable_unprepare(st->adc_clk);
 	clk_disable_unprepare(st->clk);
 	free_irq(st->irq, idev);
@@ -820,11 +1152,16 @@ static struct at91_adc_caps at91sam9260_caps = {
 };
 
 static struct at91_adc_caps at91sam9g45_caps = {
+	.has_ts = true,
 	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 	.mr_startup_mask = AT91_ADC_STARTUP_9G45,
 };
 
 static struct at91_adc_caps at91sam9x5_caps = {
+	.has_ts = true,
+	.has_tsmr = true,
+	.ts_filter_average = 3,
+	.ts_pen_detect_sensitivity = 2,
 	.start_time_use_lookup_tab = true,
 	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,	/* same as 9G45 */
 	.mr_startup_mask = AT91_ADC_STARTUP_9X5,
@@ -843,7 +1180,7 @@ static struct platform_driver at91_adc_driver = {
 	.probe = at91_adc_probe,
 	.remove = at91_adc_remove,
 	.driver = {
-		   .name = "at91_adc",
+		   .name = DRIVER_NAME,
 		   .of_match_table = of_match_ptr(at91_adc_dt_ids),
 	},
 };
-- 
1.7.10

[PATCH v2 4/4] iio: at91: introduce touch screen support in iio adc driver

[Dmitry Torokhov]

Hi Josh,

On Sun, Aug 11, 2013 at 07:04:31PM +0800, Josh Wu wrote:
> AT91 ADC hardware integrate touch screen support. So this patch add touch
> screen support for at91 adc iio driver.
> To enable touch screen support in adc, you need to add the dt parameters:
>   1. which type of touch are used? (4 or 5 wires), sample period time.
>   2. correct pressure detect threshold value.
> 
> In the meantime, since touch screen will use a interal period trigger of adc,
> so it is conflict to other hardware triggers. Driver will disable the hardware
> trigger support if touch screen is enabled.
> 
> This driver has been tested in AT91SAM9X5-EK and SAMA5D3x-EK.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> v1 --> v2:
>   1. use the multiple compatible string for different IPs.
>   2. hide the driver configuration in driver code instead of exposing them in dt.
>      Such like sample period, pen detect debounce time and etc.
>   3. add a new dt parameter: ts-pressure-threshold. this value is much related
>      with different chips and boards.
> 
>  .../devicetree/bindings/arm/atmel-adc.txt          |    7 +
>  arch/arm/mach-at91/include/mach/at91_adc.h         |   34 ++
>  drivers/iio/adc/at91_adc.c                         |  385 ++++++++++++++++++--
>  3 files changed, 402 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/atmel-adc.txt b/Documentation/devicetree/bindings/arm/atmel-adc.txt
> index 6e60781..c01eaca 100644
> --- a/Documentation/devicetree/bindings/arm/atmel-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/atmel-adc.txt
> @@ -28,6 +28,13 @@ Optional properties:
>  		       resolution will be used.
>    - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
>    - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
> +  - atmel,adc-ts-wires: Number of touch screen wires. Should be 4 or 5. If this
> +                        value is set, then adc driver will enable touch screen
> +                        support.
> +    NOTE: when adc touch screen enabled, the adc hardware trigger will be
> +          disabled. Since touch screen will occupied the trigger register.
> +  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
> +                                     make touch detect more precision.
>   
>  Optional trigger Nodes:
>    - Required properties:
> diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
> index f00926a..86f42a1 100644
> --- a/arch/arm/mach-at91/include/mach/at91_adc.h
> +++ b/arch/arm/mach-at91/include/mach/at91_adc.h
> @@ -57,8 +57,42 @@
>  #define AT91_ADC_IER		0x24		/* Interrupt Enable Register */
>  #define AT91_ADC_IDR		0x28		/* Interrupt Disable Register */
>  #define AT91_ADC_IMR		0x2C		/* Interrupt Mask Register */
> +#define		AT91_ADC_IER_PEN	(1 << 29)
> +#define		AT91_ADC_IER_NOPEN	(1 << 30)
> +#define		AT91_ADC_IER_XRDY	(1 << 20)
> +#define		AT91_ADC_IER_YRDY	(1 << 21)
> +#define		AT91_ADC_IER_PRDY	(1 << 22)
> +#define		AT91_ADC_ISR_PENS	(1 << 31)
>  
>  #define AT91_ADC_CHR(n)		(0x30 + ((n) * 4))	/* Channel Data Register N */
>  #define		AT91_ADC_DATA		(0x3ff)
>  
> +#define AT91_ADC_ACR		0x94	/* Analog Control Register */
> +#define		AT91_ADC_ACR_PENDETSENS	(0x3 << 0)	/* pull-up resistor */
> +
> +#define AT91_ADC_TSMR		0xB0
> +#define		AT91_ADC_TSMR_TSMODE	(3 << 0)	/* Touch Screen Mode */
> +#define			AT91_ADC_TSMR_TSMODE_NONE		(0 << 0)
> +#define			AT91_ADC_TSMR_TSMODE_4WIRE_NO_PRESS	(1 << 0)
> +#define			AT91_ADC_TSMR_TSMODE_4WIRE_PRESS	(2 << 0)
> +#define			AT91_ADC_TSMR_TSMODE_5WIRE		(3 << 0)
> +#define		AT91_ADC_TSMR_TSAV	(3 << 4)	/* Averages samples */
> +#define			AT91_ADC_TSMR_TSAV_(x)		((x) << 4)
> +#define		AT91_ADC_TSMR_SCTIM	(0x0f << 16)	/* Switch closure time */
> +#define		AT91_ADC_TSMR_PENDBC	(0x0f << 28)	/* Pen Debounce time */
> +#define			AT91_ADC_TSMR_PENDBC_(x)	((x) << 28)
> +#define		AT91_ADC_TSMR_NOTSDMA	(1 << 22)	/* No Touchscreen DMA */
> +#define		AT91_ADC_TSMR_PENDET_DIS	(0 << 24)	/* Pen contact detection disable */
> +#define		AT91_ADC_TSMR_PENDET_ENA	(1 << 24)	/* Pen contact detection enable */
> +
> +#define AT91_ADC_TSXPOSR	0xB4
> +#define AT91_ADC_TSYPOSR	0xB8
> +#define AT91_ADC_TSPRESSR	0xBC
> +
> +/* Trigger Register bit field */
> +#define		AT91_ADC_TRGR_TRGPER	(0xffff << 16)
> +#define			AT91_ADC_TRGR_TRGPER_(x)	((x) << 16)
> +#define		AT91_ADC_TRGR_TRGMOD	(0x7 << 0)
> +#define			AT91_ADC_TRGR_MOD_PERIOD_TRIG	(5 << 0)
> +
>  #endif
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 9b0ffff..b2495cb 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -11,6 +11,7 @@
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/io.h>
> +#include <linux/input.h>
>  #include <linux/interrupt.h>
>  #include <linux/jiffies.h>
>  #include <linux/kernel.h>
> @@ -39,12 +40,34 @@
>  #define at91_adc_writel(st, reg, val) \
>  	(writel_relaxed(val, st->reg_base + reg))
>  
> +#define DRIVER_NAME		"at91_adc"
> +#define MAX_POS_BITS		12
> +
> +#define TOUCH_SAMPLE_PERIOD_US		2000	/* 2ms */
> +#define TOUCH_PEN_DETECT_DEBOUNCE_US	200
> +
>  struct at91_adc_caps {
> +	bool	has_ts;		/* Support touch screen */
> +	bool	has_tsmr;	/* only at91sam9x5, sama5d3 have TSMR reg */
> +	/*
> +	 * Numbers of sampling data will be averaged. Can be 0~3.
> +	 * Hardware can average (2 ^ ts_filter_average) sample data.
> +	 */
> +	u8	ts_filter_average;
> +	/* Pen Detection input pull-up resistor, can be 0~3 */
> +	u8	ts_pen_detect_sensitivity;
> +
>  	bool	start_time_use_lookup_tab;
>  	u32	mr_prescal_mask;
>  	u32	mr_startup_mask;
>  };
>  
> +enum atmel_adc_ts_type {
> +	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
> +	ATMEL_ADC_TOUCHSCREEN_4WIRE,
> +	ATMEL_ADC_TOUCHSCREEN_5WIRE,
> +};
> +
>  struct at91_adc_state {
>  	struct clk		*adc_clk;
>  	u16			*buffer;
> @@ -69,6 +92,26 @@ struct at91_adc_state {
>  	bool			low_res;	/* the resolution corresponds to the lowest one */
>  	wait_queue_head_t	wq_data_avail;
>  	struct at91_adc_caps	*caps;
> +
> +	/*
> +	 * Following ADC channels are shared by touchscreen:
> +	 *
> +	 * CH0 -- Touch screen XP/UL
> +	 * CH1 -- Touch screen XM/UR
> +	 * CH2 -- Touch screen YP/LL
> +	 * CH3 -- Touch screen YM/Sense
> +	 * CH4 -- Touch screen LR(5-wire only)
> +	 *
> +	 * The bitfields below represents the reserved channel in the
> +	 * touchscreen mode.
> +	 */
> +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 0)
> +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 0)
> +	enum atmel_adc_ts_type	touchscreen_type;
> +	struct input_dev	*ts_input;
> +
> +	u16			ts_sample_period_val;
> +	u32			ts_pressure_threshold;
>  };
>  
>  static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
> @@ -103,14 +146,10 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> -static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
> +/* Handler for classic adc channel eoc trigger */
> +void handle_adc_eoc_trigger(int irq, struct iio_dev *idev)
>  {
> -	struct iio_dev *idev = private;
>  	struct at91_adc_state *st = iio_priv(idev);
> -	u32 status = at91_adc_readl(st, st->registers->status_register);
> -
> -	if (!(status & st->registers->drdy_mask))
> -		return IRQ_HANDLED;
>  
>  	if (iio_buffer_enabled(idev)) {
>  		disable_irq_nosync(irq);
> @@ -120,6 +159,114 @@ static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
>  		st->done = true;
>  		wake_up_interruptible(&st->wq_data_avail);
>  	}
> +}
> +
> +static int at91_ts_sample(struct at91_adc_state *st)
> +{
> +	unsigned int xscale, yscale, reg, z1, z2;
> +	unsigned int x, y, pres, xpos, ypos;
> +	unsigned int rxp = 1;
> +	unsigned int factor = 1000;
> +	struct iio_dev *idev = iio_priv_to_dev(st);
> +
> +	unsigned int xyz_mask_bits = st->res;
> +	unsigned int xyz_mask = (1 << xyz_mask_bits) - 1;
> +
> +	/* calculate position */
> +	/* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */
> +	reg = at91_adc_readl(st, AT91_ADC_TSXPOSR);
> +	xpos = reg & xyz_mask;
> +	x = (xpos << MAX_POS_BITS) - xpos;
> +	xscale = (reg >> 16) & xyz_mask;
> +	if (xscale != 0)
> +		x /= xscale;
> +	else
> +		dev_err(&idev->dev, "xscale == 0!!!\n");

Maybe you want to ratelimit this? Also, do you really want to continue
reporting or maybe you want to jump out early?

> +
> +	/* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */
> +	reg = at91_adc_readl(st, AT91_ADC_TSYPOSR);
> +	ypos = reg & xyz_mask;
> +	y = (ypos << MAX_POS_BITS) - ypos;
> +	yscale = (reg >> 16) & xyz_mask;
> +	if (yscale != 0)
> +		y /= yscale;
> +	else
> +		dev_err(&idev->dev, "yscale == 0!!!\n");
> +
> +	/* calculate the pressure */
> +	reg = at91_adc_readl(st, AT91_ADC_TSPRESSR);
> +	z1 = reg & xyz_mask;
> +	z2 = (reg >> 16) & xyz_mask;
> +
> +	if (z1 != 0)
> +		pres = rxp * (x * factor / 1024) * (z2 * factor / z1 - factor)
> +			/ factor;
> +	else
> +		pres = st->ts_pressure_threshold;	/* no pen contacted */
> +
> +	dev_dbg(&idev->dev, "xpos = %d, xscale = %d, ypos = %d, yscale = %d, z1 = %d, z2 = %d, press = %d\n",
> +				xpos, xscale, ypos, yscale, z1, z2, pres);
> +
> +	if (pres < st->ts_pressure_threshold) {
> +		dev_dbg(&idev->dev, "x = %d, y = %d, pressure = %d\n",
> +					x, y, pres / factor);
> +		input_report_abs(st->ts_input, ABS_X, x);
> +		input_report_abs(st->ts_input, ABS_Y, y);
> +		input_report_abs(st->ts_input, ABS_PRESSURE, pres);
> +		input_report_key(st->ts_input, BTN_TOUCH, 1);
> +		input_sync(st->ts_input);
> +	} else {
> +		dev_dbg(&idev->dev, "pressure too low: not reporting\n");
> +	}
> +
> +	return 0;
> +}
> +
> +
> +static irqreturn_t at91_adc_interrupt(int irq, void *private)
> +{
> +	struct iio_dev *idev = private;
> +	struct at91_adc_state *st = iio_priv(idev);
> +	u32 status = at91_adc_readl(st, st->registers->status_register);
> +	const uint32_t ts_data_irq_mask =
> +		AT91_ADC_IER_XRDY |
> +		AT91_ADC_IER_YRDY |
> +		AT91_ADC_IER_PRDY;
> +
> +	if (status & st->registers->drdy_mask)
> +		handle_adc_eoc_trigger(irq, idev);
> +
> +	if (status & AT91_ADC_IER_PEN) {
> +		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
> +		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_NOPEN |
> +			ts_data_irq_mask);
> +		/* Set up period trigger for sampling */
> +		at91_adc_writel(st, st->registers->trigger_register,
> +			AT91_ADC_TRGR_MOD_PERIOD_TRIG |
> +			AT91_ADC_TRGR_TRGPER_(st->ts_sample_period_val));
> +	} else if (status & AT91_ADC_IER_NOPEN) {
> +		at91_adc_writel(st, st->registers->trigger_register, 0);
> +		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_NOPEN |
> +			ts_data_irq_mask);
> +		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
> +
> +		input_report_key(st->ts_input, BTN_TOUCH, 0);
> +		input_sync(st->ts_input);
> +	} else if ((status & ts_data_irq_mask) == ts_data_irq_mask) {
> +		/* Now all touchscreen data is ready */
> +
> +		if (status & AT91_ADC_ISR_PENS) {
> +			/* validate data by pen contact */
> +			at91_ts_sample(st);
> +		} else {
> +			/* triggered by event that is no pen contact, just read
> +			 * them to clean the interrupt and discard all.
> +			 */
> +			at91_adc_readl(st, AT91_ADC_TSXPOSR);
> +			at91_adc_readl(st, AT91_ADC_TSYPOSR);
> +			at91_adc_readl(st, AT91_ADC_TSPRESSR);
> +		}
> +	}
>  
>  	return IRQ_HANDLED;
>  }
> @@ -129,6 +276,16 @@ static int at91_adc_channel_init(struct iio_dev *idev)
>  	struct at91_adc_state *st = iio_priv(idev);
>  	struct iio_chan_spec *chan_array, *timestamp;
>  	int bit, idx = 0;
> +	unsigned long rsvd_mask = 0;
> +
> +	/* If touchscreen is enable, then reserve the adc channels */
> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
> +		rsvd_mask = CHAN_MASK_TOUCHSCREEN_4WIRE;
> +	else if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_5WIRE)
> +		rsvd_mask = CHAN_MASK_TOUCHSCREEN_5WIRE;
> +
> +	/* set up the channel mask to reserve touchscreen channels */
> +	st->channels_mask &= ~rsvd_mask;
>  
>  	idev->num_channels = bitmap_weight(&st->channels_mask,
>  					   st->num_channels) + 1;
> @@ -438,6 +595,37 @@ ret:
>  
>  static const struct of_device_id at91_adc_dt_ids[];
>  
> +static int at91_adc_probe_dt_ts(struct device_node *node,
> +	struct at91_adc_state *st, struct device *dev)
> +{
> +	int ret;
> +	u32 prop;
> +
> +	ret = of_property_read_u32(node, "atmel,adc-ts-wires", &prop);
> +	if (ret)
> +		dev_info(dev, "Touchscreen not enabled.\n");
> +	else if (prop == 4)
> +		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_4WIRE;
> +	else if (prop == 5)
> +		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_5WIRE;
> +	else
> +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
> +				prop);
> +
> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_NONE)
> +		return 0;

I'd rather we separated invalid wire configuration from the absent (i.e.
disabled) case.

> +
> +	prop = 0;
> +	of_property_read_u32(node, "atmel,adc-ts-pressure-threshold", &prop);
> +	st->ts_pressure_threshold = prop;
> +	if (st->ts_pressure_threshold) {
> +		return 0;
> +	} else {
> +		dev_err(dev, "Invalid pressure threshold for the touchscreen\n");
> +		return -EINVAL;
> +	}
> +}
> +
>  static int at91_adc_probe_dt(struct at91_adc_state *st,
>  			     struct platform_device *pdev)
>  {
> @@ -561,6 +749,12 @@ static int at91_adc_probe_dt(struct at91_adc_state *st,
>  		i++;
>  	}
>  
> +	/* Check if touchscreen is supported. */
> +	if (st->caps->has_ts)
> +		return at91_adc_probe_dt_ts(node, st, &idev->dev);
> +	else
> +		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
> +
>  	return 0;
>  
>  error_ret:
> @@ -592,6 +786,114 @@ static const struct iio_info at91_adc_info = {
>  	.read_raw = &at91_adc_read_raw,
>  };
>  
> +/* Touchscreen related functions */
> +static int atmel_ts_open(struct input_dev *dev)
> +{
> +	struct at91_adc_state *st = input_get_drvdata(dev);
> +
> +	at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
> +	return 0;
> +}
> +
> +static void atmel_ts_close(struct input_dev *dev)
> +{
> +	struct at91_adc_state *st = input_get_drvdata(dev);
> +
> +	at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
> +}
> +
> +static int at91_ts_hw_init(struct at91_adc_state *st, u32 adc_clk_khz)
> +{
> +	u32 reg = 0, pendbc;
> +	int i = 0;
> +
> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
> +		reg = AT91_ADC_TSMR_TSMODE_4WIRE_PRESS;
> +	else
> +		reg = AT91_ADC_TSMR_TSMODE_5WIRE;
> +
> +	/* a Pen Detect Debounce Time is necessary for the ADC Touch to avoid
> +	 * pen detect noise.
> +	 * The formula is : Pen Detect Debounce Time = (2 ^ pendbc) / ADCClock
> +	 */
> +	pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * adc_clk_khz / 1000, 1);
> +
> +	while (pendbc >> ++i)
> +		;	/* Empty! Find the shift offset */
> +	if (abs(pendbc - (1 << i)) < abs(pendbc - (1 << (i - 1))))
> +		pendbc = i;
> +	else
> +		pendbc = i - 1;
> +
> +	if (st->caps->has_tsmr) {
> +		reg |= AT91_ADC_TSMR_TSAV_(st->caps->ts_filter_average)
> +				& AT91_ADC_TSMR_TSAV;
> +		reg |= AT91_ADC_TSMR_PENDBC_(pendbc) & AT91_ADC_TSMR_PENDBC;
> +		reg |= AT91_ADC_TSMR_NOTSDMA;
> +		reg |= AT91_ADC_TSMR_PENDET_ENA;
> +		reg |= 0x03 << 8;	/* TSFREQ, need bigger than TSAV */
> +
> +		at91_adc_writel(st, AT91_ADC_TSMR, reg);
> +	} else {
> +		/* TODO: for 9g45 which has no TSMR */
> +	}
> +
> +	/* Change adc internal resistor value for better pen detection,
> +	 * default value is 100 kOhm.
> +	 * 0 = 200 kOhm, 1 = 150 kOhm, 2 = 100 kOhm, 3 = 50 kOhm
> +	 * option only available on ES2 and higher
> +	 */
> +	at91_adc_writel(st, AT91_ADC_ACR, st->caps->ts_pen_detect_sensitivity
> +			& AT91_ADC_ACR_PENDETSENS);
> +
> +	/* Sample Peroid Time = (TRGPER + 1) / ADCClock */
> +	st->ts_sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
> +			adc_clk_khz / 1000) - 1, 1);
> +
> +	return 0;
> +}
> +
> +static int at91_ts_register(struct at91_adc_state *st)
> +{
> +	struct input_dev *input;
> +	struct iio_dev *idev = iio_priv_to_dev(st);
> +	int ret;
> +
> +	input = input_allocate_device();
> +	if (!input) {
> +		dev_err(&idev->dev, "Failed to allocate TS device!\n");
> +		return -ENOMEM;
> +	}
> +
> +	input->name = DRIVER_NAME;
> +	input->id.bustype = BUS_HOST;
> +	input->dev.parent = idev->dev.parent;

Why you are setting parent to the IIO's parent? This is confusing, maybe
you should simply pass the parent platform device into this fucntion.

> +	input->open = atmel_ts_open;
> +	input->close = atmel_ts_close;
> +
> +	__set_bit(EV_ABS, input->evbit);
> +	__set_bit(EV_KEY, input->evbit);
> +	__set_bit(BTN_TOUCH, input->keybit);
> +	input_set_abs_params(input, ABS_X, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
> +	input_set_abs_params(input, ABS_Y, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
> +	input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
> +
> +	st->ts_input = input;
> +	input_set_drvdata(input, st);
> +
> +	ret = input_register_device(input);
> +	if (ret)
> +		input_free_device(st->ts_input);
> +
> +	return ret;
> +}
> +
> +static void at91_ts_unregister(struct at91_adc_state *st)
> +{
> +	input_unregister_device(st->ts_input);
> +	input_free_device(st->ts_input);

No free after unregister please.

> +}
> +
>  static int at91_adc_probe(struct platform_device *pdev)
>  {
>  	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
> @@ -648,7 +950,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST);
>  	at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF);
>  	ret = request_irq(st->irq,
> -			  at91_adc_eoc_trigger,
> +			  at91_adc_interrupt,
>  			  0,
>  			  pdev->dev.driver->name,
>  			  idev);
> @@ -693,6 +995,10 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	mstrclk = clk_get_rate(st->clk);
>  	adc_clk = clk_get_rate(st->adc_clk);
>  	adc_clk_khz = adc_clk / 1000;
> +
> +	dev_dbg(&pdev->dev, "Master clock is set as: %d Hz, adc_clk should set as: %d Hz\n",
> +		mstrclk, adc_clk);
> +
>  	prsc = (mstrclk / (2 * adc_clk)) - 1;
>  
>  	if (!st->startup_time) {
> @@ -761,30 +1067,52 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	init_waitqueue_head(&st->wq_data_avail);
>  	mutex_init(&st->lock);
>  
> -	ret = at91_adc_buffer_init(idev);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
> -		goto error_disable_adc_clk;
> -	}
> +	/*
> +	 * Since touch screen will set trigger register as period trigger. So
> +	 * when touch screen is enabled, then we have to disable hardware
> +	 * trigger for classic adc.
> +	 */
> +	if (!st->touchscreen_type) {
> +		ret = at91_adc_buffer_init(idev);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
> +			goto error_disable_adc_clk;
> +		}
>  
> -	ret = at91_adc_trigger_init(idev);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
> -		goto error_unregister_buffer;
> +		ret = at91_adc_trigger_init(idev);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
> +			at91_adc_buffer_remove(idev);
> +			goto error_disable_adc_clk;
> +		}
> +	} else {
> +		if (!st->caps->has_tsmr) {
> +			dev_err(&pdev->dev, "We don't support non-TSMR adc\n");
> +			goto error_disable_adc_clk;
> +		}
> +
> +		ret = at91_ts_register(st);
> +		if (ret)
> +			goto error_disable_adc_clk;
> +
> +		at91_ts_hw_init(st, adc_clk_khz);
>  	}
>  
>  	ret = iio_device_register(idev);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Couldn't register the device.\n");
> -		goto error_remove_triggers;
> +		goto error_iio_device_register;
>  	}
>  
>  	return 0;
>  
> -error_remove_triggers:
> -	at91_adc_trigger_remove(idev);
> -error_unregister_buffer:
> -	at91_adc_buffer_remove(idev);
> +error_iio_device_register:
> +	if (!st->touchscreen_type) {
> +		at91_adc_trigger_remove(idev);
> +		at91_adc_buffer_remove(idev);
> +	} else {
> +		at91_ts_unregister(st);
> +	}
>  error_disable_adc_clk:
>  	clk_disable_unprepare(st->adc_clk);
>  error_disable_clk:
> @@ -803,8 +1131,12 @@ static int at91_adc_remove(struct platform_device *pdev)
>  	struct at91_adc_state *st = iio_priv(idev);
>  
>  	iio_device_unregister(idev);
> -	at91_adc_trigger_remove(idev);
> -	at91_adc_buffer_remove(idev);
> +	if (!st->touchscreen_type) {
> +		at91_adc_trigger_remove(idev);
> +		at91_adc_buffer_remove(idev);
> +	} else {
> +		at91_ts_unregister(st);
> +	}
>  	clk_disable_unprepare(st->adc_clk);
>  	clk_disable_unprepare(st->clk);
>  	free_irq(st->irq, idev);
> @@ -820,11 +1152,16 @@ static struct at91_adc_caps at91sam9260_caps = {
>  };
>  
>  static struct at91_adc_caps at91sam9g45_caps = {
> +	.has_ts = true,
>  	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  	.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>  };
>  
>  static struct at91_adc_caps at91sam9x5_caps = {
> +	.has_ts = true,
> +	.has_tsmr = true,
> +	.ts_filter_average = 3,
> +	.ts_pen_detect_sensitivity = 2,
>  	.start_time_use_lookup_tab = true,
>  	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,	/* same as 9G45 */
>  	.mr_startup_mask = AT91_ADC_STARTUP_9X5,
> @@ -843,7 +1180,7 @@ static struct platform_driver at91_adc_driver = {
>  	.probe = at91_adc_probe,
>  	.remove = at91_adc_remove,
>  	.driver = {
> -		   .name = "at91_adc",
> +		   .name = DRIVER_NAME,
>  		   .of_match_table = of_match_ptr(at91_adc_dt_ids),
>  	},
>  };
> -- 
> 1.7.10
> 

Thanks.

-- 
Dmitry

[PATCH v2 4/4] iio: at91: introduce touch screen support in iio adc driver

[Jonathan Cameron]

On 08/12/13 18:24, Dmitry Torokhov wrote:
> Hi Josh,

Hi Dmitry,

As you haven't raised it here, I am guessing you are fine with having this
chunk of input related code in an IIO driver?  I wonder if it would be
better to break this into an mfd core and child input and iio drivers.
The two parts look fairly separable to me.
Clearly that means adding some overhead, but the gain is that the input
code ends up in input and the IIO code in IIO.

What are your views on this?  I'm fairly flexible if you think it simply
isn't worth the hassle of breaking this up.  I'm more worried about setting
a precident than anything else.

Jonathan

> 
> On Sun, Aug 11, 2013 at 07:04:31PM +0800, Josh Wu wrote:
>> AT91 ADC hardware integrate touch screen support. So this patch add touch
>> screen support for at91 adc iio driver.
>> To enable touch screen support in adc, you need to add the dt parameters:
>>   1. which type of touch are used? (4 or 5 wires), sample period time.
>>   2. correct pressure detect threshold value.
>>
>> In the meantime, since touch screen will use a interal period trigger of adc,
>> so it is conflict to other hardware triggers. Driver will disable the hardware
>> trigger support if touch screen is enabled.
>>
>> This driver has been tested in AT91SAM9X5-EK and SAMA5D3x-EK.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>> v1 --> v2:
>>   1. use the multiple compatible string for different IPs.
>>   2. hide the driver configuration in driver code instead of exposing them in dt.
>>      Such like sample period, pen detect debounce time and etc.
>>   3. add a new dt parameter: ts-pressure-threshold. this value is much related
>>      with different chips and boards.
>>
>>  .../devicetree/bindings/arm/atmel-adc.txt          |    7 +
>>  arch/arm/mach-at91/include/mach/at91_adc.h         |   34 ++
>>  drivers/iio/adc/at91_adc.c                         |  385 ++++++++++++++++++--
>>  3 files changed, 402 insertions(+), 24 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/atmel-adc.txt b/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> index 6e60781..c01eaca 100644
>> --- a/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> @@ -28,6 +28,13 @@ Optional properties:
>>  		       resolution will be used.
>>    - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
>>    - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
>> +  - atmel,adc-ts-wires: Number of touch screen wires. Should be 4 or 5. If this
>> +                        value is set, then adc driver will enable touch screen
>> +                        support.
>> +    NOTE: when adc touch screen enabled, the adc hardware trigger will be
>> +          disabled. Since touch screen will occupied the trigger register.
>> +  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
>> +                                     make touch detect more precision.
>>   
>>  Optional trigger Nodes:
>>    - Required properties:
>> diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
>> index f00926a..86f42a1 100644
>> --- a/arch/arm/mach-at91/include/mach/at91_adc.h
>> +++ b/arch/arm/mach-at91/include/mach/at91_adc.h
>> @@ -57,8 +57,42 @@
>>  #define AT91_ADC_IER		0x24		/* Interrupt Enable Register */
>>  #define AT91_ADC_IDR		0x28		/* Interrupt Disable Register */
>>  #define AT91_ADC_IMR		0x2C		/* Interrupt Mask Register */
>> +#define		AT91_ADC_IER_PEN	(1 << 29)
>> +#define		AT91_ADC_IER_NOPEN	(1 << 30)
>> +#define		AT91_ADC_IER_XRDY	(1 << 20)
>> +#define		AT91_ADC_IER_YRDY	(1 << 21)
>> +#define		AT91_ADC_IER_PRDY	(1 << 22)
>> +#define		AT91_ADC_ISR_PENS	(1 << 31)
>>  
>>  #define AT91_ADC_CHR(n)		(0x30 + ((n) * 4))	/* Channel Data Register N */
>>  #define		AT91_ADC_DATA		(0x3ff)
>>  
>> +#define AT91_ADC_ACR		0x94	/* Analog Control Register */
>> +#define		AT91_ADC_ACR_PENDETSENS	(0x3 << 0)	/* pull-up resistor */
>> +
>> +#define AT91_ADC_TSMR		0xB0
>> +#define		AT91_ADC_TSMR_TSMODE	(3 << 0)	/* Touch Screen Mode */
>> +#define			AT91_ADC_TSMR_TSMODE_NONE		(0 << 0)
>> +#define			AT91_ADC_TSMR_TSMODE_4WIRE_NO_PRESS	(1 << 0)
>> +#define			AT91_ADC_TSMR_TSMODE_4WIRE_PRESS	(2 << 0)
>> +#define			AT91_ADC_TSMR_TSMODE_5WIRE		(3 << 0)
>> +#define		AT91_ADC_TSMR_TSAV	(3 << 4)	/* Averages samples */
>> +#define			AT91_ADC_TSMR_TSAV_(x)		((x) << 4)
>> +#define		AT91_ADC_TSMR_SCTIM	(0x0f << 16)	/* Switch closure time */
>> +#define		AT91_ADC_TSMR_PENDBC	(0x0f << 28)	/* Pen Debounce time */
>> +#define			AT91_ADC_TSMR_PENDBC_(x)	((x) << 28)
>> +#define		AT91_ADC_TSMR_NOTSDMA	(1 << 22)	/* No Touchscreen DMA */
>> +#define		AT91_ADC_TSMR_PENDET_DIS	(0 << 24)	/* Pen contact detection disable */
>> +#define		AT91_ADC_TSMR_PENDET_ENA	(1 << 24)	/* Pen contact detection enable */
>> +
>> +#define AT91_ADC_TSXPOSR	0xB4
>> +#define AT91_ADC_TSYPOSR	0xB8
>> +#define AT91_ADC_TSPRESSR	0xBC
>> +
>> +/* Trigger Register bit field */
>> +#define		AT91_ADC_TRGR_TRGPER	(0xffff << 16)
>> +#define			AT91_ADC_TRGR_TRGPER_(x)	((x) << 16)
>> +#define		AT91_ADC_TRGR_TRGMOD	(0x7 << 0)
>> +#define			AT91_ADC_TRGR_MOD_PERIOD_TRIG	(5 << 0)
>> +
>>  #endif
>> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>> index 9b0ffff..b2495cb 100644
>> --- a/drivers/iio/adc/at91_adc.c
>> +++ b/drivers/iio/adc/at91_adc.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/clk.h>
>>  #include <linux/err.h>
>>  #include <linux/io.h>
>> +#include <linux/input.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/jiffies.h>
>>  #include <linux/kernel.h>
>> @@ -39,12 +40,34 @@
>>  #define at91_adc_writel(st, reg, val) \
>>  	(writel_relaxed(val, st->reg_base + reg))
>>  
>> +#define DRIVER_NAME		"at91_adc"
>> +#define MAX_POS_BITS		12
>> +
>> +#define TOUCH_SAMPLE_PERIOD_US		2000	/* 2ms */
>> +#define TOUCH_PEN_DETECT_DEBOUNCE_US	200
>> +
>>  struct at91_adc_caps {
>> +	bool	has_ts;		/* Support touch screen */
>> +	bool	has_tsmr;	/* only at91sam9x5, sama5d3 have TSMR reg */
>> +	/*
>> +	 * Numbers of sampling data will be averaged. Can be 0~3.
>> +	 * Hardware can average (2 ^ ts_filter_average) sample data.
>> +	 */
>> +	u8	ts_filter_average;
>> +	/* Pen Detection input pull-up resistor, can be 0~3 */
>> +	u8	ts_pen_detect_sensitivity;
>> +
>>  	bool	start_time_use_lookup_tab;
>>  	u32	mr_prescal_mask;
>>  	u32	mr_startup_mask;
>>  };
>>  
>> +enum atmel_adc_ts_type {
>> +	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
>> +	ATMEL_ADC_TOUCHSCREEN_4WIRE,
>> +	ATMEL_ADC_TOUCHSCREEN_5WIRE,
>> +};
>> +
>>  struct at91_adc_state {
>>  	struct clk		*adc_clk;
>>  	u16			*buffer;
>> @@ -69,6 +92,26 @@ struct at91_adc_state {
>>  	bool			low_res;	/* the resolution corresponds to the lowest one */
>>  	wait_queue_head_t	wq_data_avail;
>>  	struct at91_adc_caps	*caps;
>> +
>> +	/*
>> +	 * Following ADC channels are shared by touchscreen:
>> +	 *
>> +	 * CH0 -- Touch screen XP/UL
>> +	 * CH1 -- Touch screen XM/UR
>> +	 * CH2 -- Touch screen YP/LL
>> +	 * CH3 -- Touch screen YM/Sense
>> +	 * CH4 -- Touch screen LR(5-wire only)
>> +	 *
>> +	 * The bitfields below represents the reserved channel in the
>> +	 * touchscreen mode.
>> +	 */
>> +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 0)
>> +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 0)
>> +	enum atmel_adc_ts_type	touchscreen_type;
>> +	struct input_dev	*ts_input;
>> +
>> +	u16			ts_sample_period_val;
>> +	u32			ts_pressure_threshold;
>>  };
>>  
>>  static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>> @@ -103,14 +146,10 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> -static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
>> +/* Handler for classic adc channel eoc trigger */
>> +void handle_adc_eoc_trigger(int irq, struct iio_dev *idev)
>>  {
>> -	struct iio_dev *idev = private;
>>  	struct at91_adc_state *st = iio_priv(idev);
>> -	u32 status = at91_adc_readl(st, st->registers->status_register);
>> -
>> -	if (!(status & st->registers->drdy_mask))
>> -		return IRQ_HANDLED;
>>  
>>  	if (iio_buffer_enabled(idev)) {
>>  		disable_irq_nosync(irq);
>> @@ -120,6 +159,114 @@ static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
>>  		st->done = true;
>>  		wake_up_interruptible(&st->wq_data_avail);
>>  	}
>> +}
>> +
>> +static int at91_ts_sample(struct at91_adc_state *st)
>> +{
>> +	unsigned int xscale, yscale, reg, z1, z2;
>> +	unsigned int x, y, pres, xpos, ypos;
>> +	unsigned int rxp = 1;
>> +	unsigned int factor = 1000;
>> +	struct iio_dev *idev = iio_priv_to_dev(st);
>> +
>> +	unsigned int xyz_mask_bits = st->res;
>> +	unsigned int xyz_mask = (1 << xyz_mask_bits) - 1;
>> +
>> +	/* calculate position */
>> +	/* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */
>> +	reg = at91_adc_readl(st, AT91_ADC_TSXPOSR);
>> +	xpos = reg & xyz_mask;
>> +	x = (xpos << MAX_POS_BITS) - xpos;
>> +	xscale = (reg >> 16) & xyz_mask;
>> +	if (xscale != 0)
>> +		x /= xscale;
>> +	else
>> +		dev_err(&idev->dev, "xscale == 0!!!\n");
> 
> Maybe you want to ratelimit this? Also, do you really want to continue
> reporting or maybe you want to jump out early?
> 
>> +
>> +	/* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */
>> +	reg = at91_adc_readl(st, AT91_ADC_TSYPOSR);
>> +	ypos = reg & xyz_mask;
>> +	y = (ypos << MAX_POS_BITS) - ypos;
>> +	yscale = (reg >> 16) & xyz_mask;
>> +	if (yscale != 0)
>> +		y /= yscale;
>> +	else
>> +		dev_err(&idev->dev, "yscale == 0!!!\n");
>> +
>> +	/* calculate the pressure */
>> +	reg = at91_adc_readl(st, AT91_ADC_TSPRESSR);
>> +	z1 = reg & xyz_mask;
>> +	z2 = (reg >> 16) & xyz_mask;
>> +
>> +	if (z1 != 0)
>> +		pres = rxp * (x * factor / 1024) * (z2 * factor / z1 - factor)
>> +			/ factor;
>> +	else
>> +		pres = st->ts_pressure_threshold;	/* no pen contacted */
>> +
>> +	dev_dbg(&idev->dev, "xpos = %d, xscale = %d, ypos = %d, yscale = %d, z1 = %d, z2 = %d, press = %d\n",
>> +				xpos, xscale, ypos, yscale, z1, z2, pres);
>> +
>> +	if (pres < st->ts_pressure_threshold) {
>> +		dev_dbg(&idev->dev, "x = %d, y = %d, pressure = %d\n",
>> +					x, y, pres / factor);
>> +		input_report_abs(st->ts_input, ABS_X, x);
>> +		input_report_abs(st->ts_input, ABS_Y, y);
>> +		input_report_abs(st->ts_input, ABS_PRESSURE, pres);
>> +		input_report_key(st->ts_input, BTN_TOUCH, 1);
>> +		input_sync(st->ts_input);
>> +	} else {
>> +		dev_dbg(&idev->dev, "pressure too low: not reporting\n");
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static irqreturn_t at91_adc_interrupt(int irq, void *private)
>> +{
>> +	struct iio_dev *idev = private;
>> +	struct at91_adc_state *st = iio_priv(idev);
>> +	u32 status = at91_adc_readl(st, st->registers->status_register);
>> +	const uint32_t ts_data_irq_mask =
>> +		AT91_ADC_IER_XRDY |
>> +		AT91_ADC_IER_YRDY |
>> +		AT91_ADC_IER_PRDY;
>> +
>> +	if (status & st->registers->drdy_mask)
>> +		handle_adc_eoc_trigger(irq, idev);
>> +
>> +	if (status & AT91_ADC_IER_PEN) {
>> +		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
>> +		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_NOPEN |
>> +			ts_data_irq_mask);
>> +		/* Set up period trigger for sampling */
>> +		at91_adc_writel(st, st->registers->trigger_register,
>> +			AT91_ADC_TRGR_MOD_PERIOD_TRIG |
>> +			AT91_ADC_TRGR_TRGPER_(st->ts_sample_period_val));
>> +	} else if (status & AT91_ADC_IER_NOPEN) {
>> +		at91_adc_writel(st, st->registers->trigger_register, 0);
>> +		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_NOPEN |
>> +			ts_data_irq_mask);
>> +		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
>> +
>> +		input_report_key(st->ts_input, BTN_TOUCH, 0);
>> +		input_sync(st->ts_input);
>> +	} else if ((status & ts_data_irq_mask) == ts_data_irq_mask) {
>> +		/* Now all touchscreen data is ready */
>> +
>> +		if (status & AT91_ADC_ISR_PENS) {
>> +			/* validate data by pen contact */
>> +			at91_ts_sample(st);
>> +		} else {
>> +			/* triggered by event that is no pen contact, just read
>> +			 * them to clean the interrupt and discard all.
>> +			 */
>> +			at91_adc_readl(st, AT91_ADC_TSXPOSR);
>> +			at91_adc_readl(st, AT91_ADC_TSYPOSR);
>> +			at91_adc_readl(st, AT91_ADC_TSPRESSR);
>> +		}
>> +	}
>>  
>>  	return IRQ_HANDLED;
>>  }
>> @@ -129,6 +276,16 @@ static int at91_adc_channel_init(struct iio_dev *idev)
>>  	struct at91_adc_state *st = iio_priv(idev);
>>  	struct iio_chan_spec *chan_array, *timestamp;
>>  	int bit, idx = 0;
>> +	unsigned long rsvd_mask = 0;
>> +
>> +	/* If touchscreen is enable, then reserve the adc channels */
>> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
>> +		rsvd_mask = CHAN_MASK_TOUCHSCREEN_4WIRE;
>> +	else if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_5WIRE)
>> +		rsvd_mask = CHAN_MASK_TOUCHSCREEN_5WIRE;
>> +
>> +	/* set up the channel mask to reserve touchscreen channels */
>> +	st->channels_mask &= ~rsvd_mask;
>>  
>>  	idev->num_channels = bitmap_weight(&st->channels_mask,
>>  					   st->num_channels) + 1;
>> @@ -438,6 +595,37 @@ ret:
>>  
>>  static const struct of_device_id at91_adc_dt_ids[];
>>  
>> +static int at91_adc_probe_dt_ts(struct device_node *node,
>> +	struct at91_adc_state *st, struct device *dev)
>> +{
>> +	int ret;
>> +	u32 prop;
>> +
>> +	ret = of_property_read_u32(node, "atmel,adc-ts-wires", &prop);
>> +	if (ret)
>> +		dev_info(dev, "Touchscreen not enabled.\n");
>> +	else if (prop == 4)
>> +		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_4WIRE;
>> +	else if (prop == 5)
>> +		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_5WIRE;
>> +	else
>> +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
>> +				prop);
>> +
>> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_NONE)
>> +		return 0;
> 
> I'd rather we separated invalid wire configuration from the absent (i.e.
> disabled) case.
> 
>> +
>> +	prop = 0;
>> +	of_property_read_u32(node, "atmel,adc-ts-pressure-threshold", &prop);
>> +	st->ts_pressure_threshold = prop;
>> +	if (st->ts_pressure_threshold) {
>> +		return 0;
>> +	} else {
>> +		dev_err(dev, "Invalid pressure threshold for the touchscreen\n");
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>>  static int at91_adc_probe_dt(struct at91_adc_state *st,
>>  			     struct platform_device *pdev)
>>  {
>> @@ -561,6 +749,12 @@ static int at91_adc_probe_dt(struct at91_adc_state *st,
>>  		i++;
>>  	}
>>  
>> +	/* Check if touchscreen is supported. */
>> +	if (st->caps->has_ts)
>> +		return at91_adc_probe_dt_ts(node, st, &idev->dev);
>> +	else
>> +		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
>> +
>>  	return 0;
>>  
>>  error_ret:
>> @@ -592,6 +786,114 @@ static const struct iio_info at91_adc_info = {
>>  	.read_raw = &at91_adc_read_raw,
>>  };
>>  
>> +/* Touchscreen related functions */
>> +static int atmel_ts_open(struct input_dev *dev)
>> +{
>> +	struct at91_adc_state *st = input_get_drvdata(dev);
>> +
>> +	at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
>> +	return 0;
>> +}
>> +
>> +static void atmel_ts_close(struct input_dev *dev)
>> +{
>> +	struct at91_adc_state *st = input_get_drvdata(dev);
>> +
>> +	at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
>> +}
>> +
>> +static int at91_ts_hw_init(struct at91_adc_state *st, u32 adc_clk_khz)
>> +{
>> +	u32 reg = 0, pendbc;
>> +	int i = 0;
>> +
>> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
>> +		reg = AT91_ADC_TSMR_TSMODE_4WIRE_PRESS;
>> +	else
>> +		reg = AT91_ADC_TSMR_TSMODE_5WIRE;
>> +
>> +	/* a Pen Detect Debounce Time is necessary for the ADC Touch to avoid
>> +	 * pen detect noise.
>> +	 * The formula is : Pen Detect Debounce Time = (2 ^ pendbc) / ADCClock
>> +	 */
>> +	pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * adc_clk_khz / 1000, 1);
>> +
>> +	while (pendbc >> ++i)
>> +		;	/* Empty! Find the shift offset */
>> +	if (abs(pendbc - (1 << i)) < abs(pendbc - (1 << (i - 1))))
>> +		pendbc = i;
>> +	else
>> +		pendbc = i - 1;
>> +
>> +	if (st->caps->has_tsmr) {
>> +		reg |= AT91_ADC_TSMR_TSAV_(st->caps->ts_filter_average)
>> +				& AT91_ADC_TSMR_TSAV;
>> +		reg |= AT91_ADC_TSMR_PENDBC_(pendbc) & AT91_ADC_TSMR_PENDBC;
>> +		reg |= AT91_ADC_TSMR_NOTSDMA;
>> +		reg |= AT91_ADC_TSMR_PENDET_ENA;
>> +		reg |= 0x03 << 8;	/* TSFREQ, need bigger than TSAV */
>> +
>> +		at91_adc_writel(st, AT91_ADC_TSMR, reg);
>> +	} else {
>> +		/* TODO: for 9g45 which has no TSMR */
>> +	}
>> +
>> +	/* Change adc internal resistor value for better pen detection,
>> +	 * default value is 100 kOhm.
>> +	 * 0 = 200 kOhm, 1 = 150 kOhm, 2 = 100 kOhm, 3 = 50 kOhm
>> +	 * option only available on ES2 and higher
>> +	 */
>> +	at91_adc_writel(st, AT91_ADC_ACR, st->caps->ts_pen_detect_sensitivity
>> +			& AT91_ADC_ACR_PENDETSENS);
>> +
>> +	/* Sample Peroid Time = (TRGPER + 1) / ADCClock */
>> +	st->ts_sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
>> +			adc_clk_khz / 1000) - 1, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int at91_ts_register(struct at91_adc_state *st)
>> +{
>> +	struct input_dev *input;
>> +	struct iio_dev *idev = iio_priv_to_dev(st);
>> +	int ret;
>> +
>> +	input = input_allocate_device();
>> +	if (!input) {
>> +		dev_err(&idev->dev, "Failed to allocate TS device!\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	input->name = DRIVER_NAME;
>> +	input->id.bustype = BUS_HOST;
>> +	input->dev.parent = idev->dev.parent;
> 
> Why you are setting parent to the IIO's parent? This is confusing, maybe
> you should simply pass the parent platform device into this fucntion.
> 
>> +	input->open = atmel_ts_open;
>> +	input->close = atmel_ts_close;
>> +
>> +	__set_bit(EV_ABS, input->evbit);
>> +	__set_bit(EV_KEY, input->evbit);
>> +	__set_bit(BTN_TOUCH, input->keybit);
>> +	input_set_abs_params(input, ABS_X, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
>> +	input_set_abs_params(input, ABS_Y, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
>> +	input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
>> +
>> +	st->ts_input = input;
>> +	input_set_drvdata(input, st);
>> +
>> +	ret = input_register_device(input);
>> +	if (ret)
>> +		input_free_device(st->ts_input);
>> +
>> +	return ret;
>> +}
>> +
>> +static void at91_ts_unregister(struct at91_adc_state *st)
>> +{
>> +	input_unregister_device(st->ts_input);
>> +	input_free_device(st->ts_input);
> 
> No free after unregister please.
> 
>> +}
>> +
>>  static int at91_adc_probe(struct platform_device *pdev)
>>  {
>>  	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
>> @@ -648,7 +950,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>>  	at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST);
>>  	at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF);
>>  	ret = request_irq(st->irq,
>> -			  at91_adc_eoc_trigger,
>> +			  at91_adc_interrupt,
>>  			  0,
>>  			  pdev->dev.driver->name,
>>  			  idev);
>> @@ -693,6 +995,10 @@ static int at91_adc_probe(struct platform_device *pdev)
>>  	mstrclk = clk_get_rate(st->clk);
>>  	adc_clk = clk_get_rate(st->adc_clk);
>>  	adc_clk_khz = adc_clk / 1000;
>> +
>> +	dev_dbg(&pdev->dev, "Master clock is set as: %d Hz, adc_clk should set as: %d Hz\n",
>> +		mstrclk, adc_clk);
>> +
>>  	prsc = (mstrclk / (2 * adc_clk)) - 1;
>>  
>>  	if (!st->startup_time) {
>> @@ -761,30 +1067,52 @@ static int at91_adc_probe(struct platform_device *pdev)
>>  	init_waitqueue_head(&st->wq_data_avail);
>>  	mutex_init(&st->lock);
>>  
>> -	ret = at91_adc_buffer_init(idev);
>> -	if (ret < 0) {
>> -		dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
>> -		goto error_disable_adc_clk;
>> -	}
>> +	/*
>> +	 * Since touch screen will set trigger register as period trigger. So
>> +	 * when touch screen is enabled, then we have to disable hardware
>> +	 * trigger for classic adc.
>> +	 */
>> +	if (!st->touchscreen_type) {
>> +		ret = at91_adc_buffer_init(idev);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
>> +			goto error_disable_adc_clk;
>> +		}
>>  
>> -	ret = at91_adc_trigger_init(idev);
>> -	if (ret < 0) {
>> -		dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
>> -		goto error_unregister_buffer;
>> +		ret = at91_adc_trigger_init(idev);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
>> +			at91_adc_buffer_remove(idev);
>> +			goto error_disable_adc_clk;
>> +		}
>> +	} else {
>> +		if (!st->caps->has_tsmr) {
>> +			dev_err(&pdev->dev, "We don't support non-TSMR adc\n");
>> +			goto error_disable_adc_clk;
>> +		}
>> +
>> +		ret = at91_ts_register(st);
>> +		if (ret)
>> +			goto error_disable_adc_clk;
>> +
>> +		at91_ts_hw_init(st, adc_clk_khz);
>>  	}
>>  
>>  	ret = iio_device_register(idev);
>>  	if (ret < 0) {
>>  		dev_err(&pdev->dev, "Couldn't register the device.\n");
>> -		goto error_remove_triggers;
>> +		goto error_iio_device_register;
>>  	}
>>  
>>  	return 0;
>>  
>> -error_remove_triggers:
>> -	at91_adc_trigger_remove(idev);
>> -error_unregister_buffer:
>> -	at91_adc_buffer_remove(idev);
>> +error_iio_device_register:
>> +	if (!st->touchscreen_type) {
>> +		at91_adc_trigger_remove(idev);
>> +		at91_adc_buffer_remove(idev);
>> +	} else {
>> +		at91_ts_unregister(st);
>> +	}
>>  error_disable_adc_clk:
>>  	clk_disable_unprepare(st->adc_clk);
>>  error_disable_clk:
>> @@ -803,8 +1131,12 @@ static int at91_adc_remove(struct platform_device *pdev)
>>  	struct at91_adc_state *st = iio_priv(idev);
>>  
>>  	iio_device_unregister(idev);
>> -	at91_adc_trigger_remove(idev);
>> -	at91_adc_buffer_remove(idev);
>> +	if (!st->touchscreen_type) {
>> +		at91_adc_trigger_remove(idev);
>> +		at91_adc_buffer_remove(idev);
>> +	} else {
>> +		at91_ts_unregister(st);
>> +	}
>>  	clk_disable_unprepare(st->adc_clk);
>>  	clk_disable_unprepare(st->clk);
>>  	free_irq(st->irq, idev);
>> @@ -820,11 +1152,16 @@ static struct at91_adc_caps at91sam9260_caps = {
>>  };
>>  
>>  static struct at91_adc_caps at91sam9g45_caps = {
>> +	.has_ts = true,
>>  	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>>  	.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>>  };
>>  
>>  static struct at91_adc_caps at91sam9x5_caps = {
>> +	.has_ts = true,
>> +	.has_tsmr = true,
>> +	.ts_filter_average = 3,
>> +	.ts_pen_detect_sensitivity = 2,
>>  	.start_time_use_lookup_tab = true,
>>  	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,	/* same as 9G45 */
>>  	.mr_startup_mask = AT91_ADC_STARTUP_9X5,
>> @@ -843,7 +1180,7 @@ static struct platform_driver at91_adc_driver = {
>>  	.probe = at91_adc_probe,
>>  	.remove = at91_adc_remove,
>>  	.driver = {
>> -		   .name = "at91_adc",
>> +		   .name = DRIVER_NAME,
>>  		   .of_match_table = of_match_ptr(at91_adc_dt_ids),
>>  	},
>>  };
>> -- 
>> 1.7.10
>>
> 
> Thanks.
> 

[PATCH v2 4/4] iio: at91: introduce touch screen support in iio adc driver

[Josh Wu]

Hi, Jonathan

On 8/15/2013 6:27 PM, Jonathan Cameron wrote:
>
> On 08/12/13 18:24, Dmitry Torokhov wrote:
>> Hi Josh,
> Hi Dmitry,
>
> As you haven't raised it here, I am guessing you are fine with having this
> chunk of input related code in an IIO driver?  I wonder if it would be
> better to break this into an mfd core and child input and iio drivers.
> The two parts look fairly separable to me.
> Clearly that means adding some overhead, but the gain is that the input
> code ends up in input and the IIO code in IIO.
>
> What are your views on this?  I'm fairly flexible if you think it simply
> isn't worth the hassle of breaking this up.  I'm more worried about setting
> a precident than anything else.
>
> Jonathan

Since Dmitry has no feedback yet. I am not sure is it a right time to 
send out the next version for the IIO ad91 touch patch?

[snip...]

Best Regards,
Josh Wu

[PATCH v2 4/4] iio: at91: introduce touch screen support in iio adc driver

[Josh Wu]

Dear Dmitry Torokhov,

Thank you for the review. My comment is in below.

On 8/13/2013 1:24 AM, Dmitry Torokhov wrote:
> Hi Josh,
>
> On Sun, Aug 11, 2013 at 07:04:31PM +0800, Josh Wu wrote:
>> AT91 ADC hardware integrate touch screen support. So this patch add touch
>> screen support for at91 adc iio driver.
>> To enable touch screen support in adc, you need to add the dt parameters:
>>    1. which type of touch are used? (4 or 5 wires), sample period time.
>>    2. correct pressure detect threshold value.
>>
>> In the meantime, since touch screen will use a interal period trigger of adc,
>> so it is conflict to other hardware triggers. Driver will disable the hardware
>> trigger support if touch screen is enabled.
>>
>> This driver has been tested in AT91SAM9X5-EK and SAMA5D3x-EK.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>> v1 --> v2:
>>    1. use the multiple compatible string for different IPs.
>>    2. hide the driver configuration in driver code instead of exposing them in dt.
>>       Such like sample period, pen detect debounce time and etc.
>>    3. add a new dt parameter: ts-pressure-threshold. this value is much related
>>       with different chips and boards.
>>
>>   .../devicetree/bindings/arm/atmel-adc.txt          |    7 +
>>   arch/arm/mach-at91/include/mach/at91_adc.h         |   34 ++
>>   drivers/iio/adc/at91_adc.c                         |  385 ++++++++++++++++++--
>>   3 files changed, 402 insertions(+), 24 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/atmel-adc.txt b/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> index 6e60781..c01eaca 100644
>> --- a/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/atmel-adc.txt
>> @@ -28,6 +28,13 @@ Optional properties:
>>   		       resolution will be used.
>>     - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
>>     - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
>> +  - atmel,adc-ts-wires: Number of touch screen wires. Should be 4 or 5. If this
>> +                        value is set, then adc driver will enable touch screen
>> +                        support.
>> +    NOTE: when adc touch screen enabled, the adc hardware trigger will be
>> +          disabled. Since touch screen will occupied the trigger register.
>> +  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
>> +                                     make touch detect more precision.
>>    
>>   Optional trigger Nodes:
>>     - Required properties:
>> diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
>> index f00926a..86f42a1 100644
>> --- a/arch/arm/mach-at91/include/mach/at91_adc.h
>> +++ b/arch/arm/mach-at91/include/mach/at91_adc.h
>> @@ -57,8 +57,42 @@
>>   #define AT91_ADC_IER		0x24		/* Interrupt Enable Register */
>>   #define AT91_ADC_IDR		0x28		/* Interrupt Disable Register */
>>   #define AT91_ADC_IMR		0x2C		/* Interrupt Mask Register */
>> +#define		AT91_ADC_IER_PEN	(1 << 29)
>> +#define		AT91_ADC_IER_NOPEN	(1 << 30)
>> +#define		AT91_ADC_IER_XRDY	(1 << 20)
>> +#define		AT91_ADC_IER_YRDY	(1 << 21)
>> +#define		AT91_ADC_IER_PRDY	(1 << 22)
>> +#define		AT91_ADC_ISR_PENS	(1 << 31)
>>   
>>   #define AT91_ADC_CHR(n)		(0x30 + ((n) * 4))	/* Channel Data Register N */
>>   #define		AT91_ADC_DATA		(0x3ff)
>>   
>> +#define AT91_ADC_ACR		0x94	/* Analog Control Register */
>> +#define		AT91_ADC_ACR_PENDETSENS	(0x3 << 0)	/* pull-up resistor */
>> +
>> +#define AT91_ADC_TSMR		0xB0
>> +#define		AT91_ADC_TSMR_TSMODE	(3 << 0)	/* Touch Screen Mode */
>> +#define			AT91_ADC_TSMR_TSMODE_NONE		(0 << 0)
>> +#define			AT91_ADC_TSMR_TSMODE_4WIRE_NO_PRESS	(1 << 0)
>> +#define			AT91_ADC_TSMR_TSMODE_4WIRE_PRESS	(2 << 0)
>> +#define			AT91_ADC_TSMR_TSMODE_5WIRE		(3 << 0)
>> +#define		AT91_ADC_TSMR_TSAV	(3 << 4)	/* Averages samples */
>> +#define			AT91_ADC_TSMR_TSAV_(x)		((x) << 4)
>> +#define		AT91_ADC_TSMR_SCTIM	(0x0f << 16)	/* Switch closure time */
>> +#define		AT91_ADC_TSMR_PENDBC	(0x0f << 28)	/* Pen Debounce time */
>> +#define			AT91_ADC_TSMR_PENDBC_(x)	((x) << 28)
>> +#define		AT91_ADC_TSMR_NOTSDMA	(1 << 22)	/* No Touchscreen DMA */
>> +#define		AT91_ADC_TSMR_PENDET_DIS	(0 << 24)	/* Pen contact detection disable */
>> +#define		AT91_ADC_TSMR_PENDET_ENA	(1 << 24)	/* Pen contact detection enable */
>> +
>> +#define AT91_ADC_TSXPOSR	0xB4
>> +#define AT91_ADC_TSYPOSR	0xB8
>> +#define AT91_ADC_TSPRESSR	0xBC
>> +
>> +/* Trigger Register bit field */
>> +#define		AT91_ADC_TRGR_TRGPER	(0xffff << 16)
>> +#define			AT91_ADC_TRGR_TRGPER_(x)	((x) << 16)
>> +#define		AT91_ADC_TRGR_TRGMOD	(0x7 << 0)
>> +#define			AT91_ADC_TRGR_MOD_PERIOD_TRIG	(5 << 0)
>> +
>>   #endif
>> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>> index 9b0ffff..b2495cb 100644
>> --- a/drivers/iio/adc/at91_adc.c
>> +++ b/drivers/iio/adc/at91_adc.c
>> @@ -11,6 +11,7 @@
>>   #include <linux/clk.h>
>>   #include <linux/err.h>
>>   #include <linux/io.h>
>> +#include <linux/input.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/jiffies.h>
>>   #include <linux/kernel.h>
>> @@ -39,12 +40,34 @@
>>   #define at91_adc_writel(st, reg, val) \
>>   	(writel_relaxed(val, st->reg_base + reg))
>>   
>> +#define DRIVER_NAME		"at91_adc"
>> +#define MAX_POS_BITS		12
>> +
>> +#define TOUCH_SAMPLE_PERIOD_US		2000	/* 2ms */
>> +#define TOUCH_PEN_DETECT_DEBOUNCE_US	200
>> +
>>   struct at91_adc_caps {
>> +	bool	has_ts;		/* Support touch screen */
>> +	bool	has_tsmr;	/* only at91sam9x5, sama5d3 have TSMR reg */
>> +	/*
>> +	 * Numbers of sampling data will be averaged. Can be 0~3.
>> +	 * Hardware can average (2 ^ ts_filter_average) sample data.
>> +	 */
>> +	u8	ts_filter_average;
>> +	/* Pen Detection input pull-up resistor, can be 0~3 */
>> +	u8	ts_pen_detect_sensitivity;
>> +
>>   	bool	start_time_use_lookup_tab;
>>   	u32	mr_prescal_mask;
>>   	u32	mr_startup_mask;
>>   };
>>   
>> +enum atmel_adc_ts_type {
>> +	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
>> +	ATMEL_ADC_TOUCHSCREEN_4WIRE,
>> +	ATMEL_ADC_TOUCHSCREEN_5WIRE,
>> +};
>> +
>>   struct at91_adc_state {
>>   	struct clk		*adc_clk;
>>   	u16			*buffer;
>> @@ -69,6 +92,26 @@ struct at91_adc_state {
>>   	bool			low_res;	/* the resolution corresponds to the lowest one */
>>   	wait_queue_head_t	wq_data_avail;
>>   	struct at91_adc_caps	*caps;
>> +
>> +	/*
>> +	 * Following ADC channels are shared by touchscreen:
>> +	 *
>> +	 * CH0 -- Touch screen XP/UL
>> +	 * CH1 -- Touch screen XM/UR
>> +	 * CH2 -- Touch screen YP/LL
>> +	 * CH3 -- Touch screen YM/Sense
>> +	 * CH4 -- Touch screen LR(5-wire only)
>> +	 *
>> +	 * The bitfields below represents the reserved channel in the
>> +	 * touchscreen mode.
>> +	 */
>> +#define CHAN_MASK_TOUCHSCREEN_4WIRE	(0xf << 0)
>> +#define CHAN_MASK_TOUCHSCREEN_5WIRE	(0x1f << 0)
>> +	enum atmel_adc_ts_type	touchscreen_type;
>> +	struct input_dev	*ts_input;
>> +
>> +	u16			ts_sample_period_val;
>> +	u32			ts_pressure_threshold;
>>   };
>>   
>>   static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>> @@ -103,14 +146,10 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> -static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
>> +/* Handler for classic adc channel eoc trigger */
>> +void handle_adc_eoc_trigger(int irq, struct iio_dev *idev)
>>   {
>> -	struct iio_dev *idev = private;
>>   	struct at91_adc_state *st = iio_priv(idev);
>> -	u32 status = at91_adc_readl(st, st->registers->status_register);
>> -
>> -	if (!(status & st->registers->drdy_mask))
>> -		return IRQ_HANDLED;
>>   
>>   	if (iio_buffer_enabled(idev)) {
>>   		disable_irq_nosync(irq);
>> @@ -120,6 +159,114 @@ static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
>>   		st->done = true;
>>   		wake_up_interruptible(&st->wq_data_avail);
>>   	}
>> +}
>> +
>> +static int at91_ts_sample(struct at91_adc_state *st)
>> +{
>> +	unsigned int xscale, yscale, reg, z1, z2;
>> +	unsigned int x, y, pres, xpos, ypos;
>> +	unsigned int rxp = 1;
>> +	unsigned int factor = 1000;
>> +	struct iio_dev *idev = iio_priv_to_dev(st);
>> +
>> +	unsigned int xyz_mask_bits = st->res;
>> +	unsigned int xyz_mask = (1 << xyz_mask_bits) - 1;
>> +
>> +	/* calculate position */
>> +	/* x position = (x / xscale) * max, max = 2^MAX_POS_BITS - 1 */
>> +	reg = at91_adc_readl(st, AT91_ADC_TSXPOSR);
>> +	xpos = reg & xyz_mask;
>> +	x = (xpos << MAX_POS_BITS) - xpos;
>> +	xscale = (reg >> 16) & xyz_mask;
>> +	if (xscale != 0)
>> +		x /= xscale;
>> +	else
>> +		dev_err(&idev->dev, "xscale == 0!!!\n");
> Maybe you want to ratelimit this? Also, do you really want to continue
> reporting or maybe you want to jump out early?

I will fix it by return a error code when xscale/yscale is 0 .

>
>> +
>> +	/* y position = (y / yscale) * max, max = 2^MAX_POS_BITS - 1 */
>> +	reg = at91_adc_readl(st, AT91_ADC_TSYPOSR);
>> +	ypos = reg & xyz_mask;
>> +	y = (ypos << MAX_POS_BITS) - ypos;
>> +	yscale = (reg >> 16) & xyz_mask;
>> +	if (yscale != 0)
>> +		y /= yscale;
>> +	else
>> +		dev_err(&idev->dev, "yscale == 0!!!\n");
>> +
>> +	/* calculate the pressure */
>> +	reg = at91_adc_readl(st, AT91_ADC_TSPRESSR);
>> +	z1 = reg & xyz_mask;
>> +	z2 = (reg >> 16) & xyz_mask;
>> +
>> +	if (z1 != 0)
>> +		pres = rxp * (x * factor / 1024) * (z2 * factor / z1 - factor)
>> +			/ factor;
>> +	else
>> +		pres = st->ts_pressure_threshold;	/* no pen contacted */
>> +
>> +	dev_dbg(&idev->dev, "xpos = %d, xscale = %d, ypos = %d, yscale = %d, z1 = %d, z2 = %d, press = %d\n",
>> +				xpos, xscale, ypos, yscale, z1, z2, pres);
>> +
>> +	if (pres < st->ts_pressure_threshold) {
>> +		dev_dbg(&idev->dev, "x = %d, y = %d, pressure = %d\n",
>> +					x, y, pres / factor);
>> +		input_report_abs(st->ts_input, ABS_X, x);
>> +		input_report_abs(st->ts_input, ABS_Y, y);
>> +		input_report_abs(st->ts_input, ABS_PRESSURE, pres);
>> +		input_report_key(st->ts_input, BTN_TOUCH, 1);
>> +		input_sync(st->ts_input);
>> +	} else {
>> +		dev_dbg(&idev->dev, "pressure too low: not reporting\n");
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +
>> +static irqreturn_t at91_adc_interrupt(int irq, void *private)
>> +{
>> +	struct iio_dev *idev = private;
>> +	struct at91_adc_state *st = iio_priv(idev);
>> +	u32 status = at91_adc_readl(st, st->registers->status_register);
>> +	const uint32_t ts_data_irq_mask =
>> +		AT91_ADC_IER_XRDY |
>> +		AT91_ADC_IER_YRDY |
>> +		AT91_ADC_IER_PRDY;
>> +
>> +	if (status & st->registers->drdy_mask)
>> +		handle_adc_eoc_trigger(irq, idev);
>> +
>> +	if (status & AT91_ADC_IER_PEN) {
>> +		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
>> +		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_NOPEN |
>> +			ts_data_irq_mask);
>> +		/* Set up period trigger for sampling */
>> +		at91_adc_writel(st, st->registers->trigger_register,
>> +			AT91_ADC_TRGR_MOD_PERIOD_TRIG |
>> +			AT91_ADC_TRGR_TRGPER_(st->ts_sample_period_val));
>> +	} else if (status & AT91_ADC_IER_NOPEN) {
>> +		at91_adc_writel(st, st->registers->trigger_register, 0);
>> +		at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_NOPEN |
>> +			ts_data_irq_mask);
>> +		at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
>> +
>> +		input_report_key(st->ts_input, BTN_TOUCH, 0);
>> +		input_sync(st->ts_input);
>> +	} else if ((status & ts_data_irq_mask) == ts_data_irq_mask) {
>> +		/* Now all touchscreen data is ready */
>> +
>> +		if (status & AT91_ADC_ISR_PENS) {
>> +			/* validate data by pen contact */
>> +			at91_ts_sample(st);
>> +		} else {
>> +			/* triggered by event that is no pen contact, just read
>> +			 * them to clean the interrupt and discard all.
>> +			 */
>> +			at91_adc_readl(st, AT91_ADC_TSXPOSR);
>> +			at91_adc_readl(st, AT91_ADC_TSYPOSR);
>> +			at91_adc_readl(st, AT91_ADC_TSPRESSR);
>> +		}
>> +	}
>>   
>>   	return IRQ_HANDLED;
>>   }
>> @@ -129,6 +276,16 @@ static int at91_adc_channel_init(struct iio_dev *idev)
>>   	struct at91_adc_state *st = iio_priv(idev);
>>   	struct iio_chan_spec *chan_array, *timestamp;
>>   	int bit, idx = 0;
>> +	unsigned long rsvd_mask = 0;
>> +
>> +	/* If touchscreen is enable, then reserve the adc channels */
>> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
>> +		rsvd_mask = CHAN_MASK_TOUCHSCREEN_4WIRE;
>> +	else if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_5WIRE)
>> +		rsvd_mask = CHAN_MASK_TOUCHSCREEN_5WIRE;
>> +
>> +	/* set up the channel mask to reserve touchscreen channels */
>> +	st->channels_mask &= ~rsvd_mask;
>>   
>>   	idev->num_channels = bitmap_weight(&st->channels_mask,
>>   					   st->num_channels) + 1;
>> @@ -438,6 +595,37 @@ ret:
>>   
>>   static const struct of_device_id at91_adc_dt_ids[];
>>   
>> +static int at91_adc_probe_dt_ts(struct device_node *node,
>> +	struct at91_adc_state *st, struct device *dev)
>> +{
>> +	int ret;
>> +	u32 prop;
>> +
>> +	ret = of_property_read_u32(node, "atmel,adc-ts-wires", &prop);
>> +	if (ret)
>> +		dev_info(dev, "Touchscreen not enabled.\n");
>> +	else if (prop == 4)
>> +		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_4WIRE;
>> +	else if (prop == 5)
>> +		st->touchscreen_type = ATMEL_ADC_TOUCHSCREEN_5WIRE;
>> +	else
>> +		dev_warn(dev, "Unsupported number of touchscreen wires (%d)\n",
>> +				prop);
>> +
>> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_NONE)
>> +		return 0;
> I'd rather we separated invalid wire configuration from the absent (i.e.
> disabled) case.

I am not clear for this point. Do you mean above code need to be 
separated in those two case?
Or do you mean for DT parameters it need separated parameters, one for 
wire configuration, another for touchscreen disable/enable?

I think you are the former one. What about I change above code to following:

ret = of_property_read_u32(node, "atmel,adc-ts-wires", &prop);
     if (ret) {
         dev_info(dev, "ADC Touch screen is disabled.\n");
         return 0;
     }

     switch (prop) {
     case 4:
     case 5:
         st->touchscreen_type = prop;
         break;
     default:
         dev_err(dev, "Unsupported number of touchscreen wires (%d). 
Should be 4 or 5.\n", prop);
         return -EINVAL;
     }

It is clearer in the two cases:
  - if no "atmel,adc-ts-wires" then disable touchscreen.
  - If has "atmel,adc-ts-wires", then check it is valid or not.

>
>> +
>> +	prop = 0;
>> +	of_property_read_u32(node, "atmel,adc-ts-pressure-threshold", &prop);
>> +	st->ts_pressure_threshold = prop;
>> +	if (st->ts_pressure_threshold) {
>> +		return 0;
>> +	} else {
>> +		dev_err(dev, "Invalid pressure threshold for the touchscreen\n");
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>>   static int at91_adc_probe_dt(struct at91_adc_state *st,
>>   			     struct platform_device *pdev)
>>   {
>> @@ -561,6 +749,12 @@ static int at91_adc_probe_dt(struct at91_adc_state *st,
>>   		i++;
>>   	}
>>   
>> +	/* Check if touchscreen is supported. */
>> +	if (st->caps->has_ts)
>> +		return at91_adc_probe_dt_ts(node, st, &idev->dev);
>> +	else
>> +		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
>> +
>>   	return 0;
>>   
>>   error_ret:
>> @@ -592,6 +786,114 @@ static const struct iio_info at91_adc_info = {
>>   	.read_raw = &at91_adc_read_raw,
>>   };
>>   
>> +/* Touchscreen related functions */
>> +static int atmel_ts_open(struct input_dev *dev)
>> +{
>> +	struct at91_adc_state *st = input_get_drvdata(dev);
>> +
>> +	at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
>> +	return 0;
>> +}
>> +
>> +static void atmel_ts_close(struct input_dev *dev)
>> +{
>> +	struct at91_adc_state *st = input_get_drvdata(dev);
>> +
>> +	at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
>> +}
>> +
>> +static int at91_ts_hw_init(struct at91_adc_state *st, u32 adc_clk_khz)
>> +{
>> +	u32 reg = 0, pendbc;
>> +	int i = 0;
>> +
>> +	if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
>> +		reg = AT91_ADC_TSMR_TSMODE_4WIRE_PRESS;
>> +	else
>> +		reg = AT91_ADC_TSMR_TSMODE_5WIRE;
>> +
>> +	/* a Pen Detect Debounce Time is necessary for the ADC Touch to avoid
>> +	 * pen detect noise.
>> +	 * The formula is : Pen Detect Debounce Time = (2 ^ pendbc) / ADCClock
>> +	 */
>> +	pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * adc_clk_khz / 1000, 1);
>> +
>> +	while (pendbc >> ++i)
>> +		;	/* Empty! Find the shift offset */
>> +	if (abs(pendbc - (1 << i)) < abs(pendbc - (1 << (i - 1))))
>> +		pendbc = i;
>> +	else
>> +		pendbc = i - 1;
>> +
>> +	if (st->caps->has_tsmr) {
>> +		reg |= AT91_ADC_TSMR_TSAV_(st->caps->ts_filter_average)
>> +				& AT91_ADC_TSMR_TSAV;
>> +		reg |= AT91_ADC_TSMR_PENDBC_(pendbc) & AT91_ADC_TSMR_PENDBC;
>> +		reg |= AT91_ADC_TSMR_NOTSDMA;
>> +		reg |= AT91_ADC_TSMR_PENDET_ENA;
>> +		reg |= 0x03 << 8;	/* TSFREQ, need bigger than TSAV */
>> +
>> +		at91_adc_writel(st, AT91_ADC_TSMR, reg);
>> +	} else {
>> +		/* TODO: for 9g45 which has no TSMR */
>> +	}
>> +
>> +	/* Change adc internal resistor value for better pen detection,
>> +	 * default value is 100 kOhm.
>> +	 * 0 = 200 kOhm, 1 = 150 kOhm, 2 = 100 kOhm, 3 = 50 kOhm
>> +	 * option only available on ES2 and higher
>> +	 */
>> +	at91_adc_writel(st, AT91_ADC_ACR, st->caps->ts_pen_detect_sensitivity
>> +			& AT91_ADC_ACR_PENDETSENS);
>> +
>> +	/* Sample Peroid Time = (TRGPER + 1) / ADCClock */
>> +	st->ts_sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
>> +			adc_clk_khz / 1000) - 1, 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int at91_ts_register(struct at91_adc_state *st)
>> +{
>> +	struct input_dev *input;
>> +	struct iio_dev *idev = iio_priv_to_dev(st);
>> +	int ret;
>> +
>> +	input = input_allocate_device();
>> +	if (!input) {
>> +		dev_err(&idev->dev, "Failed to allocate TS device!\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	input->name = DRIVER_NAME;
>> +	input->id.bustype = BUS_HOST;
>> +	input->dev.parent = idev->dev.parent;
> Why you are setting parent to the IIO's parent? This is confusing, maybe
> you should simply pass the parent platform device into this fucntion.

Actually In this case the idev->dev.parent is '&pdev->dev'.
So I will add one more parameter "pdev" for at91_ts_register() function.

>
>> +	input->open = atmel_ts_open;
>> +	input->close = atmel_ts_close;
>> +
>> +	__set_bit(EV_ABS, input->evbit);
>> +	__set_bit(EV_KEY, input->evbit);
>> +	__set_bit(BTN_TOUCH, input->keybit);
>> +	input_set_abs_params(input, ABS_X, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
>> +	input_set_abs_params(input, ABS_Y, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
>> +	input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
>> +
>> +	st->ts_input = input;
>> +	input_set_drvdata(input, st);
>> +
>> +	ret = input_register_device(input);
>> +	if (ret)
>> +		input_free_device(st->ts_input);
>> +
>> +	return ret;
>> +}
>> +
>> +static void at91_ts_unregister(struct at91_adc_state *st)
>> +{
>> +	input_unregister_device(st->ts_input);
>> +	input_free_device(st->ts_input);
> No free after unregister please.

I'll fix it in next version.

>
>> +}
>> +
>>   static int at91_adc_probe(struct platform_device *pdev)
>>   {
>>   	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
>> @@ -648,7 +950,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>>   	at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST);
>>   	at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF);
>>   	ret = request_irq(st->irq,
>> -			  at91_adc_eoc_trigger,
>> +			  at91_adc_interrupt,
>>   			  0,
>>   			  pdev->dev.driver->name,
>>   			  idev);
>> @@ -693,6 +995,10 @@ static int at91_adc_probe(struct platform_device *pdev)
>>   	mstrclk = clk_get_rate(st->clk);
>>   	adc_clk = clk_get_rate(st->adc_clk);
>>   	adc_clk_khz = adc_clk / 1000;
>> +
>> +	dev_dbg(&pdev->dev, "Master clock is set as: %d Hz, adc_clk should set as: %d Hz\n",
>> +		mstrclk, adc_clk);
>> +
>>   	prsc = (mstrclk / (2 * adc_clk)) - 1;
>>   
>>   	if (!st->startup_time) {
>> @@ -761,30 +1067,52 @@ static int at91_adc_probe(struct platform_device *pdev)
>>   	init_waitqueue_head(&st->wq_data_avail);
>>   	mutex_init(&st->lock);
>>   
>> -	ret = at91_adc_buffer_init(idev);
>> -	if (ret < 0) {
>> -		dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
>> -		goto error_disable_adc_clk;
>> -	}
>> +	/*
>> +	 * Since touch screen will set trigger register as period trigger. So
>> +	 * when touch screen is enabled, then we have to disable hardware
>> +	 * trigger for classic adc.
>> +	 */
>> +	if (!st->touchscreen_type) {
>> +		ret = at91_adc_buffer_init(idev);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
>> +			goto error_disable_adc_clk;
>> +		}
>>   
>> -	ret = at91_adc_trigger_init(idev);
>> -	if (ret < 0) {
>> -		dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
>> -		goto error_unregister_buffer;
>> +		ret = at91_adc_trigger_init(idev);
>> +		if (ret < 0) {
>> +			dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
>> +			at91_adc_buffer_remove(idev);
>> +			goto error_disable_adc_clk;
>> +		}
>> +	} else {
>> +		if (!st->caps->has_tsmr) {
>> +			dev_err(&pdev->dev, "We don't support non-TSMR adc\n");
>> +			goto error_disable_adc_clk;
>> +		}
>> +
>> +		ret = at91_ts_register(st);
>> +		if (ret)
>> +			goto error_disable_adc_clk;
>> +
>> +		at91_ts_hw_init(st, adc_clk_khz);
>>   	}
>>   
>>   	ret = iio_device_register(idev);
>>   	if (ret < 0) {
>>   		dev_err(&pdev->dev, "Couldn't register the device.\n");
>> -		goto error_remove_triggers;
>> +		goto error_iio_device_register;
>>   	}
>>   
>>   	return 0;
>>   
>> -error_remove_triggers:
>> -	at91_adc_trigger_remove(idev);
>> -error_unregister_buffer:
>> -	at91_adc_buffer_remove(idev);
>> +error_iio_device_register:
>> +	if (!st->touchscreen_type) {
>> +		at91_adc_trigger_remove(idev);
>> +		at91_adc_buffer_remove(idev);
>> +	} else {
>> +		at91_ts_unregister(st);
>> +	}
>>   error_disable_adc_clk:
>>   	clk_disable_unprepare(st->adc_clk);
>>   error_disable_clk:
>> @@ -803,8 +1131,12 @@ static int at91_adc_remove(struct platform_device *pdev)
>>   	struct at91_adc_state *st = iio_priv(idev);
>>   
>>   	iio_device_unregister(idev);
>> -	at91_adc_trigger_remove(idev);
>> -	at91_adc_buffer_remove(idev);
>> +	if (!st->touchscreen_type) {
>> +		at91_adc_trigger_remove(idev);
>> +		at91_adc_buffer_remove(idev);
>> +	} else {
>> +		at91_ts_unregister(st);
>> +	}
>>   	clk_disable_unprepare(st->adc_clk);
>>   	clk_disable_unprepare(st->clk);
>>   	free_irq(st->irq, idev);
>> @@ -820,11 +1152,16 @@ static struct at91_adc_caps at91sam9260_caps = {
>>   };
>>   
>>   static struct at91_adc_caps at91sam9g45_caps = {
>> +	.has_ts = true,
>>   	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>>   	.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>>   };
>>   
>>   static struct at91_adc_caps at91sam9x5_caps = {
>> +	.has_ts = true,
>> +	.has_tsmr = true,
>> +	.ts_filter_average = 3,
>> +	.ts_pen_detect_sensitivity = 2,
>>   	.start_time_use_lookup_tab = true,
>>   	.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,	/* same as 9G45 */
>>   	.mr_startup_mask = AT91_ADC_STARTUP_9X5,
>> @@ -843,7 +1180,7 @@ static struct platform_driver at91_adc_driver = {
>>   	.probe = at91_adc_probe,
>>   	.remove = at91_adc_remove,
>>   	.driver = {
>> -		   .name = "at91_adc",
>> +		   .name = DRIVER_NAME,
>>   		   .of_match_table = of_match_ptr(at91_adc_dt_ids),
>>   	},
>>   };
>> -- 
>> 1.7.10
>>
> Thanks.
>

Thank you.
Best Regards,
Josh Wu