am335x: TSC & ADC reworking including DT pieces, take 4

am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

I believe the whole thing should go via the MFD tree. It touches also
input & iio subsystem. I collected ACKs where I got some in the meantime.

I added Lee Jones because I hear no sign of life from Samuel.

The following changes since commit d683b96b072dc4680fc74964eca77e6a23d1fa6e:

  Linux 3.10-rc4 (2013-06-02 17:11:17 +0900)

are available in the git repository at:

  git://breakpoint.cc/bigeasy/linux tags/am335x_tsc-adc

for you to fetch changes up to fe12425dd7e93db2dfdfa4eb9289036100cb0338:

  iio/ti_am335x_adc: check if we found the value (2013-06-11 13:11:35 +0200)

----------------------------------------------------------------
A complete refurbished series inclunding:
- DT support for the MFD, TSC and ADC driver & platform device support,
  which has no users, has been killed.
- iio_map from last series is gone and replaced by proper nodes in the
  device tree.
- suspend fixes which means correct data structs are taken and no
  interrupt storm
- fifo split which should problem with TSC & ADC beeing used at the same
  time
- The ADC channels are now checked before blindly applied. That means the
  touch part reads X, Y and Z coordinates and does not mix them up. Same
  goes for the IIO ADC driver.
- The IIO ADC driver now creates files named in_voltageX_raw where X
  represents the ADC line instead of a number starting at 0. A read from
  this file can return -EBUSY in case touch is busy and the ADC didn't
  collect a value.

----------------------------------------------------------------
Pantelis Antoniou (2):
      iio/ti_tscadc: provide datasheet_name and scan_type
      mfd/ti_tscadc: deal with partial activation

Patil, Rachna (7):
      input/ti_am33x_tsc: Step enable bits made configurable
      input/ti_am33x_tsc: Order of TSC wires, made configurable
      input/ti_am33x_tsc: remove unwanted fifo flush
      input/ti_am33x_tsc: Add DT support
      iio/ti_am335x_adc: Add DT support
      mfd/ti_am335x_tscadc: Add DT support
      arm/am33xx: add TSC/ADC mfd device support

Sebastian Andrzej Siewior (13):
      mfd/ti_am335x_tscadc: remove regmap
      mfd & input & iio/ti_am335x_adc: use one structure for ti_tscadc_dev
      input/ti_am33x_tsc: remove platform_data support
      iio/ti_am335x_adc: remove platform_data support
      mfd/ti_am335x_tscadc: remove platform_data support
      input & mfd: ti_am335x_tsc remove remaining platform data pieces
      mfd & input/ti_am335x_tsc: rename device from tsc to TI-am335x-tsc
      mfd & iio/ti_am335x_adc: rename device from tiadc to TI-am335x-adc
      input/ti_am335x_adc: use only FIFO0 and clean up a little
      input/ti_am335x_tsc: ACK the HW_PEN irq in ISR
      input/ti_am335x_tsc: return IRQ_NONE if there was no IRQ for us
      iio/ti_am335x_adc: Allow to specify input line
      iio/ti_am335x_adc: check if we found the value

 .../bindings/input/touchscreen/ti-tsc-adc.txt      |   44 +++
 arch/arm/boot/dts/am335x-evm.dts                   |   14 +
 arch/arm/boot/dts/am33xx.dtsi                      |   18 ++
 drivers/iio/adc/ti_am335x_adc.c                    |  132 ++++++---
 drivers/input/touchscreen/ti_am335x_tsc.c          |  288 ++++++++++++++------
 drivers/mfd/ti_am335x_tscadc.c                     |  133 ++++++---
 include/linux/input/ti_am335x_tsc.h                |   23 --
 include/linux/mfd/ti_am335x_tscadc.h               |   43 +--
 include/linux/platform_data/ti_am335x_adc.h        |   14 -
 9 files changed, 494 insertions(+), 215 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
 delete mode 100644 include/linux/input/ti_am335x_tsc.h
 delete mode 100644 include/linux/platform_data/ti_am335x_adc.h

Sebastian

am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

I believe the whole thing should go via the MFD tree. It touches also
input & iio subsystem. I collected ACKs where I got some in the meantime.

I added Lee Jones because I hear no sign of life from Samuel.

The following changes since commit d683b96b072dc4680fc74964eca77e6a23d1fa6e:

  Linux 3.10-rc4 (2013-06-02 17:11:17 +0900)

are available in the git repository at:

  git://breakpoint.cc/bigeasy/linux tags/am335x_tsc-adc

for you to fetch changes up to fe12425dd7e93db2dfdfa4eb9289036100cb0338:

  iio/ti_am335x_adc: check if we found the value (2013-06-11 13:11:35 +0200)

----------------------------------------------------------------
A complete refurbished series inclunding:
- DT support for the MFD, TSC and ADC driver & platform device support,
  which has no users, has been killed.
- iio_map from last series is gone and replaced by proper nodes in the
  device tree.
- suspend fixes which means correct data structs are taken and no
  interrupt storm
- fifo split which should problem with TSC & ADC beeing used at the same
  time
- The ADC channels are now checked before blindly applied. That means the
  touch part reads X, Y and Z coordinates and does not mix them up. Same
  goes for the IIO ADC driver.
- The IIO ADC driver now creates files named in_voltageX_raw where X
  represents the ADC line instead of a number starting at 0. A read from
  this file can return -EBUSY in case touch is busy and the ADC didn't
  collect a value.

----------------------------------------------------------------
Pantelis Antoniou (2):
      iio/ti_tscadc: provide datasheet_name and scan_type
      mfd/ti_tscadc: deal with partial activation

Patil, Rachna (7):
      input/ti_am33x_tsc: Step enable bits made configurable
      input/ti_am33x_tsc: Order of TSC wires, made configurable
      input/ti_am33x_tsc: remove unwanted fifo flush
      input/ti_am33x_tsc: Add DT support
      iio/ti_am335x_adc: Add DT support
      mfd/ti_am335x_tscadc: Add DT support
      arm/am33xx: add TSC/ADC mfd device support

Sebastian Andrzej Siewior (13):
      mfd/ti_am335x_tscadc: remove regmap
      mfd & input & iio/ti_am335x_adc: use one structure for ti_tscadc_dev
      input/ti_am33x_tsc: remove platform_data support
      iio/ti_am335x_adc: remove platform_data support
      mfd/ti_am335x_tscadc: remove platform_data support
      input & mfd: ti_am335x_tsc remove remaining platform data pieces
      mfd & input/ti_am335x_tsc: rename device from tsc to TI-am335x-tsc
      mfd & iio/ti_am335x_adc: rename device from tiadc to TI-am335x-adc
      input/ti_am335x_adc: use only FIFO0 and clean up a little
      input/ti_am335x_tsc: ACK the HW_PEN irq in ISR
      input/ti_am335x_tsc: return IRQ_NONE if there was no IRQ for us
      iio/ti_am335x_adc: Allow to specify input line
      iio/ti_am335x_adc: check if we found the value

 .../bindings/input/touchscreen/ti-tsc-adc.txt      |   44 +++
 arch/arm/boot/dts/am335x-evm.dts                   |   14 +
 arch/arm/boot/dts/am33xx.dtsi                      |   18 ++
 drivers/iio/adc/ti_am335x_adc.c                    |  132 ++++++---
 drivers/input/touchscreen/ti_am335x_tsc.c          |  288 ++++++++++++++------
 drivers/mfd/ti_am335x_tscadc.c                     |  133 ++++++---
 include/linux/input/ti_am335x_tsc.h                |   23 --
 include/linux/mfd/ti_am335x_tscadc.h               |   43 +--
 include/linux/platform_data/ti_am335x_adc.h        |   14 -
 9 files changed, 494 insertions(+), 215 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
 delete mode 100644 include/linux/input/ti_am335x_tsc.h
 delete mode 100644 include/linux/platform_data/ti_am335x_adc.h

Sebastian

[PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

The MFD part uses regmap without caching and is the only user of the
regmap. The child drivers, that is input(touch) and iio(adc), use direct
reg access.
There is a patch which converts them to use regmap as well but I see no
benefit at all doing this. There is a direct MMIO bus access with no
need to cache values like for I2C or SPI devices. Furthermore, most (if
not all) of the access done by the touch & ADC driver read volatile
register.
Therefore this patch removes regmap part of the driver.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mfd/ti_am335x_tscadc.c       |   23 ++---------------------
 include/linux/mfd/ti_am335x_tscadc.h |    1 -
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index e9f3fb5..804e61e 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -19,7 +19,6 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/clk.h>
-#include <linux/regmap.h>
 #include <linux/mfd/core.h>
 #include <linux/pm_runtime.h>
 
@@ -29,25 +28,15 @@
 
 static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int reg)
 {
-	unsigned int val;
-
-	regmap_read(tsadc->regmap_tscadc, reg, &val);
-	return val;
+	return readl(tsadc->tscadc_base + reg);
 }
 
 static void tscadc_writel(struct ti_tscadc_dev *tsadc, unsigned int reg,
 					unsigned int val)
 {
-	regmap_write(tsadc->regmap_tscadc, reg, val);
+	writel(val, tsadc->tscadc_base + reg);
 }
 
-static const struct regmap_config tscadc_regmap_config = {
-	.name = "ti_tscadc",
-	.reg_bits = 32,
-	.reg_stride = 4,
-	.val_bits = 32,
-};
-
 static void tscadc_idle_config(struct ti_tscadc_dev *config)
 {
 	unsigned int idleconfig;
@@ -121,14 +110,6 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	tscadc->regmap_tscadc = devm_regmap_init_mmio(&pdev->dev,
-			tscadc->tscadc_base, &tscadc_regmap_config);
-	if (IS_ERR(tscadc->regmap_tscadc)) {
-		dev_err(&pdev->dev, "regmap init failed\n");
-		err = PTR_ERR(tscadc->regmap_tscadc);
-		goto ret;
-	}
-
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index c79ad5d..dc75c34 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -137,7 +137,6 @@ struct mfd_tscadc_board {
 
 struct ti_tscadc_dev {
 	struct device *dev;
-	struct regmap *regmap_tscadc;
 	void __iomem *tscadc_base;
 	int irq;
 	struct mfd_cell cells[TSCADC_CELLS];
-- 
1.7.10.4

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 01:30:47PM +0200, Sebastian Andrzej Siewior wrote:
> The MFD part uses regmap without caching and is the only user of the
> regmap. The child drivers, that is input(touch) and iio(adc), use direct
> reg access.
> There is a patch which converts them to use regmap as well but I see no
> benefit at all doing this. There is a direct MMIO bus access with no
> need to cache values like for I2C or SPI devices. Furthermore, most (if
> not all) of the access done by the touch & ADC driver read volatile
> register.
> Therefore this patch removes regmap part of the driver.
NAK. Using regmap is better than open coding your register accesses, and
the children not using this API is not a reason for the MFD driver to do
the same.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

>> Therefore this patch removes regmap part of the driver.
> NAK. Using regmap is better than open coding your register accesses, and
> the children not using this API is not a reason for the MFD driver to do
> the same.

There is no advantage over using regmap in the first place. It goes
through a few layers, uses no caching because almost all registers are
volatile and this is a direct bus. In the end it complicates more than
it helps.

> 
> Cheers,
> Samuel.
> 

Sebastian

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

>> Therefore this patch removes regmap part of the driver.
> NAK. Using regmap is better than open coding your register accesses, and
> the children not using this API is not a reason for the MFD driver to do
> the same.

There is no advantage over using regmap in the first place. It goes
through a few layers, uses no caching because almost all registers are
volatile and this is a direct bus. In the end it complicates more than
it helps.

> 
> Cheers,
> Samuel.
> 

Sebastian

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Mark Brown]

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

On Tue, Jun 11, 2013 at 04:34:53PM +0200, Sebastian Andrzej Siewior wrote:

> >> Therefore this patch removes regmap part of the driver.

> > NAK. Using regmap is better than open coding your register accesses, and
> > the children not using this API is not a reason for the MFD driver to do
> > the same.

> There is no advantage over using regmap in the first place. It goes
> through a few layers, uses no caching because almost all registers are
> volatile and this is a direct bus. In the end it complicates more than
> it helps.

It does give you tracepoints and debugfs.  If it's making things at all
complicated we need to look at why that is and figure out how to fix
that since it's probably an issue for other users.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Mark Brown]

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

On Tue, Jun 11, 2013 at 04:34:53PM +0200, Sebastian Andrzej Siewior wrote:

> >> Therefore this patch removes regmap part of the driver.

> > NAK. Using regmap is better than open coding your register accesses, and
> > the children not using this API is not a reason for the MFD driver to do
> > the same.

> There is no advantage over using regmap in the first place. It goes
> through a few layers, uses no caching because almost all registers are
> volatile and this is a direct bus. In the end it complicates more than
> it helps.

It does give you tracepoints and debugfs.  If it's making things at all
complicated we need to look at why that is and figure out how to fix
that since it's probably an issue for other users.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

On 06/14/2013 03:53 PM, Mark Brown wrote:
> On Tue, Jun 11, 2013 at 04:34:53PM +0200, Sebastian Andrzej Siewior
> wrote:
> 
> It does give you tracepoints and debugfs.  If it's making things at
> all complicated we need to look at why that is and figure out how
> to fix that since it's probably an issue for other users.

debugfs are tracepoints is our offer? Let me check the price one more
time.

A simply mmio read does right now:
- lock + unlock.
  each time you chase another pointer plus enable/disable interrupts
  plus you have to save flags in another structure.

- _regmap_read()
  We check a few variables and then we go after reg_read and we end up
  in _regmap_raw_read().
  Here we call _regmap_range_lookup() which should return NULL. Next
  thing we invoke map->format.format_reg(). Finally we can call
  map->bus->read() which brings us to regmap_mmio_read().
  At the end we invoke map->format.parse_val().

write looks most likely the same.

A simple register read invokes 5 functions pointers. I am not counting
the function arguments in between and I am also not counting the number
of arguments which are involved and take pointer as well. This is a lot
of stuff that is done for a simple read of an mmio.

I understand that most of this may not be expensive in total if it
comes to SPI or I2C and all the goodies like reg caching and one
interface which deals with SPI and I2C. I also understand that some
chips have a non standard interface and are either BE or LE. We have
similar things on USB where people wired the BUS wrongly either at the
BUS level or at the SoC level so some PowerPC have an in-core ehci
controller but its registers are BE and not LE like it should be. The
solution here was variable check a simple swap() in that case.
I like abstractions but this gone a little too far I think.

This is a lot of for a simple mmio access. In terms of performance: If
I add a trace point to my read and write I have still less code which
is called and it can be disabled. This regmap overhead is always there
chasing pointers.

As I said before: I doubt that you get this regmap access in an one GiB
network driver and in turn remove their trace points to register access.

Please don't get me wrong: It is still nice for slow buses and this ADC
driver isn't anything close to high performance like a 1GiB network
driver but it adds a lot of unwanted overhead which I prefer not to
have.

Sebastian

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

On 06/14/2013 03:53 PM, Mark Brown wrote:
> On Tue, Jun 11, 2013 at 04:34:53PM +0200, Sebastian Andrzej Siewior
> wrote:
> 
> It does give you tracepoints and debugfs.  If it's making things at
> all complicated we need to look at why that is and figure out how
> to fix that since it's probably an issue for other users.

debugfs are tracepoints is our offer? Let me check the price one more
time.

A simply mmio read does right now:
- lock + unlock.
  each time you chase another pointer plus enable/disable interrupts
  plus you have to save flags in another structure.

- _regmap_read()
  We check a few variables and then we go after reg_read and we end up
  in _regmap_raw_read().
  Here we call _regmap_range_lookup() which should return NULL. Next
  thing we invoke map->format.format_reg(). Finally we can call
  map->bus->read() which brings us to regmap_mmio_read().
  At the end we invoke map->format.parse_val().

write looks most likely the same.

A simple register read invokes 5 functions pointers. I am not counting
the function arguments in between and I am also not counting the number
of arguments which are involved and take pointer as well. This is a lot
of stuff that is done for a simple read of an mmio.

I understand that most of this may not be expensive in total if it
comes to SPI or I2C and all the goodies like reg caching and one
interface which deals with SPI and I2C. I also understand that some
chips have a non standard interface and are either BE or LE. We have
similar things on USB where people wired the BUS wrongly either at the
BUS level or at the SoC level so some PowerPC have an in-core ehci
controller but its registers are BE and not LE like it should be. The
solution here was variable check a simple swap() in that case.
I like abstractions but this gone a little too far I think.

This is a lot of for a simple mmio access. In terms of performance: If
I add a trace point to my read and write I have still less code which
is called and it can be disabled. This regmap overhead is always there
chasing pointers.

As I said before: I doubt that you get this regmap access in an one GiB
network driver and in turn remove their trace points to register access.

Please don't get me wrong: It is still nice for slow buses and this ADC
driver isn't anything close to high performance like a 1GiB network
driver but it adds a lot of unwanted overhead which I prefer not to
have.

Sebastian

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Mark Brown]

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

On Mon, Jun 17, 2013 at 01:41:40PM +0200, Sebastian Andrzej Siewior wrote:
> On 06/14/2013 03:53 PM, Mark Brown wrote:

> > It does give you tracepoints and debugfs.  If it's making things at
> > all complicated we need to look at why that is and figure out how
> > to fix that since it's probably an issue for other users.

> debugfs are tracepoints is our offer? Let me check the price one more
> time.

OK...

> This is a lot of for a simple mmio access. In terms of performance: If
> I add a trace point to my read and write I have still less code which
> is called and it can be disabled. This regmap overhead is always there
> chasing pointers.

Equally well what you're implementing here is support for something
that's typically implemented with an I2C or SPI control interface so
you're already going to be quite a way ahead of the norm.  This is part
of what's confusing me, usually for this application things aren't that
bad performance wise even on a massively slower bus.

If all you're saying here is that there's some overhead that's fine if a
bit surprising, but the way you're talking made it sound like there was
some issue that made the API actually unusable.

> As I said before: I doubt that you get this regmap access in an one GiB
> network driver and in turn remove their trace points to register access.

Well, of course for that sort of thing the general trick is not to talk
to the hardware at all and do as much as possible with memory that the
hardware can DMA - there's actually still non-trivial costs in talking
over the buses.

> Please don't get me wrong: It is still nice for slow buses and this ADC
> driver isn't anything close to high performance like a 1GiB network
> driver but it adds a lot of unwanted overhead which I prefer not to
> have.

OK, but equally well remember that from a subsystem maintainer point of
view having things factored out is a win in itself; for example with the
MFDs locking on the register I/O has been a persistent issue in the past.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Mark Brown]

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

On Mon, Jun 17, 2013 at 01:41:40PM +0200, Sebastian Andrzej Siewior wrote:
> On 06/14/2013 03:53 PM, Mark Brown wrote:

> > It does give you tracepoints and debugfs.  If it's making things at
> > all complicated we need to look at why that is and figure out how
> > to fix that since it's probably an issue for other users.

> debugfs are tracepoints is our offer? Let me check the price one more
> time.

OK...

> This is a lot of for a simple mmio access. In terms of performance: If
> I add a trace point to my read and write I have still less code which
> is called and it can be disabled. This regmap overhead is always there
> chasing pointers.

Equally well what you're implementing here is support for something
that's typically implemented with an I2C or SPI control interface so
you're already going to be quite a way ahead of the norm.  This is part
of what's confusing me, usually for this application things aren't that
bad performance wise even on a massively slower bus.

If all you're saying here is that there's some overhead that's fine if a
bit surprising, but the way you're talking made it sound like there was
some issue that made the API actually unusable.

> As I said before: I doubt that you get this regmap access in an one GiB
> network driver and in turn remove their trace points to register access.

Well, of course for that sort of thing the general trick is not to talk
to the hardware at all and do as much as possible with memory that the
hardware can DMA - there's actually still non-trivial costs in talking
over the buses.

> Please don't get me wrong: It is still nice for slow buses and this ADC
> driver isn't anything close to high performance like a 1GiB network
> driver but it adds a lot of unwanted overhead which I prefer not to
> have.

OK, but equally well remember that from a subsystem maintainer point of
view having things factored out is a win in itself; for example with the
MFDs locking on the register I/O has been a persistent issue in the past.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

Sorry for the long pause…

On 06/17/2013 06:03 PM, Mark Brown wrote:
>> This is a lot of for a simple mmio access. In terms of
>> performance: If I add a trace point to my read and write I have
>> still less code which is called and it can be disabled. This
>> regmap overhead is always there chasing pointers.
> 
> Equally well what you're implementing here is support for
> something that's typically implemented with an I2C or SPI control
> interface so you're already going to be quite a way ahead of the
> norm.  This is part of what's confusing me, usually for this
> application things aren't that bad performance wise even on a
> massively slower bus.
> 
> If all you're saying here is that there's some overhead that's fine
> if a bit surprising, but the way you're talking made it sound like
> there was some issue that made the API actually unusable.

No, sorry for that confusion. I had a problem with the locking in irq
context, changed the driver & pointed out the problem, been told that
there is a fix, applied it, never complained again.

This is simply about I am forced to use regmap and I don't agree with
it.

>> As I said before: I doubt that you get this regmap access in an
>> one GiB network driver and in turn remove their trace points to
>> register access.
> 
> Well, of course for that sort of thing the general trick is not to
> talk to the hardware at all and do as much as possible with memory
> that the hardware can DMA - there's actually still non-trivial
> costs in talking over the buses.

That is true but even while doing DMA you have enough handshake with
the HW to trigger the transfer and if you are lucky your DMA
descriptors are in cached memory.

>> Please don't get me wrong: It is still nice for slow buses and
>> this ADC driver isn't anything close to high performance like a
>> 1GiB network driver but it adds a lot of unwanted overhead which
>> I prefer not to have.
> 
> OK, but equally well remember that from a subsystem maintainer
> point of view having things factored out is a win in itself; for
> example with the MFDs locking on the register I/O has been a
> persistent issue in the past.

I agree with that. But:

The driver here does not use atomic updates but read followed by write
so your locking here is futile. So the API/regmap alone does not make
it right. And look: the MFD part uses regmap. Its children (IIO &
input) do not use it. After I told this Samuel he said that it is okay.
So here I am. Using regmap in MFD which is only used once on init and
never again. It has regmap.

The register access in both child driver is split making sure they do
not use the same ones. I added one function which writes a common
register. That one is reset after an operation and needs to be written
by the currently active child so the HW continues to work. Haven't seen
anything close to it in regmap.

I ask, politely I hope, to get this patch in and remove regmap since
none of its features are used in this driver.

Sebastian

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

Sorry for the long pause…

On 06/17/2013 06:03 PM, Mark Brown wrote:
>> This is a lot of for a simple mmio access. In terms of
>> performance: If I add a trace point to my read and write I have
>> still less code which is called and it can be disabled. This
>> regmap overhead is always there chasing pointers.
> 
> Equally well what you're implementing here is support for
> something that's typically implemented with an I2C or SPI control
> interface so you're already going to be quite a way ahead of the
> norm.  This is part of what's confusing me, usually for this
> application things aren't that bad performance wise even on a
> massively slower bus.
> 
> If all you're saying here is that there's some overhead that's fine
> if a bit surprising, but the way you're talking made it sound like
> there was some issue that made the API actually unusable.

No, sorry for that confusion. I had a problem with the locking in irq
context, changed the driver & pointed out the problem, been told that
there is a fix, applied it, never complained again.

This is simply about I am forced to use regmap and I don't agree with
it.

>> As I said before: I doubt that you get this regmap access in an
>> one GiB network driver and in turn remove their trace points to
>> register access.
> 
> Well, of course for that sort of thing the general trick is not to
> talk to the hardware at all and do as much as possible with memory
> that the hardware can DMA - there's actually still non-trivial
> costs in talking over the buses.

That is true but even while doing DMA you have enough handshake with
the HW to trigger the transfer and if you are lucky your DMA
descriptors are in cached memory.

>> Please don't get me wrong: It is still nice for slow buses and
>> this ADC driver isn't anything close to high performance like a
>> 1GiB network driver but it adds a lot of unwanted overhead which
>> I prefer not to have.
> 
> OK, but equally well remember that from a subsystem maintainer
> point of view having things factored out is a win in itself; for
> example with the MFDs locking on the register I/O has been a
> persistent issue in the past.

I agree with that. But:

The driver here does not use atomic updates but read followed by write
so your locking here is futile. So the API/regmap alone does not make
it right. And look: the MFD part uses regmap. Its children (IIO &
input) do not use it. After I told this Samuel he said that it is okay.
So here I am. Using regmap in MFD which is only used once on init and
never again. It has regmap.

The register access in both child driver is split making sure they do
not use the same ones. I added one function which writes a common
register. That one is reset after an operation and needs to be written
by the currently active child so the HW continues to work. Haven't seen
anything close to it in regmap.

I ask, politely I hope, to get this patch in and remove regmap since
none of its features are used in this driver.

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

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Mark Brown]

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

On Thu, Jul 04, 2013 at 11:02:41AM +0200, Sebastian Andrzej Siewior wrote:

> The driver here does not use atomic updates but read followed by write
> so your locking here is futile. So the API/regmap alone does not make

Doesn't that sound like the driver ought to be using a r/m/w primitive
though?

> it right. And look: the MFD part uses regmap. Its children (IIO &
> input) do not use it. After I told this Samuel he said that it is okay.

Again I think the point here was that they probably ought to do so.

But I guess if you're saying there's no problem that's fine...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

On 07/04/2013 12:45 PM, Mark Brown wrote:
> On Thu, Jul 04, 2013 at 11:02:41AM +0200, Sebastian Andrzej Siewior
> wrote:
> 
>> The driver here does not use atomic updates but read followed by
>> write so your locking here is futile. So the API/regmap alone
>> does not make
> 
> Doesn't that sound like the driver ought to be using a r/m/w
> primitive though?

It does this in the init phase before the child devices are created so
no harm is done. I just wanted to say that regmap alone does not help
as long as the use simply replaces all reads & writes with regmap reads
& writes.

> 
>> it right. And look: the MFD part uses regmap. Its children (IIO
>> & input) do not use it. After I told this Samuel he said that it
>> is okay.
> 
> Again I think the point here was that they probably ought to do
> so.

It didn't sound that way.

> But I guess if you're saying there's no problem that's fine...
Thank you.

Samuel, is it okay if I repost the patch? It can wait till past -rc1 if
you are willing to take it.

Sebastian

Re: [PATCH 01/22] mfd/ti_am335x_tscadc: remove regmap

[Sebastian Andrzej Siewior]

On 07/04/2013 12:45 PM, Mark Brown wrote:
> On Thu, Jul 04, 2013 at 11:02:41AM +0200, Sebastian Andrzej Siewior
> wrote:
> 
>> The driver here does not use atomic updates but read followed by
>> write so your locking here is futile. So the API/regmap alone
>> does not make
> 
> Doesn't that sound like the driver ought to be using a r/m/w
> primitive though?

It does this in the init phase before the child devices are created so
no harm is done. I just wanted to say that regmap alone does not help
as long as the use simply replaces all reads & writes with regmap reads
& writes.

> 
>> it right. And look: the MFD part uses regmap. Its children (IIO
>> & input) do not use it. After I told this Samuel he said that it
>> is okay.
> 
> Again I think the point here was that they probably ought to do
> so.

It didn't sound that way.

> But I guess if you're saying there's no problem that's fine...
Thank you.

Samuel, is it okay if I repost the patch? It can wait till past -rc1 if
you are willing to take it.

Sebastian

[PATCH 02/22] mfd & input & iio/ti_am335x_adc: use one structure for ti_tscadc_dev

[Sebastian Andrzej Siewior]

The mfd driver creates platform data for the child devices and it is the
ti_tscadc_dev struct. This struct is copied for the two devices.
The copy of the structure makes a common lock in this structure a little
less usefull. Therefore the platform data is not a pointer to the
structure and the same structure is used.
While doing the change I noticed that the suspend/resume code assumes
the wrong pointer for ti_tscadc_dev and this has been fixed as well.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c           |    5 +++--
 drivers/input/touchscreen/ti_am335x_tsc.c |   16 +++++++++-------
 drivers/mfd/ti_am335x_tscadc.c            |    8 ++++----
 include/linux/mfd/ti_am335x_tscadc.h      |    7 +++++++
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 5f9a7e7..9db352e 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -140,7 +140,7 @@ static int tiadc_probe(struct platform_device *pdev)
 {
 	struct iio_dev		*indio_dev;
 	struct tiadc_device	*adc_dev;
-	struct ti_tscadc_dev	*tscadc_dev = pdev->dev.platform_data;
+	struct ti_tscadc_dev	*tscadc_dev = ti_tscadc_dev_get(pdev);
 	struct mfd_tscadc_board	*pdata;
 	int			err;
 
@@ -205,9 +205,10 @@ static int tiadc_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
-	struct ti_tscadc_dev *tscadc_dev = dev->platform_data;
+	struct ti_tscadc_dev *tscadc_dev;
 	unsigned int idle;
 
+	tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
 	if (!device_may_wakeup(tscadc_dev->dev)) {
 		idle = tiadc_readl(adc_dev, REG_CTRL);
 		idle &= ~(CNTRLREG_TSCSSENB);
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 51e7b87..16077d3 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -262,7 +262,7 @@ static int titsc_probe(struct platform_device *pdev)
 {
 	struct titsc *ts_dev;
 	struct input_dev *input_dev;
-	struct ti_tscadc_dev *tscadc_dev = pdev->dev.platform_data;
+	struct ti_tscadc_dev *tscadc_dev = ti_tscadc_dev_get(pdev);
 	struct mfd_tscadc_board	*pdata;
 	int err;
 
@@ -329,8 +329,8 @@ static int titsc_probe(struct platform_device *pdev)
 
 static int titsc_remove(struct platform_device *pdev)
 {
-	struct ti_tscadc_dev *tscadc_dev = pdev->dev.platform_data;
-	struct titsc *ts_dev = tscadc_dev->tsc;
+	struct titsc *ts_dev = platform_get_drvdata(pdev);
+	u32 steps;
 
 	free_irq(ts_dev->irq, ts_dev);
 
@@ -344,10 +344,11 @@ static int titsc_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM
 static int titsc_suspend(struct device *dev)
 {
-	struct ti_tscadc_dev *tscadc_dev = dev->platform_data;
-	struct titsc *ts_dev = tscadc_dev->tsc;
+	struct titsc *ts_dev = dev_get_drvdata(dev);
+	struct ti_tscadc_dev *tscadc_dev;
 	unsigned int idle;
 
+	tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
 	if (device_may_wakeup(tscadc_dev->dev)) {
 		idle = titsc_readl(ts_dev, REG_IRQENABLE);
 		titsc_writel(ts_dev, REG_IRQENABLE,
@@ -359,9 +360,10 @@ static int titsc_suspend(struct device *dev)
 
 static int titsc_resume(struct device *dev)
 {
-	struct ti_tscadc_dev *tscadc_dev = dev->platform_data;
-	struct titsc *ts_dev = tscadc_dev->tsc;
+	struct titsc *ts_dev = dev_get_drvdata(dev);
+	struct ti_tscadc_dev *tscadc_dev;
 
+	tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
 	if (device_may_wakeup(tscadc_dev->dev)) {
 		titsc_writel(ts_dev, REG_IRQWAKEUP,
 				0x00);
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 804e61e..490b2bd 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -157,14 +157,14 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	/* TSC Cell */
 	cell = &tscadc->cells[TSC_CELL];
 	cell->name = "tsc";
-	cell->platform_data = tscadc;
-	cell->pdata_size = sizeof(*tscadc);
+	cell->platform_data = &tscadc;
+	cell->pdata_size = sizeof(tscadc);
 
 	/* ADC Cell */
 	cell = &tscadc->cells[ADC_CELL];
 	cell->name = "tiadc";
-	cell->platform_data = tscadc;
-	cell->pdata_size = sizeof(*tscadc);
+	cell->platform_data = &tscadc;
+	cell->pdata_size = sizeof(tscadc);
 
 	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
 			TSCADC_CELLS, NULL, 0, NULL);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index dc75c34..5e430fe 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -148,4 +148,11 @@ struct ti_tscadc_dev {
 	struct adc_device *adc;
 };
 
+static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
+{
+	struct ti_tscadc_dev **tscadc_dev = p->dev.platform_data;
+
+	return *tscadc_dev;
+}
+
 #endif
-- 
1.7.10.4

[PATCH 03/22] input/ti_am33x_tsc: Step enable bits made configurable

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

Current code has hard coded value written to
step enable bits. Now the bits are updated based
on how many steps are needed to be configured got
from platform data.

The user needs to take care not to exceed
the count more than 16. While using ADC and TSC
one should take care to set this parameter correctly.

Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: add lock, move to MFD, use in TSC & ADC]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c           |   20 ++++++++++++++++++--
 drivers/input/touchscreen/ti_am335x_tsc.c |   12 ++++++++++--
 drivers/mfd/ti_am335x_tscadc.c            |   29 ++++++++++++++++++++++++++++-
 include/linux/mfd/ti_am335x_tscadc.h      |    8 ++++++--
 4 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 9db352e..543b9c4 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -42,10 +42,20 @@ static void tiadc_writel(struct tiadc_device *adc, unsigned int reg,
 	writel(val, adc->mfd_tscadc->tscadc_base + reg);
 }
 
+static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
+{
+	u32 step_en;
+
+	step_en = ((1 << adc_dev->channels) - 1);
+	step_en <<= TOTAL_STEPS - adc_dev->channels + 1;
+	return step_en;
+}
+
 static void tiadc_step_config(struct tiadc_device *adc_dev)
 {
 	unsigned int stepconfig;
 	int i, channels = 0, steps;
+	u32 step_en;
 
 	/*
 	 * There are 16 configurable steps and 8 analog input
@@ -69,7 +79,8 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 				STEPCONFIG_OPENDLY);
 		channels++;
 	}
-	tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB);
+	step_en = get_adc_step_mask(adc_dev);
+	am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
 }
 
 static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
@@ -127,7 +138,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 		if (i == chan->channel)
 			*val = readx1 & 0xfff;
 	}
-	tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB);
+	am335x_tsc_se_update(adc_dev->mfd_tscadc);
 
 	return IIO_VAL_INT;
 }
@@ -191,10 +202,15 @@ static int tiadc_probe(struct platform_device *pdev)
 static int tiadc_remove(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	u32 step_en;
 
 	iio_device_unregister(indio_dev);
 	tiadc_channels_remove(indio_dev);
 
+	step_en = get_adc_step_mask(adc_dev);
+	am335x_tsc_se_clr(adc_dev->mfd_tscadc, step_en);
+
 	iio_device_free(indio_dev);
 
 	return 0;
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 16077d3..23d6a4d 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -57,6 +57,7 @@ static void titsc_writel(struct titsc *tsc, unsigned int reg,
 static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
+	unsigned int	stepenable = 0;
 	int i, total_steps;
 
 	/* Configure the Step registers */
@@ -128,7 +129,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2),
 			STEPCONFIG_OPENDLY);
 
-	titsc_writel(ts_dev, REG_SE, STPENB_STEPENB_TC);
+	/* The steps1 … end and bit 0 for TS_Charge */
+	stepenable = (1 << (total_steps + 2)) - 1;
+	am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
 }
 
 static void titsc_read_coordinates(struct titsc *ts_dev,
@@ -250,7 +253,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 
 	titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
 
-	titsc_writel(ts_dev, REG_SE, STPENB_STEPENB_TC);
+	am335x_tsc_se_update(ts_dev->mfd_tscadc);
 	return IRQ_HANDLED;
 }
 
@@ -334,6 +337,11 @@ static int titsc_remove(struct platform_device *pdev)
 
 	free_irq(ts_dev->irq, ts_dev);
 
+	/* total steps followed by the enable mask */
+	steps = 2 * ts_dev->steps_to_configure + 2;
+	steps = (1 << steps) - 1;
+	am335x_tsc_se_clr(ts_dev->mfd_tscadc, steps);
+
 	input_unregister_device(ts_dev->input);
 
 	platform_set_drvdata(pdev, NULL);
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 490b2bd..7b091c4 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -37,6 +37,32 @@ static void tscadc_writel(struct ti_tscadc_dev *tsadc, unsigned int reg,
 	writel(val, tsadc->tscadc_base + reg);
 }
 
+void am335x_tsc_se_update(struct ti_tscadc_dev *tsadc)
+{
+	tscadc_writel(tsadc, REG_SE, tsadc->reg_se_cache);
+}
+EXPORT_SYMBOL_GPL(am335x_tsc_se_update);
+
+void am335x_tsc_se_set(struct ti_tscadc_dev *tsadc, u32 val)
+{
+	spin_lock(&tsadc->reg_lock);
+	tsadc->reg_se_cache |= val;
+	spin_unlock(&tsadc->reg_lock);
+
+	am335x_tsc_se_update(tsadc);
+}
+EXPORT_SYMBOL_GPL(am335x_tsc_se_set);
+
+void am335x_tsc_se_clr(struct ti_tscadc_dev *tsadc, u32 val)
+{
+	spin_lock(&tsadc->reg_lock);
+	tsadc->reg_se_cache &= ~val;
+	spin_unlock(&tsadc->reg_lock);
+
+	am335x_tsc_se_update(tsadc);
+}
+EXPORT_SYMBOL_GPL(am335x_tsc_se_clr);
+
 static void tscadc_idle_config(struct ti_tscadc_dev *config)
 {
 	unsigned int idleconfig;
@@ -110,6 +136,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	spin_lock_init(&tscadc->reg_lock);
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -220,7 +247,7 @@ static int tscadc_resume(struct device *dev)
 			CNTRLREG_STEPID | CNTRLREG_4WIRE;
 	tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
 	tscadc_idle_config(tscadc_dev);
-	tscadc_writel(tscadc_dev, REG_SE, STPENB_STEPENB);
+	am335x_tsc_se_update(tscadc_dev);
 	restore = tscadc_readl(tscadc_dev, REG_CTRL);
 	tscadc_writel(tscadc_dev, REG_CTRL,
 			(restore | CNTRLREG_TSCSSENB));
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 5e430fe..eeead15 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -46,8 +46,6 @@
 /* Step Enable */
 #define STEPENB_MASK		(0x1FFFF << 0)
 #define STEPENB(val)		((val) << 0)
-#define STPENB_STEPENB		STEPENB(0x1FFFF)
-#define STPENB_STEPENB_TC	STEPENB(0x1FFF)
 
 /* IRQ enable */
 #define IRQENB_HW_PEN		BIT(0)
@@ -140,6 +138,8 @@ struct ti_tscadc_dev {
 	void __iomem *tscadc_base;
 	int irq;
 	struct mfd_cell cells[TSCADC_CELLS];
+	u32 reg_se_cache;
+	spinlock_t reg_lock;
 
 	/* tsc device */
 	struct titsc *tsc;
@@ -155,4 +155,8 @@ static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
 	return *tscadc_dev;
 }
 
+void am335x_tsc_se_update(struct ti_tscadc_dev *tsadc);
+void am335x_tsc_se_set(struct ti_tscadc_dev *tsadc, u32 val);
+void am335x_tsc_se_clr(struct ti_tscadc_dev *tsadc, u32 val);
+
 #endif
-- 
1.7.10.4

[PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

The current driver expected touchscreen input
wires(XP,XN,YP,YN) to be connected in a particular order.
Making changes to accept this as platform data.

Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: larger rework of the patch, no config[4][4] array, smaller
          sized config_inp, no regbit_map(), shrinked
	  titsc_config_wires(), no config redo on resume, config_inp and
	  friends are now u32]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |  102 ++++++++++++++++++++++++-----
 include/linux/input/ti_am335x_tsc.h       |   12 ++++
 include/linux/mfd/ti_am335x_tscadc.h      |   10 ++-
 3 files changed, 105 insertions(+), 19 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 23d6a4d..56c6220 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -33,6 +33,13 @@
 #define SEQ_SETTLE		275
 #define MAX_12BIT		((1 << 12) - 1)
 
+static const int config_pins[] = {
+	XPP,
+	XNN,
+	YPP,
+	YNN,
+};
+
 struct titsc {
 	struct input_dev	*input;
 	struct ti_tscadc_dev	*mfd_tscadc;
@@ -41,6 +48,9 @@ struct titsc {
 	unsigned int		x_plate_resistance;
 	bool			pen_down;
 	int			steps_to_configure;
+	u32			config_inp[4];
+	u32			bit_xp, bit_xn, bit_yp, bit_yn;
+	u32			inp_xp, inp_xn, inp_yp, inp_yn;
 };
 
 static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -54,6 +64,58 @@ static void titsc_writel(struct titsc *tsc, unsigned int reg,
 	writel(val, tsc->mfd_tscadc->tscadc_base + reg);
 }
 
+static int titsc_config_wires(struct titsc *ts_dev)
+{
+	u32 analog_line[4];
+	u32 wire_order[4];
+	int i, bit_cfg;
+
+	for (i = 0; i < 4; i++) {
+		/*
+		 * Get the order in which TSC wires are attached
+		 * w.r.t. each of the analog input lines on the EVM.
+		 */
+		analog_line[i] = (ts_dev->config_inp[i] & 0xF0) >> 4;
+		wire_order[i] = ts_dev->config_inp[i] & 0x0F;
+		if (WARN_ON(analog_line[i] > 7))
+			return -EINVAL;
+		if (WARN_ON(wire_order[i] > ARRAY_SIZE(config_pins)))
+			return -EINVAL;
+	}
+
+	for (i = 0; i < 4; i++) {
+		int an_line;
+		int wi_order;
+
+		an_line = analog_line[i];
+		wi_order = wire_order[i];
+		bit_cfg = config_pins[wi_order];
+		if (bit_cfg == 0)
+			return -EINVAL;
+		switch (wi_order) {
+		case 0:
+			ts_dev->bit_xp = bit_cfg;
+			ts_dev->inp_xp = an_line;
+			break;
+
+		case 1:
+			ts_dev->bit_xn = bit_cfg;
+			ts_dev->inp_xn = an_line;
+			break;
+
+		case 2:
+			ts_dev->bit_yp = bit_cfg;
+			ts_dev->inp_yp = an_line;
+			break;
+		case 3:
+			ts_dev->bit_yn = bit_cfg;
+			ts_dev->inp_yn = an_line;
+			break;
+		}
+	}
+	return 0;
+}
+
 static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
@@ -64,18 +126,18 @@ static void titsc_step_config(struct titsc *ts_dev)
 	total_steps = 2 * ts_dev->steps_to_configure;
 
 	config = STEPCONFIG_MODE_HWSYNC |
-			STEPCONFIG_AVG_16 | STEPCONFIG_XPP;
+			STEPCONFIG_AVG_16 | ts_dev->bit_xp;
 	switch (ts_dev->wires) {
 	case 4:
-		config |= STEPCONFIG_INP_AN2 | STEPCONFIG_XNN;
+		config |= STEPCONFIG_INP(ts_dev->inp_yp) | ts_dev->bit_xn;
 		break;
 	case 5:
-		config |= STEPCONFIG_YNN |
-				STEPCONFIG_INP_AN4 | STEPCONFIG_XNN |
-				STEPCONFIG_YPP;
+		config |= ts_dev->bit_yn |
+				STEPCONFIG_INP_AN4 | ts_dev->bit_xn |
+				ts_dev->bit_yp;
 		break;
 	case 8:
-		config |= STEPCONFIG_INP_AN2 | STEPCONFIG_XNN;
+		config |= STEPCONFIG_INP(ts_dev->inp_yp) | ts_dev->bit_xn;
 		break;
 	}
 
@@ -86,18 +148,18 @@ static void titsc_step_config(struct titsc *ts_dev)
 
 	config = 0;
 	config = STEPCONFIG_MODE_HWSYNC |
-			STEPCONFIG_AVG_16 | STEPCONFIG_YNN |
+			STEPCONFIG_AVG_16 | ts_dev->bit_yn |
 			STEPCONFIG_INM_ADCREFM | STEPCONFIG_FIFO1;
 	switch (ts_dev->wires) {
 	case 4:
-		config |= STEPCONFIG_YPP;
+		config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp);
 		break;
 	case 5:
-		config |= STEPCONFIG_XPP | STEPCONFIG_INP_AN4 |
-				STEPCONFIG_XNP | STEPCONFIG_YPN;
+		config |= ts_dev->bit_xp | STEPCONFIG_INP_AN4 |
+				ts_dev->bit_xn | ts_dev->bit_yp;
 		break;
 	case 8:
-		config |= STEPCONFIG_YPP;
+		config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp);
 		break;
 	}
 
@@ -108,9 +170,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 
 	config = 0;
 	/* Charge step configuration */
-	config = STEPCONFIG_XPP | STEPCONFIG_YNN |
+	config = ts_dev->bit_xp | ts_dev->bit_yn |
 			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
-			STEPCHARGE_INM_AN1 | STEPCHARGE_INP_AN1;
+			STEPCHARGE_INM_AN1 | STEPCHARGE_INP(ts_dev->inp_yp);
 
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
@@ -118,13 +180,14 @@ static void titsc_step_config(struct titsc *ts_dev)
 	config = 0;
 	/* Configure to calculate pressure */
 	config = STEPCONFIG_MODE_HWSYNC |
-			STEPCONFIG_AVG_16 | STEPCONFIG_YPP |
-			STEPCONFIG_XNN | STEPCONFIG_INM_ADCREFM;
+			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
+			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
+			STEPCONFIG_INP(ts_dev->inp_xp);
 	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 1), config);
 	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 1),
 			STEPCONFIG_OPENDLY);
 
-	config |= STEPCONFIG_INP_AN3 | STEPCONFIG_FIFO1;
+	config |= STEPCONFIG_INP(ts_dev->inp_yn) | STEPCONFIG_FIFO1;
 	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 2), config);
 	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2),
 			STEPCONFIG_OPENDLY);
@@ -292,6 +355,8 @@ static int titsc_probe(struct platform_device *pdev)
 	ts_dev->wires = pdata->tsc_init->wires;
 	ts_dev->x_plate_resistance = pdata->tsc_init->x_plate_resistance;
 	ts_dev->steps_to_configure = pdata->tsc_init->steps_to_configure;
+	memcpy(ts_dev->config_inp, pdata->tsc_init->wire_config,
+			sizeof(pdata->tsc_init->wire_config));
 
 	err = request_irq(ts_dev->irq, titsc_irq,
 			  0, pdev->dev.driver->name, ts_dev);
@@ -301,6 +366,11 @@ static int titsc_probe(struct platform_device *pdev)
 	}
 
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
+	err = titsc_config_wires(ts_dev);
+	if (err) {
+		dev_err(&pdev->dev, "wrong i/p wire configuration\n");
+		goto err_free_irq;
+	}
 	titsc_step_config(ts_dev);
 	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->steps_to_configure);
 
diff --git a/include/linux/input/ti_am335x_tsc.h b/include/linux/input/ti_am335x_tsc.h
index 49269a2..6a66b4d 100644
--- a/include/linux/input/ti_am335x_tsc.h
+++ b/include/linux/input/ti_am335x_tsc.h
@@ -12,12 +12,24 @@
  *			A step configured to read a single
  *			co-ordinate value, can be applied
  *			more number of times for better results.
+ * @wire_config:	Different EVM's could have a different order
+ *			for connecting wires on touchscreen.
+ *			We need to provide an 8 bit number where in
+ *			the 1st four bits represent the analog lines
+ *			and the next 4 bits represent positive/
+ *			negative terminal on that input line.
+ *			Notations to represent the input lines and
+ *			terminals resoectively is as follows:
+ *			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
+ *			XP  = 0, XN = 1, YP = 2, YN = 3.
+ *
  */
 
 struct tsc_data {
 	int wires;
 	int x_plate_resistance;
 	int steps_to_configure;
+	int wire_config[10];
 };
 
 #endif
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index eeead15..2d78af8 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -71,8 +71,6 @@
 #define STEPCONFIG_INM_ADCREFM	STEPCONFIG_INM(8)
 #define STEPCONFIG_INP_MASK	(0xF << 19)
 #define STEPCONFIG_INP(val)	((val) << 19)
-#define STEPCONFIG_INP_AN2	STEPCONFIG_INP(2)
-#define STEPCONFIG_INP_AN3	STEPCONFIG_INP(3)
 #define STEPCONFIG_INP_AN4	STEPCONFIG_INP(4)
 #define STEPCONFIG_INP_ADCREFM	STEPCONFIG_INP(8)
 #define STEPCONFIG_FIFO1	BIT(26)
@@ -94,7 +92,6 @@
 #define STEPCHARGE_INM_AN1	STEPCHARGE_INM(1)
 #define STEPCHARGE_INP_MASK	(0xF << 19)
 #define STEPCHARGE_INP(val)	((val) << 19)
-#define STEPCHARGE_INP_AN1	STEPCHARGE_INP(1)
 #define STEPCHARGE_RFM_MASK	(3 << 23)
 #define STEPCHARGE_RFM(val)	((val) << 23)
 #define STEPCHARGE_RFM_XNUR	STEPCHARGE_RFM(1)
@@ -116,6 +113,13 @@
 #define CNTRLREG_8WIRE		CNTRLREG_AFE_CTRL(3)
 #define CNTRLREG_TSCENB		BIT(7)
 
+#define XPP			STEPCONFIG_XPP
+#define XNP			STEPCONFIG_XNP
+#define XNN			STEPCONFIG_XNN
+#define YPP			STEPCONFIG_YPP
+#define YPN			STEPCONFIG_YPN
+#define YNN			STEPCONFIG_YNN
+
 #define ADC_CLK			3000000
 #define	MAX_CLK_DIV		7
 #define TOTAL_STEPS		16
-- 
1.7.10.4

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 01:30:50PM +0200, Sebastian Andrzej Siewior wrote:
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index eeead15..2d78af8 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -71,8 +71,6 @@
>  #define STEPCONFIG_INM_ADCREFM	STEPCONFIG_INM(8)
>  #define STEPCONFIG_INP_MASK	(0xF << 19)
>  #define STEPCONFIG_INP(val)	((val) << 19)
> -#define STEPCONFIG_INP_AN2	STEPCONFIG_INP(2)
> -#define STEPCONFIG_INP_AN3	STEPCONFIG_INP(3)
>  #define STEPCONFIG_INP_AN4	STEPCONFIG_INP(4)
>  #define STEPCONFIG_INP_ADCREFM	STEPCONFIG_INP(8)
>  #define STEPCONFIG_FIFO1	BIT(26)
> @@ -94,7 +92,6 @@
>  #define STEPCHARGE_INM_AN1	STEPCHARGE_INM(1)
>  #define STEPCHARGE_INP_MASK	(0xF << 19)
>  #define STEPCHARGE_INP(val)	((val) << 19)
> -#define STEPCHARGE_INP_AN1	STEPCHARGE_INP(1)
>  #define STEPCHARGE_RFM_MASK	(3 << 23)
>  #define STEPCHARGE_RFM(val)	((val) << 23)
>  #define STEPCHARGE_RFM_XNUR	STEPCHARGE_RFM(1)
> @@ -116,6 +113,13 @@
>  #define CNTRLREG_8WIRE		CNTRLREG_AFE_CTRL(3)
>  #define CNTRLREG_TSCENB		BIT(7)
>  
> +#define XPP			STEPCONFIG_XPP
> +#define XNP			STEPCONFIG_XNP
> +#define XNN			STEPCONFIG_XNN
> +#define YPP			STEPCONFIG_YPP
> +#define YPN			STEPCONFIG_YPN
> +#define YNN			STEPCONFIG_YNN
What is that for ? Isn't STEPCONFIG_XPP explicit enough ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 01:30:50PM +0200, Sebastian Andrzej Siewior wrote:
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index eeead15..2d78af8 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -71,8 +71,6 @@
>  #define STEPCONFIG_INM_ADCREFM	STEPCONFIG_INM(8)
>  #define STEPCONFIG_INP_MASK	(0xF << 19)
>  #define STEPCONFIG_INP(val)	((val) << 19)
> -#define STEPCONFIG_INP_AN2	STEPCONFIG_INP(2)
> -#define STEPCONFIG_INP_AN3	STEPCONFIG_INP(3)
>  #define STEPCONFIG_INP_AN4	STEPCONFIG_INP(4)
>  #define STEPCONFIG_INP_ADCREFM	STEPCONFIG_INP(8)
>  #define STEPCONFIG_FIFO1	BIT(26)
> @@ -94,7 +92,6 @@
>  #define STEPCHARGE_INM_AN1	STEPCHARGE_INM(1)
>  #define STEPCHARGE_INP_MASK	(0xF << 19)
>  #define STEPCHARGE_INP(val)	((val) << 19)
> -#define STEPCHARGE_INP_AN1	STEPCHARGE_INP(1)
>  #define STEPCHARGE_RFM_MASK	(3 << 23)
>  #define STEPCHARGE_RFM(val)	((val) << 23)
>  #define STEPCHARGE_RFM_XNUR	STEPCHARGE_RFM(1)
> @@ -116,6 +113,13 @@
>  #define CNTRLREG_8WIRE		CNTRLREG_AFE_CTRL(3)
>  #define CNTRLREG_TSCENB		BIT(7)
>  
> +#define XPP			STEPCONFIG_XPP
> +#define XNP			STEPCONFIG_XNP
> +#define XNN			STEPCONFIG_XNN
> +#define YPP			STEPCONFIG_YPP
> +#define YPN			STEPCONFIG_YPN
> +#define YNN			STEPCONFIG_YNN
What is that for ? Isn't STEPCONFIG_XPP explicit enough ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 01:30:50PM +0200, Sebastian Andrzej Siewior wrote:
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index eeead15..2d78af8 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -116,6 +113,13 @@
>>  #define CNTRLREG_8WIRE		CNTRLREG_AFE_CTRL(3)
>>  #define CNTRLREG_TSCENB		BIT(7)
>>  
>> +#define XPP			STEPCONFIG_XPP
>> +#define XNP			STEPCONFIG_XNP
>> +#define XNN			STEPCONFIG_XNN
>> +#define YPP			STEPCONFIG_YPP
>> +#define YPN			STEPCONFIG_YPN
>> +#define YNN			STEPCONFIG_YNN
> What is that for ? Isn't STEPCONFIG_XPP explicit enough ?

Yeah :P It was added by the original author of the patch, I have no
problem getting rid of it.

> 
> Cheers,
> Samuel.

Sebastian

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sekhar Nori]

On 6/11/2013 5:00 PM, Sebastian Andrzej Siewior wrote:
> From: "Patil, Rachna" <rachna@ti.com>
> 
> The current driver expected touchscreen input
> wires(XP,XN,YP,YN) to be connected in a particular order.
> Making changes to accept this as platform data

The platform data part of this driver will never get used since it is
used on DT-only platforms (and future platforms will all be DT-only).
You should get rid of it as it will save you some code.

Thanks,
Sekhar

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sekhar Nori]

On 6/11/2013 5:00 PM, Sebastian Andrzej Siewior wrote:
> From: "Patil, Rachna" <rachna-l0cyMroinI0@public.gmane.org>
> 
> The current driver expected touchscreen input
> wires(XP,XN,YP,YN) to be connected in a particular order.
> Making changes to accept this as platform data

The platform data part of this driver will never get used since it is
used on DT-only platforms (and future platforms will all be DT-only).
You should get rid of it as it will save you some code.

Thanks,
Sekhar

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sebastian Andrzej Siewior]

On 07/04/2013 01:14 PM, Sekhar Nori wrote:
> 
> On 6/11/2013 5:00 PM, Sebastian Andrzej Siewior wrote:
>> From: "Patil, Rachna" <rachna@ti.com>
>>
>> The current driver expected touchscreen input
>> wires(XP,XN,YP,YN) to be connected in a particular order.
>> Making changes to accept this as platform data
> 
> The platform data part of this driver will never get used since it is
> used on DT-only platforms (and future platforms will all be DT-only).
> You should get rid of it as it will save you some code.

If you follow the series you will notice that the platform bits are
removed later. Should I have overlooked something please say so.

> 
> Thanks,
> Sekhar
> 

Sebastian

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sebastian Andrzej Siewior]

On 07/04/2013 01:14 PM, Sekhar Nori wrote:
> 
> On 6/11/2013 5:00 PM, Sebastian Andrzej Siewior wrote:
>> From: "Patil, Rachna" <rachna-l0cyMroinI0@public.gmane.org>
>>
>> The current driver expected touchscreen input
>> wires(XP,XN,YP,YN) to be connected in a particular order.
>> Making changes to accept this as platform data
> 
> The platform data part of this driver will never get used since it is
> used on DT-only platforms (and future platforms will all be DT-only).
> You should get rid of it as it will save you some code.

If you follow the series you will notice that the platform bits are
removed later. Should I have overlooked something please say so.

> 
> Thanks,
> Sekhar
> 

Sebastian

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sekhar Nori]

On 7/4/2013 5:03 PM, Sebastian Andrzej Siewior wrote:
> On 07/04/2013 01:14 PM, Sekhar Nori wrote:
>>
>> On 6/11/2013 5:00 PM, Sebastian Andrzej Siewior wrote:
>>> From: "Patil, Rachna" <rachna@ti.com>
>>>
>>> The current driver expected touchscreen input
>>> wires(XP,XN,YP,YN) to be connected in a particular order.
>>> Making changes to accept this as platform data
>>
>> The platform data part of this driver will never get used since it is
>> used on DT-only platforms (and future platforms will all be DT-only).
>> You should get rid of it as it will save you some code.
> 
> If you follow the series you will notice that the platform bits are
> removed later. Should I have overlooked something please say so.

Yes, I noticed that after sending the mail. To me it does not make sense
to make changes to accept something as platform data only to remove
platform data itself later.

May be reorder the series to move this to after platform data removal -
that way any platform data related changes in the patch will have to go
away.

Thanks,
Sekhar

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sekhar Nori]

On 7/4/2013 5:03 PM, Sebastian Andrzej Siewior wrote:
> On 07/04/2013 01:14 PM, Sekhar Nori wrote:
>>
>> On 6/11/2013 5:00 PM, Sebastian Andrzej Siewior wrote:
>>> From: "Patil, Rachna" <rachna-l0cyMroinI0@public.gmane.org>
>>>
>>> The current driver expected touchscreen input
>>> wires(XP,XN,YP,YN) to be connected in a particular order.
>>> Making changes to accept this as platform data
>>
>> The platform data part of this driver will never get used since it is
>> used on DT-only platforms (and future platforms will all be DT-only).
>> You should get rid of it as it will save you some code.
> 
> If you follow the series you will notice that the platform bits are
> removed later. Should I have overlooked something please say so.

Yes, I noticed that after sending the mail. To me it does not make sense
to make changes to accept something as platform data only to remove
platform data itself later.

May be reorder the series to move this to after platform data removal -
that way any platform data related changes in the patch will have to go
away.

Thanks,
Sekhar

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sebastian Andrzej Siewior]

On 07/04/2013 03:39 PM, Sekhar Nori wrote:
> Yes, I noticed that after sending the mail. To me it does not make sense
> to make changes to accept something as platform data only to remove
> platform data itself later.

The patches were made earlier and it was easier that way to take
everything and simple remove the platform part later.

> May be reorder the series to move this to after platform data removal -
> that way any platform data related changes in the patch will have to go
> away.
> 
> Thanks,
> Sekhar

Sebastian

Re: [PATCH 04/22] input/ti_am33x_tsc: Order of TSC wires, made configurable

[Sekhar Nori]

On 7/4/2013 7:20 PM, Sebastian Andrzej Siewior wrote:
> On 07/04/2013 03:39 PM, Sekhar Nori wrote:
>> Yes, I noticed that after sending the mail. To me it does not make sense
>> to make changes to accept something as platform data only to remove
>> platform data itself later.
> 
> The patches were made earlier and it was easier that way to take
> everything and simple remove the platform part later.

Okay, then. If the maintainers do not have objection, I am fine too!

Regards,
Sekhar

[PATCH 05/22] input/ti_am33x_tsc: remove unwanted fifo flush

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

When touchscreen and ADC are used together, this
unwanted fifo flush leads to loss of ADC data.

Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 56c6220..b2f8a46 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -252,8 +252,6 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 	unsigned int x = 0, y = 0;
 	unsigned int z1, z2, z;
 	unsigned int fsm;
-	unsigned int fifo1count, fifo0count;
-	int i;
 
 	status = titsc_readl(ts_dev, REG_IRQSTATUS);
 	if (status & IRQENB_FIFO0THRES) {
@@ -262,14 +260,6 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 		z1 = titsc_readl(ts_dev, REG_FIFO0) & 0xfff;
 		z2 = titsc_readl(ts_dev, REG_FIFO1) & 0xfff;
 
-		fifo1count = titsc_readl(ts_dev, REG_FIFO1CNT);
-		for (i = 0; i < fifo1count; i++)
-			titsc_readl(ts_dev, REG_FIFO1);
-
-		fifo0count = titsc_readl(ts_dev, REG_FIFO0CNT);
-		for (i = 0; i < fifo0count; i++)
-			titsc_readl(ts_dev, REG_FIFO0);
-
 		if (ts_dev->pen_down && z1 != 0 && z2 != 0) {
 			/*
 			 * Calculate pressure using formula
-- 
1.7.10.4

[PATCH 06/22] input/ti_am33x_tsc: Add DT support

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

Add DT support for client touchscreen driver

[ panto@antoniou-consulting.com : use of_get_child_by_name
	instead of of_find_node_by_name ]

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[ bigeasy: shift the code to the left, shirnk titsc_parse_dt() by not
           using temp value, change a binding and document them in
	   ti-tsc-adc.txt which also contains ADC binding which will be
	   used later, addd DT binding to mfd driver so we our DT node
	   without of_get_child_by_name(), rename steps_to_configure to
	   coordinate_readouts because this is what it really does,
	   don't overrire err after calling  titsc_parse_dt() or
	   titsc_parse_pdata()]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 .../bindings/input/touchscreen/ti-tsc-adc.txt      |   44 ++++++++
 drivers/input/touchscreen/ti_am335x_tsc.c          |  105 +++++++++++++++-----
 drivers/mfd/ti_am335x_tscadc.c                     |    1 +
 3 files changed, 127 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
new file mode 100644
index 0000000..491c97b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
@@ -0,0 +1,44 @@
+* TI - TSC ADC (Touschscreen and analog digital converter)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Required properties:
+- child "tsc"
+	ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
+		  support on the platform.
+	ti,x-plate-resistance: X plate resistance
+	ti,coordiante-readouts: The sequencer supports a total of 16
+				programmable steps each step is used to
+				read a single coordinate. A single
+                                readout is enough but multiple reads can
+				increase the quality.
+				A value of 5 means, 5 reads for X, 5 for
+				Y and 2 for Z (always). This utilises 12
+				of the 16 software steps available. The
+				remaining 4 can be used by the ADC.
+	ti,wire-config: Different boards could have a different order for
+			connecting wires on touchscreen. We need to provide an
+			8 bit number where in the 1st four bits represent the
+			analog lines and the next 4 bits represent positive/
+			negative terminal on that input line. Notations to
+			represent the input lines and terminals resoectively
+			is as follows:
+			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
+			XP  = 0, XN = 1, YP = 2, YN = 3.
+- child "adc"
+	ti,adc-channels: List of analog inputs available for ADC.
+			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
+
+Example:
+	tscadc: tscadc@44e0d000 {
+		compatible = "ti,am3359-tscadc";
+		tsc {
+			ti,wires = <4>;
+			ti,x-plate-resistance = <200>;
+			ti,coordiante-readouts = <5>;
+			ti,wire-config = <0x00 0x11 0x22 0x33>;
+		};
+
+		adc {
+			ti,adc-channels = <4 5 6 7>;
+		};
+	}
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index b2f8a46..a44d2c4 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -26,6 +26,8 @@
 #include <linux/io.h>
 #include <linux/input/ti_am335x_tsc.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 
@@ -47,7 +49,7 @@ struct titsc {
 	unsigned int		wires;
 	unsigned int		x_plate_resistance;
 	bool			pen_down;
-	int			steps_to_configure;
+	int			coordinate_readouts;
 	u32			config_inp[4];
 	u32			bit_xp, bit_xn, bit_yp, bit_yn;
 	u32			inp_xp, inp_xn, inp_yp, inp_yn;
@@ -123,7 +125,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	int i, total_steps;
 
 	/* Configure the Step registers */
-	total_steps = 2 * ts_dev->steps_to_configure;
+	total_steps = 2 * ts_dev->coordinate_readouts;
 
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_xp;
@@ -141,7 +143,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = 1; i <= ts_dev->steps_to_configure; i++) {
+	for (i = 1; i <= ts_dev->coordinate_readouts; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -163,7 +165,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = (ts_dev->steps_to_configure + 1); i <= total_steps; i++) {
+	for (i = (ts_dev->coordinate_readouts + 1); i <= total_steps; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -218,7 +220,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 		read = titsc_readl(ts_dev, REG_FIFO0);
 		channel = read & 0xf0000;
 		channel = channel >> 0x10;
-		if ((channel >= 0) && (channel < ts_dev->steps_to_configure)) {
+		if ((channel >= 0) && (channel < ts_dev->coordinate_readouts)) {
 			read &= 0xfff;
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
@@ -231,8 +233,8 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 		read = titsc_readl(ts_dev, REG_FIFO1);
 		channel = read & 0xf0000;
 		channel = channel >> 0x10;
-		if ((channel >= ts_dev->steps_to_configure) &&
-			(channel < (2 * ts_dev->steps_to_configure - 1))) {
+		if ((channel >= ts_dev->coordinate_readouts) &&
+			(channel < (2 * ts_dev->coordinate_readouts - 1))) {
 			read &= 0xfff;
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
@@ -310,6 +312,59 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int titsc_parse_dt(struct platform_device *pdev,
+					struct titsc *ts_dev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	int err;
+
+	if (!node)
+		return -EINVAL;
+
+	err = of_property_read_u32(node, "ti,wires", &ts_dev->wires);
+	if (err < 0)
+		return err;
+	switch (ts_dev->wires) {
+	case 4:
+	case 5:
+	case 8:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(node, "ti,x-plate-resistance",
+			&ts_dev->x_plate_resistance);
+	if (err < 0)
+		return err;
+
+	err = of_property_read_u32(node, "ti,coordiante-readouts",
+			&ts_dev->coordinate_readouts);
+	if (err < 0)
+		return err;
+
+	return of_property_read_u32_array(node, "ti,wire-config",
+			ts_dev->config_inp, ARRAY_SIZE(ts_dev->config_inp));
+}
+
+static int titsc_parse_pdata(struct ti_tscadc_dev *tscadc_dev,
+					struct titsc *ts_dev)
+{
+	struct mfd_tscadc_board	*pdata = tscadc_dev->dev->platform_data;
+
+	if (!pdata)
+		return -EINVAL;
+
+	ts_dev->wires = pdata->tsc_init->wires;
+	ts_dev->x_plate_resistance =
+		pdata->tsc_init->x_plate_resistance;
+	ts_dev->steps_to_configure =
+		pdata->tsc_init->steps_to_configure;
+	memcpy(ts_dev->config_inp, pdata->tsc_init->wire_config,
+		sizeof(pdata->tsc_init->wire_config));
+	return 0;
+}
+
 /*
  * The functions for inserting/removing driver as a module.
  */
@@ -319,16 +374,8 @@ static int titsc_probe(struct platform_device *pdev)
 	struct titsc *ts_dev;
 	struct input_dev *input_dev;
 	struct ti_tscadc_dev *tscadc_dev = ti_tscadc_dev_get(pdev);
-	struct mfd_tscadc_board	*pdata;
 	int err;
 
-	pdata = tscadc_dev->dev->platform_data;
-
-	if (!pdata) {
-		dev_err(&pdev->dev, "Could not find platform data\n");
-		return -EINVAL;
-	}
-
 	/* Allocate memory for device */
 	ts_dev = kzalloc(sizeof(struct titsc), GFP_KERNEL);
 	input_dev = input_allocate_device();
@@ -342,11 +389,16 @@ static int titsc_probe(struct platform_device *pdev)
 	ts_dev->mfd_tscadc = tscadc_dev;
 	ts_dev->input = input_dev;
 	ts_dev->irq = tscadc_dev->irq;
-	ts_dev->wires = pdata->tsc_init->wires;
-	ts_dev->x_plate_resistance = pdata->tsc_init->x_plate_resistance;
-	ts_dev->steps_to_configure = pdata->tsc_init->steps_to_configure;
-	memcpy(ts_dev->config_inp, pdata->tsc_init->wire_config,
-			sizeof(pdata->tsc_init->wire_config));
+
+	if (tscadc_dev->dev->platform_data)
+		err = titsc_parse_pdata(tscadc_dev, ts_dev);
+	else
+		err = titsc_parse_dt(pdev, ts_dev);
+
+	if (err) {
+		dev_err(&pdev->dev, "Could not find valid DT data.\n");
+		goto err_free_mem;
+	}
 
 	err = request_irq(ts_dev->irq, titsc_irq,
 			  0, pdev->dev.driver->name, ts_dev);
@@ -362,7 +414,7 @@ static int titsc_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 	titsc_step_config(ts_dev);
-	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->steps_to_configure);
+	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->coordinate_readouts);
 
 	input_dev->name = "ti-tsc";
 	input_dev->dev.parent = &pdev->dev;
@@ -398,7 +450,7 @@ static int titsc_remove(struct platform_device *pdev)
 	free_irq(ts_dev->irq, ts_dev);
 
 	/* total steps followed by the enable mask */
-	steps = 2 * ts_dev->steps_to_configure + 2;
+	steps = 2 * ts_dev->coordinate_readouts + 2;
 	steps = (1 << steps) - 1;
 	am335x_tsc_se_clr(ts_dev->mfd_tscadc, steps);
 
@@ -439,7 +491,7 @@ static int titsc_resume(struct device *dev)
 	}
 	titsc_step_config(ts_dev);
 	titsc_writel(ts_dev, REG_FIFO0THR,
-			ts_dev->steps_to_configure);
+			ts_dev->coordinate_readouts);
 	return 0;
 }
 
@@ -452,6 +504,12 @@ static const struct dev_pm_ops titsc_pm_ops = {
 #define TITSC_PM_OPS NULL
 #endif
 
+static const struct of_device_id ti_tsc_dt_ids[] = {
+	{ .compatible = "ti,am3359-tsc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_tsc_dt_ids);
+
 static struct platform_driver ti_tsc_driver = {
 	.probe	= titsc_probe,
 	.remove	= titsc_remove,
@@ -459,6 +517,7 @@ static struct platform_driver ti_tsc_driver = {
 		.name   = "tsc",
 		.owner	= THIS_MODULE,
 		.pm	= TITSC_PM_OPS,
+		.of_match_table = of_match_ptr(ti_tsc_dt_ids),
 	},
 };
 module_platform_driver(ti_tsc_driver);
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 7b091c4..15a01f6 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -184,6 +184,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	/* TSC Cell */
 	cell = &tscadc->cells[TSC_CELL];
 	cell->name = "tsc";
+	cell->of_compatible = "ti,am3359-tsc";
 	cell->platform_data = &tscadc;
 	cell->pdata_size = sizeof(tscadc);
 
-- 
1.7.10.4

[PATCH 06/22] input/ti_am33x_tsc: Add DT support

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna-l0cyMroinI0@public.gmane.org>

Add DT support for client touchscreen driver

[ panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org : use of_get_child_by_name
	instead of of_find_node_by_name ]

Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
Signed-off-by: Patil, Rachna <rachna-l0cyMroinI0@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
[ bigeasy: shift the code to the left, shirnk titsc_parse_dt() by not
           using temp value, change a binding and document them in
	   ti-tsc-adc.txt which also contains ADC binding which will be
	   used later, addd DT binding to mfd driver so we our DT node
	   without of_get_child_by_name(), rename steps_to_configure to
	   coordinate_readouts because this is what it really does,
	   don't overrire err after calling  titsc_parse_dt() or
	   titsc_parse_pdata()]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 .../bindings/input/touchscreen/ti-tsc-adc.txt      |   44 ++++++++
 drivers/input/touchscreen/ti_am335x_tsc.c          |  105 +++++++++++++++-----
 drivers/mfd/ti_am335x_tscadc.c                     |    1 +
 3 files changed, 127 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
new file mode 100644
index 0000000..491c97b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
@@ -0,0 +1,44 @@
+* TI - TSC ADC (Touschscreen and analog digital converter)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Required properties:
+- child "tsc"
+	ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
+		  support on the platform.
+	ti,x-plate-resistance: X plate resistance
+	ti,coordiante-readouts: The sequencer supports a total of 16
+				programmable steps each step is used to
+				read a single coordinate. A single
+                                readout is enough but multiple reads can
+				increase the quality.
+				A value of 5 means, 5 reads for X, 5 for
+				Y and 2 for Z (always). This utilises 12
+				of the 16 software steps available. The
+				remaining 4 can be used by the ADC.
+	ti,wire-config: Different boards could have a different order for
+			connecting wires on touchscreen. We need to provide an
+			8 bit number where in the 1st four bits represent the
+			analog lines and the next 4 bits represent positive/
+			negative terminal on that input line. Notations to
+			represent the input lines and terminals resoectively
+			is as follows:
+			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
+			XP  = 0, XN = 1, YP = 2, YN = 3.
+- child "adc"
+	ti,adc-channels: List of analog inputs available for ADC.
+			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
+
+Example:
+	tscadc: tscadc@44e0d000 {
+		compatible = "ti,am3359-tscadc";
+		tsc {
+			ti,wires = <4>;
+			ti,x-plate-resistance = <200>;
+			ti,coordiante-readouts = <5>;
+			ti,wire-config = <0x00 0x11 0x22 0x33>;
+		};
+
+		adc {
+			ti,adc-channels = <4 5 6 7>;
+		};
+	}
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index b2f8a46..a44d2c4 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -26,6 +26,8 @@
 #include <linux/io.h>
 #include <linux/input/ti_am335x_tsc.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 
@@ -47,7 +49,7 @@ struct titsc {
 	unsigned int		wires;
 	unsigned int		x_plate_resistance;
 	bool			pen_down;
-	int			steps_to_configure;
+	int			coordinate_readouts;
 	u32			config_inp[4];
 	u32			bit_xp, bit_xn, bit_yp, bit_yn;
 	u32			inp_xp, inp_xn, inp_yp, inp_yn;
@@ -123,7 +125,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	int i, total_steps;
 
 	/* Configure the Step registers */
-	total_steps = 2 * ts_dev->steps_to_configure;
+	total_steps = 2 * ts_dev->coordinate_readouts;
 
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_xp;
@@ -141,7 +143,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = 1; i <= ts_dev->steps_to_configure; i++) {
+	for (i = 1; i <= ts_dev->coordinate_readouts; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -163,7 +165,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = (ts_dev->steps_to_configure + 1); i <= total_steps; i++) {
+	for (i = (ts_dev->coordinate_readouts + 1); i <= total_steps; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -218,7 +220,7 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 		read = titsc_readl(ts_dev, REG_FIFO0);
 		channel = read & 0xf0000;
 		channel = channel >> 0x10;
-		if ((channel >= 0) && (channel < ts_dev->steps_to_configure)) {
+		if ((channel >= 0) && (channel < ts_dev->coordinate_readouts)) {
 			read &= 0xfff;
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
@@ -231,8 +233,8 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 		read = titsc_readl(ts_dev, REG_FIFO1);
 		channel = read & 0xf0000;
 		channel = channel >> 0x10;
-		if ((channel >= ts_dev->steps_to_configure) &&
-			(channel < (2 * ts_dev->steps_to_configure - 1))) {
+		if ((channel >= ts_dev->coordinate_readouts) &&
+			(channel < (2 * ts_dev->coordinate_readouts - 1))) {
 			read &= 0xfff;
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
@@ -310,6 +312,59 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static int titsc_parse_dt(struct platform_device *pdev,
+					struct titsc *ts_dev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	int err;
+
+	if (!node)
+		return -EINVAL;
+
+	err = of_property_read_u32(node, "ti,wires", &ts_dev->wires);
+	if (err < 0)
+		return err;
+	switch (ts_dev->wires) {
+	case 4:
+	case 5:
+	case 8:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(node, "ti,x-plate-resistance",
+			&ts_dev->x_plate_resistance);
+	if (err < 0)
+		return err;
+
+	err = of_property_read_u32(node, "ti,coordiante-readouts",
+			&ts_dev->coordinate_readouts);
+	if (err < 0)
+		return err;
+
+	return of_property_read_u32_array(node, "ti,wire-config",
+			ts_dev->config_inp, ARRAY_SIZE(ts_dev->config_inp));
+}
+
+static int titsc_parse_pdata(struct ti_tscadc_dev *tscadc_dev,
+					struct titsc *ts_dev)
+{
+	struct mfd_tscadc_board	*pdata = tscadc_dev->dev->platform_data;
+
+	if (!pdata)
+		return -EINVAL;
+
+	ts_dev->wires = pdata->tsc_init->wires;
+	ts_dev->x_plate_resistance =
+		pdata->tsc_init->x_plate_resistance;
+	ts_dev->steps_to_configure =
+		pdata->tsc_init->steps_to_configure;
+	memcpy(ts_dev->config_inp, pdata->tsc_init->wire_config,
+		sizeof(pdata->tsc_init->wire_config));
+	return 0;
+}
+
 /*
  * The functions for inserting/removing driver as a module.
  */
@@ -319,16 +374,8 @@ static int titsc_probe(struct platform_device *pdev)
 	struct titsc *ts_dev;
 	struct input_dev *input_dev;
 	struct ti_tscadc_dev *tscadc_dev = ti_tscadc_dev_get(pdev);
-	struct mfd_tscadc_board	*pdata;
 	int err;
 
-	pdata = tscadc_dev->dev->platform_data;
-
-	if (!pdata) {
-		dev_err(&pdev->dev, "Could not find platform data\n");
-		return -EINVAL;
-	}
-
 	/* Allocate memory for device */
 	ts_dev = kzalloc(sizeof(struct titsc), GFP_KERNEL);
 	input_dev = input_allocate_device();
@@ -342,11 +389,16 @@ static int titsc_probe(struct platform_device *pdev)
 	ts_dev->mfd_tscadc = tscadc_dev;
 	ts_dev->input = input_dev;
 	ts_dev->irq = tscadc_dev->irq;
-	ts_dev->wires = pdata->tsc_init->wires;
-	ts_dev->x_plate_resistance = pdata->tsc_init->x_plate_resistance;
-	ts_dev->steps_to_configure = pdata->tsc_init->steps_to_configure;
-	memcpy(ts_dev->config_inp, pdata->tsc_init->wire_config,
-			sizeof(pdata->tsc_init->wire_config));
+
+	if (tscadc_dev->dev->platform_data)
+		err = titsc_parse_pdata(tscadc_dev, ts_dev);
+	else
+		err = titsc_parse_dt(pdev, ts_dev);
+
+	if (err) {
+		dev_err(&pdev->dev, "Could not find valid DT data.\n");
+		goto err_free_mem;
+	}
 
 	err = request_irq(ts_dev->irq, titsc_irq,
 			  0, pdev->dev.driver->name, ts_dev);
@@ -362,7 +414,7 @@ static int titsc_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 	titsc_step_config(ts_dev);
-	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->steps_to_configure);
+	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->coordinate_readouts);
 
 	input_dev->name = "ti-tsc";
 	input_dev->dev.parent = &pdev->dev;
@@ -398,7 +450,7 @@ static int titsc_remove(struct platform_device *pdev)
 	free_irq(ts_dev->irq, ts_dev);
 
 	/* total steps followed by the enable mask */
-	steps = 2 * ts_dev->steps_to_configure + 2;
+	steps = 2 * ts_dev->coordinate_readouts + 2;
 	steps = (1 << steps) - 1;
 	am335x_tsc_se_clr(ts_dev->mfd_tscadc, steps);
 
@@ -439,7 +491,7 @@ static int titsc_resume(struct device *dev)
 	}
 	titsc_step_config(ts_dev);
 	titsc_writel(ts_dev, REG_FIFO0THR,
-			ts_dev->steps_to_configure);
+			ts_dev->coordinate_readouts);
 	return 0;
 }
 
@@ -452,6 +504,12 @@ static const struct dev_pm_ops titsc_pm_ops = {
 #define TITSC_PM_OPS NULL
 #endif
 
+static const struct of_device_id ti_tsc_dt_ids[] = {
+	{ .compatible = "ti,am3359-tsc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_tsc_dt_ids);
+
 static struct platform_driver ti_tsc_driver = {
 	.probe	= titsc_probe,
 	.remove	= titsc_remove,
@@ -459,6 +517,7 @@ static struct platform_driver ti_tsc_driver = {
 		.name   = "tsc",
 		.owner	= THIS_MODULE,
 		.pm	= TITSC_PM_OPS,
+		.of_match_table = of_match_ptr(ti_tsc_dt_ids),
 	},
 };
 module_platform_driver(ti_tsc_driver);
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 7b091c4..15a01f6 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -184,6 +184,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	/* TSC Cell */
 	cell = &tscadc->cells[TSC_CELL];
 	cell->name = "tsc";
+	cell->of_compatible = "ti,am3359-tsc";
 	cell->platform_data = &tscadc;
 	cell->pdata_size = sizeof(tscadc);
 
-- 
1.7.10.4

[PATCH 07/22] input/ti_am33x_tsc: remove platform_data support

[Sebastian Andrzej Siewior]

This patch removes access to platform data mfd_tscadc_board because the
platform is DT only.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |   25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index a44d2c4..66c5a26 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -24,7 +24,6 @@
 #include <linux/clk.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
-#include <linux/input/ti_am335x_tsc.h>
 #include <linux/delay.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -347,24 +346,6 @@ static int titsc_parse_dt(struct platform_device *pdev,
 			ts_dev->config_inp, ARRAY_SIZE(ts_dev->config_inp));
 }
 
-static int titsc_parse_pdata(struct ti_tscadc_dev *tscadc_dev,
-					struct titsc *ts_dev)
-{
-	struct mfd_tscadc_board	*pdata = tscadc_dev->dev->platform_data;
-
-	if (!pdata)
-		return -EINVAL;
-
-	ts_dev->wires = pdata->tsc_init->wires;
-	ts_dev->x_plate_resistance =
-		pdata->tsc_init->x_plate_resistance;
-	ts_dev->steps_to_configure =
-		pdata->tsc_init->steps_to_configure;
-	memcpy(ts_dev->config_inp, pdata->tsc_init->wire_config,
-		sizeof(pdata->tsc_init->wire_config));
-	return 0;
-}
-
 /*
  * The functions for inserting/removing driver as a module.
  */
@@ -390,11 +371,7 @@ static int titsc_probe(struct platform_device *pdev)
 	ts_dev->input = input_dev;
 	ts_dev->irq = tscadc_dev->irq;
 
-	if (tscadc_dev->dev->platform_data)
-		err = titsc_parse_pdata(tscadc_dev, ts_dev);
-	else
-		err = titsc_parse_dt(pdev, ts_dev);
-
+	err = titsc_parse_dt(pdev, ts_dev);
 	if (err) {
 		dev_err(&pdev->dev, "Could not find valid DT data.\n");
 		goto err_free_mem;
-- 
1.7.10.4

[PATCH 08/22] iio/ti_am335x_adc: Add DT support

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

Add DT support for client ADC driver.

[ panto@antoniou-consulting.com : use of_get_child_by_name
	instead of of_find_node_by_name ]

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: shift the code to the left, fix clean up on
          of_get_child_by_name() failer, provide OF binding in mfd, use
	  own of node, get rid of of_get_child_by_name()]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c |   29 ++++++++++++++++++++++++-----
 drivers/mfd/ti_am335x_tscadc.c  |    1 +
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 543b9c4..b24402c 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -22,6 +22,8 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/iio/iio.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 #include <linux/platform_data/ti_am335x_adc.h>
@@ -152,11 +154,12 @@ static int tiadc_probe(struct platform_device *pdev)
 	struct iio_dev		*indio_dev;
 	struct tiadc_device	*adc_dev;
 	struct ti_tscadc_dev	*tscadc_dev = ti_tscadc_dev_get(pdev);
-	struct mfd_tscadc_board	*pdata;
+	struct mfd_tscadc_board	*pdata = tscadc_dev->dev->platform_data;
+	struct device_node	*node = pdev->dev.of_node;
 	int			err;
+	u32			val32;
 
-	pdata = tscadc_dev->dev->platform_data;
-	if (!pdata || !pdata->adc_init) {
+	if (!pdata && !node) {
 		dev_err(&pdev->dev, "Could not find platform data\n");
 		return -EINVAL;
 	}
@@ -169,8 +172,17 @@ static int tiadc_probe(struct platform_device *pdev)
 	}
 	adc_dev = iio_priv(indio_dev);
 
-	adc_dev->mfd_tscadc = tscadc_dev;
-	adc_dev->channels = pdata->adc_init->adc_channels;
+	adc_dev->mfd_tscadc = ti_tscadc_dev_get(pdev);
+
+	if (pdata)
+		adc_dev->channels = pdata->adc_init->adc_channels;
+	else {
+		err = of_property_read_u32(node,
+				"ti,adc-channels", &val32);
+		if (err < 0)
+			goto err_free_device;
+		adc_dev->channels = val32;
+	}
 
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->name = dev_name(&pdev->dev);
@@ -260,11 +272,18 @@ static const struct dev_pm_ops tiadc_pm_ops = {
 #define TIADC_PM_OPS NULL
 #endif
 
+static const struct of_device_id ti_adc_dt_ids[] = {
+	{ .compatible = "ti,am3359-adc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_adc_dt_ids);
+
 static struct platform_driver tiadc_driver = {
 	.driver = {
 		.name   = "tiadc",
 		.owner	= THIS_MODULE,
 		.pm	= TIADC_PM_OPS,
+		.of_match_table = of_match_ptr(ti_adc_dt_ids),
 	},
 	.probe	= tiadc_probe,
 	.remove	= tiadc_remove,
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 15a01f6..eaaa253 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -191,6 +191,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	/* ADC Cell */
 	cell = &tscadc->cells[ADC_CELL];
 	cell->name = "tiadc";
+	cell->of_compatible = "ti,am3359-adc";
 	cell->platform_data = &tscadc;
 	cell->pdata_size = sizeof(tscadc);
 
-- 
1.7.10.4

[PATCH 09/22] iio/ti_am335x_adc: remove platform_data support

[Sebastian Andrzej Siewior]

This patch removes access to platform data mfd_tscadc_board because the
platform is DT only.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c |   21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index b24402c..2868c0c 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -26,7 +26,6 @@
 #include <linux/of_device.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
-#include <linux/platform_data/ti_am335x_adc.h>
 
 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
@@ -153,14 +152,12 @@ static int tiadc_probe(struct platform_device *pdev)
 {
 	struct iio_dev		*indio_dev;
 	struct tiadc_device	*adc_dev;
-	struct ti_tscadc_dev	*tscadc_dev = ti_tscadc_dev_get(pdev);
-	struct mfd_tscadc_board	*pdata = tscadc_dev->dev->platform_data;
 	struct device_node	*node = pdev->dev.of_node;
 	int			err;
 	u32			val32;
 
-	if (!pdata && !node) {
-		dev_err(&pdev->dev, "Could not find platform data\n");
+	if (!node) {
+		dev_err(&pdev->dev, "Could not find valid DT data.\n");
 		return -EINVAL;
 	}
 
@@ -174,15 +171,11 @@ static int tiadc_probe(struct platform_device *pdev)
 
 	adc_dev->mfd_tscadc = ti_tscadc_dev_get(pdev);
 
-	if (pdata)
-		adc_dev->channels = pdata->adc_init->adc_channels;
-	else {
-		err = of_property_read_u32(node,
-				"ti,adc-channels", &val32);
-		if (err < 0)
-			goto err_free_device;
-		adc_dev->channels = val32;
-	}
+	err = of_property_read_u32(node,
+			"ti,adc-channels", &val32);
+	if (err < 0)
+		goto err_free_device;
+	adc_dev->channels = val32;
 
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->name = dev_name(&pdev->dev);
-- 
1.7.10.4

[PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

Make changes to add DT support in the MFD core driver.

[ panto@antoniou-consulting.com : use of_get_child_by_name
	instead of of_find_node_by_name ]

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: module alias, rename to ti,am3359-tscadc as it was tested on
          AM3359]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mfd/ti_am335x_tscadc.c |   32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index eaaa253..aae5e55 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -21,6 +21,8 @@
 #include <linux/clk.h>
 #include <linux/mfd/core.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 #include <linux/input/ti_am335x_tsc.h>
@@ -79,20 +81,31 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	struct resource		*res;
 	struct clk		*clk;
 	struct mfd_tscadc_board	*pdata = pdev->dev.platform_data;
+	struct device_node	*node = pdev->dev.of_node;
 	struct mfd_cell		*cell;
 	int			err, ctrl;
 	int			clk_value, clock_rate;
-	int			tsc_wires, adc_channels = 0, total_channels;
+	int			tsc_wires = 0, adc_channels = 0, total_channels;
 
-	if (!pdata) {
+	if (!pdata && !pdev->dev.of_node) {
 		dev_err(&pdev->dev, "Could not find platform data\n");
 		return -EINVAL;
 	}
 
-	if (pdata->adc_init)
-		adc_channels = pdata->adc_init->adc_channels;
+	if (pdev->dev.platform_data) {
+		if (pdata->tsc_init)
+			tsc_wires = pdata->tsc_init->wires;
+
+		if (pdata->adc_init)
+			adc_channels = pdata->adc_init->adc_channels;
+	} else {
+		node = of_get_child_by_name(pdev->dev.of_node, "tsc");
+		of_property_read_u32(node, "ti,wires", &tsc_wires);
+
+		node = of_get_child_by_name(pdev->dev.of_node, "adc");
+		of_property_read_u32(node, "ti,adc-channels", &adc_channels);
+	}
 
-	tsc_wires = pdata->tsc_init->wires;
 	total_channels = tsc_wires + adc_channels;
 
 	if (total_channels > 8) {
@@ -266,11 +279,18 @@ static const struct dev_pm_ops tscadc_pm_ops = {
 #define TSCADC_PM_OPS NULL
 #endif
 
+static const struct of_device_id ti_tscadc_dt_ids[] = {
+	{ .compatible = "ti,am3359-tscadc", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_tscadc_dt_ids);
+
 static struct platform_driver ti_tscadc_driver = {
 	.driver = {
-		.name   = "ti_tscadc",
+		.name   = "ti_am3359-tscadc",
 		.owner	= THIS_MODULE,
 		.pm	= TSCADC_PM_OPS,
+		.of_match_table = of_match_ptr(ti_tscadc_dt_ids),
 	},
 	.probe	= ti_tscadc_probe,
 	.remove	= ti_tscadc_remove,
-- 
1.7.10.4

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 01:30:56PM +0200, Sebastian Andrzej Siewior wrote:
> From: "Patil, Rachna" <rachna@ti.com>
> 
> Make changes to add DT support in the MFD core driver.
Which changes ?


> [ panto@antoniou-consulting.com : use of_get_child_by_name
> 	instead of of_find_node_by_name ]
I have no idea where that is coming from. Please remove this kind of
stuff and build a proper commit log instead.


> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> Signed-off-by: Patil, Rachna <rachna@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> [bigeasy: module alias, rename to ti,am3359-tscadc as it was tested on
>           AM3359]
I honestly can't tell if this is a change from the last version of your
patchset or a description of this patch changes in general.
This is cluttering your commit logs, please remove this as well.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 01:30:56PM +0200, Sebastian Andrzej Siewior wrote:
>> From: "Patil, Rachna" <rachna@ti.com>
>>
>> Make changes to add DT support in the MFD core driver.
> Which changes ?

So rewrite the commit log…

>> [ panto@antoniou-consulting.com : use of_get_child_by_name
>> 	instead of of_find_node_by_name ]
> I have no idea where that is coming from. Please remove this kind of
> stuff and build a proper commit log instead.
> 
> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> Signed-off-by: Patil, Rachna <rachna@ti.com>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> [bigeasy: module alias, rename to ti,am3359-tscadc as it was tested on
>>           AM3359]
> I honestly can't tell if this is a change from the last version of your
> patchset or a description of this patch changes in general.
> This is cluttering your commit logs, please remove this as well.

I took the original patch. Every change I made to it because people
asked to merge changes into the patch where the problem occurred I
added it here before my sign-off.

In the end I would like not to post a patch with "From: != me" and
don't make change which the original author did not do. Also dropping
their authorship isn't nice. What could we agree on?

> Cheers,
> Samuel.

Sebastian

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 01:30:56PM +0200, Sebastian Andrzej Siewior wrote:
>> From: "Patil, Rachna" <rachna@ti.com>
>>
>> Make changes to add DT support in the MFD core driver.
> Which changes ?

So rewrite the commit log…

>> [ panto@antoniou-consulting.com : use of_get_child_by_name
>> 	instead of of_find_node_by_name ]
> I have no idea where that is coming from. Please remove this kind of
> stuff and build a proper commit log instead.
> 
> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> Signed-off-by: Patil, Rachna <rachna@ti.com>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> [bigeasy: module alias, rename to ti,am3359-tscadc as it was tested on
>>           AM3359]
> I honestly can't tell if this is a change from the last version of your
> patchset or a description of this patch changes in general.
> This is cluttering your commit logs, please remove this as well.

I took the original patch. Every change I made to it because people
asked to merge changes into the patch where the problem occurred I
added it here before my sign-off.

In the end I would like not to post a patch with "From: != me" and
don't make change which the original author did not do. Also dropping
their authorship isn't nice. What could we agree on?

> Cheers,
> Samuel.

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

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 04:42:30PM +0200, Sebastian Andrzej Siewior wrote:
> In the end I would like not to post a patch with "From: != me" and
> don't make change which the original author did not do. Also dropping
> their authorship isn't nice. What could we agree on?
You probably don't want to change authorship unless the final patch is
really far from the original one. In which case you can change it and
mention the original author name in the commit log.
If you have only made a few changes on top of the original patch, please
say so explicitely in the commit log. Don't bother if we're talking
about small changes or cosmetic ones. But your commit log has to be
readable and clear.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Sebastian Andrzej Siewior]

On 06/11/2013 05:05 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 04:42:30PM +0200, Sebastian Andrzej Siewior wrote:
>> In the end I would like not to post a patch with "From: != me" and
>> don't make change which the original author did not do. Also dropping
>> their authorship isn't nice. What could we agree on?
> You probably don't want to change authorship unless the final patch is
> really far from the original one. In which case you can change it and
> mention the original author name in the commit log.
> If you have only made a few changes on top of the original patch, please
> say so explicitely in the commit log. Don't bother if we're talking
> about small changes or cosmetic ones. But your commit log has to be
> readable and clear.

Commit 06c9494 on of many examples where cosmetic are recorded. But to
make some progress I rewrite the commit log to resolve this and don't
add anything to the sign-off area.

> Cheers,
> Samuel.

Sebastian

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Sebastian Andrzej Siewior]

On 06/11/2013 05:05 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 04:42:30PM +0200, Sebastian Andrzej Siewior wrote:
>> In the end I would like not to post a patch with "From: != me" and
>> don't make change which the original author did not do. Also dropping
>> their authorship isn't nice. What could we agree on?
> You probably don't want to change authorship unless the final patch is
> really far from the original one. In which case you can change it and
> mention the original author name in the commit log.
> If you have only made a few changes on top of the original patch, please
> say so explicitely in the commit log. Don't bother if we're talking
> about small changes or cosmetic ones. But your commit log has to be
> readable and clear.

Commit 06c9494 on of many examples where cosmetic are recorded. But to
make some progress I rewrite the commit log to resolve this and don't
add anything to the sign-off area.

> Cheers,
> Samuel.

Sebastian

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Lee Jones]

On Tue, 11 Jun 2013, Sebastian Andrzej Siewior wrote:

> On 06/11/2013 05:05 PM, Samuel Ortiz wrote:
> > Hi Sebastian,
> 
> Hi Samuel,
> 
> > On Tue, Jun 11, 2013 at 04:42:30PM +0200, Sebastian Andrzej Siewior wrote:
> >> In the end I would like not to post a patch with "From: != me" and
> >> don't make change which the original author did not do. Also dropping
> >> their authorship isn't nice. What could we agree on?
> > You probably don't want to change authorship unless the final patch is
> > really far from the original one. In which case you can change it and
> > mention the original author name in the commit log.
> > If you have only made a few changes on top of the original patch, please
> > say so explicitely in the commit log. Don't bother if we're talking
> > about small changes or cosmetic ones. But your commit log has to be
> > readable and clear.
> 
> Commit 06c9494 on of many examples where cosmetic are recorded. But to
> make some progress I rewrite the commit log to resolve this and don't
> add anything to the sign-off area.

If you are submitting the patch, you still have to add your own SOB.

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

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Lee Jones]

On Tue, 11 Jun 2013, Sebastian Andrzej Siewior wrote:

> On 06/11/2013 05:05 PM, Samuel Ortiz wrote:
> > Hi Sebastian,
> 
> Hi Samuel,
> 
> > On Tue, Jun 11, 2013 at 04:42:30PM +0200, Sebastian Andrzej Siewior wrote:
> >> In the end I would like not to post a patch with "From: != me" and
> >> don't make change which the original author did not do. Also dropping
> >> their authorship isn't nice. What could we agree on?
> > You probably don't want to change authorship unless the final patch is
> > really far from the original one. In which case you can change it and
> > mention the original author name in the commit log.
> > If you have only made a few changes on top of the original patch, please
> > say so explicitely in the commit log. Don't bother if we're talking
> > about small changes or cosmetic ones. But your commit log has to be
> > readable and clear.
> 
> Commit 06c9494 on of many examples where cosmetic are recorded. But to
> make some progress I rewrite the commit log to resolve this and don't
> add anything to the sign-off area.

If you are submitting the patch, you still have to add your own SOB.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Lee Jones]

> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> Signed-off-by: Patil, Rachna <rachna@ti.com>
> >> Signed-off-by: Felipe Balbi <balbi@ti.com>
> >> [bigeasy: module alias, rename to ti,am3359-tscadc as it was tested on
> >>           AM3359]
> > I honestly can't tell if this is a change from the last version of your
> > patchset or a description of this patch changes in general.
> > This is cluttering your commit logs, please remove this as well.
> 
> I took the original patch. Every change I made to it because people
> asked to merge changes into the patch where the problem occurred I
> added it here before my sign-off.
> 
> In the end I would like not to post a patch with "From: != me" and
> don't make change which the original author did not do. Also dropping
> their authorship isn't nice. What could we agree on?

Generally speaking, if it is necessary to merge various author's
patches into one, then you can the merger will tend to take authorship
of the commit. Note that just because you are the author of the
commit, it doesn't mean you authored the patch.

I also use the rule of thumb that if you make significant changes to a
patch, then you can also assume authorship too. I'll leave the 'how
much is significant' to your own good judgement.

If you're just making a few fixups, then just add your SOB in the
normal way. That should be enough reward for a mere few patch fixes.

Adding little 'I-did-this' notes to the commit log should mostly be
avoided IMO.

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

Re: [PATCH 10/22] mfd/ti_am335x_tscadc: Add DT support

[Lee Jones]

> >> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> >> Signed-off-by: Patil, Rachna <rachna-l0cyMroinI0@public.gmane.org>
> >> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> >> [bigeasy: module alias, rename to ti,am3359-tscadc as it was tested on
> >>           AM3359]
> > I honestly can't tell if this is a change from the last version of your
> > patchset or a description of this patch changes in general.
> > This is cluttering your commit logs, please remove this as well.
> 
> I took the original patch. Every change I made to it because people
> asked to merge changes into the patch where the problem occurred I
> added it here before my sign-off.
> 
> In the end I would like not to post a patch with "From: != me" and
> don't make change which the original author did not do. Also dropping
> their authorship isn't nice. What could we agree on?

Generally speaking, if it is necessary to merge various author's
patches into one, then you can the merger will tend to take authorship
of the commit. Note that just because you are the author of the
commit, it doesn't mean you authored the patch.

I also use the rule of thumb that if you make significant changes to a
patch, then you can also assume authorship too. I'll leave the 'how
much is significant' to your own good judgement.

If you're just making a few fixups, then just add your SOB in the
normal way. That should be enough reward for a mere few patch fixes.

Adding little 'I-did-this' notes to the commit log should mostly be
avoided IMO.

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

[PATCH 11/22] mfd/ti_am335x_tscadc: remove platform_data support

[Sebastian Andrzej Siewior]

This patch removes access to platform data mfd_tscadc_board because the
platform is DT only.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mfd/ti_am335x_tscadc.c |   23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index aae5e55..f9ad26f 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -25,8 +25,6 @@
 #include <linux/of_device.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
-#include <linux/input/ti_am335x_tsc.h>
-#include <linux/platform_data/ti_am335x_adc.h>
 
 static unsigned int tscadc_readl(struct ti_tscadc_dev *tsadc, unsigned int reg)
 {
@@ -80,31 +78,22 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	struct ti_tscadc_dev	*tscadc;
 	struct resource		*res;
 	struct clk		*clk;
-	struct mfd_tscadc_board	*pdata = pdev->dev.platform_data;
 	struct device_node	*node = pdev->dev.of_node;
 	struct mfd_cell		*cell;
 	int			err, ctrl;
 	int			clk_value, clock_rate;
 	int			tsc_wires = 0, adc_channels = 0, total_channels;
 
-	if (!pdata && !pdev->dev.of_node) {
-		dev_err(&pdev->dev, "Could not find platform data\n");
+	if (!pdev->dev.of_node) {
+		dev_err(&pdev->dev, "Could not find valid DT data.\n");
 		return -EINVAL;
 	}
 
-	if (pdev->dev.platform_data) {
-		if (pdata->tsc_init)
-			tsc_wires = pdata->tsc_init->wires;
+	node = of_get_child_by_name(pdev->dev.of_node, "tsc");
+	of_property_read_u32(node, "ti,wires", &tsc_wires);
 
-		if (pdata->adc_init)
-			adc_channels = pdata->adc_init->adc_channels;
-	} else {
-		node = of_get_child_by_name(pdev->dev.of_node, "tsc");
-		of_property_read_u32(node, "ti,wires", &tsc_wires);
-
-		node = of_get_child_by_name(pdev->dev.of_node, "adc");
-		of_property_read_u32(node, "ti,adc-channels", &adc_channels);
-	}
+	node = of_get_child_by_name(pdev->dev.of_node, "adc");
+	of_property_read_u32(node, "ti,adc-channels", &adc_channels);
 
 	total_channels = tsc_wires + adc_channels;
 
-- 
1.7.10.4

[PATCH 12/22] iio/ti_tscadc: provide datasheet_name and scan_type

[Sebastian Andrzej Siewior]

From: Pantelis Antoniou <panto@antoniou-consulting.com>

This patch provides the members "datasheet_name" and scan_type. This is
the remaining part of the earlier patch where I (bigeasy) removed iio_map
because it is now supplied by the device tree.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: use static AIN[0-8] names, use kcalloc(), removed iio_map,
	patch description]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c |   29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 2868c0c..9939810 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -24,6 +24,8 @@
 #include <linux/iio/iio.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 
@@ -84,29 +86,46 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 	am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
 }
 
+static const char * const chan_name_ain[] = {
+	"AIN0",
+	"AIN1",
+	"AIN2",
+	"AIN3",
+	"AIN4",
+	"AIN5",
+	"AIN6",
+	"AIN7",
+};
+
 static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
 {
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	struct iio_chan_spec *chan_array;
+	struct iio_chan_spec *chan;
 	int i;
 
 	indio_dev->num_channels = channels;
-	chan_array = kcalloc(indio_dev->num_channels,
+	chan_array = kcalloc(channels,
 			sizeof(struct iio_chan_spec), GFP_KERNEL);
-
 	if (chan_array == NULL)
 		return -ENOMEM;
 
-	for (i = 0; i < (indio_dev->num_channels); i++) {
-		struct iio_chan_spec *chan = chan_array + i;
+	chan = chan_array;
+	for (i = 0; i < channels; i++, chan++) {
+
 		chan->type = IIO_VOLTAGE;
 		chan->indexed = 1;
 		chan->channel = i;
 		chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+		chan->datasheet_name = chan_name_ain[i];
+		chan->scan_type.sign = 'u';
+		chan->scan_type.realbits = 12;
+		chan->scan_type.storagebits = 32;
 	}
 
 	indio_dev->channels = chan_array;
 
-	return indio_dev->num_channels;
+	return 0;
 }
 
 static void tiadc_channels_remove(struct iio_dev *indio_dev)
-- 
1.7.10.4

[PATCH 13/22] mfd/ti_tscadc: deal with partial activation

[Sebastian Andrzej Siewior]

From: Pantelis Antoniou <panto@antoniou-consulting.com>

Fix the mfd device in the case where a subdevice might not be activated.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: split out this chunk from the orignal patch, check for neither
          ADC nor TSC channels]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mfd/ti_am335x_tscadc.c       |   38 ++++++++++++++++++++++------------
 include/linux/mfd/ti_am335x_tscadc.h |    8 +++----
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index f9ad26f..74e4694 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -96,11 +96,14 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	of_property_read_u32(node, "ti,adc-channels", &adc_channels);
 
 	total_channels = tsc_wires + adc_channels;
-
 	if (total_channels > 8) {
 		dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
 		return -EINVAL;
 	}
+	if (total_channels == 0) {
+		dev_err(&pdev->dev, "Need atleast one channel.\n");
+		return -EINVAL;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -183,28 +186,37 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	ctrl |= CNTRLREG_TSCSSENB;
 	tscadc_writel(tscadc, REG_CTRL, ctrl);
 
+	tscadc->used_cells = 0;
+	tscadc->tsc_cell = -1;
+	tscadc->adc_cell = -1;
+
 	/* TSC Cell */
-	cell = &tscadc->cells[TSC_CELL];
-	cell->name = "tsc";
-	cell->of_compatible = "ti,am3359-tsc";
-	cell->platform_data = &tscadc;
-	cell->pdata_size = sizeof(tscadc);
+	if (tsc_wires > 0) {
+		tscadc->tsc_cell = tscadc->used_cells;
+		cell = &tscadc->cells[tscadc->used_cells++];
+		cell->name = "tsc";
+		cell->of_compatible = "ti,am3359-tsc";
+		cell->platform_data = &tscadc;
+		cell->pdata_size = sizeof(tscadc);
+	}
 
 	/* ADC Cell */
-	cell = &tscadc->cells[ADC_CELL];
-	cell->name = "tiadc";
-	cell->of_compatible = "ti,am3359-adc";
-	cell->platform_data = &tscadc;
-	cell->pdata_size = sizeof(tscadc);
+	if (adc_channels > 0) {
+		tscadc->adc_cell = tscadc->used_cells;
+		cell = &tscadc->cells[tscadc->used_cells++];
+		cell->name = "tiadc";
+		cell->of_compatible = "ti,am3359-adc";
+		cell->platform_data = &tscadc;
+		cell->pdata_size = sizeof(tscadc);
+	}
 
 	err = mfd_add_devices(&pdev->dev, pdev->id, tscadc->cells,
-			TSCADC_CELLS, NULL, 0, NULL);
+			tscadc->used_cells, NULL, 0, NULL);
 	if (err < 0)
 		goto err_disable_clk;
 
 	device_init_wakeup(&pdev->dev, true);
 	platform_set_drvdata(pdev, tscadc);
-
 	return 0;
 
 err_disable_clk:
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 2d78af8..b6e7ac6 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -127,11 +127,6 @@
 
 #define TSCADC_CELLS		2
 
-enum tscadc_cells {
-	TSC_CELL,
-	ADC_CELL,
-};
-
 struct mfd_tscadc_board {
 	struct tsc_data *tsc_init;
 	struct adc_data *adc_init;
@@ -141,6 +136,9 @@ struct ti_tscadc_dev {
 	struct device *dev;
 	void __iomem *tscadc_base;
 	int irq;
+	int used_cells;	/* 1-2 */
+	int tsc_cell;	/* -1 if not used */
+	int adc_cell;	/* -1 if not used */
 	struct mfd_cell cells[TSCADC_CELLS];
 	u32 reg_se_cache;
 	spinlock_t reg_lock;
-- 
1.7.10.4

[PATCH 14/22] arm/am33xx: add TSC/ADC mfd device support

[Sebastian Andrzej Siewior]

From: "Patil, Rachna" <rachna@ti.com>

Add support for core multifunctional device along
with its clients touchscreen and ADC.

[ panto@antoniou-consulting.com : make sure status is
	set to 'disabled' in dtsi file. ]

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Patil, Rachna <rachna@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
[bigeasy: add 'status = "okay"']
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 arch/arm/boot/dts/am335x-evm.dts |   14 ++++++++++++++
 arch/arm/boot/dts/am33xx.dtsi    |   18 ++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 0423298..26fea97 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -244,3 +244,17 @@
 &cpsw_emac1 {
 	phy_id = <&davinci_mdio>, <1>;
 };
+
+&tscadc {
+	status = "okay";
+	tsc {
+		ti,wires = <4>;
+		ti,x-plate-resistance = <200>;
+		ti,coordiante-readouts = <5>;
+		ti,wire-config = <0x00 0x11 0x22 0x33>;
+	};
+
+	adc {
+		ti,adc-channels = <4>;
+	};
+};
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 1460d9b..4ad7797 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -404,6 +404,24 @@
 			ti,hwmods = "wkup_m3";
 		};
 
+		tscadc: tscadc@44e0d000 {
+			compatible = "ti,am3359-tscadc";
+			reg = <0x44e0d000 0x1000>;
+			interrupt-parent = <&intc>;
+			interrupts = <16>;
+			ti,hwmods = "adc_tsc";
+			status = "disabled";
+
+			tsc {
+				compatible = "ti,am3359-tsc";
+			};
+			am335x_adc: adc {
+				#io-channel-cells = <1>;
+				compatible = "ti,am3359-adc";
+			};
+
+		};
+
 		gpmc: gpmc@50000000 {
 			compatible = "ti,am3352-gpmc";
 			ti,hwmods = "gpmc";
-- 
1.7.10.4

Re: [PATCH 14/22] arm/am33xx: add TSC/ADC mfd device support

[Sekhar Nori]

On 6/11/2013 5:01 PM, Sebastian Andrzej Siewior wrote:
> From: "Patil, Rachna" <rachna@ti.com>
> 
> Add support for core multifunctional device along
> with its clients touchscreen and ADC.
> 
> [ panto@antoniou-consulting.com : make sure status is
> 	set to 'disabled' in dtsi file. ]
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> Signed-off-by: Patil, Rachna <rachna@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> [bigeasy: add 'status = "okay"']
> Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> ---
>  arch/arm/boot/dts/am335x-evm.dts |   14 ++++++++++++++
>  arch/arm/boot/dts/am33xx.dtsi    |   18 ++++++++++++++++++
>  2 files changed, 32 insertions(+)

> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 1460d9b..4ad7797 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -404,6 +404,24 @@
>  			ti,hwmods = "wkup_m3";
>  		};
>  
> +		tscadc: tscadc@44e0d000 {
> +			compatible = "ti,am3359-tscadc";
> +			reg = <0x44e0d000 0x1000>;
> +			interrupt-parent = <&intc>;

interrupt-parent can be dropped since it will be inherited from parent.

Thanks,
Sekhar

Re: [PATCH 14/22] arm/am33xx: add TSC/ADC mfd device support

[Sekhar Nori]

On 6/11/2013 5:01 PM, Sebastian Andrzej Siewior wrote:
> From: "Patil, Rachna" <rachna-l0cyMroinI0@public.gmane.org>
> 
> Add support for core multifunctional device along
> with its clients touchscreen and ADC.
> 
> [ panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org : make sure status is
> 	set to 'disabled' in dtsi file. ]
> 
> Signed-off-by: Pantelis Antoniou <panto-wVdstyuyKrO8r51toPun2/C9HSW9iNxf@public.gmane.org>
> Signed-off-by: Patil, Rachna <rachna-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> [bigeasy: add 'status = "okay"']
> Signed-off-by: Sebastian Andrzej Siewior <sebastian-E0PNVn5OA6ohrxcnuTQ+TQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/am335x-evm.dts |   14 ++++++++++++++
>  arch/arm/boot/dts/am33xx.dtsi    |   18 ++++++++++++++++++
>  2 files changed, 32 insertions(+)

> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 1460d9b..4ad7797 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -404,6 +404,24 @@
>  			ti,hwmods = "wkup_m3";
>  		};
>  
> +		tscadc: tscadc@44e0d000 {
> +			compatible = "ti,am3359-tscadc";
> +			reg = <0x44e0d000 0x1000>;
> +			interrupt-parent = <&intc>;

interrupt-parent can be dropped since it will be inherited from parent.

Thanks,
Sekhar

Re: [PATCH 14/22] arm/am33xx: add TSC/ADC mfd device support

[Sebastian Andrzej Siewior]

On 07/04/2013 03:49 PM, Sekhar Nori wrote:
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index 1460d9b..4ad7797 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -404,6 +404,24 @@
>>  			ti,hwmods = "wkup_m3";
>>  		};
>>  
>> +		tscadc: tscadc@44e0d000 {
>> +			compatible = "ti,am3359-tscadc";
>> +			reg = <0x44e0d000 0x1000>;
>> +			interrupt-parent = <&intc>;
> 
> interrupt-parent can be dropped since it will be inherited from parent.

That is true. I prepare a patch for that after the merge window.

> 
> Thanks,
> Sekhar

Sebastian

Re: [PATCH 14/22] arm/am33xx: add TSC/ADC mfd device support

[Sebastian Andrzej Siewior]

On 07/04/2013 03:49 PM, Sekhar Nori wrote:
>> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
>> index 1460d9b..4ad7797 100644
>> --- a/arch/arm/boot/dts/am33xx.dtsi
>> +++ b/arch/arm/boot/dts/am33xx.dtsi
>> @@ -404,6 +404,24 @@
>>  			ti,hwmods = "wkup_m3";
>>  		};
>>  
>> +		tscadc: tscadc@44e0d000 {
>> +			compatible = "ti,am3359-tscadc";
>> +			reg = <0x44e0d000 0x1000>;
>> +			interrupt-parent = <&intc>;
> 
> interrupt-parent can be dropped since it will be inherited from parent.

That is true. I prepare a patch for that after the merge window.

> 
> Thanks,
> Sekhar

Sebastian

[PATCH 15/22] input & mfd: ti_am335x_tsc remove remaining platform data pieces

[Sebastian Andrzej Siewior]

The two header files removed here are unused and have no users as this
platform was never used with platform devices.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/input/ti_am335x_tsc.h         |   35 ---------------------------
 include/linux/mfd/ti_am335x_tscadc.h        |    5 ----
 include/linux/platform_data/ti_am335x_adc.h |   14 -----------
 3 files changed, 54 deletions(-)
 delete mode 100644 include/linux/input/ti_am335x_tsc.h
 delete mode 100644 include/linux/platform_data/ti_am335x_adc.h

diff --git a/include/linux/input/ti_am335x_tsc.h b/include/linux/input/ti_am335x_tsc.h
deleted file mode 100644
index 6a66b4d..0000000
--- a/include/linux/input/ti_am335x_tsc.h
+++ /dev/null
@@ -1,35 +0,0 @@
-#ifndef __LINUX_TI_AM335X_TSC_H
-#define __LINUX_TI_AM335X_TSC_H
-
-/**
- * struct tsc_data	Touchscreen wire configuration
- * @wires:		Wires refer to application modes
- *			i.e. 4/5/8 wire touchscreen support
- *			on the platform.
- * @x_plate_resistance:	X plate resistance.
- * @steps_to_configure:	The sequencer supports a total of
- *			16 programmable steps.
- *			A step configured to read a single
- *			co-ordinate value, can be applied
- *			more number of times for better results.
- * @wire_config:	Different EVM's could have a different order
- *			for connecting wires on touchscreen.
- *			We need to provide an 8 bit number where in
- *			the 1st four bits represent the analog lines
- *			and the next 4 bits represent positive/
- *			negative terminal on that input line.
- *			Notations to represent the input lines and
- *			terminals resoectively is as follows:
- *			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
- *			XP  = 0, XN = 1, YP = 2, YN = 3.
- *
- */
-
-struct tsc_data {
-	int wires;
-	int x_plate_resistance;
-	int steps_to_configure;
-	int wire_config[10];
-};
-
-#endif
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index b6e7ac6..cd8686b 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -127,11 +127,6 @@
 
 #define TSCADC_CELLS		2
 
-struct mfd_tscadc_board {
-	struct tsc_data *tsc_init;
-	struct adc_data *adc_init;
-};
-
 struct ti_tscadc_dev {
 	struct device *dev;
 	void __iomem *tscadc_base;
diff --git a/include/linux/platform_data/ti_am335x_adc.h b/include/linux/platform_data/ti_am335x_adc.h
deleted file mode 100644
index e41d583..0000000
--- a/include/linux/platform_data/ti_am335x_adc.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef __LINUX_TI_AM335X_ADC_H
-#define __LINUX_TI_AM335X_ADC_H
-
-/**
- * struct adc_data	ADC Input information
- * @adc_channels:	Number of analog inputs
- *			available for ADC.
- */
-
-struct adc_data {
-	unsigned int adc_channels;
-};
-
-#endif
-- 
1.7.10.4

[PATCH 16/22] mfd & input/ti_am335x_tsc: rename device from tsc to TI-am335x-tsc

[Sebastian Andrzej Siewior]

tsc is a very generic name. This patch adds a TI and HW prefix to it
less generic.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |    2 +-
 drivers/mfd/ti_am335x_tscadc.c            |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 66c5a26..d6643cb 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -491,7 +491,7 @@ static struct platform_driver ti_tsc_driver = {
 	.probe	= titsc_probe,
 	.remove	= titsc_remove,
 	.driver	= {
-		.name   = "tsc",
+		.name   = "TI-am335x-tsc",
 		.owner	= THIS_MODULE,
 		.pm	= TITSC_PM_OPS,
 		.of_match_table = of_match_ptr(ti_tsc_dt_ids),
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index 74e4694..bf33134 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -194,7 +194,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	if (tsc_wires > 0) {
 		tscadc->tsc_cell = tscadc->used_cells;
 		cell = &tscadc->cells[tscadc->used_cells++];
-		cell->name = "tsc";
+		cell->name = "TI-am335x-tsc";
 		cell->of_compatible = "ti,am3359-tsc";
 		cell->platform_data = &tscadc;
 		cell->pdata_size = sizeof(tscadc);
-- 
1.7.10.4

[PATCH 17/22] mfd & iio/ti_am335x_adc: rename device from tiadc to TI-am335x-adc

[Sebastian Andrzej Siewior]

TI-adc reads a little better compared to tiadc. And if we add am335x to
it then we have the same naming scheme as the tsc side.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c |    3 +--
 drivers/mfd/ti_am335x_tscadc.c  |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 9939810..4bec91e 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -292,7 +292,7 @@ MODULE_DEVICE_TABLE(of, ti_adc_dt_ids);
 
 static struct platform_driver tiadc_driver = {
 	.driver = {
-		.name   = "tiadc",
+		.name   = "TI-am335x-adc",
 		.owner	= THIS_MODULE,
 		.pm	= TIADC_PM_OPS,
 		.of_match_table = of_match_ptr(ti_adc_dt_ids),
@@ -300,7 +300,6 @@ static struct platform_driver tiadc_driver = {
 	.probe	= tiadc_probe,
 	.remove	= tiadc_remove,
 };
-
 module_platform_driver(tiadc_driver);
 
 MODULE_DESCRIPTION("TI ADC controller driver");
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index bf33134..d3f48d2 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -204,7 +204,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	if (adc_channels > 0) {
 		tscadc->adc_cell = tscadc->used_cells;
 		cell = &tscadc->cells[tscadc->used_cells++];
-		cell->name = "tiadc";
+		cell->name = "TI-am335x-adc";
 		cell->of_compatible = "ti,am3359-adc";
 		cell->platform_data = &tscadc;
 		cell->pdata_size = sizeof(tscadc);
-- 
1.7.10.4

[PATCH 17/22] mfd & iio/ti_am335x_adc: rename device from tiadc to TI-am335x-adc

[Sebastian Andrzej Siewior]

TI-adc reads a little better compared to tiadc. And if we add am335x to
it then we have the same naming scheme as the tsc side.

Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 drivers/iio/adc/ti_am335x_adc.c |    3 +--
 drivers/mfd/ti_am335x_tscadc.c  |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 9939810..4bec91e 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -292,7 +292,7 @@ MODULE_DEVICE_TABLE(of, ti_adc_dt_ids);
 
 static struct platform_driver tiadc_driver = {
 	.driver = {
-		.name   = "tiadc",
+		.name   = "TI-am335x-adc",
 		.owner	= THIS_MODULE,
 		.pm	= TIADC_PM_OPS,
 		.of_match_table = of_match_ptr(ti_adc_dt_ids),
@@ -300,7 +300,6 @@ static struct platform_driver tiadc_driver = {
 	.probe	= tiadc_probe,
 	.remove	= tiadc_remove,
 };
-
 module_platform_driver(tiadc_driver);
 
 MODULE_DESCRIPTION("TI ADC controller driver");
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index bf33134..d3f48d2 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -204,7 +204,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	if (adc_channels > 0) {
 		tscadc->adc_cell = tscadc->used_cells;
 		cell = &tscadc->cells[tscadc->used_cells++];
-		cell->name = "tiadc";
+		cell->name = "TI-am335x-adc";
 		cell->of_compatible = "ti,am3359-adc";
 		cell->platform_data = &tscadc;
 		cell->pdata_size = sizeof(tscadc);
-- 
1.7.10.4

[PATCH 18/22] input/ti_am335x_adc: use only FIFO0 and clean up a little

[Sebastian Andrzej Siewior]

The driver programs a threshold of "coordinate_readouts" say 5. The
REG_FIFO0THR registers says it should it be programmed to "threshold
minus one". The driver does not expect just 5 coordinates but 5 * 2 + 2.
Multiplied by two because 5 for X and 5 for Y and plus 2 because we have
two Z.
The whole thing kind of works because It reads the 5 coordinates for X
and Y from FIFO0 and FIFO1 and the last element in each FIFO is ignored
within the loop and read later.
Nothing guaranties that FIFO1 is ready by the time it is read. In fact I
could see that that FIFO1 reaturns for Y channels 8,9, 10, 12, 6 and for
Y channel 7 for Z. The problem is that channel 7 and channel 12 got
somehow mixed up.
The other Problem is that FIFO1 is also used by the IIO part leading to
wrong results if both (tsc & adc) are used.

The patch tries to clean up the whole thing a little:
- Remove the +1 and -1 in REG_STEPCONFIG, REG_STEPDELAY and its counter
  part in the for loop. This is just confusing.

- Use only FIFO0 in TSC. The fifo has space for 64 entries so should be
  fine.

- Read the whole FIFO in one function and check the channel.

- in case we dawdle around, make sure we only read a multiple of our
  coordinate set. On the second interrupt we will cleanup the remaining
  enties.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c           |    2 +-
 drivers/input/touchscreen/ti_am335x_tsc.c |   78 +++++++++++++++--------------
 include/linux/mfd/ti_am335x_tscadc.h      |    4 +-
 3 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 4bec91e..307a7c0 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -75,7 +75,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 
 	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
 
-	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
+	for (i = steps; i < TOTAL_STEPS; i++) {
 		tiadc_writel(adc_dev, REG_STEPCONFIG(i),
 				stepconfig | STEPCONFIG_INP(channels));
 		tiadc_writel(adc_dev, REG_STEPDELAY(i),
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index d6643cb..29febe9 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -120,11 +120,9 @@ static int titsc_config_wires(struct titsc *ts_dev)
 static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
-	unsigned int	stepenable = 0;
-	int i, total_steps;
-
-	/* Configure the Step registers */
-	total_steps = 2 * ts_dev->coordinate_readouts;
+	int i;
+	int end_step;
+	u32 stepenable;
 
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_xp;
@@ -142,7 +140,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = 1; i <= ts_dev->coordinate_readouts; i++) {
+	/* 1 … coordinate_readouts is for X */
+	end_step = ts_dev->coordinate_readouts;
+	for (i = 0; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -150,7 +150,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	config = 0;
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yn |
-			STEPCONFIG_INM_ADCREFM | STEPCONFIG_FIFO1;
+			STEPCONFIG_INM_ADCREFM;
 	switch (ts_dev->wires) {
 	case 4:
 		config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp);
@@ -164,12 +164,13 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = (ts_dev->coordinate_readouts + 1); i <= total_steps; i++) {
+	/* coordinate_readouts … coordinate_readouts * 2 is for Y */
+	end_step = ts_dev->coordinate_readouts * 2;
+	for (i = ts_dev->coordinate_readouts; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
 
-	config = 0;
 	/* Charge step configuration */
 	config = ts_dev->bit_xp | ts_dev->bit_yn |
 			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
@@ -178,35 +179,39 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
-	config = 0;
-	/* Configure to calculate pressure */
+	/* coordinate_readouts * 2 … coordinate_readouts * 2 + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
 			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
 			STEPCONFIG_INP(ts_dev->inp_xp);
-	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 1), config);
-	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 1),
+	titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
+	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
-	config |= STEPCONFIG_INP(ts_dev->inp_yn) | STEPCONFIG_FIFO1;
-	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 2), config);
-	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2),
+	end_step++;
+	config |= STEPCONFIG_INP(ts_dev->inp_yn);
+	titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
+	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
 	/* The steps1 … end and bit 0 for TS_Charge */
-	stepenable = (1 << (total_steps + 2)) - 1;
+	stepenable = (1 << (end_step + 2)) - 1;
 	am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
 }
 
 static void titsc_read_coordinates(struct titsc *ts_dev,
-				    unsigned int *x, unsigned int *y)
+		u32 *x, u32 *y, u32 *z1, u32 *z2)
 {
 	unsigned int fifocount = titsc_readl(ts_dev, REG_FIFO0CNT);
 	unsigned int prev_val_x = ~0, prev_val_y = ~0;
 	unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
 	unsigned int read, diff;
 	unsigned int i, channel;
+	unsigned int creads = ts_dev->coordinate_readouts;
 
+	*z1 = *z2 = 0;
+	if (fifocount % (creads * 2 + 2))
+		fifocount -= fifocount % (creads * 2 + 2);
 	/*
 	 * Delta filter is used to remove large variations in sampled
 	 * values from ADC. The filter tries to predict where the next
@@ -215,32 +220,32 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 	 * algorithm compares the difference with that of a present value,
 	 * if true the value is reported to the sub system.
 	 */
-	for (i = 0; i < fifocount - 1; i++) {
+	for (i = 0; i < fifocount; i++) {
 		read = titsc_readl(ts_dev, REG_FIFO0);
-		channel = read & 0xf0000;
-		channel = channel >> 0x10;
-		if ((channel >= 0) && (channel < ts_dev->coordinate_readouts)) {
-			read &= 0xfff;
+
+		channel = (read & 0xf0000) >> 16;
+		read &= 0xfff;
+		if (channel < creads) {
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
 				prev_diff_x = diff;
 				*x = read;
 			}
 			prev_val_x = read;
-		}
 
-		read = titsc_readl(ts_dev, REG_FIFO1);
-		channel = read & 0xf0000;
-		channel = channel >> 0x10;
-		if ((channel >= ts_dev->coordinate_readouts) &&
-			(channel < (2 * ts_dev->coordinate_readouts - 1))) {
-			read &= 0xfff;
+		} else if (channel < creads * 2) {
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
 				prev_diff_y = diff;
 				*y = read;
 			}
 			prev_val_y = read;
+
+		} else if (channel < creads * 2 + 1) {
+			*z1 = read;
+
+		} else if (channel < creads * 2 + 2) {
+			*z2 = read;
 		}
 	}
 }
@@ -256,10 +261,8 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 
 	status = titsc_readl(ts_dev, REG_IRQSTATUS);
 	if (status & IRQENB_FIFO0THRES) {
-		titsc_read_coordinates(ts_dev, &x, &y);
 
-		z1 = titsc_readl(ts_dev, REG_FIFO0) & 0xfff;
-		z2 = titsc_readl(ts_dev, REG_FIFO1) & 0xfff;
+		titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
 
 		if (ts_dev->pen_down && z1 != 0 && z2 != 0) {
 			/*
@@ -267,10 +270,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 			 * Resistance(touch) = x plate resistance *
 			 * x postion/4096 * ((z2 / z1) - 1)
 			 */
-			z = z2 - z1;
+			z = z1 - z2;
 			z *= x;
 			z *= ts_dev->x_plate_resistance;
-			z /= z1;
+			z /= z2;
 			z = (z + 2047) >> 12;
 
 			if (z <= MAX_12BIT) {
@@ -391,7 +394,8 @@ static int titsc_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 	titsc_step_config(ts_dev);
-	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->coordinate_readouts);
+	titsc_writel(ts_dev, REG_FIFO0THR,
+			ts_dev->coordinate_readouts * 2 + 2 - 1);
 
 	input_dev->name = "ti-tsc";
 	input_dev->dev.parent = &pdev->dev;
@@ -468,7 +472,7 @@ static int titsc_resume(struct device *dev)
 	}
 	titsc_step_config(ts_dev);
 	titsc_writel(ts_dev, REG_FIFO0THR,
-			ts_dev->coordinate_readouts);
+			ts_dev->coordinate_readouts * 2 + 2 - 1);
 	return 0;
 }
 
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index cd8686b..fe6223c 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -30,8 +30,8 @@
 #define REG_IDLECONFIG		0x058
 #define REG_CHARGECONFIG	0x05C
 #define REG_CHARGEDELAY		0x060
-#define REG_STEPCONFIG(n)	(0x64 + ((n - 1) * 8))
-#define REG_STEPDELAY(n)	(0x68 + ((n - 1) * 8))
+#define REG_STEPCONFIG(n)	(0x64 + ((n) * 8))
+#define REG_STEPDELAY(n)	(0x68 + ((n) * 8))
 #define REG_FIFO0CNT		0xE4
 #define REG_FIFO0THR		0xE8
 #define REG_FIFO1CNT		0xF0
-- 
1.7.10.4

[PATCH 19/22] input/ti_am335x_tsc: ACK the HW_PEN irq in ISR

[Sebastian Andrzej Siewior]

The interrupt source IRQENB_HW_PEN is enabled in suspend and suposed to
be used as a wake up source. Once this interrupt source is unmaksed, the
devices ends up in ISR and never continues.
This change ACKs the interrupt and disables it so the system does not
freeze.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 29febe9..84859da 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -308,6 +308,12 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 		irqclr |= IRQENB_PENUP;
 	}
 
+	if (status & IRQENB_HW_PEN) {
+
+		titsc_writel(ts_dev, REG_IRQWAKEUP, 0x00);
+		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
+	}
+
 	titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
 
 	am335x_tsc_se_update(ts_dev->mfd_tscadc);
-- 
1.7.10.4

[PATCH 20/22] input/ti_am335x_tsc: return IRQ_NONE if there was no IRQ for us

[Sebastian Andrzej Siewior]

The previous patch ("input/ti_am335x_tsc: ACK the HW_PEN irq in ISR")
acked the interrupt so we don't freeze if we don't handle an enabled
interrupt source. The interrupt core has a mechanism for this and to get
it work one should only say that it handled an interrupt if it is
actually the case.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/input/touchscreen/ti_am335x_tsc.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 84859da..6327169 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -314,10 +314,12 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
 	}
 
-	titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
-
-	am335x_tsc_se_update(ts_dev->mfd_tscadc);
-	return IRQ_HANDLED;
+	if (irqclr) {
+		titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
+		am335x_tsc_se_update(ts_dev->mfd_tscadc);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
 }
 
 static int titsc_parse_dt(struct platform_device *pdev,
-- 
1.7.10.4

[PATCH 21/22] iio/ti_am335x_adc: Allow to specify input line

[Sebastian Andrzej Siewior]

The TSC part allows to specify the input lines. The IIO part assumes
that it usues always the last few, that means if IIO has adc-channels
set to 2 it will use channel 6 and 7. However it might make sense to use
only 6.
This patch changes the device property (which was introduced recently
and was never in an official release) in a way that the user can specify
which of the AIN lines should be used. In Addition to this, the name is
now AINx where x is the channel number i.e. for AIN6 we would have 6.
Prior this, it always started counting at 0 which is confusing. In
addition to this, it also checks for correct step number during reading
and does not rely on proper FIFO depth.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/boot/dts/am335x-evm.dts |    2 +-
 drivers/iio/adc/ti_am335x_adc.c  |   57 +++++++++++++++++++++++++-------------
 drivers/mfd/ti_am335x_tscadc.c   |   20 +++++++++++--
 3 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 26fea97..0fa4c7f 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -255,6 +255,6 @@
 	};
 
 	adc {
-		ti,adc-channels = <4>;
+		ti,adc-channels = <4 5 6 7>;
 	};
 };
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 307a7c0..8ffe52d 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -32,6 +32,8 @@
 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
 	int channels;
+	u8 channel_line[8];
+	u8 channel_step[8];
 };
 
 static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
@@ -57,7 +59,7 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
 static void tiadc_step_config(struct tiadc_device *adc_dev)
 {
 	unsigned int stepconfig;
-	int i, channels = 0, steps;
+	int i, steps;
 	u32 step_en;
 
 	/*
@@ -71,16 +73,18 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 	 */
 
 	steps = TOTAL_STEPS - adc_dev->channels;
-	channels = TOTAL_CHANNELS - adc_dev->channels;
-
 	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
 
-	for (i = steps; i < TOTAL_STEPS; i++) {
-		tiadc_writel(adc_dev, REG_STEPCONFIG(i),
-				stepconfig | STEPCONFIG_INP(channels));
-		tiadc_writel(adc_dev, REG_STEPDELAY(i),
+	for (i = 0; i < adc_dev->channels; i++) {
+		int chan;
+
+		chan = adc_dev->channel_line[i];
+		tiadc_writel(adc_dev, REG_STEPCONFIG(steps),
+				stepconfig | STEPCONFIG_INP(chan));
+		tiadc_writel(adc_dev, REG_STEPDELAY(steps),
 				STEPCONFIG_OPENDLY);
-		channels++;
+		adc_dev->channel_step[i] = steps;
+		steps++;
 	}
 	step_en = get_adc_step_mask(adc_dev);
 	am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
@@ -115,9 +119,9 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
 
 		chan->type = IIO_VOLTAGE;
 		chan->indexed = 1;
-		chan->channel = i;
+		chan->channel = adc_dev->channel_line[i];
 		chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
-		chan->datasheet_name = chan_name_ain[i];
+		chan->datasheet_name = chan_name_ain[chan->channel];
 		chan->scan_type.sign = 'u';
 		chan->scan_type.realbits = 12;
 		chan->scan_type.storagebits = 32;
@@ -139,7 +143,8 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	int i;
-	unsigned int fifo1count, readx1;
+	unsigned int fifo1count, read;
+	u32 step = UINT_MAX;
 
 	/*
 	 * When the sub-system is first enabled,
@@ -152,11 +157,20 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	 * Hence we need to flush out this data.
 	 */
 
+	for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) {
+		if (chan->channel == adc_dev->channel_line[i]) {
+			step = adc_dev->channel_step[i];
+			break;
+		}
+	}
+	if (WARN_ON_ONCE(step == UINT_MAX))
+		return -EINVAL;
+
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
 	for (i = 0; i < fifo1count; i++) {
-		readx1 = tiadc_readl(adc_dev, REG_FIFO1);
-		if (i == chan->channel)
-			*val = readx1 & 0xfff;
+		read = tiadc_readl(adc_dev, REG_FIFO1);
+		if (read >> 16 == step)
+			*val = read & 0xfff;
 	}
 	am335x_tsc_se_update(adc_dev->mfd_tscadc);
 
@@ -172,8 +186,11 @@ static int tiadc_probe(struct platform_device *pdev)
 	struct iio_dev		*indio_dev;
 	struct tiadc_device	*adc_dev;
 	struct device_node	*node = pdev->dev.of_node;
+	struct property		*prop;
+	const __be32		*cur;
 	int			err;
-	u32			val32;
+	u32			val;
+	int			channels = 0;
 
 	if (!node) {
 		dev_err(&pdev->dev, "Could not find valid DT data.\n");
@@ -190,11 +207,11 @@ static int tiadc_probe(struct platform_device *pdev)
 
 	adc_dev->mfd_tscadc = ti_tscadc_dev_get(pdev);
 
-	err = of_property_read_u32(node,
-			"ti,adc-channels", &val32);
-	if (err < 0)
-		goto err_free_device;
-	adc_dev->channels = val32;
+	of_property_for_each_u32(node, "ti,adc-channels", prop, cur, val) {
+		adc_dev->channel_line[channels] = val;
+		channels++;
+	}
+	adc_dev->channels = channels;
 
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->name = dev_name(&pdev->dev);
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index d3f48d2..d19b517 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -80,9 +80,13 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 	struct clk		*clk;
 	struct device_node	*node = pdev->dev.of_node;
 	struct mfd_cell		*cell;
+	struct property         *prop;
+	const __be32            *cur;
+	u32			val;
 	int			err, ctrl;
 	int			clk_value, clock_rate;
 	int			tsc_wires = 0, adc_channels = 0, total_channels;
+	int			readouts = 0;
 
 	if (!pdev->dev.of_node) {
 		dev_err(&pdev->dev, "Could not find valid DT data.\n");
@@ -91,10 +95,17 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 
 	node = of_get_child_by_name(pdev->dev.of_node, "tsc");
 	of_property_read_u32(node, "ti,wires", &tsc_wires);
+	of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
 
 	node = of_get_child_by_name(pdev->dev.of_node, "adc");
-	of_property_read_u32(node, "ti,adc-channels", &adc_channels);
-
+	of_property_for_each_u32(node, "ti,adc-channels", prop, cur, val) {
+		adc_channels++;
+		if (val > 7) {
+			dev_err(&pdev->dev, " PIN numbers are 0..7 (not %d)\n",
+					val);
+			return -EINVAL;
+		}
+	}
 	total_channels = tsc_wires + adc_channels;
 	if (total_channels > 8) {
 		dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
@@ -105,6 +116,11 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	if (readouts * 2 + 2 + adc_channels > 16) {
+		dev_err(&pdev->dev, "Too many step configurations requested\n");
+		return -EINVAL;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "no memory resource defined.\n");
-- 
1.7.10.4

[PATCH 22/22] iio/ti_am335x_adc: check if we found the value

[Sebastian Andrzej Siewior]

Usually we get all the values we wanted but it is possible, that te ADC
unit is busy performing the conversation for the HW events. In that case
-EBUSY is returned and the user may re-call the function.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 8ffe52d..4427e8e 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -145,6 +145,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	int i;
 	unsigned int fifo1count, read;
 	u32 step = UINT_MAX;
+	bool found = false;
 
 	/*
 	 * When the sub-system is first enabled,
@@ -169,11 +170,14 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
 	for (i = 0; i < fifo1count; i++) {
 		read = tiadc_readl(adc_dev, REG_FIFO1);
-		if (read >> 16 == step)
+		if (read >> 16 == step) {
 			*val = read & 0xfff;
+			found = true;
+		}
 	}
 	am335x_tsc_se_update(adc_dev->mfd_tscadc);
-
+	if (found == false)
+		return -EBUSY;
 	return IIO_VAL_INT;
 }
 
-- 
1.7.10.4

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Lee Jones]

> I believe the whole thing should go via the MFD tree. It touches also
> input & iio subsystem. I collected ACKs where I got some in the meantime.
> 
> I added Lee Jones because I hear no sign of life from Samuel.

Unfortunately I can't be of any added help here, as I also send my
pull-requests though Sam.

Sorry I couldn't be of any more use Sebastian.

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

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Lee Jones]

> I believe the whole thing should go via the MFD tree. It touches also
> input & iio subsystem. I collected ACKs where I got some in the meantime.
> 
> I added Lee Jones because I hear no sign of life from Samuel.

Unfortunately I can't be of any added help here, as I also send my
pull-requests though Sam.

Sorry I couldn't be of any more use Sebastian.

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

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Lars-Peter Clausen]

On 06/11/2013 02:05 PM, Lee Jones wrote:
>> I believe the whole thing should go via the MFD tree. It touches also
>> input & iio subsystem. I collected ACKs where I got some in the meantime.
>>
>> I added Lee Jones because I hear no sign of life from Samuel.
> 
> Unfortunately I can't be of any added help here, as I also send my
> pull-requests though Sam.
> 
> Sorry I couldn't be of any more use Sebastian.
> 

It sometimes takes him a week or two to respond, but Samuel is pretty reliable
when it comes to merging patches, so I wouldn't worry about it.

- Lars

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Lars-Peter Clausen]

On 06/11/2013 02:05 PM, Lee Jones wrote:
>> I believe the whole thing should go via the MFD tree. It touches also
>> input & iio subsystem. I collected ACKs where I got some in the meantime.
>>
>> I added Lee Jones because I hear no sign of life from Samuel.
> 
> Unfortunately I can't be of any added help here, as I also send my
> pull-requests though Sam.
> 
> Sorry I couldn't be of any more use Sebastian.
> 

It sometimes takes him a week or two to respond, but Samuel is pretty reliable
when it comes to merging patches, so I wouldn't worry about it.

- Lars

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
> I believe the whole thing should go via the MFD tree. It touches also
> input & iio subsystem. I collected ACKs where I got some in the meantime.
Please fix your commit logs, and your subject lines. It should be e.g.
mfd: input: ti_am335x_adc: Blablabla

if it's mostly an mfd patch that also touches an input driver.

Then, this is a pretty big patchset, with iio, input and mfd all mixed
together and it is likely to create merge conflicts.
>From what I can see from it, and please correct me if I'm
wrong, the iio and input changes depend on the mfd ones, and not the
other way around. If that's so, I'm going to ask you to reshuffle your
patch set and separate the MFD changes from the iio and input ones. I'll
take the MFD ones and will create an immutable branch for Jonathan and
Dmitry to pull from and apply the iio and input changes on top of it.
Merge conflicts should be mostly avoided that way.
AFAICT, only patch #2 should be kept with input and iio bits mixed
together with MFD as otherwise this would introduce functional breakage.
Otherwise, all MFD bits from the other patches could be either separated
or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
#17).

Does that sound doable to you ?

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
>> I believe the whole thing should go via the MFD tree. It touches also
>> input & iio subsystem. I collected ACKs where I got some in the meantime.
> Please fix your commit logs, and your subject lines. It should be e.g.
> mfd: input: ti_am335x_adc: Blablabla
> 
> if it's mostly an mfd patch that also touches an input driver.

I usually do "subsystem / driver: short description" but if you want
the ":" as delimiter I can do this.

> Then, this is a pretty big patchset, with iio, input and mfd all mixed
> together and it is likely to create merge conflicts.
They somehow depend on each other. Otherwise I would have sent three
series, one per subsystem.

>>From what I can see from it, and please correct me if I'm
> wrong, the iio and input changes depend on the mfd ones, and not the
> other way around. If that's so, I'm going to ask you to reshuffle your
> patch set and separate the MFD changes from the iio and input ones. I'll
> take the MFD ones and will create an immutable branch for Jonathan and
> Dmitry to pull from and apply the iio and input changes on top of it.
> Merge conflicts should be mostly avoided that way.
> AFAICT, only patch #2 should be kept with input and iio bits mixed
> together with MFD as otherwise this would introduce functional breakage.
> Otherwise, all MFD bits from the other patches could be either separated
> or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
> #17).
> 
> Does that sound doable to you ?

The device renaming shouldn't matter since I added DT nodes for the mfd
child devices earlier. But then the of_compatible assignments should
go hand in hand. However if I split this then the driver won't work
but then it does not now as well (because there is no platform_data
provider in tree).

Still. There is #18 which reworks the "step addressing" and involves
changes in both (iio & input) at the same time.
There is #21. Adding this to the initial "DT support" patch would be bad
I think because it requires some changes on the iio side which have
nothing to do with DT. Putting the iio changes before DT would require
to make some change to platform-data part which will go away anyway.

So I started collecting ACKs from input and iio to avoid this split. If
you really want the split then I will start doing so…

> Cheers,
> Samuel.

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/11/2013 04:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
>> I believe the whole thing should go via the MFD tree. It touches also
>> input & iio subsystem. I collected ACKs where I got some in the meantime.
> Please fix your commit logs, and your subject lines. It should be e.g.
> mfd: input: ti_am335x_adc: Blablabla
> 
> if it's mostly an mfd patch that also touches an input driver.

I usually do "subsystem / driver: short description" but if you want
the ":" as delimiter I can do this.

> Then, this is a pretty big patchset, with iio, input and mfd all mixed
> together and it is likely to create merge conflicts.
They somehow depend on each other. Otherwise I would have sent three
series, one per subsystem.

>>>From what I can see from it, and please correct me if I'm
> wrong, the iio and input changes depend on the mfd ones, and not the
> other way around. If that's so, I'm going to ask you to reshuffle your
> patch set and separate the MFD changes from the iio and input ones. I'll
> take the MFD ones and will create an immutable branch for Jonathan and
> Dmitry to pull from and apply the iio and input changes on top of it.
> Merge conflicts should be mostly avoided that way.
> AFAICT, only patch #2 should be kept with input and iio bits mixed
> together with MFD as otherwise this would introduce functional breakage.
> Otherwise, all MFD bits from the other patches could be either separated
> or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
> #17).
> 
> Does that sound doable to you ?

The device renaming shouldn't matter since I added DT nodes for the mfd
child devices earlier. But then the of_compatible assignments should
go hand in hand. However if I split this then the driver won't work
but then it does not now as well (because there is no platform_data
provider in tree).

Still. There is #18 which reworks the "step addressing" and involves
changes in both (iio & input) at the same time.
There is #21. Adding this to the initial "DT support" patch would be bad
I think because it requires some changes on the iio side which have
nothing to do with DT. Putting the iio changes before DT would require
to make some change to platform-data part which will go away anyway.

So I started collecting ACKs from input and iio to avoid this split. If
you really want the split then I will start doing so…

> Cheers,
> Samuel.

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 05:29:22PM +0200, Sebastian Andrzej Siewior wrote:
> > Then, this is a pretty big patchset, with iio, input and mfd all mixed
> > together and it is likely to create merge conflicts.
> They somehow depend on each other. Otherwise I would have sent three
> series, one per subsystem.
Of course they depend on each other, but the dependency is mostly for
iio and input to depend on the MFD changes.


> >>From what I can see from it, and please correct me if I'm
> > wrong, the iio and input changes depend on the mfd ones, and not the
> > other way around. If that's so, I'm going to ask you to reshuffle your
> > patch set and separate the MFD changes from the iio and input ones. I'll
> > take the MFD ones and will create an immutable branch for Jonathan and
> > Dmitry to pull from and apply the iio and input changes on top of it.
> > Merge conflicts should be mostly avoided that way.
> > AFAICT, only patch #2 should be kept with input and iio bits mixed
> > together with MFD as otherwise this would introduce functional breakage.
> > Otherwise, all MFD bits from the other patches could be either separated
> > or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
> > #17).
> > 
> > Does that sound doable to you ?
> 
> The device renaming shouldn't matter since I added DT nodes for the mfd
> child devices earlier. But then the of_compatible assignments should
> go hand in hand. However if I split this then the driver won't work
> but then it does not now as well (because there is no platform_data
> provider in tree).
> 
> Still. There is #18 which reworks the "step addressing" and involves
> changes in both (iio & input) at the same time.
Would splitting iio and input break anything there ?


> There is #21. Adding this to the initial "DT support" patch would be bad
> I think because it requires some changes on the iio side which have
> nothing to do with DT. Putting the iio changes before DT would require
> to make some change to platform-data part which will go away anyway.
Wouldn't it workif you split this one into an MFD+dts file changes and
another one for the iio changes ?

 
> So I started collecting ACKs from input and iio to avoid this split. If
> you really want the split then I will start doing so…
I think it would be nicer, yes.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Sebastian,

On Tue, Jun 11, 2013 at 05:29:22PM +0200, Sebastian Andrzej Siewior wrote:
> > Then, this is a pretty big patchset, with iio, input and mfd all mixed
> > together and it is likely to create merge conflicts.
> They somehow depend on each other. Otherwise I would have sent three
> series, one per subsystem.
Of course they depend on each other, but the dependency is mostly for
iio and input to depend on the MFD changes.


> >>From what I can see from it, and please correct me if I'm
> > wrong, the iio and input changes depend on the mfd ones, and not the
> > other way around. If that's so, I'm going to ask you to reshuffle your
> > patch set and separate the MFD changes from the iio and input ones. I'll
> > take the MFD ones and will create an immutable branch for Jonathan and
> > Dmitry to pull from and apply the iio and input changes on top of it.
> > Merge conflicts should be mostly avoided that way.
> > AFAICT, only patch #2 should be kept with input and iio bits mixed
> > together with MFD as otherwise this would introduce functional breakage.
> > Otherwise, all MFD bits from the other patches could be either separated
> > or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
> > #17).
> > 
> > Does that sound doable to you ?
> 
> The device renaming shouldn't matter since I added DT nodes for the mfd
> child devices earlier. But then the of_compatible assignments should
> go hand in hand. However if I split this then the driver won't work
> but then it does not now as well (because there is no platform_data
> provider in tree).
> 
> Still. There is #18 which reworks the "step addressing" and involves
> changes in both (iio & input) at the same time.
Would splitting iio and input break anything there ?


> There is #21. Adding this to the initial "DT support" patch would be bad
> I think because it requires some changes on the iio side which have
> nothing to do with DT. Putting the iio changes before DT would require
> to make some change to platform-data part which will go away anyway.
Wouldn't it workif you split this one into an MFD+dts file changes and
another one for the iio changes ?

 
> So I started collecting ACKs from input and iio to avoid this split. If
> you really want the split then I will start doing so…
I think it would be nicer, yes.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/11/2013 06:10 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 05:29:22PM +0200, Sebastian Andrzej Siewior wrote:
>>> Then, this is a pretty big patchset, with iio, input and mfd all mixed
>>> together and it is likely to create merge conflicts.
>> They somehow depend on each other. Otherwise I would have sent three
>> series, one per subsystem.
> Of course they depend on each other, but the dependency is mostly for
> iio and input to depend on the MFD changes.

Except for the one #18 as mentioned below.

>> Still. There is #18 which reworks the "step addressing" and involves
>> changes in both (iio & input) at the same time.
> Would splitting iio and input break anything there ?

Yes. Once the header files is modified without the two .c files the
driver is not working. To fix this I need another patch making sure
input + iio does not the header files and another to user it (once
everything is merged).

>> There is #21. Adding this to the initial "DT support" patch would be bad
>> I think because it requires some changes on the iio side which have
>> nothing to do with DT. Putting the iio changes before DT would require
>> to make some change to platform-data part which will go away anyway.
> Wouldn't it workif you split this one into an MFD+dts file changes and
> another one for the iio changes ?

It would work in general. The first few DT-iio patches wouldn't make
sense but then why not.

>> So I started collecting ACKs from input and iio to avoid this split. If
>> you really want the split then I will start doing so…
> I think it would be nicer, yes.

Nicer. I see. Please tell me what you think about #1 because I really
would like to drop regmap and then I can start reshuffle the series :)

> 
> Cheers,
> Samuel.
> 

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/11/2013 06:10 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

> On Tue, Jun 11, 2013 at 05:29:22PM +0200, Sebastian Andrzej Siewior wrote:
>>> Then, this is a pretty big patchset, with iio, input and mfd all mixed
>>> together and it is likely to create merge conflicts.
>> They somehow depend on each other. Otherwise I would have sent three
>> series, one per subsystem.
> Of course they depend on each other, but the dependency is mostly for
> iio and input to depend on the MFD changes.

Except for the one #18 as mentioned below.

>> Still. There is #18 which reworks the "step addressing" and involves
>> changes in both (iio & input) at the same time.
> Would splitting iio and input break anything there ?

Yes. Once the header files is modified without the two .c files the
driver is not working. To fix this I need another patch making sure
input + iio does not the header files and another to user it (once
everything is merged).

>> There is #21. Adding this to the initial "DT support" patch would be bad
>> I think because it requires some changes on the iio side which have
>> nothing to do with DT. Putting the iio changes before DT would require
>> to make some change to platform-data part which will go away anyway.
> Wouldn't it workif you split this one into an MFD+dts file changes and
> another one for the iio changes ?

It would work in general. The first few DT-iio patches wouldn't make
sense but then why not.

>> So I started collecting ACKs from input and iio to avoid this split. If
>> you really want the split then I will start doing so…
> I think it would be nicer, yes.

Nicer. I see. Please tell me what you think about #1 because I really
would like to drop regmap and then I can start reshuffle the series :)

> 
> Cheers,
> Samuel.
> 

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

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Mark Brown]

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

On Tue, Jun 11, 2013 at 05:29:22PM +0200, Sebastian Andrzej Siewior wrote:
> On 06/11/2013 04:23 PM, Samuel Ortiz wrote:

> > Please fix your commit logs, and your subject lines. It should be e.g.
> > mfd: input: ti_am335x_adc: Blablabla

> > if it's mostly an mfd patch that also touches an input driver.

> I usually do "subsystem / driver: short description" but if you want
> the ":" as delimiter I can do this.

You should always aim to be consistent with the style used by the code
you're updating - do a git log and make sure your patches don't have
changelogs that are obivously using a different style.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Mark Brown]

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

On Tue, Jun 11, 2013 at 05:29:22PM +0200, Sebastian Andrzej Siewior wrote:
> On 06/11/2013 04:23 PM, Samuel Ortiz wrote:

> > Please fix your commit logs, and your subject lines. It should be e.g.
> > mfd: input: ti_am335x_adc: Blablabla

> > if it's mostly an mfd patch that also touches an input driver.

> I usually do "subsystem / driver: short description" but if you want
> the ":" as delimiter I can do this.

You should always aim to be consistent with the style used by the code
you're updating - do a git log and make sure your patches don't have
changelogs that are obivously using a different style.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Dmitry Torokhov]

Hi Samuel,

On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
> Hi Sebastian,
> 
> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
> > I believe the whole thing should go via the MFD tree. It touches also
> > input & iio subsystem. I collected ACKs where I got some in the meantime.
> Please fix your commit logs, and your subject lines. It should be e.g.
> mfd: input: ti_am335x_adc: Blablabla
> 
> if it's mostly an mfd patch that also touches an input driver.
> 
> Then, this is a pretty big patchset, with iio, input and mfd all mixed
> together and it is likely to create merge conflicts.
> From what I can see from it, and please correct me if I'm
> wrong, the iio and input changes depend on the mfd ones, and not the
> other way around. If that's so, I'm going to ask you to reshuffle your
> patch set and separate the MFD changes from the iio and input ones. I'll
> take the MFD ones and will create an immutable branch for Jonathan and
> Dmitry to pull from and apply the iio and input changes on top of it.
> Merge conflicts should be mostly avoided that way.

I purposely left this driver alone as I expected it would be merged
through MFD tree, so there should not be any merging issues on my side.

Thanks.

-- 
Dmitry

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Dmitry Torokhov]

Hi Samuel,

On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
> Hi Sebastian,
> 
> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
> > I believe the whole thing should go via the MFD tree. It touches also
> > input & iio subsystem. I collected ACKs where I got some in the meantime.
> Please fix your commit logs, and your subject lines. It should be e.g.
> mfd: input: ti_am335x_adc: Blablabla
> 
> if it's mostly an mfd patch that also touches an input driver.
> 
> Then, this is a pretty big patchset, with iio, input and mfd all mixed
> together and it is likely to create merge conflicts.
> From what I can see from it, and please correct me if I'm
> wrong, the iio and input changes depend on the mfd ones, and not the
> other way around. If that's so, I'm going to ask you to reshuffle your
> patch set and separate the MFD changes from the iio and input ones. I'll
> take the MFD ones and will create an immutable branch for Jonathan and
> Dmitry to pull from and apply the iio and input changes on top of it.
> Merge conflicts should be mostly avoided that way.

I purposely left this driver alone as I expected it would be merged
through MFD tree, so there should not be any merging issues on my side.

Thanks.

-- 
Dmitry

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Dmitry,

On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
> > Hi Sebastian,
> > 
> > On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
> > > I believe the whole thing should go via the MFD tree. It touches also
> > > input & iio subsystem. I collected ACKs where I got some in the meantime.
> > Please fix your commit logs, and your subject lines. It should be e.g.
> > mfd: input: ti_am335x_adc: Blablabla
> > 
> > if it's mostly an mfd patch that also touches an input driver.
> > 
> > Then, this is a pretty big patchset, with iio, input and mfd all mixed
> > together and it is likely to create merge conflicts.
> > From what I can see from it, and please correct me if I'm
> > wrong, the iio and input changes depend on the mfd ones, and not the
> > other way around. If that's so, I'm going to ask you to reshuffle your
> > patch set and separate the MFD changes from the iio and input ones. I'll
> > take the MFD ones and will create an immutable branch for Jonathan and
> > Dmitry to pull from and apply the iio and input changes on top of it.
> > Merge conflicts should be mostly avoided that way.
> 
> I purposely left this driver alone as I expected it would be merged
> through MFD tree, so there should not be any merging issues on my side.
Thanks for the notice.
Jonathan, can you guarantee the same for the iio parts ?

Sabastian, hold on before reworking your patchset.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Dmitry,

On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
> > Hi Sebastian,
> > 
> > On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
> > > I believe the whole thing should go via the MFD tree. It touches also
> > > input & iio subsystem. I collected ACKs where I got some in the meantime.
> > Please fix your commit logs, and your subject lines. It should be e.g.
> > mfd: input: ti_am335x_adc: Blablabla
> > 
> > if it's mostly an mfd patch that also touches an input driver.
> > 
> > Then, this is a pretty big patchset, with iio, input and mfd all mixed
> > together and it is likely to create merge conflicts.
> > From what I can see from it, and please correct me if I'm
> > wrong, the iio and input changes depend on the mfd ones, and not the
> > other way around. If that's so, I'm going to ask you to reshuffle your
> > patch set and separate the MFD changes from the iio and input ones. I'll
> > take the MFD ones and will create an immutable branch for Jonathan and
> > Dmitry to pull from and apply the iio and input changes on top of it.
> > Merge conflicts should be mostly avoided that way.
> 
> I purposely left this driver alone as I expected it would be merged
> through MFD tree, so there should not be any merging issues on my side.
Thanks for the notice.
Jonathan, can you guarantee the same for the iio parts ?

Sabastian, hold on before reworking your patchset.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Jonathan Cameron]

Samuel Ortiz <sameo@linux.intel.com> wrote:

>Hi Dmitry,
>
>On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
>> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
>> > Hi Sebastian,
>> > 
>> > On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior
>wrote:
>> > > I believe the whole thing should go via the MFD tree. It touches
>also
>> > > input & iio subsystem. I collected ACKs where I got some in the
>meantime.
>> > Please fix your commit logs, and your subject lines. It should be
>e.g.
>> > mfd: input: ti_am335x_adc: Blablabla
>> > 
>> > if it's mostly an mfd patch that also touches an input driver.
>> > 
>> > Then, this is a pretty big patchset, with iio, input and mfd all
>mixed
>> > together and it is likely to create merge conflicts.
>> > From what I can see from it, and please correct me if I'm
>> > wrong, the iio and input changes depend on the mfd ones, and not
>the
>> > other way around. If that's so, I'm going to ask you to reshuffle
>your
>> > patch set and separate the MFD changes from the iio and input ones.
>I'll
>> > take the MFD ones and will create an immutable branch for Jonathan
>and
>> > Dmitry to pull from and apply the iio and input changes on top of
>it.
>> > Merge conflicts should be mostly avoided that way.
>> 
>> I purposely left this driver alone as I expected it would be merged
>> through MFD tree, so there should not be any merging issues on my
>side.
>Thanks for the notice.
>Jonathan, can you guarantee the same for the iio parts ?
I have also been avoiding taking any of these and there are unlikely to be any iio wide changes merging at this stage in cycle.  Hence these going through MFD is best plan.

Lars raised a couple of issues so would be best to wait for his ack if he hasn't already looked at this revision.

Thanks

Jonathan. 
>
>Sabastian, hold on before reworking your patchset.
>
>Cheers,
>Samuel.

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Jonathan Cameron]

Samuel Ortiz <sameo@linux.intel.com> wrote:

>Hi Dmitry,
>
>On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
>> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
>> > Hi Sebastian,
>> > 
>> > On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior
>wrote:
>> > > I believe the whole thing should go via the MFD tree. It touches
>also
>> > > input & iio subsystem. I collected ACKs where I got some in the
>meantime.
>> > Please fix your commit logs, and your subject lines. It should be
>e.g.
>> > mfd: input: ti_am335x_adc: Blablabla
>> > 
>> > if it's mostly an mfd patch that also touches an input driver.
>> > 
>> > Then, this is a pretty big patchset, with iio, input and mfd all
>mixed
>> > together and it is likely to create merge conflicts.
>> > From what I can see from it, and please correct me if I'm
>> > wrong, the iio and input changes depend on the mfd ones, and not
>the
>> > other way around. If that's so, I'm going to ask you to reshuffle
>your
>> > patch set and separate the MFD changes from the iio and input ones.
>I'll
>> > take the MFD ones and will create an immutable branch for Jonathan
>and
>> > Dmitry to pull from and apply the iio and input changes on top of
>it.
>> > Merge conflicts should be mostly avoided that way.
>> 
>> I purposely left this driver alone as I expected it would be merged
>> through MFD tree, so there should not be any merging issues on my
>side.
>Thanks for the notice.
>Jonathan, can you guarantee the same for the iio parts ?
I have also been avoiding taking any of these and there are unlikely to be any iio wide changes merging at this stage in cycle.  Hence these going through MFD is best plan.

Lars raised a couple of issues so would be best to wait for his ack if he hasn't already looked at this revision.

Thanks

Jonathan. 
>
>Sabastian, hold on before reworking your patchset.
>
>Cheers,
>Samuel.

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Lars-Peter Clausen]

On 06/11/2013 06:27 PM, Jonathan Cameron wrote:
> 
> 
> Samuel Ortiz <sameo@linux.intel.com> wrote:
> 
>> Hi Dmitry,
>>
>> On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
>>> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
>>>> Hi Sebastian,
>>>>
>>>> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior
>> wrote:
>>>>> I believe the whole thing should go via the MFD tree. It touches
>> also
>>>>> input & iio subsystem. I collected ACKs where I got some in the
>> meantime.
>>>> Please fix your commit logs, and your subject lines. It should be
>> e.g.
>>>> mfd: input: ti_am335x_adc: Blablabla
>>>>
>>>> if it's mostly an mfd patch that also touches an input driver.
>>>>
>>>> Then, this is a pretty big patchset, with iio, input and mfd all
>> mixed
>>>> together and it is likely to create merge conflicts.
>>>> From what I can see from it, and please correct me if I'm
>>>> wrong, the iio and input changes depend on the mfd ones, and not
>> the
>>>> other way around. If that's so, I'm going to ask you to reshuffle
>> your
>>>> patch set and separate the MFD changes from the iio and input ones.
>> I'll
>>>> take the MFD ones and will create an immutable branch for Jonathan
>> and
>>>> Dmitry to pull from and apply the iio and input changes on top of
>> it.
>>>> Merge conflicts should be mostly avoided that way.
>>>
>>> I purposely left this driver alone as I expected it would be merged
>>> through MFD tree, so there should not be any merging issues on my
>> side.
>> Thanks for the notice.
>> Jonathan, can you guarantee the same for the iio parts ?
> I have also been avoiding taking any of these and there are unlikely to be any iio wide changes merging at this stage in cycle.  Hence these going through MFD is best plan.
> 
> Lars raised a couple of issues so would be best to wait for his ack if he hasn't already looked at this revision.

The IIO bits look fine to me in this version.

- Lars

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Lars-Peter Clausen]

On 06/11/2013 06:27 PM, Jonathan Cameron wrote:
> 
> 
> Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 
>> Hi Dmitry,
>>
>> On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
>>> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
>>>> Hi Sebastian,
>>>>
>>>> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior
>> wrote:
>>>>> I believe the whole thing should go via the MFD tree. It touches
>> also
>>>>> input & iio subsystem. I collected ACKs where I got some in the
>> meantime.
>>>> Please fix your commit logs, and your subject lines. It should be
>> e.g.
>>>> mfd: input: ti_am335x_adc: Blablabla
>>>>
>>>> if it's mostly an mfd patch that also touches an input driver.
>>>>
>>>> Then, this is a pretty big patchset, with iio, input and mfd all
>> mixed
>>>> together and it is likely to create merge conflicts.
>>>> From what I can see from it, and please correct me if I'm
>>>> wrong, the iio and input changes depend on the mfd ones, and not
>> the
>>>> other way around. If that's so, I'm going to ask you to reshuffle
>> your
>>>> patch set and separate the MFD changes from the iio and input ones.
>> I'll
>>>> take the MFD ones and will create an immutable branch for Jonathan
>> and
>>>> Dmitry to pull from and apply the iio and input changes on top of
>> it.
>>>> Merge conflicts should be mostly avoided that way.
>>>
>>> I purposely left this driver alone as I expected it would be merged
>>> through MFD tree, so there should not be any merging issues on my
>> side.
>> Thanks for the notice.
>> Jonathan, can you guarantee the same for the iio parts ?
> I have also been avoiding taking any of these and there are unlikely to be any iio wide changes merging at this stage in cycle.  Hence these going through MFD is best plan.
> 
> Lars raised a couple of issues so would be best to wait for his ack if he hasn't already looked at this revision.

The IIO bits look fine to me in this version.

- Lars

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Jonathan,

On Tue, Jun 11, 2013 at 05:27:48PM +0100, Jonathan Cameron wrote:
> Samuel Ortiz <sameo@linux.intel.com> wrote:
> >On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
> >> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
> >> > Hi Sebastian,
> >> > 
> >> > On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior
> >wrote:
> >> > > I believe the whole thing should go via the MFD tree. It touches
> >also
> >> > > input & iio subsystem. I collected ACKs where I got some in the
> >meantime.
> >> > Please fix your commit logs, and your subject lines. It should be
> >e.g.
> >> > mfd: input: ti_am335x_adc: Blablabla
> >> > 
> >> > if it's mostly an mfd patch that also touches an input driver.
> >> > 
> >> > Then, this is a pretty big patchset, with iio, input and mfd all
> >mixed
> >> > together and it is likely to create merge conflicts.
> >> > From what I can see from it, and please correct me if I'm
> >> > wrong, the iio and input changes depend on the mfd ones, and not
> >the
> >> > other way around. If that's so, I'm going to ask you to reshuffle
> >your
> >> > patch set and separate the MFD changes from the iio and input ones.
> >I'll
> >> > take the MFD ones and will create an immutable branch for Jonathan
> >and
> >> > Dmitry to pull from and apply the iio and input changes on top of
> >it.
> >> > Merge conflicts should be mostly avoided that way.
> >> 
> >> I purposely left this driver alone as I expected it would be merged
> >> through MFD tree, so there should not be any merging issues on my
> >side.
> >Thanks for the notice.
> >Jonathan, can you guarantee the same for the iio parts ?
> I have also been avoiding taking any of these and there are unlikely to be any iio wide changes merging at this stage in cycle.  Hence these going through MFD is best plan.
>
Thanks. Then I'm willing to try and see if linux-next does not complain
too hard about that.

Sebastian, please address the commit log and cosmetic issues I pointed out,
keep the regmap code and I'll pull this patchset in.
If further down the road we get some nasty merge conflicts from
linux-next, I might ask you to re-work it. Let's see.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Jonathan,

On Tue, Jun 11, 2013 at 05:27:48PM +0100, Jonathan Cameron wrote:
> Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> >On Tue, Jun 11, 2013 at 09:04:16AM -0700, Dmitry Torokhov wrote:
> >> On Tue, Jun 11, 2013 at 04:23:58PM +0200, Samuel Ortiz wrote:
> >> > Hi Sebastian,
> >> > 
> >> > On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior
> >wrote:
> >> > > I believe the whole thing should go via the MFD tree. It touches
> >also
> >> > > input & iio subsystem. I collected ACKs where I got some in the
> >meantime.
> >> > Please fix your commit logs, and your subject lines. It should be
> >e.g.
> >> > mfd: input: ti_am335x_adc: Blablabla
> >> > 
> >> > if it's mostly an mfd patch that also touches an input driver.
> >> > 
> >> > Then, this is a pretty big patchset, with iio, input and mfd all
> >mixed
> >> > together and it is likely to create merge conflicts.
> >> > From what I can see from it, and please correct me if I'm
> >> > wrong, the iio and input changes depend on the mfd ones, and not
> >the
> >> > other way around. If that's so, I'm going to ask you to reshuffle
> >your
> >> > patch set and separate the MFD changes from the iio and input ones.
> >I'll
> >> > take the MFD ones and will create an immutable branch for Jonathan
> >and
> >> > Dmitry to pull from and apply the iio and input changes on top of
> >it.
> >> > Merge conflicts should be mostly avoided that way.
> >> 
> >> I purposely left this driver alone as I expected it would be merged
> >> through MFD tree, so there should not be any merging issues on my
> >side.
> >Thanks for the notice.
> >Jonathan, can you guarantee the same for the iio parts ?
> I have also been avoiding taking any of these and there are unlikely to be any iio wide changes merging at this stage in cycle.  Hence these going through MFD is best plan.
>
Thanks. Then I'm willing to try and see if linux-next does not complain
too hard about that.

Sebastian, please address the commit log and cosmetic issues I pointed out,
keep the regmap code and I'll pull this patchset in.
If further down the road we get some nasty merge conflicts from
linux-next, I might ask you to re-work it. Let's see.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/11/2013 07:55 PM, Samuel Ortiz wrote:
> Hi Jonathan,

Hi Samuel,

> Sebastian, please address the commit log and cosmetic issues I pointed out,
> keep the regmap code and I'll pull this patchset in.

By keep the regmap code you mean I am allowed to remove the regmap
usage in mfd (keep patch #1) or do you insist on adding its usage in
iio and input?

> Cheers,
> Samuel.

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Sebastian,

On Wed, Jun 12, 2013 at 03:29:43PM +0200, Sebastian Andrzej Siewior wrote:
> On 06/11/2013 07:55 PM, Samuel Ortiz wrote:
> > Hi Jonathan,
> 
> Hi Samuel,
> 
> > Sebastian, please address the commit log and cosmetic issues I pointed out,
> > keep the regmap code and I'll pull this patchset in.
>
> By keep the regmap code you mean I am allowed to remove the regmap
> usage in mfd (keep patch #1) or do you insist on adding its usage in
> iio and input?
I insist on keeping it on the MFD driver, i.e. _not_ keeping patch #1.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

Hi Sebastian,

On Wed, Jun 12, 2013 at 03:29:43PM +0200, Sebastian Andrzej Siewior wrote:
> On 06/11/2013 07:55 PM, Samuel Ortiz wrote:
> > Hi Jonathan,
> 
> Hi Samuel,
> 
> > Sebastian, please address the commit log and cosmetic issues I pointed out,
> > keep the regmap code and I'll pull this patchset in.
>
> By keep the regmap code you mean I am allowed to remove the regmap
> usage in mfd (keep patch #1) or do you insist on adding its usage in
> iio and input?
I insist on keeping it on the MFD driver, i.e. _not_ keeping patch #1.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/12/2013 03:50 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

>> By keep the regmap code you mean I am allowed to remove the regmap
>> usage in mfd (keep patch #1) or do you insist on adding its usage in
>> iio and input?
> I insist on keeping it on the MFD driver, i.e. _not_ keeping patch #1.

This forces me redo a few things and most likely adding it to the
iio and input driver to be consistent here.

Could you please give a reason why using the regmap here is a good
thing? I mentioned a few why it is not and why is better to leave it
out.

> Cheers,
> Samuel.

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/12/2013 03:50 PM, Samuel Ortiz wrote:
> Hi Sebastian,

Hi Samuel,

>> By keep the regmap code you mean I am allowed to remove the regmap
>> usage in mfd (keep patch #1) or do you insist on adding its usage in
>> iio and input?
> I insist on keeping it on the MFD driver, i.e. _not_ keeping patch #1.

This forces me redo a few things and most likely adding it to the
iio and input driver to be consistent here.

Could you please give a reason why using the regmap here is a good
thing? I mentioned a few why it is not and why is better to leave it
out.

> Cheers,
> Samuel.

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Samuel Ortiz]

On Wed, Jun 12, 2013 at 04:02:00PM +0200, Sebastian Andrzej Siewior wrote:
> >> By keep the regmap code you mean I am allowed to remove the regmap
> >> usage in mfd (keep patch #1) or do you insist on adding its usage in
> >> iio and input?
> > I insist on keeping it on the MFD driver, i.e. _not_ keeping patch #1.
> 
> This forces me redo a few things and most likely adding it to the
> iio and input driver to be consistent here.
I'm not asking for that. The current MFD code uses regmap fine without
the iio and input using it, I don't see why you would have to add regmap
support there.

> Could you please give a reason why using the regmap here is a good
> thing? I mentioned a few why it is not and why is better to leave it
> out.
Yes, and I'm not convinced obviously. regmap prevents you from open
coding your MMIO access, and this is the right approach.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Sebastian Andrzej Siewior]

On 06/12/2013 04:41 PM, Samuel Ortiz wrote:
>> This forces me redo a few things and most likely adding it to the
>> iio and input driver to be consistent here.
> I'm not asking for that. The current MFD code uses regmap fine without
> the iio and input using it, I don't see why you would have to add regmap
> support there.

Right. Since no reg-cache is used then this should be fine then.

>> Could you please give a reason why using the regmap here is a good
>> thing? I mentioned a few why it is not and why is better to leave it
>> out.
> Yes, and I'm not convinced obviously. regmap prevents you from open
> coding your MMIO access, and this is the right approach.

I am not convinced that adding another layer of indirection for doing
the same thing is a good approach. Pointer chasing _is_ expensive.
_None_ of the regmap features are used here.
I would understand if I would re-implement register caching or a
wrapper across multiple buses but nothing of this is the case.
The current code even ignores the return value.

So, are we going to convert all drivers to use regmap now?

> Cheers,
> Samuel.
> 

Sebastian

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Jonathan Cameron]

On 06/11/2013 03:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,
> 
> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
>> I believe the whole thing should go via the MFD tree. It touches also
>> input & iio subsystem. I collected ACKs where I got some in the meantime.
> Please fix your commit logs, and your subject lines. It should be e.g.
> mfd: input: ti_am335x_adc: Blablabla
> 
> if it's mostly an mfd patch that also touches an input driver.
> 
> Then, this is a pretty big patchset, with iio, input and mfd all mixed
> together and it is likely to create merge conflicts.
> From what I can see from it, and please correct me if I'm
> wrong, the iio and input changes depend on the mfd ones, and not the
> other way around. If that's so, I'm going to ask you to reshuffle your
> patch set and separate the MFD changes from the iio and input ones. I'll
> take the MFD ones and will create an immutable branch for Jonathan and
> Dmitry to pull from and apply the iio and input changes on top of it.
> Merge conflicts should be mostly avoided that way.

I'd just like to note for future reference that I would prefer Samuels
approach of such a branch for future cases where things touch on iio
and another subsystem.

Now as I've expressed I am happy with this set going through mfd
but there is never anything wrong with agreeing how things 'should'
be done ;)

> AFAICT, only patch #2 should be kept with input and iio bits mixed
> together with MFD as otherwise this would introduce functional breakage.
> Otherwise, all MFD bits from the other patches could be either separated
> or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
> #17).

> 
> Does that sound doable to you ?
> 
> Cheers,
> Samuel.
> 

Re: am335x: TSC & ADC reworking including DT pieces, take 4

[Jonathan Cameron]

On 06/11/2013 03:23 PM, Samuel Ortiz wrote:
> Hi Sebastian,
> 
> On Tue, Jun 11, 2013 at 01:30:46PM +0200, Sebastian Andrzej Siewior wrote:
>> I believe the whole thing should go via the MFD tree. It touches also
>> input & iio subsystem. I collected ACKs where I got some in the meantime.
> Please fix your commit logs, and your subject lines. It should be e.g.
> mfd: input: ti_am335x_adc: Blablabla
> 
> if it's mostly an mfd patch that also touches an input driver.
> 
> Then, this is a pretty big patchset, with iio, input and mfd all mixed
> together and it is likely to create merge conflicts.
> From what I can see from it, and please correct me if I'm
> wrong, the iio and input changes depend on the mfd ones, and not the
> other way around. If that's so, I'm going to ask you to reshuffle your
> patch set and separate the MFD changes from the iio and input ones. I'll
> take the MFD ones and will create an immutable branch for Jonathan and
> Dmitry to pull from and apply the iio and input changes on top of it.
> Merge conflicts should be mostly avoided that way.

I'd just like to note for future reference that I would prefer Samuels
approach of such a branch for future cases where things touch on iio
and another subsystem.

Now as I've expressed I am happy with this set going through mfd
but there is never anything wrong with agreeing how things 'should'
be done ;)

> AFAICT, only patch #2 should be kept with input and iio bits mixed
> together with MFD as otherwise this would introduce functional breakage.
> Otherwise, all MFD bits from the other patches could be either separated
> or merged together (e.g. MFD bits from patches #6 and #8, and #16 and
> #17).

> 
> Does that sound doable to you ?
> 
> Cheers,
> Samuel.
> 

AM335x tsc & adc, dt + cleanup take 3

[Sebastian Andrzej Siewior]

Hi Samuel,

can please take this via your mfd tree? I collected a few input & iio ACKs
for some of these patches. Unless ofcource someone wants a redo…

The following changes since commit d683b96b072dc4680fc74964eca77e6a23d1fa6e:

  Linux 3.10-rc4 (2013-06-02 17:11:17 +0900)

are available in the git repository at:

  git://git.breakpoint.cc/bigeasy/linux.git tags/am335x_tsc-adc

for you to fetch changes up to d1b713a3307101952215631046ced892397d1df4:

  iio/ti_am335x_adc: check if we found the value (2013-06-05 17:51:33 +0200)

----------------------------------------------------------------
A complete refurbished series inclunding:
- DT support for the MFD, TSC and ADC driver & platform device support,
  which has no users, has been killed
- suspend fixes which means correct data structs are taken and no
  interrupt storm
- fifo split which should problem with TSC & ADC beeing used at the same
  time
- The ADC channels are now checked before blindly applied. That means the
  touch part reads X, Y and Z coordinates and does not mix them up. Same
  goes for the IIO ADC driver.
- The IIO ADC driver now creates files named in_voltageX_raw where X
  represents the ADC line instead of a number starting at 0. A read from
  this file can return -EBUSY in case touch is busy and the ADC didn't
  collect a value.

----------------------------------------------------------------
Pantelis Antoniou (2):
      iio/ti_tscadc: Update with IIO map interface
      mfd/ti_tscadc: deal with partial activation

Patil, Rachna (7):
      input/ti_am33x_tsc: Step enable bits made configurable
      input/ti_am33x_tsc: Order of TSC wires, made configurable
      input/ti_am33x_tsc: remove unwanted fifo flush
      input/ti_am33x_tsc: Add DT support
      iio/ti_am335x_adc: Add DT support
      mfd/ti_am335x_tscadc: Add DT support
      arm/am33xx: add TSC/ADC mfd device support

Sebastian Andrzej Siewior (13):
      mfd/ti_am335x_tscadc: remove regmap
      mfd & input & iio/ti_am335x_adc: use one structure for ti_tscadc_dev
      input/ti_am33x_tsc: remove platform_data support
      iio/ti_am335x_adc: remove platform_data support
      mfd/ti_am335x_tscadc: remove platform_data support
      input & mfd: ti_am335x_tsc remove remaining platform data pieces
      mfd & input/ti_am335x_tsc: rename device from tsc to TI-am335x-tsc
      mfd & iio/ti_am335x_adc: rename device from tiadc to TI-am335x-adc
      input/ti_am335x_adc: use only FIFO0 and clean up a little
      input/ti_am335x_tsc: ACK the HW_PEN irq in ISR
      input/ti_am335x_tsc: return IRQ_NONE if there was no IRQ for us
      iio/ti_am335x_adc: Allow to specify input line
      iio/ti_am335x_adc: check if we found the value

 .../bindings/input/touchscreen/ti-tsc-adc.txt      |   44 +++
 arch/arm/boot/dts/am335x-evm.dts                   |   14 +
 arch/arm/boot/dts/am33xx.dtsi                      |    9 +
 drivers/iio/adc/ti_am335x_adc.c                    |  160 ++++++++---
 drivers/input/touchscreen/ti_am335x_tsc.c          |  286 ++++++++++++++------
 drivers/mfd/ti_am335x_tscadc.c                     |  131 ++++++---
 include/linux/input/ti_am335x_tsc.h                |   23 --
 include/linux/mfd/ti_am335x_tscadc.h               |   43 +--
 include/linux/platform_data/ti_am335x_adc.h        |   14 -
 9 files changed, 509 insertions(+), 215 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
 delete mode 100644 include/linux/input/ti_am335x_tsc.h
 delete mode 100644 include/linux/platform_data/ti_am335x_adc.h

Sebastian

[PATCH 18/22] input/ti_am335x_adc: use only FIFO0 and clean up a little

[Sebastian Andrzej Siewior]

The driver programs a threshold of "coordiante_readouts" say 5. The
REG_FIFO0THR registers says it should it be programmed to "threshold
minus one". The driver does not expect just 5 coordinates but 5 * 2 + 2.
Multiplied by two because 5 for X and 5 for Y and plus 2 because we have
two Z.
The whole thing kind of works because It reads the 5 coordinates for X
and Y from FIFO0 and FIFO1 and the last element in each FIFO is ignored
within the loop and read later.
Nothing guaranties that FIFO1 is ready by the time it is read. In fact I
could see that that FIFO1 reaturns for Y channels 8,9, 10, 12, 6 and for
Y channel 7 for Z. The problem is that channel 7 and channel 12 got
somehow mixed up.
The other Problem is that FIFO1 is also used by the IIO part leading to
wrong results if both (tsc & adc) are used.

The patch tries to clean up the whole thing a little:
- Remove the +1 and -1 in REG_STEPCONFIG, REG_STEPDELAY and its counter
  part in the for loop. This is just confusing.

- Use only FIFO0 in TSC. The fifo has space for 64 entries so should be
  fine.

- Read the whole FIFO in one function and check the channel.

- in case we dawdle around, make sure we only read a multiple of our
  coordinate set. On the second interrupt we will cleanup the remaining
  enties.

Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 drivers/iio/adc/ti_am335x_adc.c           |    2 +-
 drivers/input/touchscreen/ti_am335x_tsc.c |   78 +++++++++++++++--------------
 include/linux/mfd/ti_am335x_tscadc.h      |    4 +-
 3 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 38f2415..031f8e5 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -76,7 +76,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 
 	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
 
-	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
+	for (i = steps; i < TOTAL_STEPS; i++) {
 		tiadc_writel(adc_dev, REG_STEPCONFIG(i),
 				stepconfig | STEPCONFIG_INP(channels));
 		tiadc_writel(adc_dev, REG_STEPDELAY(i),
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 7a76b14..fa9692c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -120,11 +120,9 @@ static int titsc_config_wires(struct titsc *ts_dev)
 static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
-	unsigned int	stepenable = 0;
-	int i, total_steps;
-
-	/* Configure the Step registers */
-	total_steps = 2 * ts_dev->coordiante_readouts;
+	int i;
+	int end_step;
+	u32 stepenable;
 
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_xp;
@@ -142,7 +140,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = 1; i <= ts_dev->coordiante_readouts; i++) {
+	/* 1 … coordiante_readouts is for X */
+	end_step = ts_dev->coordiante_readouts;
+	for (i = 0; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -150,7 +150,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	config = 0;
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yn |
-			STEPCONFIG_INM_ADCREFM | STEPCONFIG_FIFO1;
+			STEPCONFIG_INM_ADCREFM;
 	switch (ts_dev->wires) {
 	case 4:
 		config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp);
@@ -164,12 +164,13 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = (ts_dev->coordiante_readouts + 1); i <= total_steps; i++) {
+	/* coordiante_readouts … coordiante_readouts * 2 is for Y */
+	end_step = ts_dev->coordiante_readouts * 2;
+	for (i = ts_dev->coordiante_readouts; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
 
-	config = 0;
 	/* Charge step configuration */
 	config = ts_dev->bit_xp | ts_dev->bit_yn |
 			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
@@ -178,35 +179,39 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
-	config = 0;
-	/* Configure to calculate pressure */
+	/* coordiante_readouts * 2 … coordiante_readouts * 2 + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
 			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
 			STEPCONFIG_INP(ts_dev->inp_xp);
-	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 1), config);
-	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 1),
+	titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
+	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
-	config |= STEPCONFIG_INP(ts_dev->inp_yn) | STEPCONFIG_FIFO1;
-	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 2), config);
-	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2),
+	end_step++;
+	config |= STEPCONFIG_INP(ts_dev->inp_yn);
+	titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
+	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
 	/* The steps1 … end and bit 0 for TS_Charge */
-	stepenable = (1 << (total_steps + 2)) - 1;
+	stepenable = (1 << (end_step + 2)) - 1;
 	am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
 }
 
 static void titsc_read_coordinates(struct titsc *ts_dev,
-				    unsigned int *x, unsigned int *y)
+		u32 *x, u32 *y, u32 *z1, u32 *z2)
 {
 	unsigned int fifocount = titsc_readl(ts_dev, REG_FIFO0CNT);
 	unsigned int prev_val_x = ~0, prev_val_y = ~0;
 	unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
 	unsigned int read, diff;
 	unsigned int i, channel;
+	unsigned int creads = ts_dev->coordiante_readouts;
 
+	*z1 = *z2 = 0;
+	if (fifocount % (creads * 2 + 2))
+		fifocount -= fifocount % (creads * 2 + 2);
 	/*
 	 * Delta filter is used to remove large variations in sampled
 	 * values from ADC. The filter tries to predict where the next
@@ -215,32 +220,32 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 	 * algorithm compares the difference with that of a present value,
 	 * if true the value is reported to the sub system.
 	 */
-	for (i = 0; i < fifocount - 1; i++) {
+	for (i = 0; i < fifocount; i++) {
 		read = titsc_readl(ts_dev, REG_FIFO0);
-		channel = read & 0xf0000;
-		channel = channel >> 0x10;
-		if ((channel >= 0) && (channel < ts_dev->coordiante_readouts)) {
-			read &= 0xfff;
+
+		channel = (read & 0xf0000) >> 16;
+		read &= 0xfff;
+		if (channel < creads) {
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
 				prev_diff_x = diff;
 				*x = read;
 			}
 			prev_val_x = read;
-		}
 
-		read = titsc_readl(ts_dev, REG_FIFO1);
-		channel = read & 0xf0000;
-		channel = channel >> 0x10;
-		if ((channel >= ts_dev->coordiante_readouts) &&
-			(channel < (2 * ts_dev->coordiante_readouts - 1))) {
-			read &= 0xfff;
+		} else if (channel < creads * 2) {
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
 				prev_diff_y = diff;
 				*y = read;
 			}
 			prev_val_y = read;
+
+		} else if (channel < creads * 2 + 1) {
+			*z1 = read;
+
+		} else if (channel < creads * 2 + 2) {
+			*z2 = read;
 		}
 	}
 }
@@ -256,10 +261,8 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 
 	status = titsc_readl(ts_dev, REG_IRQSTATUS);
 	if (status & IRQENB_FIFO0THRES) {
-		titsc_read_coordinates(ts_dev, &x, &y);
 
-		z1 = titsc_readl(ts_dev, REG_FIFO0) & 0xfff;
-		z2 = titsc_readl(ts_dev, REG_FIFO1) & 0xfff;
+		titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
 
 		if (ts_dev->pen_down && z1 != 0 && z2 != 0) {
 			/*
@@ -267,10 +270,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 			 * Resistance(touch) = x plate resistance *
 			 * x postion/4096 * ((z2 / z1) - 1)
 			 */
-			z = z2 - z1;
+			z = z1 - z2;
 			z *= x;
 			z *= ts_dev->x_plate_resistance;
-			z /= z1;
+			z /= z2;
 			z = (z + 2047) >> 12;
 
 			if (z <= MAX_12BIT) {
@@ -395,7 +398,8 @@ static int titsc_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 	titsc_step_config(ts_dev);
-	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->coordiante_readouts);
+	titsc_writel(ts_dev, REG_FIFO0THR,
+			ts_dev->coordiante_readouts * 2 + 2 - 1);
 
 	input_dev->name = "ti-tsc";
 	input_dev->dev.parent = &pdev->dev;
@@ -472,7 +476,7 @@ static int titsc_resume(struct device *dev)
 	}
 	titsc_step_config(ts_dev);
 	titsc_writel(ts_dev, REG_FIFO0THR,
-			ts_dev->coordiante_readouts);
+			ts_dev->coordiante_readouts * 2 + 2 - 1);
 	return 0;
 }
 
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index cd8686b..fe6223c 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -30,8 +30,8 @@
 #define REG_IDLECONFIG		0x058
 #define REG_CHARGECONFIG	0x05C
 #define REG_CHARGEDELAY		0x060
-#define REG_STEPCONFIG(n)	(0x64 + ((n - 1) * 8))
-#define REG_STEPDELAY(n)	(0x68 + ((n - 1) * 8))
+#define REG_STEPCONFIG(n)	(0x64 + ((n) * 8))
+#define REG_STEPDELAY(n)	(0x68 + ((n) * 8))
 #define REG_FIFO0CNT		0xE4
 #define REG_FIFO0THR		0xE8
 #define REG_FIFO1CNT		0xF0
-- 
1.7.10.4

[PATCH 18/22] input/ti_am335x_adc: use only FIFO0 and clean up a little

[Sebastian Andrzej Siewior]

The driver programs a threshold of "coordiante_readouts" say 5. The
REG_FIFO0THR registers says it should it be programmed to "threshold
minus one". The driver does not expect just 5 coordinates but 5 * 2 + 2.
Multiplied by two because 5 for X and 5 for Y and plus 2 because we have
two Z.
The whole thing kind of works because It reads the 5 coordinates for X
and Y from FIFO0 and FIFO1 and the last element in each FIFO is ignored
within the loop and read later.
Nothing guaranties that FIFO1 is ready by the time it is read. In fact I
could see that that FIFO1 reaturns for Y channels 8,9, 10, 12, 6 and for
Y channel 7 for Z. The problem is that channel 7 and channel 12 got
somehow mixed up.
The other Problem is that FIFO1 is also used by the IIO part leading to
wrong results if both (tsc & adc) are used.

The patch tries to clean up the whole thing a little:
- Remove the +1 and -1 in REG_STEPCONFIG, REG_STEPDELAY and its counter
  part in the for loop. This is just confusing.

- Use only FIFO0 in TSC. The fifo has space for 64 entries so should be
  fine.

- Read the whole FIFO in one function and check the channel.

- in case we dawdle around, make sure we only read a multiple of our
  coordinate set. On the second interrupt we will cleanup the remaining
  enties.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/iio/adc/ti_am335x_adc.c           |    2 +-
 drivers/input/touchscreen/ti_am335x_tsc.c |   78 +++++++++++++++--------------
 include/linux/mfd/ti_am335x_tscadc.h      |    4 +-
 3 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 38f2415..031f8e5 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -76,7 +76,7 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
 
 	stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
 
-	for (i = (steps + 1); i <= TOTAL_STEPS; i++) {
+	for (i = steps; i < TOTAL_STEPS; i++) {
 		tiadc_writel(adc_dev, REG_STEPCONFIG(i),
 				stepconfig | STEPCONFIG_INP(channels));
 		tiadc_writel(adc_dev, REG_STEPDELAY(i),
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 7a76b14..fa9692c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -120,11 +120,9 @@ static int titsc_config_wires(struct titsc *ts_dev)
 static void titsc_step_config(struct titsc *ts_dev)
 {
 	unsigned int	config;
-	unsigned int	stepenable = 0;
-	int i, total_steps;
-
-	/* Configure the Step registers */
-	total_steps = 2 * ts_dev->coordiante_readouts;
+	int i;
+	int end_step;
+	u32 stepenable;
 
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_xp;
@@ -142,7 +140,9 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = 1; i <= ts_dev->coordiante_readouts; i++) {
+	/* 1 … coordiante_readouts is for X */
+	end_step = ts_dev->coordiante_readouts;
+	for (i = 0; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
@@ -150,7 +150,7 @@ static void titsc_step_config(struct titsc *ts_dev)
 	config = 0;
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yn |
-			STEPCONFIG_INM_ADCREFM | STEPCONFIG_FIFO1;
+			STEPCONFIG_INM_ADCREFM;
 	switch (ts_dev->wires) {
 	case 4:
 		config |= ts_dev->bit_yp | STEPCONFIG_INP(ts_dev->inp_xp);
@@ -164,12 +164,13 @@ static void titsc_step_config(struct titsc *ts_dev)
 		break;
 	}
 
-	for (i = (ts_dev->coordiante_readouts + 1); i <= total_steps; i++) {
+	/* coordiante_readouts … coordiante_readouts * 2 is for Y */
+	end_step = ts_dev->coordiante_readouts * 2;
+	for (i = ts_dev->coordiante_readouts; i < end_step; i++) {
 		titsc_writel(ts_dev, REG_STEPCONFIG(i), config);
 		titsc_writel(ts_dev, REG_STEPDELAY(i), STEPCONFIG_OPENDLY);
 	}
 
-	config = 0;
 	/* Charge step configuration */
 	config = ts_dev->bit_xp | ts_dev->bit_yn |
 			STEPCHARGE_RFP_XPUL | STEPCHARGE_RFM_XNUR |
@@ -178,35 +179,39 @@ static void titsc_step_config(struct titsc *ts_dev)
 	titsc_writel(ts_dev, REG_CHARGECONFIG, config);
 	titsc_writel(ts_dev, REG_CHARGEDELAY, CHARGEDLY_OPENDLY);
 
-	config = 0;
-	/* Configure to calculate pressure */
+	/* coordiante_readouts * 2 … coordiante_readouts * 2 + 2 is for Z */
 	config = STEPCONFIG_MODE_HWSYNC |
 			STEPCONFIG_AVG_16 | ts_dev->bit_yp |
 			ts_dev->bit_xn | STEPCONFIG_INM_ADCREFM |
 			STEPCONFIG_INP(ts_dev->inp_xp);
-	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 1), config);
-	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 1),
+	titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
+	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
-	config |= STEPCONFIG_INP(ts_dev->inp_yn) | STEPCONFIG_FIFO1;
-	titsc_writel(ts_dev, REG_STEPCONFIG(total_steps + 2), config);
-	titsc_writel(ts_dev, REG_STEPDELAY(total_steps + 2),
+	end_step++;
+	config |= STEPCONFIG_INP(ts_dev->inp_yn);
+	titsc_writel(ts_dev, REG_STEPCONFIG(end_step), config);
+	titsc_writel(ts_dev, REG_STEPDELAY(end_step),
 			STEPCONFIG_OPENDLY);
 
 	/* The steps1 … end and bit 0 for TS_Charge */
-	stepenable = (1 << (total_steps + 2)) - 1;
+	stepenable = (1 << (end_step + 2)) - 1;
 	am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
 }
 
 static void titsc_read_coordinates(struct titsc *ts_dev,
-				    unsigned int *x, unsigned int *y)
+		u32 *x, u32 *y, u32 *z1, u32 *z2)
 {
 	unsigned int fifocount = titsc_readl(ts_dev, REG_FIFO0CNT);
 	unsigned int prev_val_x = ~0, prev_val_y = ~0;
 	unsigned int prev_diff_x = ~0, prev_diff_y = ~0;
 	unsigned int read, diff;
 	unsigned int i, channel;
+	unsigned int creads = ts_dev->coordiante_readouts;
 
+	*z1 = *z2 = 0;
+	if (fifocount % (creads * 2 + 2))
+		fifocount -= fifocount % (creads * 2 + 2);
 	/*
 	 * Delta filter is used to remove large variations in sampled
 	 * values from ADC. The filter tries to predict where the next
@@ -215,32 +220,32 @@ static void titsc_read_coordinates(struct titsc *ts_dev,
 	 * algorithm compares the difference with that of a present value,
 	 * if true the value is reported to the sub system.
 	 */
-	for (i = 0; i < fifocount - 1; i++) {
+	for (i = 0; i < fifocount; i++) {
 		read = titsc_readl(ts_dev, REG_FIFO0);
-		channel = read & 0xf0000;
-		channel = channel >> 0x10;
-		if ((channel >= 0) && (channel < ts_dev->coordiante_readouts)) {
-			read &= 0xfff;
+
+		channel = (read & 0xf0000) >> 16;
+		read &= 0xfff;
+		if (channel < creads) {
 			diff = abs(read - prev_val_x);
 			if (diff < prev_diff_x) {
 				prev_diff_x = diff;
 				*x = read;
 			}
 			prev_val_x = read;
-		}
 
-		read = titsc_readl(ts_dev, REG_FIFO1);
-		channel = read & 0xf0000;
-		channel = channel >> 0x10;
-		if ((channel >= ts_dev->coordiante_readouts) &&
-			(channel < (2 * ts_dev->coordiante_readouts - 1))) {
-			read &= 0xfff;
+		} else if (channel < creads * 2) {
 			diff = abs(read - prev_val_y);
 			if (diff < prev_diff_y) {
 				prev_diff_y = diff;
 				*y = read;
 			}
 			prev_val_y = read;
+
+		} else if (channel < creads * 2 + 1) {
+			*z1 = read;
+
+		} else if (channel < creads * 2 + 2) {
+			*z2 = read;
 		}
 	}
 }
@@ -256,10 +261,8 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 
 	status = titsc_readl(ts_dev, REG_IRQSTATUS);
 	if (status & IRQENB_FIFO0THRES) {
-		titsc_read_coordinates(ts_dev, &x, &y);
 
-		z1 = titsc_readl(ts_dev, REG_FIFO0) & 0xfff;
-		z2 = titsc_readl(ts_dev, REG_FIFO1) & 0xfff;
+		titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
 
 		if (ts_dev->pen_down && z1 != 0 && z2 != 0) {
 			/*
@@ -267,10 +270,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
 			 * Resistance(touch) = x plate resistance *
 			 * x postion/4096 * ((z2 / z1) - 1)
 			 */
-			z = z2 - z1;
+			z = z1 - z2;
 			z *= x;
 			z *= ts_dev->x_plate_resistance;
-			z /= z1;
+			z /= z2;
 			z = (z + 2047) >> 12;
 
 			if (z <= MAX_12BIT) {
@@ -395,7 +398,8 @@ static int titsc_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 	titsc_step_config(ts_dev);
-	titsc_writel(ts_dev, REG_FIFO0THR, ts_dev->coordiante_readouts);
+	titsc_writel(ts_dev, REG_FIFO0THR,
+			ts_dev->coordiante_readouts * 2 + 2 - 1);
 
 	input_dev->name = "ti-tsc";
 	input_dev->dev.parent = &pdev->dev;
@@ -472,7 +476,7 @@ static int titsc_resume(struct device *dev)
 	}
 	titsc_step_config(ts_dev);
 	titsc_writel(ts_dev, REG_FIFO0THR,
-			ts_dev->coordiante_readouts);
+			ts_dev->coordiante_readouts * 2 + 2 - 1);
 	return 0;
 }
 
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index cd8686b..fe6223c 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -30,8 +30,8 @@
 #define REG_IDLECONFIG		0x058
 #define REG_CHARGECONFIG	0x05C
 #define REG_CHARGEDELAY		0x060
-#define REG_STEPCONFIG(n)	(0x64 + ((n - 1) * 8))
-#define REG_STEPDELAY(n)	(0x68 + ((n - 1) * 8))
+#define REG_STEPCONFIG(n)	(0x64 + ((n) * 8))
+#define REG_STEPDELAY(n)	(0x68 + ((n) * 8))
 #define REG_FIFO0CNT		0xE4
 #define REG_FIFO0THR		0xE8
 #define REG_FIFO1CNT		0xF0
-- 
1.7.10.4