[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

This series for the i2c-davinci driver implements both a register dump and
a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
registers which is, so far, the da850 and da830 based platforms.

I2C "controller timed out" messages were being observed by both myself on the
da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I 
thought it was the existing pulse-SCL routine but was quickly proven wrong; my
apologies to John Povey and Philby John for jumping to conclusions.

A discussion was spawned on the e2e forums [2] where Brad Griffis was
instrumental in the development of the recovery routine proposed by this patch
series. It was pointed out by him that the da850's i2c controller has the
ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
The recovery routine implemented by the patch series toggles the SCL pin and 
reads the SDA state using the ICPFUNC registers.

The changes in this series are staged in increments of features and each patch
depends on the changes introduced by the patch before it with the exception of 
patch 2/6 which is largely independent of 1/6.

The real meat of the series in in patch 4/6. First we introduce a register
dump routine in 1/6 since this information was requested immeadiately by Brad
Griffis when the conversation began; then the i2c-davinci platform data 
structure's comments are converted to kerneldoc in 2/6. Then in 3/6 a level of
indirection is introduced so that the implementation of the recovery procedure
can be switched at runtime; this level of indirection is used in the subsequent
patch, 4/6, to execute a pulsed-SCL recovery routine using the ICPFUNC
registers if they are present. Where the presence of the registers is indicated
by the platform in a has_pfunc flag in platform data. Finally since all da8xx
platforms' i2c controllers have the ICPFUNC registers, the da8xx utility
function to register i2c controllers is modified to set the flag so that the
new recovery routine is used in all da8xx platforms.

I developed this patch against v2.6.38 but I have also tested it against 
v2.6.39-rc1: it applies cleanly and the recovery routine is executed as it was
in v2.6.38.

When creating this series I noticed that there are obvious similarities between
the existing recovery routine implemented by Philby John and John Povey and the
recovery routine proposed in this series. Testing was performed using the
ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
was found that the recovery did not work. The method described initially by Brad
Griffis had the initial state of SCL high and delays of 5us with a maximum of
8 pulses with a check of SDA each time as compared to the existing routine with
undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested
and found that if I changed the initial state of SCL or the number of pulses
(Bastian had success with 16) that the recovery did not occur as expected;
furthermore Brad pointed out that it was important to check the state of SDA
after each pulse to see if the slave had released the line. Indeed, adding the
check of SDA resulted in a quicker recovery. On my da850evm, at least, the
recovery took only one SCL pulse every time.

It would be nice to consolidate the two recovery routines and -- since they
are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
also be used by bitbanging i2c masters. I did not undertake the former because
I don't have access to the hardware on which the existing recovery routine was
tested. I did not undertake the latter since 1) it seems premature until the
recovery routines are consolidated (or not) and 2) it would require more
changes of a broad scope which I feared might mire the review process of this
series which is, at its core, a functioning workaround of an observed problem
with da850evm hardware (plus some regsiter dumps). It is my hope that both the
consolidation of the recovery routines and the extraction to common bitbanging
code can be done in a later series.

[1] http://permalink.gmane.org/gmane.linux.davinci/22291
[2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx

Ben Gardiner (6):
  i2c-davinci: register dump before attempted bus recovery
  i2c-davinci: convert davinci_i2c_platform_data to kerneldoc
  i2c-davinci: introduce a dev-> function pointer for scl pulsing
  i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio
  i2c-davinci: if device has pfunc register, dump that group also
  da8xx: enable the use of the ICPFUNC in i2c-davinci

 arch/arm/mach-davinci/devices-da8xx.c    |    6 +
 arch/arm/mach-davinci/include/mach/i2c.h |   21 +++-
 drivers/i2c/busses/i2c-davinci.c         |  155 ++++++++++++++++++++++++++++--
 3 files changed, 170 insertions(+), 12 deletions(-)

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

This series for the i2c-davinci driver implements both a register dump and
a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
registers which is, so far, the da850 and da830 based platforms.

I2C "controller timed out" messages were being observed by both myself on the
da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I 
thought it was the existing pulse-SCL routine but was quickly proven wrong; my
apologies to John Povey and Philby John for jumping to conclusions.

A discussion was spawned on the e2e forums [2] where Brad Griffis was
instrumental in the development of the recovery routine proposed by this patch
series. It was pointed out by him that the da850's i2c controller has the
ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
The recovery routine implemented by the patch series toggles the SCL pin and 
reads the SDA state using the ICPFUNC registers.

The changes in this series are staged in increments of features and each patch
depends on the changes introduced by the patch before it with the exception of 
patch 2/6 which is largely independent of 1/6.

The real meat of the series in in patch 4/6. First we introduce a register
dump routine in 1/6 since this information was requested immeadiately by Brad
Griffis when the conversation began; then the i2c-davinci platform data 
structure's comments are converted to kerneldoc in 2/6. Then in 3/6 a level of
indirection is introduced so that the implementation of the recovery procedure
can be switched at runtime; this level of indirection is used in the subsequent
patch, 4/6, to execute a pulsed-SCL recovery routine using the ICPFUNC
registers if they are present. Where the presence of the registers is indicated
by the platform in a has_pfunc flag in platform data. Finally since all da8xx
platforms' i2c controllers have the ICPFUNC registers, the da8xx utility
function to register i2c controllers is modified to set the flag so that the
new recovery routine is used in all da8xx platforms.

I developed this patch against v2.6.38 but I have also tested it against 
v2.6.39-rc1: it applies cleanly and the recovery routine is executed as it was
in v2.6.38.

When creating this series I noticed that there are obvious similarities between
the existing recovery routine implemented by Philby John and John Povey and the
recovery routine proposed in this series. Testing was performed using the
ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
was found that the recovery did not work. The method described initially by Brad
Griffis had the initial state of SCL high and delays of 5us with a maximum of
8 pulses with a check of SDA each time as compared to the existing routine with
undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested
and found that if I changed the initial state of SCL or the number of pulses
(Bastian had success with 16) that the recovery did not occur as expected;
furthermore Brad pointed out that it was important to check the state of SDA
after each pulse to see if the slave had released the line. Indeed, adding the
check of SDA resulted in a quicker recovery. On my da850evm, at least, the
recovery took only one SCL pulse every time.

It would be nice to consolidate the two recovery routines and -- since they
are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
also be used by bitbanging i2c masters. I did not undertake the former because
I don't have access to the hardware on which the existing recovery routine was
tested. I did not undertake the latter since 1) it seems premature until the
recovery routines are consolidated (or not) and 2) it would require more
changes of a broad scope which I feared might mire the review process of this
series which is, at its core, a functioning workaround of an observed problem
with da850evm hardware (plus some regsiter dumps). It is my hope that both the
consolidation of the recovery routines and the extraction to common bitbanging
code can be done in a later series.

[1] http://permalink.gmane.org/gmane.linux.davinci/22291
[2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx

Ben Gardiner (6):
  i2c-davinci: register dump before attempted bus recovery
  i2c-davinci: convert davinci_i2c_platform_data to kerneldoc
  i2c-davinci: introduce a dev-> function pointer for scl pulsing
  i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio
  i2c-davinci: if device has pfunc register, dump that group also
  da8xx: enable the use of the ICPFUNC in i2c-davinci

 arch/arm/mach-davinci/devices-da8xx.c    |    6 +
 arch/arm/mach-davinci/include/mach/i2c.h |   21 +++-
 drivers/i2c/busses/i2c-davinci.c         |  155 ++++++++++++++++++++++++++++--
 3 files changed, 170 insertions(+), 12 deletions(-)

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

This series for the i2c-davinci driver implements both a register dump and
a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
registers which is, so far, the da850 and da830 based platforms.

I2C "controller timed out" messages were being observed by both myself on the
da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I 
thought it was the existing pulse-SCL routine but was quickly proven wrong; my
apologies to John Povey and Philby John for jumping to conclusions.

A discussion was spawned on the e2e forums [2] where Brad Griffis was
instrumental in the development of the recovery routine proposed by this patch
series. It was pointed out by him that the da850's i2c controller has the
ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
The recovery routine implemented by the patch series toggles the SCL pin and 
reads the SDA state using the ICPFUNC registers.

The changes in this series are staged in increments of features and each patch
depends on the changes introduced by the patch before it with the exception of 
patch 2/6 which is largely independent of 1/6.

The real meat of the series in in patch 4/6. First we introduce a register
dump routine in 1/6 since this information was requested immeadiately by Brad
Griffis when the conversation began; then the i2c-davinci platform data 
structure's comments are converted to kerneldoc in 2/6. Then in 3/6 a level of
indirection is introduced so that the implementation of the recovery procedure
can be switched at runtime; this level of indirection is used in the subsequent
patch, 4/6, to execute a pulsed-SCL recovery routine using the ICPFUNC
registers if they are present. Where the presence of the registers is indicated
by the platform in a has_pfunc flag in platform data. Finally since all da8xx
platforms' i2c controllers have the ICPFUNC registers, the da8xx utility
function to register i2c controllers is modified to set the flag so that the
new recovery routine is used in all da8xx platforms.

I developed this patch against v2.6.38 but I have also tested it against 
v2.6.39-rc1: it applies cleanly and the recovery routine is executed as it was
in v2.6.38.

When creating this series I noticed that there are obvious similarities between
the existing recovery routine implemented by Philby John and John Povey and the
recovery routine proposed in this series. Testing was performed using the
ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
was found that the recovery did not work. The method described initially by Brad
Griffis had the initial state of SCL high and delays of 5us with a maximum of
8 pulses with a check of SDA each time as compared to the existing routine with
undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested
and found that if I changed the initial state of SCL or the number of pulses
(Bastian had success with 16) that the recovery did not occur as expected;
furthermore Brad pointed out that it was important to check the state of SDA
after each pulse to see if the slave had released the line. Indeed, adding the
check of SDA resulted in a quicker recovery. On my da850evm, at least, the
recovery took only one SCL pulse every time.

It would be nice to consolidate the two recovery routines and -- since they
are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
also be used by bitbanging i2c masters. I did not undertake the former because
I don't have access to the hardware on which the existing recovery routine was
tested. I did not undertake the latter since 1) it seems premature until the
recovery routines are consolidated (or not) and 2) it would require more
changes of a broad scope which I feared might mire the review process of this
series which is, at its core, a functioning workaround of an observed problem
with da850evm hardware (plus some regsiter dumps). It is my hope that both the
consolidation of the recovery routines and the extraction to common bitbanging
code can be done in a later series.

[1] http://permalink.gmane.org/gmane.linux.davinci/22291
[2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx

Ben Gardiner (6):
  i2c-davinci: register dump before attempted bus recovery
  i2c-davinci: convert davinci_i2c_platform_data to kerneldoc
  i2c-davinci: introduce a dev-> function pointer for scl pulsing
  i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio
  i2c-davinci: if device has pfunc register, dump that group also
  da8xx: enable the use of the ICPFUNC in i2c-davinci

 arch/arm/mach-davinci/devices-da8xx.c    |    6 +
 arch/arm/mach-davinci/include/mach/i2c.h |   21 +++-
 drivers/i2c/busses/i2c-davinci.c         |  155 ++++++++++++++++++++++++++++--
 3 files changed, 170 insertions(+), 12 deletions(-)

[PATCH 1/6] i2c-davinci: register dump before attempted bus recovery

[Ben Gardiner]

Produce a dump of the register contents of the I2C controller (at the debug
level) whenever an i2c recovery is initiated.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---

The motivation for the introduction of this additional debugging information is
that when I2C recovery on da850 began discussion in the e2e forums [1] a
register dump was requested.

[1] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx#350610

 drivers/i2c/busses/i2c-davinci.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5795c83..7011222 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -133,6 +133,37 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
 	return __raw_readw(i2c_dev->base + reg);
 }
 
+static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
+{
+	dev_dbg(dev->dev, "PSC  = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
+	dev_dbg(dev->dev, "CLKL = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
+	dev_dbg(dev->dev, "CLKH = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
+	dev_dbg(dev->dev, "MDR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG));
+
+	dev_dbg(dev->dev, "OAR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_OAR_REG));
+	dev_dbg(dev->dev, "IMR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG));
+	dev_dbg(dev->dev, "STR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG));
+	dev_dbg(dev->dev, "CNT = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CNT_REG));
+	dev_dbg(dev->dev, "DRR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_DRR_REG));
+	dev_dbg(dev->dev, "SAR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_SAR_REG));
+	dev_dbg(dev->dev, "DXR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_DXR_REG));
+	dev_dbg(dev->dev, "IVR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
+	dev_dbg(dev->dev, "EMDR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
+}
+
 /* Generate a pulse on the i2c clock pin. */
 static void generic_i2c_clock_pulse(unsigned int scl_pin)
 {
@@ -157,6 +188,7 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	u32 flag = 0;
 	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 
+	i2c_davinci_dump_regs(dev);
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
 	/* Send NACK to the slave */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
-- 
1.7.1

[PATCH 1/6] i2c-davinci: register dump before attempted bus recovery

[Ben Gardiner]

Produce a dump of the register contents of the I2C controller (at the debug
level) whenever an i2c recovery is initiated.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---

The motivation for the introduction of this additional debugging information is
that when I2C recovery on da850 began discussion in the e2e forums [1] a
register dump was requested.

[1] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx#350610

 drivers/i2c/busses/i2c-davinci.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5795c83..7011222 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -133,6 +133,37 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
 	return __raw_readw(i2c_dev->base + reg);
 }
 
+static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
+{
+	dev_dbg(dev->dev, "PSC  = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
+	dev_dbg(dev->dev, "CLKL = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
+	dev_dbg(dev->dev, "CLKH = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
+	dev_dbg(dev->dev, "MDR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG));
+
+	dev_dbg(dev->dev, "OAR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_OAR_REG));
+	dev_dbg(dev->dev, "IMR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG));
+	dev_dbg(dev->dev, "STR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG));
+	dev_dbg(dev->dev, "CNT = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CNT_REG));
+	dev_dbg(dev->dev, "DRR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_DRR_REG));
+	dev_dbg(dev->dev, "SAR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_SAR_REG));
+	dev_dbg(dev->dev, "DXR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_DXR_REG));
+	dev_dbg(dev->dev, "IVR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
+	dev_dbg(dev->dev, "EMDR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
+}
+
 /* Generate a pulse on the i2c clock pin. */
 static void generic_i2c_clock_pulse(unsigned int scl_pin)
 {
@@ -157,6 +188,7 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	u32 flag = 0;
 	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 
+	i2c_davinci_dump_regs(dev);
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
 	/* Send NACK to the slave */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
-- 
1.7.1

[PATCH 1/6] i2c-davinci: register dump before attempted bus recovery

[Ben Gardiner]

Produce a dump of the register contents of the I2C controller (at the debug
level) whenever an i2c recovery is initiated.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---

The motivation for the introduction of this additional debugging information is
that when I2C recovery on da850 began discussion in the e2e forums [1] a
register dump was requested.

[1] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx#350610

 drivers/i2c/busses/i2c-davinci.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5795c83..7011222 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -133,6 +133,37 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
 	return __raw_readw(i2c_dev->base + reg);
 }
 
+static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
+{
+	dev_dbg(dev->dev, "PSC  = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
+	dev_dbg(dev->dev, "CLKL = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
+	dev_dbg(dev->dev, "CLKH = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
+	dev_dbg(dev->dev, "MDR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG));
+
+	dev_dbg(dev->dev, "OAR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_OAR_REG));
+	dev_dbg(dev->dev, "IMR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_IMR_REG));
+	dev_dbg(dev->dev, "STR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_STR_REG));
+	dev_dbg(dev->dev, "CNT = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_CNT_REG));
+	dev_dbg(dev->dev, "DRR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_DRR_REG));
+	dev_dbg(dev->dev, "SAR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_SAR_REG));
+	dev_dbg(dev->dev, "DXR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_DXR_REG));
+	dev_dbg(dev->dev, "IVR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
+	dev_dbg(dev->dev, "EMDR = %08x\n",
+		davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
+}
+
 /* Generate a pulse on the i2c clock pin. */
 static void generic_i2c_clock_pulse(unsigned int scl_pin)
 {
@@ -157,6 +188,7 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	u32 flag = 0;
 	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 
+	i2c_davinci_dump_regs(dev);
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
 	/* Send NACK to the slave */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
-- 
1.7.1

[PATCH 2/6] i2c-davinci: convert davinci_i2c_platform_data to kerneldoc

[Ben Gardiner]

The davinci_i2c_platform_data struct is mach-specific but still deserves
to be documented using kernel doc.

Signed-off-by: Ben Gardiner<bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/include/mach/i2c.h |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/i2c.h b/arch/arm/mach-davinci/include/mach/i2c.h
index 2312d19..ab07a44 100644
--- a/arch/arm/mach-davinci/include/mach/i2c.h
+++ b/arch/arm/mach-davinci/include/mach/i2c.h
@@ -12,12 +12,19 @@
 #ifndef __ASM_ARCH_I2C_H
 #define __ASM_ARCH_I2C_H
 
-/* All frequencies are expressed in kHz */
+/**
+ * davinci_i2c_platform_data - Platform data for I2C master device on DaVinci
+ *
+ * @bus_freq:	standard bus frequency (kHz)
+ * @bus_delay:	post-transaction delay (usec)
+ * @sda_pin:	GPIO pin ID to use for SDA
+ * @scl_pin:	GPIO pin ID to use for SCL
+ */
 struct davinci_i2c_platform_data {
-	unsigned int	bus_freq;	/* standard bus frequency (kHz) */
-	unsigned int	bus_delay;	/* post-transaction delay (usec) */
-	unsigned int    sda_pin;        /* GPIO pin ID to use for SDA */
-	unsigned int    scl_pin;        /* GPIO pin ID to use for SCL */
+	unsigned int	bus_freq;
+	unsigned int	bus_delay;
+	unsigned int	sda_pin;
+	unsigned int	scl_pin;
 };
 
 /* for board setup code */
-- 
1.7.1

[PATCH 2/6] i2c-davinci: convert davinci_i2c_platform_data to kerneldoc

[Ben Gardiner]

The davinci_i2c_platform_data struct is mach-specific but still deserves
to be documented using kernel doc.

Signed-off-by: Ben Gardiner<bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/include/mach/i2c.h |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/i2c.h b/arch/arm/mach-davinci/include/mach/i2c.h
index 2312d19..ab07a44 100644
--- a/arch/arm/mach-davinci/include/mach/i2c.h
+++ b/arch/arm/mach-davinci/include/mach/i2c.h
@@ -12,12 +12,19 @@
 #ifndef __ASM_ARCH_I2C_H
 #define __ASM_ARCH_I2C_H
 
-/* All frequencies are expressed in kHz */
+/**
+ * davinci_i2c_platform_data - Platform data for I2C master device on DaVinci
+ *
+ * @bus_freq:	standard bus frequency (kHz)
+ * @bus_delay:	post-transaction delay (usec)
+ * @sda_pin:	GPIO pin ID to use for SDA
+ * @scl_pin:	GPIO pin ID to use for SCL
+ */
 struct davinci_i2c_platform_data {
-	unsigned int	bus_freq;	/* standard bus frequency (kHz) */
-	unsigned int	bus_delay;	/* post-transaction delay (usec) */
-	unsigned int    sda_pin;        /* GPIO pin ID to use for SDA */
-	unsigned int    scl_pin;        /* GPIO pin ID to use for SCL */
+	unsigned int	bus_freq;
+	unsigned int	bus_delay;
+	unsigned int	sda_pin;
+	unsigned int	scl_pin;
 };
 
 /* for board setup code */
-- 
1.7.1

[PATCH 2/6] i2c-davinci: convert davinci_i2c_platform_data to kerneldoc

[Ben Gardiner]

The davinci_i2c_platform_data struct is mach-specific but still deserves
to be documented using kernel doc.

Signed-off-by: Ben Gardiner<bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/include/mach/i2c.h |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/i2c.h b/arch/arm/mach-davinci/include/mach/i2c.h
index 2312d19..ab07a44 100644
--- a/arch/arm/mach-davinci/include/mach/i2c.h
+++ b/arch/arm/mach-davinci/include/mach/i2c.h
@@ -12,12 +12,19 @@
 #ifndef __ASM_ARCH_I2C_H
 #define __ASM_ARCH_I2C_H
 
-/* All frequencies are expressed in kHz */
+/**
+ * davinci_i2c_platform_data - Platform data for I2C master device on DaVinci
+ *
+ * @bus_freq:	standard bus frequency (kHz)
+ * @bus_delay:	post-transaction delay (usec)
+ * @sda_pin:	GPIO pin ID to use for SDA
+ * @scl_pin:	GPIO pin ID to use for SCL
+ */
 struct davinci_i2c_platform_data {
-	unsigned int	bus_freq;	/* standard bus frequency (kHz) */
-	unsigned int	bus_delay;	/* post-transaction delay (usec) */
-	unsigned int    sda_pin;        /* GPIO pin ID to use for SDA */
-	unsigned int    scl_pin;        /* GPIO pin ID to use for SCL */
+	unsigned int	bus_freq;
+	unsigned int	bus_delay;
+	unsigned int	sda_pin;
+	unsigned int	scl_pin;
 };
 
 /* for board setup code */
-- 
1.7.1

[PATCH 3/6] i2c-davinci: introduce a dev-> function pointer for scl pulsing

[Ben Gardiner]

The current implementation of the I2C recovery routine checks whether
platform data ->scl_pin has been assigned and if so, the generic_i2c_clock_pulse
routine is executed.

In preparation for an alternative recovery routine; introduce a pulse_scl
function pointer to the driver private structure and assign it the value of
generic_i2c_clock_pulse on init if the scl_pin is assigned in platform data.

Signed-off-by: Ben Gardiner<bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 drivers/i2c/busses/i2c-davinci.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 7011222..0a2c697 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -110,6 +110,7 @@ struct davinci_i2c_dev {
 	int			stop;
 	u8			terminate;
 	struct i2c_adapter	adapter;
+	void			(*pulse_scl) (struct davinci_i2c_dev *dev);
 #ifdef CONFIG_CPU_FREQ
 	struct completion	xfr_complete;
 	struct notifier_block	freq_transition;
@@ -165,16 +166,17 @@ static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 }
 
 /* Generate a pulse on the i2c clock pin. */
-static void generic_i2c_clock_pulse(unsigned int scl_pin)
+static void generic_i2c_clock_pulse(struct davinci_i2c_dev *dev)
 {
+	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 	u16 i;
 
-	if (scl_pin) {
+	if (pdata->scl_pin) {
 		/* Send high and low on the SCL line */
 		for (i = 0; i < 9; i++) {
-			gpio_set_value(scl_pin, 0);
+			gpio_set_value(pdata->scl_pin, 0);
 			udelay(20);
-			gpio_set_value(scl_pin, 1);
+			gpio_set_value(pdata->scl_pin, 1);
 			udelay(20);
 		}
 	}
@@ -186,7 +188,6 @@ static void generic_i2c_clock_pulse(unsigned int scl_pin)
 static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 {
 	u32 flag = 0;
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 
 	i2c_davinci_dump_regs(dev);
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
@@ -195,8 +196,8 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	flag |=  DAVINCI_I2C_MDR_NACK;
 	/* write the data into mode register */
 	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
-	if (pdata)
-		generic_i2c_clock_pulse(pdata->scl_pin);
+	if (dev->pulse_scl)
+		dev->pulse_scl(dev);
 	/* Send STOP */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
 	flag |= DAVINCI_I2C_MDR_STP;
@@ -669,6 +670,7 @@ static struct i2c_algorithm i2c_davinci_algo = {
 
 static int davinci_i2c_probe(struct platform_device *pdev)
 {
+	struct davinci_i2c_platform_data *pdata;
 	struct davinci_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem, *irq, *ioarea;
@@ -751,6 +753,10 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
+	pdata = dev->dev->platform_data;
+	if (pdata->scl_pin)
+		dev->pulse_scl = generic_i2c_clock_pulse;
+
 	return 0;
 
 err_free_irq:
-- 
1.7.1

[PATCH 3/6] i2c-davinci: introduce a dev-> function pointer for scl pulsing

[Ben Gardiner]

The current implementation of the I2C recovery routine checks whether
platform data ->scl_pin has been assigned and if so, the generic_i2c_clock_pulse
routine is executed.

In preparation for an alternative recovery routine; introduce a pulse_scl
function pointer to the driver private structure and assign it the value of
generic_i2c_clock_pulse on init if the scl_pin is assigned in platform data.

Signed-off-by: Ben Gardiner<bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org>
Cc: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>

---
 drivers/i2c/busses/i2c-davinci.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 7011222..0a2c697 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -110,6 +110,7 @@ struct davinci_i2c_dev {
 	int			stop;
 	u8			terminate;
 	struct i2c_adapter	adapter;
+	void			(*pulse_scl) (struct davinci_i2c_dev *dev);
 #ifdef CONFIG_CPU_FREQ
 	struct completion	xfr_complete;
 	struct notifier_block	freq_transition;
@@ -165,16 +166,17 @@ static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 }
 
 /* Generate a pulse on the i2c clock pin. */
-static void generic_i2c_clock_pulse(unsigned int scl_pin)
+static void generic_i2c_clock_pulse(struct davinci_i2c_dev *dev)
 {
+	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 	u16 i;
 
-	if (scl_pin) {
+	if (pdata->scl_pin) {
 		/* Send high and low on the SCL line */
 		for (i = 0; i < 9; i++) {
-			gpio_set_value(scl_pin, 0);
+			gpio_set_value(pdata->scl_pin, 0);
 			udelay(20);
-			gpio_set_value(scl_pin, 1);
+			gpio_set_value(pdata->scl_pin, 1);
 			udelay(20);
 		}
 	}
@@ -186,7 +188,6 @@ static void generic_i2c_clock_pulse(unsigned int scl_pin)
 static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 {
 	u32 flag = 0;
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 
 	i2c_davinci_dump_regs(dev);
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
@@ -195,8 +196,8 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	flag |=  DAVINCI_I2C_MDR_NACK;
 	/* write the data into mode register */
 	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
-	if (pdata)
-		generic_i2c_clock_pulse(pdata->scl_pin);
+	if (dev->pulse_scl)
+		dev->pulse_scl(dev);
 	/* Send STOP */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
 	flag |= DAVINCI_I2C_MDR_STP;
@@ -669,6 +670,7 @@ static struct i2c_algorithm i2c_davinci_algo = {
 
 static int davinci_i2c_probe(struct platform_device *pdev)
 {
+	struct davinci_i2c_platform_data *pdata;
 	struct davinci_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem, *irq, *ioarea;
@@ -751,6 +753,10 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
+	pdata = dev->dev->platform_data;
+	if (pdata->scl_pin)
+		dev->pulse_scl = generic_i2c_clock_pulse;
+
 	return 0;
 
 err_free_irq:
-- 
1.7.1

[PATCH 3/6] i2c-davinci: introduce a dev-> function pointer for scl pulsing

[Ben Gardiner]

The current implementation of the I2C recovery routine checks whether
platform data ->scl_pin has been assigned and if so, the generic_i2c_clock_pulse
routine is executed.

In preparation for an alternative recovery routine; introduce a pulse_scl
function pointer to the driver private structure and assign it the value of
generic_i2c_clock_pulse on init if the scl_pin is assigned in platform data.

Signed-off-by: Ben Gardiner<bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 drivers/i2c/busses/i2c-davinci.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 7011222..0a2c697 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -110,6 +110,7 @@ struct davinci_i2c_dev {
 	int			stop;
 	u8			terminate;
 	struct i2c_adapter	adapter;
+	void			(*pulse_scl) (struct davinci_i2c_dev *dev);
 #ifdef CONFIG_CPU_FREQ
 	struct completion	xfr_complete;
 	struct notifier_block	freq_transition;
@@ -165,16 +166,17 @@ static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 }
 
 /* Generate a pulse on the i2c clock pin. */
-static void generic_i2c_clock_pulse(unsigned int scl_pin)
+static void generic_i2c_clock_pulse(struct davinci_i2c_dev *dev)
 {
+	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 	u16 i;
 
-	if (scl_pin) {
+	if (pdata->scl_pin) {
 		/* Send high and low on the SCL line */
 		for (i = 0; i < 9; i++) {
-			gpio_set_value(scl_pin, 0);
+			gpio_set_value(pdata->scl_pin, 0);
 			udelay(20);
-			gpio_set_value(scl_pin, 1);
+			gpio_set_value(pdata->scl_pin, 1);
 			udelay(20);
 		}
 	}
@@ -186,7 +188,6 @@ static void generic_i2c_clock_pulse(unsigned int scl_pin)
 static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 {
 	u32 flag = 0;
-	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
 
 	i2c_davinci_dump_regs(dev);
 	dev_err(dev->dev, "initiating i2c bus recovery\n");
@@ -195,8 +196,8 @@ static void i2c_recover_bus(struct davinci_i2c_dev *dev)
 	flag |=  DAVINCI_I2C_MDR_NACK;
 	/* write the data into mode register */
 	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
-	if (pdata)
-		generic_i2c_clock_pulse(pdata->scl_pin);
+	if (dev->pulse_scl)
+		dev->pulse_scl(dev);
 	/* Send STOP */
 	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
 	flag |= DAVINCI_I2C_MDR_STP;
@@ -669,6 +670,7 @@ static struct i2c_algorithm i2c_davinci_algo = {
 
 static int davinci_i2c_probe(struct platform_device *pdev)
 {
+	struct davinci_i2c_platform_data *pdata;
 	struct davinci_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem, *irq, *ioarea;
@@ -751,6 +753,10 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
+	pdata = dev->dev->platform_data;
+	if (pdata->scl_pin)
+		dev->pulse_scl = generic_i2c_clock_pulse;
+
 	return 0;
 
 err_free_irq:
-- 
1.7.1

[PATCH 4/6] i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio

[Ben Gardiner]

On the da850evm, with the default config of polling keys enabled, several
"controller timed out" errors were observed on the console; as well as the
same error observed on custom hardware when communicating with a i2c
touchscreen device. [1]

Discussion of the causes and potential workarounds began on the e2e forums [2]
where Brad Griffis pointed out that the da850 (and da830) has an i2c controller
whose SCL and SDA may be manipulated as GPIOs by using the ICPFUNC registers
of the i2c controller. He further suggested a means of using this feature to
send clock pulses on the I2C bus to free the frozen slave device.

Implement the suggested procedure by toggling SCL and checking SDA using the
ICPFUNC registers of the I2C controller when present. Allow platforms to
indicate the presence of the ICPFUNC registers with a has_pfunc platform data
flag.

[1] http://permalink.gmane.org/gmane.linux.davinci/22291
[2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Jon Povey <jon.povey@racelogic.co.uk>
Cc: Philby John <pjohn@in.mvista.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/include/mach/i2c.h |    6 ++-
 drivers/i2c/busses/i2c-davinci.c         |   88 ++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/i2c.h b/arch/arm/mach-davinci/include/mach/i2c.h
index ab07a44..858c5c6 100644
--- a/arch/arm/mach-davinci/include/mach/i2c.h
+++ b/arch/arm/mach-davinci/include/mach/i2c.h
@@ -19,12 +19,16 @@
  * @bus_delay:	post-transaction delay (usec)
  * @sda_pin:	GPIO pin ID to use for SDA
  * @scl_pin:	GPIO pin ID to use for SCL
+ * @has_pfunc:	set this to true if the i2c controller on your chip has a
+ *		ICPFUNC register which allows the SDA and SCL pins to be
+ *		controlled as gpio (like in DA850)
  */
 struct davinci_i2c_platform_data {
 	unsigned int	bus_freq;
 	unsigned int	bus_delay;
 	unsigned int	sda_pin;
 	unsigned int	scl_pin;
+	bool		has_pfunc;
 };
 
 /* for board setup code */
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 0a2c697..5bdc98c 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -65,6 +65,11 @@
 #define DAVINCI_I2C_IVR_REG	0x28
 #define DAVINCI_I2C_EMDR_REG	0x2c
 #define DAVINCI_I2C_PSC_REG	0x30
+#define DAVINCI_I2C_PFUNC_REG	0x48
+#define DAVINCI_I2C_PDIR_REG	0x4c
+#define DAVINCI_I2C_PDIN_REG	0x50
+#define DAVINCI_I2C_DSET_REG	0x58
+#define DAVINCI_I2C_DCLR_REG	0x5c
 
 #define DAVINCI_I2C_IVR_AAS	0x07
 #define DAVINCI_I2C_IVR_SCD	0x06
@@ -98,6 +103,29 @@
 #define DAVINCI_I2C_IMR_NACK	BIT(1)
 #define DAVINCI_I2C_IMR_AL	BIT(0)
 
+/* set SDA and SCL as GPIO */
+#define DAVINCI_I2C_PFUNC_PFUNC0	BIT(0)
+
+/* set SCL as output when used as GPIO*/
+#define DAVINCI_I2C_PDIR_PDIR0	BIT(0)
+/* set SDA as output when used as GPIO*/
+#define DAVINCI_I2C_PDIR_PDIR1	BIT(1)
+
+/* read SCL GPIO level */
+#define DAVINCI_I2C_PDIN_PDIN0 BIT(0)
+/* read SDA GPIO level */
+#define DAVINCI_I2C_PDIN_PDIN1 BIT(1)
+
+/*set the SCL GPIO high */
+#define DAVINCI_I2C_DSET_PDSET0	BIT(0)
+/*set the SDA GPIO high */
+#define DAVINCI_I2C_DSET_PDSET1	BIT(1)
+
+/* set the SCL GPIO low */
+#define DAVINCI_I2C_DCLR_PDCLR0	BIT(0)
+/* set the SDA GPIO low */
+#define DAVINCI_I2C_DCLR_PDCLR1	BIT(1)
+
 struct davinci_i2c_dev {
 	struct device           *dev;
 	void __iomem		*base;
@@ -182,6 +210,64 @@ static void generic_i2c_clock_pulse(struct davinci_i2c_dev *dev)
 	}
 }
 
+static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
+								int val);
+
+/* Generate gpio a pulse on the i2c clock pin. */
+static void i2c_davinci_pfunc_i2c_clock_pulse(struct davinci_i2c_dev *dev)
+{
+	u32 flag = 0;
+	u16 i;
+
+	davinci_i2c_reset_ctrl(dev, 0);
+
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0x00);
+
+	/* SCL output, SDA input */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PDIR_REG,
+				DAVINCI_I2C_PDIR_PDIR0);
+
+	/* change to GPIO mode */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG,
+				DAVINCI_I2C_PFUNC_PFUNC0);
+
+	/* SCL high */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+				DAVINCI_I2C_DSET_PDSET0);
+	udelay(5);
+	for (i = 0; i < 16; i++) {
+		/* SCL low */
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,
+					DAVINCI_I2C_DCLR_PDCLR0);
+		udelay(5);
+		/* SCL high */
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+					DAVINCI_I2C_DSET_PDSET0);
+		udelay(5);
+
+		/* read the state of SDA */
+		flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
+		if (flag & DAVINCI_I2C_PDIN_PDIN1) {
+			dev_dbg(dev->dev, "recovered after %d SCL pulses",
+					i + 1);
+			break;
+		}
+	}
+
+	/* change back to I2C mode */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0);
+
+	/* take the I2C dev out of reset */
+	davinci_i2c_reset_ctrl(dev, 1);
+
+	/* read the state of SDA */
+	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
+	if (flag & DAVINCI_I2C_PDIN_PDIN1)
+		return;
+
+	dev_err(dev->dev, "I2C slave will not release SDA.\n");
+}
+
 /* This routine does i2c bus recovery as specified in the
  * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
  */
@@ -756,6 +842,8 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	pdata = dev->dev->platform_data;
 	if (pdata->scl_pin)
 		dev->pulse_scl = generic_i2c_clock_pulse;
+	else if (pdata->has_pfunc)
+		dev->pulse_scl = i2c_davinci_pfunc_i2c_clock_pulse;
 
 	return 0;
 
-- 
1.7.1

[PATCH 4/6] i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio

[Ben Gardiner]

On the da850evm, with the default config of polling keys enabled, several
"controller timed out" errors were observed on the console; as well as the
same error observed on custom hardware when communicating with a i2c
touchscreen device. [1]

Discussion of the causes and potential workarounds began on the e2e forums [2]
where Brad Griffis pointed out that the da850 (and da830) has an i2c controller
whose SCL and SDA may be manipulated as GPIOs by using the ICPFUNC registers
of the i2c controller. He further suggested a means of using this feature to
send clock pulses on the I2C bus to free the frozen slave device.

Implement the suggested procedure by toggling SCL and checking SDA using the
ICPFUNC registers of the I2C controller when present. Allow platforms to
indicate the presence of the ICPFUNC registers with a has_pfunc platform data
flag.

[1] http://permalink.gmane.org/gmane.linux.davinci/22291
[2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Jon Povey <jon.povey@racelogic.co.uk>
Cc: Philby John <pjohn@in.mvista.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/include/mach/i2c.h |    6 ++-
 drivers/i2c/busses/i2c-davinci.c         |   88 ++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/i2c.h b/arch/arm/mach-davinci/include/mach/i2c.h
index ab07a44..858c5c6 100644
--- a/arch/arm/mach-davinci/include/mach/i2c.h
+++ b/arch/arm/mach-davinci/include/mach/i2c.h
@@ -19,12 +19,16 @@
  * @bus_delay:	post-transaction delay (usec)
  * @sda_pin:	GPIO pin ID to use for SDA
  * @scl_pin:	GPIO pin ID to use for SCL
+ * @has_pfunc:	set this to true if the i2c controller on your chip has a
+ *		ICPFUNC register which allows the SDA and SCL pins to be
+ *		controlled as gpio (like in DA850)
  */
 struct davinci_i2c_platform_data {
 	unsigned int	bus_freq;
 	unsigned int	bus_delay;
 	unsigned int	sda_pin;
 	unsigned int	scl_pin;
+	bool		has_pfunc;
 };
 
 /* for board setup code */
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 0a2c697..5bdc98c 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -65,6 +65,11 @@
 #define DAVINCI_I2C_IVR_REG	0x28
 #define DAVINCI_I2C_EMDR_REG	0x2c
 #define DAVINCI_I2C_PSC_REG	0x30
+#define DAVINCI_I2C_PFUNC_REG	0x48
+#define DAVINCI_I2C_PDIR_REG	0x4c
+#define DAVINCI_I2C_PDIN_REG	0x50
+#define DAVINCI_I2C_DSET_REG	0x58
+#define DAVINCI_I2C_DCLR_REG	0x5c
 
 #define DAVINCI_I2C_IVR_AAS	0x07
 #define DAVINCI_I2C_IVR_SCD	0x06
@@ -98,6 +103,29 @@
 #define DAVINCI_I2C_IMR_NACK	BIT(1)
 #define DAVINCI_I2C_IMR_AL	BIT(0)
 
+/* set SDA and SCL as GPIO */
+#define DAVINCI_I2C_PFUNC_PFUNC0	BIT(0)
+
+/* set SCL as output when used as GPIO*/
+#define DAVINCI_I2C_PDIR_PDIR0	BIT(0)
+/* set SDA as output when used as GPIO*/
+#define DAVINCI_I2C_PDIR_PDIR1	BIT(1)
+
+/* read SCL GPIO level */
+#define DAVINCI_I2C_PDIN_PDIN0 BIT(0)
+/* read SDA GPIO level */
+#define DAVINCI_I2C_PDIN_PDIN1 BIT(1)
+
+/*set the SCL GPIO high */
+#define DAVINCI_I2C_DSET_PDSET0	BIT(0)
+/*set the SDA GPIO high */
+#define DAVINCI_I2C_DSET_PDSET1	BIT(1)
+
+/* set the SCL GPIO low */
+#define DAVINCI_I2C_DCLR_PDCLR0	BIT(0)
+/* set the SDA GPIO low */
+#define DAVINCI_I2C_DCLR_PDCLR1	BIT(1)
+
 struct davinci_i2c_dev {
 	struct device           *dev;
 	void __iomem		*base;
@@ -182,6 +210,64 @@ static void generic_i2c_clock_pulse(struct davinci_i2c_dev *dev)
 	}
 }
 
+static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
+								int val);
+
+/* Generate gpio a pulse on the i2c clock pin. */
+static void i2c_davinci_pfunc_i2c_clock_pulse(struct davinci_i2c_dev *dev)
+{
+	u32 flag = 0;
+	u16 i;
+
+	davinci_i2c_reset_ctrl(dev, 0);
+
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0x00);
+
+	/* SCL output, SDA input */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PDIR_REG,
+				DAVINCI_I2C_PDIR_PDIR0);
+
+	/* change to GPIO mode */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG,
+				DAVINCI_I2C_PFUNC_PFUNC0);
+
+	/* SCL high */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+				DAVINCI_I2C_DSET_PDSET0);
+	udelay(5);
+	for (i = 0; i < 16; i++) {
+		/* SCL low */
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,
+					DAVINCI_I2C_DCLR_PDCLR0);
+		udelay(5);
+		/* SCL high */
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+					DAVINCI_I2C_DSET_PDSET0);
+		udelay(5);
+
+		/* read the state of SDA */
+		flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
+		if (flag & DAVINCI_I2C_PDIN_PDIN1) {
+			dev_dbg(dev->dev, "recovered after %d SCL pulses",
+					i + 1);
+			break;
+		}
+	}
+
+	/* change back to I2C mode */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0);
+
+	/* take the I2C dev out of reset */
+	davinci_i2c_reset_ctrl(dev, 1);
+
+	/* read the state of SDA */
+	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
+	if (flag & DAVINCI_I2C_PDIN_PDIN1)
+		return;
+
+	dev_err(dev->dev, "I2C slave will not release SDA.\n");
+}
+
 /* This routine does i2c bus recovery as specified in the
  * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
  */
@@ -756,6 +842,8 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	pdata = dev->dev->platform_data;
 	if (pdata->scl_pin)
 		dev->pulse_scl = generic_i2c_clock_pulse;
+	else if (pdata->has_pfunc)
+		dev->pulse_scl = i2c_davinci_pfunc_i2c_clock_pulse;
 
 	return 0;
 
-- 
1.7.1

[PATCH 4/6] i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio

[Ben Gardiner]

On the da850evm, with the default config of polling keys enabled, several
"controller timed out" errors were observed on the console; as well as the
same error observed on custom hardware when communicating with a i2c
touchscreen device. [1]

Discussion of the causes and potential workarounds began on the e2e forums [2]
where Brad Griffis pointed out that the da850 (and da830) has an i2c controller
whose SCL and SDA may be manipulated as GPIOs by using the ICPFUNC registers
of the i2c controller. He further suggested a means of using this feature to
send clock pulses on the I2C bus to free the frozen slave device.

Implement the suggested procedure by toggling SCL and checking SDA using the
ICPFUNC registers of the I2C controller when present. Allow platforms to
indicate the presence of the ICPFUNC registers with a has_pfunc platform data
flag.

[1] http://permalink.gmane.org/gmane.linux.davinci/22291
[2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Jon Povey <jon.povey@racelogic.co.uk>
Cc: Philby John <pjohn@in.mvista.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/include/mach/i2c.h |    6 ++-
 drivers/i2c/busses/i2c-davinci.c         |   88 ++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/include/mach/i2c.h b/arch/arm/mach-davinci/include/mach/i2c.h
index ab07a44..858c5c6 100644
--- a/arch/arm/mach-davinci/include/mach/i2c.h
+++ b/arch/arm/mach-davinci/include/mach/i2c.h
@@ -19,12 +19,16 @@
  * @bus_delay:	post-transaction delay (usec)
  * @sda_pin:	GPIO pin ID to use for SDA
  * @scl_pin:	GPIO pin ID to use for SCL
+ * @has_pfunc:	set this to true if the i2c controller on your chip has a
+ *		ICPFUNC register which allows the SDA and SCL pins to be
+ *		controlled as gpio (like in DA850)
  */
 struct davinci_i2c_platform_data {
 	unsigned int	bus_freq;
 	unsigned int	bus_delay;
 	unsigned int	sda_pin;
 	unsigned int	scl_pin;
+	bool		has_pfunc;
 };
 
 /* for board setup code */
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 0a2c697..5bdc98c 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -65,6 +65,11 @@
 #define DAVINCI_I2C_IVR_REG	0x28
 #define DAVINCI_I2C_EMDR_REG	0x2c
 #define DAVINCI_I2C_PSC_REG	0x30
+#define DAVINCI_I2C_PFUNC_REG	0x48
+#define DAVINCI_I2C_PDIR_REG	0x4c
+#define DAVINCI_I2C_PDIN_REG	0x50
+#define DAVINCI_I2C_DSET_REG	0x58
+#define DAVINCI_I2C_DCLR_REG	0x5c
 
 #define DAVINCI_I2C_IVR_AAS	0x07
 #define DAVINCI_I2C_IVR_SCD	0x06
@@ -98,6 +103,29 @@
 #define DAVINCI_I2C_IMR_NACK	BIT(1)
 #define DAVINCI_I2C_IMR_AL	BIT(0)
 
+/* set SDA and SCL as GPIO */
+#define DAVINCI_I2C_PFUNC_PFUNC0	BIT(0)
+
+/* set SCL as output when used as GPIO*/
+#define DAVINCI_I2C_PDIR_PDIR0	BIT(0)
+/* set SDA as output when used as GPIO*/
+#define DAVINCI_I2C_PDIR_PDIR1	BIT(1)
+
+/* read SCL GPIO level */
+#define DAVINCI_I2C_PDIN_PDIN0 BIT(0)
+/* read SDA GPIO level */
+#define DAVINCI_I2C_PDIN_PDIN1 BIT(1)
+
+/*set the SCL GPIO high */
+#define DAVINCI_I2C_DSET_PDSET0	BIT(0)
+/*set the SDA GPIO high */
+#define DAVINCI_I2C_DSET_PDSET1	BIT(1)
+
+/* set the SCL GPIO low */
+#define DAVINCI_I2C_DCLR_PDCLR0	BIT(0)
+/* set the SDA GPIO low */
+#define DAVINCI_I2C_DCLR_PDCLR1	BIT(1)
+
 struct davinci_i2c_dev {
 	struct device           *dev;
 	void __iomem		*base;
@@ -182,6 +210,64 @@ static void generic_i2c_clock_pulse(struct davinci_i2c_dev *dev)
 	}
 }
 
+static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
+								int val);
+
+/* Generate gpio a pulse on the i2c clock pin. */
+static void i2c_davinci_pfunc_i2c_clock_pulse(struct davinci_i2c_dev *dev)
+{
+	u32 flag = 0;
+	u16 i;
+
+	davinci_i2c_reset_ctrl(dev, 0);
+
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0x00);
+
+	/* SCL output, SDA input */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PDIR_REG,
+				DAVINCI_I2C_PDIR_PDIR0);
+
+	/* change to GPIO mode */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG,
+				DAVINCI_I2C_PFUNC_PFUNC0);
+
+	/* SCL high */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+				DAVINCI_I2C_DSET_PDSET0);
+	udelay(5);
+	for (i = 0; i < 16; i++) {
+		/* SCL low */
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,
+					DAVINCI_I2C_DCLR_PDCLR0);
+		udelay(5);
+		/* SCL high */
+		davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+					DAVINCI_I2C_DSET_PDSET0);
+		udelay(5);
+
+		/* read the state of SDA */
+		flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
+		if (flag & DAVINCI_I2C_PDIN_PDIN1) {
+			dev_dbg(dev->dev, "recovered after %d SCL pulses",
+					i + 1);
+			break;
+		}
+	}
+
+	/* change back to I2C mode */
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_PFUNC_REG, 0);
+
+	/* take the I2C dev out of reset */
+	davinci_i2c_reset_ctrl(dev, 1);
+
+	/* read the state of SDA */
+	flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG);
+	if (flag & DAVINCI_I2C_PDIN_PDIN1)
+		return;
+
+	dev_err(dev->dev, "I2C slave will not release SDA.\n");
+}
+
 /* This routine does i2c bus recovery as specified in the
  * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
  */
@@ -756,6 +842,8 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	pdata = dev->dev->platform_data;
 	if (pdata->scl_pin)
 		dev->pulse_scl = generic_i2c_clock_pulse;
+	else if (pdata->has_pfunc)
+		dev->pulse_scl = i2c_davinci_pfunc_i2c_clock_pulse;
 
 	return 0;
 
-- 
1.7.1

[PATCH 5/6] i2c-davinci: if device has pfunc register, dump that group also

[Ben Gardiner]

If the platform has indicated that this I2c controller has the ICPFUNC
regsiters then also dump their contents when producing a debug register dump.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 drivers/i2c/busses/i2c-davinci.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5bdc98c..a084e50 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -164,6 +164,8 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
 
 static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 {
+	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
+
 	dev_dbg(dev->dev, "PSC  = %08x\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
 	dev_dbg(dev->dev, "CLKL = %08x\n",
@@ -191,6 +193,19 @@ static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
 	dev_dbg(dev->dev, "EMDR = %08x\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
+
+	if (pdata->has_pfunc) {
+		dev_dbg(dev->dev, "PFUNC = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PFUNC_REG));
+		dev_dbg(dev->dev, "PDIR = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIR_REG));
+		dev_dbg(dev->dev, "PDIN = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG));
+		dev_dbg(dev->dev, "DSET = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_DSET_REG));
+		dev_dbg(dev->dev, "DCLR = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_DCLR_REG));
+	}
 }
 
 /* Generate a pulse on the i2c clock pin. */
-- 
1.7.1

[PATCH 5/6] i2c-davinci: if device has pfunc register, dump that group also

[Ben Gardiner]

If the platform has indicated that this I2c controller has the ICPFUNC
regsiters then also dump their contents when producing a debug register dump.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 drivers/i2c/busses/i2c-davinci.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5bdc98c..a084e50 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -164,6 +164,8 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
 
 static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 {
+	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
+
 	dev_dbg(dev->dev, "PSC  = %08x\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
 	dev_dbg(dev->dev, "CLKL = %08x\n",
@@ -191,6 +193,19 @@ static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
 	dev_dbg(dev->dev, "EMDR = %08x\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
+
+	if (pdata->has_pfunc) {
+		dev_dbg(dev->dev, "PFUNC = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PFUNC_REG));
+		dev_dbg(dev->dev, "PDIR = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIR_REG));
+		dev_dbg(dev->dev, "PDIN = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG));
+		dev_dbg(dev->dev, "DSET = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_DSET_REG));
+		dev_dbg(dev->dev, "DCLR = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_DCLR_REG));
+	}
 }
 
 /* Generate a pulse on the i2c clock pin. */
-- 
1.7.1

[PATCH 5/6] i2c-davinci: if device has pfunc register, dump that group also

[Ben Gardiner]

If the platform has indicated that this I2c controller has the ICPFUNC
regsiters then also dump their contents when producing a debug register dump.

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Bastian Ruppert <Bastian.Ruppert@sewerin.de>
Cc: Brad Griffis <bgriffis@ti.com>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 drivers/i2c/busses/i2c-davinci.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 5bdc98c..a084e50 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -164,6 +164,8 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
 
 static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 {
+	struct davinci_i2c_platform_data *pdata = dev->dev->platform_data;
+
 	dev_dbg(dev->dev, "PSC  = %08x\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
 	dev_dbg(dev->dev, "CLKL = %08x\n",
@@ -191,6 +193,19 @@ static void i2c_davinci_dump_regs(struct davinci_i2c_dev *dev)
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_IVR_REG));
 	dev_dbg(dev->dev, "EMDR = %08x\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_EMDR_REG));
+
+	if (pdata->has_pfunc) {
+		dev_dbg(dev->dev, "PFUNC = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PFUNC_REG));
+		dev_dbg(dev->dev, "PDIR = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIR_REG));
+		dev_dbg(dev->dev, "PDIN = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_PDIN_REG));
+		dev_dbg(dev->dev, "DSET = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_DSET_REG));
+		dev_dbg(dev->dev, "DCLR = %08x\n",
+			davinci_i2c_read_reg(dev, DAVINCI_I2C_DCLR_REG));
+	}
 }
 
 /* Generate a pulse on the i2c clock pin. */
-- 
1.7.1

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

Both the da850 and da830 have an I2C controller which has the ICPFUNC
registers. Indicate this for all da830 and da850 boards by setting the
has_pfunc flag true in the da8xx utility setup routine for registering the
I2C controller

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index beda8a4..da01558 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
 	else
 		return -EINVAL;
 
+	/*
+	 * Both the DA850 and DA830 have an I2C controller which has the
+	 * ICPFUNC et. al. registers
+	 */
+	pdata->has_pfunc = 1;
+
 	pdev->dev.platform_data = pdata;
 	return platform_device_register(pdev);
 }
-- 
1.7.1

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

Both the da850 and da830 have an I2C controller which has the ICPFUNC
registers. Indicate this for all da830 and da850 boards by setting the
has_pfunc flag true in the da8xx utility setup routine for registering the
I2C controller

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index beda8a4..da01558 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
 	else
 		return -EINVAL;
 
+	/*
+	 * Both the DA850 and DA830 have an I2C controller which has the
+	 * ICPFUNC et. al. registers
+	 */
+	pdata->has_pfunc = 1;
+
 	pdev->dev.platform_data = pdata;
 	return platform_device_register(pdev);
 }
-- 
1.7.1

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

Both the da850 and da830 have an I2C controller which has the ICPFUNC
registers. Indicate this for all da830 and da850 boards by setting the
has_pfunc flag true in the da8xx utility setup routine for registering the
I2C controller

Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Ben Dooks <ben-linux@fluff.org>

---
 arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index beda8a4..da01558 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
 	else
 		return -EINVAL;
 
+	/*
+	 * Both the DA850 and DA830 have an I2C controller which has the
+	 * ICPFUNC et. al. registers
+	 */
+	pdata->has_pfunc = 1;
+
 	pdev->dev.platform_data = pdata;
 	return platform_device_register(pdev);
 }
-- 
1.7.1

RE: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

Hi Ben,

On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
> Both the da850 and da830 have an I2C controller which has the ICPFUNC
> registers. Indicate this for all da830 and da850 boards by setting the
> has_pfunc flag true in the da8xx utility setup routine for registering the
> I2C controller
> 
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
> 
> ---
>  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index beda8a4..da01558 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
>  	else
>  		return -EINVAL;
>  
> +	/*
> +	 * Both the DA850 and DA830 have an I2C controller which has the
> +	 * ICPFUNC et. al. registers
> +	 */
> +	pdata->has_pfunc = 1;

The I2C driver implements a default platform data
so it should actually be legal for a DA8x board to
pass NULL platform data. In that case this line
will crash. You should either check for pdata to
be NULL or just let each board choose whether it
needs recovery (I think the better option).

Thanks,
Sekhar

RE: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

Hi Ben,

On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
> Both the da850 and da830 have an I2C controller which has the ICPFUNC
> registers. Indicate this for all da830 and da850 boards by setting the
> has_pfunc flag true in the da8xx utility setup routine for registering the
> I2C controller
> 
> Signed-off-by: Ben Gardiner <bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org>
> Cc: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> 
> ---
>  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index beda8a4..da01558 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
>  	else
>  		return -EINVAL;
>  
> +	/*
> +	 * Both the DA850 and DA830 have an I2C controller which has the
> +	 * ICPFUNC et. al. registers
> +	 */
> +	pdata->has_pfunc = 1;

The I2C driver implements a default platform data
so it should actually be legal for a DA8x board to
pass NULL platform data. In that case this line
will crash. You should either check for pdata to
be NULL or just let each board choose whether it
needs recovery (I think the better option).

Thanks,
Sekhar

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

Hi Ben,

On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
> Both the da850 and da830 have an I2C controller which has the ICPFUNC
> registers. Indicate this for all da830 and da850 boards by setting the
> has_pfunc flag true in the da8xx utility setup routine for registering the
> I2C controller
> 
> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
> 
> ---
>  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index beda8a4..da01558 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
>  	else
>  		return -EINVAL;
>  
> +	/*
> +	 * Both the DA850 and DA830 have an I2C controller which has the
> +	 * ICPFUNC et. al. registers
> +	 */
> +	pdata->has_pfunc = 1;

The I2C driver implements a default platform data
so it should actually be legal for a DA8x board to
pass NULL platform data. In that case this line
will crash. You should either check for pdata to
be NULL or just let each board choose whether it
needs recovery (I think the better option).

Thanks,
Sekhar

Re: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

Hi Sekhar,

On Wed, Apr 13, 2011 at 11:10 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
>> Both the da850 and da830 have an I2C controller which has the ICPFUNC
>> registers. Indicate this for all da830 and da850 boards by setting the
>> has_pfunc flag true in the da8xx utility setup routine for registering the
>> I2C controller
>>
>> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Ben Dooks <ben-linux@fluff.org>
>>
>> ---
>>  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index beda8a4..da01558 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
>>       else
>>               return -EINVAL;
>>
>> +     /*
>> +      * Both the DA850 and DA830 have an I2C controller which has the
>> +      * ICPFUNC et. al. registers
>> +      */
>> +     pdata->has_pfunc = 1;
>
> The I2C driver implements a default platform data
> so it should actually be legal for a DA8x board to
> pass NULL platform data. In that case this line
> will crash.

Good catch, thanks.

> [...] You should either check for pdata to
> be NULL or just let each board choose whether it
> needs recovery (I think the better option).

I understand "check for pdata to be NULL."  If you think it is the
better option I'd be happy to implement it but I don't understand how
to implement "let each board choose whether it needs recovery."

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

Hi Sekhar,

On Wed, Apr 13, 2011 at 11:10 AM, Nori, Sekhar <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
>> Both the da850 and da830 have an I2C controller which has the ICPFUNC
>> registers. Indicate this for all da830 and da850 boards by setting the
>> has_pfunc flag true in the da8xx utility setup routine for registering the
>> I2C controller
>>
>> Signed-off-by: Ben Gardiner <bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org>
>> Cc: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
>> Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
>>
>> ---
>>  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index beda8a4..da01558 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
>>       else
>>               return -EINVAL;
>>
>> +     /*
>> +      * Both the DA850 and DA830 have an I2C controller which has the
>> +      * ICPFUNC et. al. registers
>> +      */
>> +     pdata->has_pfunc = 1;
>
> The I2C driver implements a default platform data
> so it should actually be legal for a DA8x board to
> pass NULL platform data. In that case this line
> will crash.

Good catch, thanks.

> [...] You should either check for pdata to
> be NULL or just let each board choose whether it
> needs recovery (I think the better option).

I understand "check for pdata to be NULL."  If you think it is the
better option I'd be happy to implement it but I don't understand how
to implement "let each board choose whether it needs recovery."

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

Hi Sekhar,

On Wed, Apr 13, 2011 at 11:10 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
>> Both the da850 and da830 have an I2C controller which has the ICPFUNC
>> registers. Indicate this for all da830 and da850 boards by setting the
>> has_pfunc flag true in the da8xx utility setup routine for registering the
>> I2C controller
>>
>> Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> Cc: Ben Dooks <ben-linux@fluff.org>
>>
>> ---
>> ?arch/arm/mach-davinci/devices-da8xx.c | ? ?6 ++++++
>> ?1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>> index beda8a4..da01558 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
>> ? ? ? else
>> ? ? ? ? ? ? ? return -EINVAL;
>>
>> + ? ? /*
>> + ? ? ?* Both the DA850 and DA830 have an I2C controller which has the
>> + ? ? ?* ICPFUNC et. al. registers
>> + ? ? ?*/
>> + ? ? pdata->has_pfunc = 1;
>
> The I2C driver implements a default platform data
> so it should actually be legal for a DA8x board to
> pass NULL platform data. In that case this line
> will crash.

Good catch, thanks.

> [...] You should either check for pdata to
> be NULL or just let each board choose whether it
> needs recovery (I think the better option).

I understand "check for pdata to be NULL."  If you think it is the
better option I'd be happy to implement it but I don't understand how
to implement "let each board choose whether it needs recovery."

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

RE: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

On Wed, Apr 13, 2011 at 20:52:05, Ben Gardiner wrote:

> >> --- a/arch/arm/mach-davinci/devices-da8xx.c
> >> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> >> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
> >>       else
> >>               return -EINVAL;
> >>
> >> +     /*
> >> +      * Both the DA850 and DA830 have an I2C controller which has the
> >> +      * ICPFUNC et. al. registers
> >> +      */
> >> +     pdata->has_pfunc = 1;
> >
> > The I2C driver implements a default platform data
> > so it should actually be legal for a DA8x board to
> > pass NULL platform data. In that case this line
> > will crash.
> 
> Good catch, thanks.
> 
> > [...] You should either check for pdata to
> > be NULL or just let each board choose whether it
> > needs recovery (I think the better option).
> 
> I understand "check for pdata to be NULL."  If you think it is the
> better option I'd be happy to implement it but I don't understand how
> to implement "let each board choose whether it needs recovery."

This is done by just setting has_pfunc = 1 in the
davinci_i2c_platform_data defined in the board file.

Thanks,
Sekhar

RE: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

On Wed, Apr 13, 2011 at 20:52:05, Ben Gardiner wrote:

> >> --- a/arch/arm/mach-davinci/devices-da8xx.c
> >> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> >> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
> >>       else
> >>               return -EINVAL;
> >>
> >> +     /*
> >> +      * Both the DA850 and DA830 have an I2C controller which has the
> >> +      * ICPFUNC et. al. registers
> >> +      */
> >> +     pdata->has_pfunc = 1;
> >
> > The I2C driver implements a default platform data
> > so it should actually be legal for a DA8x board to
> > pass NULL platform data. In that case this line
> > will crash.
> 
> Good catch, thanks.
> 
> > [...] You should either check for pdata to
> > be NULL or just let each board choose whether it
> > needs recovery (I think the better option).
> 
> I understand "check for pdata to be NULL."  If you think it is the
> better option I'd be happy to implement it but I don't understand how
> to implement "let each board choose whether it needs recovery."

This is done by just setting has_pfunc = 1 in the
davinci_i2c_platform_data defined in the board file.

Thanks,
Sekhar

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

On Wed, Apr 13, 2011 at 20:52:05, Ben Gardiner wrote:

> >> --- a/arch/arm/mach-davinci/devices-da8xx.c
> >> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> >> @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
> >> ? ? ? else
> >> ? ? ? ? ? ? ? return -EINVAL;
> >>
> >> + ? ? /*
> >> + ? ? ?* Both the DA850 and DA830 have an I2C controller which has the
> >> + ? ? ?* ICPFUNC et. al. registers
> >> + ? ? ?*/
> >> + ? ? pdata->has_pfunc = 1;
> >
> > The I2C driver implements a default platform data
> > so it should actually be legal for a DA8x board to
> > pass NULL platform data. In that case this line
> > will crash.
> 
> Good catch, thanks.
> 
> > [...] You should either check for pdata to
> > be NULL or just let each board choose whether it
> > needs recovery (I think the better option).
> 
> I understand "check for pdata to be NULL."  If you think it is the
> better option I'd be happy to implement it but I don't understand how
> to implement "let each board choose whether it needs recovery."

This is done by just setting has_pfunc = 1 in the
davinci_i2c_platform_data defined in the board file.

Thanks,
Sekhar

Re: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

On Wed, Apr 13, 2011 at 11:42 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> On Wed, Apr 13, 2011 at 20:52:05, Ben Gardiner wrote:
>> I understand "check for pdata to be NULL."  If you think it is the
>> better option I'd be happy to implement it but I don't understand how
>> to implement "let each board choose whether it needs recovery."
>
> This is done by just setting has_pfunc = 1 in the
> davinci_i2c_platform_data defined in the board file.

Ok -- now I get it, thanks. I spin a V2 shortly.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

On Wed, Apr 13, 2011 at 11:42 AM, Nori, Sekhar <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> On Wed, Apr 13, 2011 at 20:52:05, Ben Gardiner wrote:
>> I understand "check for pdata to be NULL."  If you think it is the
>> better option I'd be happy to implement it but I don't understand how
>> to implement "let each board choose whether it needs recovery."
>
> This is done by just setting has_pfunc = 1 in the
> davinci_i2c_platform_data defined in the board file.

Ok -- now I get it, thanks. I spin a V2 shortly.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Ben Gardiner]

On Wed, Apr 13, 2011 at 11:42 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
> On Wed, Apr 13, 2011 at 20:52:05, Ben Gardiner wrote:
>> I understand "check for pdata to be NULL." ?If you think it is the
>> better option I'd be happy to implement it but I don't understand how
>> to implement "let each board choose whether it needs recovery."
>
> This is done by just setting has_pfunc = 1 in the
> davinci_i2c_platform_data defined in the board file.

Ok -- now I get it, thanks. I spin a V2 shortly.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

RE: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Nori, Sekhar]

Hi Ben,

On Wed, Apr 06, 2011 at 03:08:03, Ben Gardiner wrote:
> This series for the i2c-davinci driver implements both a register dump and
> a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
> registers which is, so far, the da850 and da830 based platforms.
> 
> I2C "controller timed out" messages were being observed by both myself on the
> da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I 
> thought it was the existing pulse-SCL routine but was quickly proven wrong; my
> apologies to John Povey and Philby John for jumping to conclusions.
> 
> A discussion was spawned on the e2e forums [2] where Brad Griffis was
> instrumental in the development of the recovery routine proposed by this patch
> series. It was pointed out by him that the da850's i2c controller has the
> ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
> The recovery routine implemented by the patch series toggles the SCL pin and 
> reads the SDA state using the ICPFUNC registers.

[...]

> 
> When creating this series I noticed that there are obvious similarities between
> the existing recovery routine implemented by Philby John and John Povey and the
> recovery routine proposed in this series. Testing was performed using the
> ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
> was found that the recovery did not work. The method described initially by Brad
> Griffis had the initial state of SCL high and delays of 5us with a maximum 
of
> 8 pulses with a check of SDA each time as compared to the existing routine with
> undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested

I wonder how these values (5us or 20us) are derived. The document[1]
referred to in the existing code says that "the master should send
9 clock pulses". I think 5us suggested by Brad assumes a 100KHz
bus. If that is so, this number should be derived out of the bus_freq
platform data.

[1] http://www.nxp.com/documents/user_manual/UM10204.pdf

> and found that if I changed the initial state of SCL or the number of pulses
> (Bastian had success with 16) that the recovery did not occur as expected;

Was this testing done with the original PinMuxed GPIO based approach or with
the new internal GPIO based approach?

> furthermore Brad pointed out that it was important to check the state of SDA
> after each pulse to see if the slave had released the line. Indeed, adding the
> check of SDA resulted in a quicker recovery. On my da850evm, at least, the
> recovery took only one SCL pulse every time.

Yes, the document referred to above suggests this too.

I guess the existing recovery mechanism and the new ICPFUNC
based approach shouldn't be functionally different - correct?

In that case, it would really be worthwhile to fix the existing
recovery method as well.

> 
> It would be nice to consolidate the two recovery routines and -- since they
> are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
> also be used by bitbanging i2c masters. I did not undertake the former because
> I don't have access to the hardware on which the existing recovery routine was
> tested.

I think if you see issues with existing code, you should just
go ahead and fix. AFAICT, the existing approach should have
worked for you. You can test on your hardware and those with
other hardware can test your patches as well and send their
acks.

Thanks,
Sekhar

> I did not undertake the latter since 1) it seems premature until the
> recovery routines are consolidated (or not) and 2) it would require more
> changes of a broad scope which I feared might mire the review process of this
> series which is, at its core, a functioning workaround of an observed problem
> with da850evm hardware (plus some regsiter dumps). It is my hope that both the
> consolidation of the recovery routines and the extraction to common bitbanging
> code can be done in a later series.
> 
> [1] http://permalink.gmane.org/gmane.linux.davinci/22291
> [2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx
> 
> Ben Gardiner (6):
>   i2c-davinci: register dump before attempted bus recovery
>   i2c-davinci: convert davinci_i2c_platform_data to kerneldoc
>   i2c-davinci: introduce a dev-> function pointer for scl pulsing
>   i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio
>   i2c-davinci: if device has pfunc register, dump that group also
>   da8xx: enable the use of the ICPFUNC in i2c-davinci
> 
>  arch/arm/mach-davinci/devices-da8xx.c    |    6 +
>  arch/arm/mach-davinci/include/mach/i2c.h |   21 +++-
>  drivers/i2c/busses/i2c-davinci.c         |  155 ++++++++++++++++++++++++++++--
>  3 files changed, 170 insertions(+), 12 deletions(-)
> 
> 

RE: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Nori, Sekhar]

Hi Ben,

On Wed, Apr 06, 2011 at 03:08:03, Ben Gardiner wrote:
> This series for the i2c-davinci driver implements both a register dump and
> a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
> registers which is, so far, the da850 and da830 based platforms.
> 
> I2C "controller timed out" messages were being observed by both myself on the
> da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I 
> thought it was the existing pulse-SCL routine but was quickly proven wrong; my
> apologies to John Povey and Philby John for jumping to conclusions.
> 
> A discussion was spawned on the e2e forums [2] where Brad Griffis was
> instrumental in the development of the recovery routine proposed by this patch
> series. It was pointed out by him that the da850's i2c controller has the
> ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
> The recovery routine implemented by the patch series toggles the SCL pin and 
> reads the SDA state using the ICPFUNC registers.

[...]

> 
> When creating this series I noticed that there are obvious similarities between
> the existing recovery routine implemented by Philby John and John Povey and the
> recovery routine proposed in this series. Testing was performed using the
> ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
> was found that the recovery did not work. The method described initially by Brad
> Griffis had the initial state of SCL high and delays of 5us with a maximum 
of
> 8 pulses with a check of SDA each time as compared to the existing routine with
> undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested

I wonder how these values (5us or 20us) are derived. The document[1]
referred to in the existing code says that "the master should send
9 clock pulses". I think 5us suggested by Brad assumes a 100KHz
bus. If that is so, this number should be derived out of the bus_freq
platform data.

[1] http://www.nxp.com/documents/user_manual/UM10204.pdf

> and found that if I changed the initial state of SCL or the number of pulses
> (Bastian had success with 16) that the recovery did not occur as expected;

Was this testing done with the original PinMuxed GPIO based approach or with
the new internal GPIO based approach?

> furthermore Brad pointed out that it was important to check the state of SDA
> after each pulse to see if the slave had released the line. Indeed, adding the
> check of SDA resulted in a quicker recovery. On my da850evm, at least, the
> recovery took only one SCL pulse every time.

Yes, the document referred to above suggests this too.

I guess the existing recovery mechanism and the new ICPFUNC
based approach shouldn't be functionally different - correct?

In that case, it would really be worthwhile to fix the existing
recovery method as well.

> 
> It would be nice to consolidate the two recovery routines and -- since they
> are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
> also be used by bitbanging i2c masters. I did not undertake the former because
> I don't have access to the hardware on which the existing recovery routine was
> tested.

I think if you see issues with existing code, you should just
go ahead and fix. AFAICT, the existing approach should have
worked for you. You can test on your hardware and those with
other hardware can test your patches as well and send their
acks.

Thanks,
Sekhar

> I did not undertake the latter since 1) it seems premature until the
> recovery routines are consolidated (or not) and 2) it would require more
> changes of a broad scope which I feared might mire the review process of this
> series which is, at its core, a functioning workaround of an observed problem
> with da850evm hardware (plus some regsiter dumps). It is my hope that both the
> consolidation of the recovery routines and the extraction to common bitbanging
> code can be done in a later series.
> 
> [1] http://permalink.gmane.org/gmane.linux.davinci/22291
> [2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx
> 
> Ben Gardiner (6):
>   i2c-davinci: register dump before attempted bus recovery
>   i2c-davinci: convert davinci_i2c_platform_data to kerneldoc
>   i2c-davinci: introduce a dev-> function pointer for scl pulsing
>   i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio
>   i2c-davinci: if device has pfunc register, dump that group also
>   da8xx: enable the use of the ICPFUNC in i2c-davinci
> 
>  arch/arm/mach-davinci/devices-da8xx.c    |    6 +
>  arch/arm/mach-davinci/include/mach/i2c.h |   21 +++-
>  drivers/i2c/busses/i2c-davinci.c         |  155 ++++++++++++++++++++++++++++--
>  3 files changed, 170 insertions(+), 12 deletions(-)
> 
> 

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Nori, Sekhar]

Hi Ben,

On Wed, Apr 06, 2011 at 03:08:03, Ben Gardiner wrote:
> This series for the i2c-davinci driver implements both a register dump and
> a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
> registers which is, so far, the da850 and da830 based platforms.
> 
> I2C "controller timed out" messages were being observed by both myself on the
> da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I 
> thought it was the existing pulse-SCL routine but was quickly proven wrong; my
> apologies to John Povey and Philby John for jumping to conclusions.
> 
> A discussion was spawned on the e2e forums [2] where Brad Griffis was
> instrumental in the development of the recovery routine proposed by this patch
> series. It was pointed out by him that the da850's i2c controller has the
> ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
> The recovery routine implemented by the patch series toggles the SCL pin and 
> reads the SDA state using the ICPFUNC registers.

[...]

> 
> When creating this series I noticed that there are obvious similarities between
> the existing recovery routine implemented by Philby John and John Povey and the
> recovery routine proposed in this series. Testing was performed using the
> ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
> was found that the recovery did not work. The method described initially by Brad
> Griffis had the initial state of SCL high and delays of 5us with a maximum 
of
> 8 pulses with a check of SDA each time as compared to the existing routine with
> undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested

I wonder how these values (5us or 20us) are derived. The document[1]
referred to in the existing code says that "the master should send
9 clock pulses". I think 5us suggested by Brad assumes a 100KHz
bus. If that is so, this number should be derived out of the bus_freq
platform data.

[1] http://www.nxp.com/documents/user_manual/UM10204.pdf

> and found that if I changed the initial state of SCL or the number of pulses
> (Bastian had success with 16) that the recovery did not occur as expected;

Was this testing done with the original PinMuxed GPIO based approach or with
the new internal GPIO based approach?

> furthermore Brad pointed out that it was important to check the state of SDA
> after each pulse to see if the slave had released the line. Indeed, adding the
> check of SDA resulted in a quicker recovery. On my da850evm, at least, the
> recovery took only one SCL pulse every time.

Yes, the document referred to above suggests this too.

I guess the existing recovery mechanism and the new ICPFUNC
based approach shouldn't be functionally different - correct?

In that case, it would really be worthwhile to fix the existing
recovery method as well.

> 
> It would be nice to consolidate the two recovery routines and -- since they
> are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
> also be used by bitbanging i2c masters. I did not undertake the former because
> I don't have access to the hardware on which the existing recovery routine was
> tested.

I think if you see issues with existing code, you should just
go ahead and fix. AFAICT, the existing approach should have
worked for you. You can test on your hardware and those with
other hardware can test your patches as well and send their
acks.

Thanks,
Sekhar

> I did not undertake the latter since 1) it seems premature until the
> recovery routines are consolidated (or not) and 2) it would require more
> changes of a broad scope which I feared might mire the review process of this
> series which is, at its core, a functioning workaround of an observed problem
> with da850evm hardware (plus some regsiter dumps). It is my hope that both the
> consolidation of the recovery routines and the extraction to common bitbanging
> code can be done in a later series.
> 
> [1] http://permalink.gmane.org/gmane.linux.davinci/22291
> [2] http://e2e.ti.com/support/dsp/omap_applications_processors/f/42/p/99895/350610.aspx
> 
> Ben Gardiner (6):
>   i2c-davinci: register dump before attempted bus recovery
>   i2c-davinci: convert davinci_i2c_platform_data to kerneldoc
>   i2c-davinci: introduce a dev-> function pointer for scl pulsing
>   i2c-davinci: use the DA8xx's ICPFUNC to toggle I2C as gpio
>   i2c-davinci: if device has pfunc register, dump that group also
>   da8xx: enable the use of the ICPFUNC in i2c-davinci
> 
>  arch/arm/mach-davinci/devices-da8xx.c    |    6 +
>  arch/arm/mach-davinci/include/mach/i2c.h |   21 +++-
>  drivers/i2c/busses/i2c-davinci.c         |  155 ++++++++++++++++++++++++++++--
>  3 files changed, 170 insertions(+), 12 deletions(-)
> 
> 

Re: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Mike Williamson]

On Wed, Apr 13, 2011 at 11:10 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
>
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
> > Both the da850 and da830 have an I2C controller which has the ICPFUNC
> > registers. Indicate this for all da830 and da850 boards by setting the
> > has_pfunc flag true in the da8xx utility setup routine for registering the
> > I2C controller
> >
> > Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Ben Dooks <ben-linux@fluff.org>
> >
> > ---
> >  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index beda8a4..da01558 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
> >       else
> >               return -EINVAL;
> >
> > +     /*
> > +      * Both the DA850 and DA830 have an I2C controller which has the
> > +      * ICPFUNC et. al. registers
> > +      */
> > +     pdata->has_pfunc = 1;
>
> The I2C driver implements a default platform data
> so it should actually be legal for a DA8x board to
> pass NULL platform data. In that case this line
> will crash. You should either check for pdata to
> be NULL or just let each board choose whether it
> needs recovery (I think the better option).
>

I actually had a problem with using NULL for pdata with davinci I2C
and had submitted a patch here that sort of fell on the floor.  The
problem was that the i2c_davinci_calc_clk_dividers is using
platform_data without a check as described above.  So at the moment
using NULL doesn't really work, best as I can tell...

https://lkml.org/lkml/2010/9/4/119

FWIW.

-Mike

Re: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Mike Williamson]

On Wed, Apr 13, 2011 at 11:10 AM, Nori, Sekhar <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
>
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
> > Both the da850 and da830 have an I2C controller which has the ICPFUNC
> > registers. Indicate this for all da830 and da850 boards by setting the
> > has_pfunc flag true in the da8xx utility setup routine for registering the
> > I2C controller
> >
> > Signed-off-by: Ben Gardiner <bengardiner-ScDXFp4xN3PN+rMO2ozGnw@public.gmane.org>
> > Cc: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
> > Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> >
> > ---
> >  arch/arm/mach-davinci/devices-da8xx.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index beda8a4..da01558 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
> >       else
> >               return -EINVAL;
> >
> > +     /*
> > +      * Both the DA850 and DA830 have an I2C controller which has the
> > +      * ICPFUNC et. al. registers
> > +      */
> > +     pdata->has_pfunc = 1;
>
> The I2C driver implements a default platform data
> so it should actually be legal for a DA8x board to
> pass NULL platform data. In that case this line
> will crash. You should either check for pdata to
> be NULL or just let each board choose whether it
> needs recovery (I think the better option).
>

I actually had a problem with using NULL for pdata with davinci I2C
and had submitted a patch here that sort of fell on the floor.  The
problem was that the i2c_davinci_calc_clk_dividers is using
platform_data without a check as described above.  So at the moment
using NULL doesn't really work, best as I can tell...

https://lkml.org/lkml/2010/9/4/119

FWIW.

-Mike

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Mike Williamson]

On Wed, Apr 13, 2011 at 11:10 AM, Nori, Sekhar <nsekhar@ti.com> wrote:
>
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:09, Ben Gardiner wrote:
> > Both the da850 and da830 have an I2C controller which has the ICPFUNC
> > registers. Indicate this for all da830 and da850 boards by setting the
> > has_pfunc flag true in the da8xx utility setup routine for registering the
> > I2C controller
> >
> > Signed-off-by: Ben Gardiner <bengardiner@nanometrics.ca>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Ben Dooks <ben-linux@fluff.org>
> >
> > ---
> > ?arch/arm/mach-davinci/devices-da8xx.c | ? ?6 ++++++
> > ?1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index beda8a4..da01558 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -324,6 +324,12 @@ int __init da8xx_register_i2c(int instance,
> > ? ? ? else
> > ? ? ? ? ? ? ? return -EINVAL;
> >
> > + ? ? /*
> > + ? ? ?* Both the DA850 and DA830 have an I2C controller which has the
> > + ? ? ?* ICPFUNC et. al. registers
> > + ? ? ?*/
> > + ? ? pdata->has_pfunc = 1;
>
> The I2C driver implements a default platform data
> so it should actually be legal for a DA8x board to
> pass NULL platform data. In that case this line
> will crash. You should either check for pdata to
> be NULL or just let each board choose whether it
> needs recovery (I think the better option).
>

I actually had a problem with using NULL for pdata with davinci I2C
and had submitted a patch here that sort of fell on the floor.  The
problem was that the i2c_davinci_calc_clk_dividers is using
platform_data without a check as described above.  So at the moment
using NULL doesn't really work, best as I can tell...

https://lkml.org/lkml/2010/9/4/119

FWIW.

-Mike

RE: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

Hi Mike,

On Thu, Apr 14, 2011 at 04:34:34, Mike Williamson wrote:

> > The I2C driver implements a default platform data
> > so it should actually be legal for a DA8x board to
> > pass NULL platform data. In that case this line
> > will crash. You should either check for pdata to
> > be NULL or just let each board choose whether it
> > needs recovery (I think the better option).
> >
> 
> I actually had a problem with using NULL for pdata with davinci I2C
> and had submitted a patch here that sort of fell on the floor.  The
> problem was that the i2c_davinci_calc_clk_dividers is using
> platform_data without a check as described above.  So at the moment
> using NULL doesn't really work, best as I can tell...
> 
> https://lkml.org/lkml/2010/9/4/119

Looking at this mail chain, it looks like Ben was actually OK
with your first submission and just wanted you to update the
patch description saying that dev->platform_data is accessed
later in the code and that is why it needs to be updated.

I liked your first attempt better and may be you should
try another submission with the patch description updated.

Thanks,
Sekhar

RE: [PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

Hi Mike,

On Thu, Apr 14, 2011 at 04:34:34, Mike Williamson wrote:

> > The I2C driver implements a default platform data
> > so it should actually be legal for a DA8x board to
> > pass NULL platform data. In that case this line
> > will crash. You should either check for pdata to
> > be NULL or just let each board choose whether it
> > needs recovery (I think the better option).
> >
> 
> I actually had a problem with using NULL for pdata with davinci I2C
> and had submitted a patch here that sort of fell on the floor.  The
> problem was that the i2c_davinci_calc_clk_dividers is using
> platform_data without a check as described above.  So at the moment
> using NULL doesn't really work, best as I can tell...
> 
> https://lkml.org/lkml/2010/9/4/119

Looking at this mail chain, it looks like Ben was actually OK
with your first submission and just wanted you to update the
patch description saying that dev->platform_data is accessed
later in the code and that is why it needs to be updated.

I liked your first attempt better and may be you should
try another submission with the patch description updated.

Thanks,
Sekhar

[PATCH 6/6] da8xx: enable the use of the ICPFUNC in i2c-davinci

[Nori, Sekhar]

Hi Mike,

On Thu, Apr 14, 2011 at 04:34:34, Mike Williamson wrote:

> > The I2C driver implements a default platform data
> > so it should actually be legal for a DA8x board to
> > pass NULL platform data. In that case this line
> > will crash. You should either check for pdata to
> > be NULL or just let each board choose whether it
> > needs recovery (I think the better option).
> >
> 
> I actually had a problem with using NULL for pdata with davinci I2C
> and had submitted a patch here that sort of fell on the floor.  The
> problem was that the i2c_davinci_calc_clk_dividers is using
> platform_data without a check as described above.  So at the moment
> using NULL doesn't really work, best as I can tell...
> 
> https://lkml.org/lkml/2010/9/4/119

Looking at this mail chain, it looks like Ben was actually OK
with your first submission and just wanted you to update the
patch description saying that dev->platform_data is accessed
later in the code and that is why it needs to be updated.

I liked your first attempt better and may be you should
try another submission with the patch description updated.

Thanks,
Sekhar

Re: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

Hi Sekhar,

Thank you for reviewing the series.

On Wed, Apr 13, 2011 at 12:25 PM, Nori, Sekhar <nsekhar@ti.com> wrote:
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:03, Ben Gardiner wrote:
>> This series for the i2c-davinci driver implements both a register dump and
>> a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
>> registers which is, so far, the da850 and da830 based platforms.
>>
>> I2C "controller timed out" messages were being observed by both myself on the
>> da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I
>> thought it was the existing pulse-SCL routine but was quickly proven wrong; my
>> apologies to John Povey and Philby John for jumping to conclusions.
>>
>> A discussion was spawned on the e2e forums [2] where Brad Griffis was
>> instrumental in the development of the recovery routine proposed by this patch
>> series. It was pointed out by him that the da850's i2c controller has the
>> ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
>> The recovery routine implemented by the patch series toggles the SCL pin and
>> reads the SDA state using the ICPFUNC registers.
>
> [...]
>
>>
>> When creating this series I noticed that there are obvious similarities between
>> the existing recovery routine implemented by Philby John and John Povey and the
>> recovery routine proposed in this series. Testing was performed using the
>> ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
>> was found that the recovery did not work. The method described initially by Brad
>> Griffis had the initial state of SCL high and delays of 5us with a maximum
> of
>> 8 pulses with a check of SDA each time as compared to the existing routine with
>> undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested
>
> I wonder how these values (5us or 20us) are derived. The document[1]
> referred to in the existing code says that "the master should send
> 9 clock pulses". I think 5us suggested by Brad assumes a 100KHz
> bus. If that is so, this number should be derived out of the bus_freq
> platform data.
>
> [1] http://www.nxp.com/documents/user_manual/UM10204.pdf

I'm not sure how the 20us in the existing method was derived -- I
wonder if Philby John or John Povey could comment? I agree that the
5us suggested by Brad assumes a 100KHz bus -- I could certainly derive
this delay from the bus_freq platform data for the new recovery
procedure since board-da850-evm.c sets bus_freq to 100 kHz.

But I was apprehensive to do so for the existing recovery procedure
since the two (in-tree) boards that are assigning the scl_pin member
are dm355 and dm644x which assign bus_freq values of 400kHz and 20kHz,
respectively. These would require delays of 1.25us and 25us,
respectively; neither of which is the hard-coded 20us. To complicate
matters further, dm644x (20kHz) sets bus_delay to 100us with a comment
that "the msp430 uses a slow bitbanged i2c implementation [...] which
requires 100us of idle after i2c writes [...]." If I derived the
clock-pulse delays from bus_freq the dm644x would probably still work
but if the clock-pulse width was at all important for dm355 then the
recovery procedure could fail there. I don't want to introduce
instabilities into other platforms.

>> and found that if I changed the initial state of SCL or the number of pulses
>> (Bastian had success with 16) that the recovery did not occur as expected;
>
> Was this testing done with the original PinMuxed GPIO based approach or with
> the new internal GPIO based approach?

This testing was performed using the 'new' ICPFUNC-based GPIO toggling.

>> furthermore Brad pointed out that it was important to check the state of SDA
>> after each pulse to see if the slave had released the line. Indeed, adding the
>> check of SDA resulted in a quicker recovery. On my da850evm, at least, the
>> recovery took only one SCL pulse every time.
>
> Yes, the document referred to above suggests this too.
>
> I guess the existing recovery mechanism and the new ICPFUNC
> based approach shouldn't be functionally different - correct?

At the outset I had hoped so too; but what we have in the result is a
method proven to work on the problem hardware here that is now
different than the existing method.

> In that case, it would really be worthwhile to fix the existing
> recovery method as well.
>
>>
>> It would be nice to consolidate the two recovery routines and -- since they
>> are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
>> also be used by bitbanging i2c masters. I did not undertake the former because
>> I don't have access to the hardware on which the existing recovery routine was
>> tested.
>
> I think if you see issues with existing code, you should just
> go ahead and fix. AFAICT, the existing approach should have
> worked for you. You can test on your hardware and those with
> other hardware can test your patches as well and send their
> acks.

Ok -- I did not want to break behaviour on other hardware with my
changes --but I respect your expert opinion coming from the experience
you have in merging patches as a kernel mach maintainer. I will
re-spin (not immeadiately -- I don't expect to be able to return to
development of this patch series for another couple weeks) another
version, this time replacing the existing recovery procedure with the
tested method here. Also, allowing the individual da8xx boards to
specify whether they enable the recovery procedure as you requested in
reviewing 6/6 of this series. I would still prefer to do the
extraction to i2c-algo-bit in a separate series.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

Hi Sekhar,

Thank you for reviewing the series.

On Wed, Apr 13, 2011 at 12:25 PM, Nori, Sekhar <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:03, Ben Gardiner wrote:
>> This series for the i2c-davinci driver implements both a register dump and
>> a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
>> registers which is, so far, the da850 and da830 based platforms.
>>
>> I2C "controller timed out" messages were being observed by both myself on the
>> da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I
>> thought it was the existing pulse-SCL routine but was quickly proven wrong; my
>> apologies to John Povey and Philby John for jumping to conclusions.
>>
>> A discussion was spawned on the e2e forums [2] where Brad Griffis was
>> instrumental in the development of the recovery routine proposed by this patch
>> series. It was pointed out by him that the da850's i2c controller has the
>> ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
>> The recovery routine implemented by the patch series toggles the SCL pin and
>> reads the SDA state using the ICPFUNC registers.
>
> [...]
>
>>
>> When creating this series I noticed that there are obvious similarities between
>> the existing recovery routine implemented by Philby John and John Povey and the
>> recovery routine proposed in this series. Testing was performed using the
>> ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
>> was found that the recovery did not work. The method described initially by Brad
>> Griffis had the initial state of SCL high and delays of 5us with a maximum
> of
>> 8 pulses with a check of SDA each time as compared to the existing routine with
>> undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested
>
> I wonder how these values (5us or 20us) are derived. The document[1]
> referred to in the existing code says that "the master should send
> 9 clock pulses". I think 5us suggested by Brad assumes a 100KHz
> bus. If that is so, this number should be derived out of the bus_freq
> platform data.
>
> [1] http://www.nxp.com/documents/user_manual/UM10204.pdf

I'm not sure how the 20us in the existing method was derived -- I
wonder if Philby John or John Povey could comment? I agree that the
5us suggested by Brad assumes a 100KHz bus -- I could certainly derive
this delay from the bus_freq platform data for the new recovery
procedure since board-da850-evm.c sets bus_freq to 100 kHz.

But I was apprehensive to do so for the existing recovery procedure
since the two (in-tree) boards that are assigning the scl_pin member
are dm355 and dm644x which assign bus_freq values of 400kHz and 20kHz,
respectively. These would require delays of 1.25us and 25us,
respectively; neither of which is the hard-coded 20us. To complicate
matters further, dm644x (20kHz) sets bus_delay to 100us with a comment
that "the msp430 uses a slow bitbanged i2c implementation [...] which
requires 100us of idle after i2c writes [...]." If I derived the
clock-pulse delays from bus_freq the dm644x would probably still work
but if the clock-pulse width was at all important for dm355 then the
recovery procedure could fail there. I don't want to introduce
instabilities into other platforms.

>> and found that if I changed the initial state of SCL or the number of pulses
>> (Bastian had success with 16) that the recovery did not occur as expected;
>
> Was this testing done with the original PinMuxed GPIO based approach or with
> the new internal GPIO based approach?

This testing was performed using the 'new' ICPFUNC-based GPIO toggling.

>> furthermore Brad pointed out that it was important to check the state of SDA
>> after each pulse to see if the slave had released the line. Indeed, adding the
>> check of SDA resulted in a quicker recovery. On my da850evm, at least, the
>> recovery took only one SCL pulse every time.
>
> Yes, the document referred to above suggests this too.
>
> I guess the existing recovery mechanism and the new ICPFUNC
> based approach shouldn't be functionally different - correct?

At the outset I had hoped so too; but what we have in the result is a
method proven to work on the problem hardware here that is now
different than the existing method.

> In that case, it would really be worthwhile to fix the existing
> recovery method as well.
>
>>
>> It would be nice to consolidate the two recovery routines and -- since they
>> are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
>> also be used by bitbanging i2c masters. I did not undertake the former because
>> I don't have access to the hardware on which the existing recovery routine was
>> tested.
>
> I think if you see issues with existing code, you should just
> go ahead and fix. AFAICT, the existing approach should have
> worked for you. You can test on your hardware and those with
> other hardware can test your patches as well and send their
> acks.

Ok -- I did not want to break behaviour on other hardware with my
changes --but I respect your expert opinion coming from the experience
you have in merging patches as a kernel mach maintainer. I will
re-spin (not immeadiately -- I don't expect to be able to return to
development of this patch series for another couple weeks) another
version, this time replacing the existing recovery procedure with the
tested method here. Also, allowing the individual da8xx boards to
specify whether they enable the recovery procedure as you requested in
reviewing 6/6 of this series. I would still prefer to do the
extraction to i2c-algo-bit in a separate series.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

Hi Sekhar,

Thank you for reviewing the series.

On Wed, Apr 13, 2011 at 12:25 PM, Nori, Sekhar <nsekhar@ti.com> wrote:
> Hi Ben,
>
> On Wed, Apr 06, 2011 at 03:08:03, Ben Gardiner wrote:
>> This series for the i2c-davinci driver implements both a register dump and
>> a pulsed-SCL recovery of the bus on those controllers that have ICPFUNC
>> registers which is, so far, the da850 and da830 based platforms.
>>
>> I2C "controller timed out" messages were being observed by both myself on the
>> da850evm and Bastian Ruppert on some custom da850 hardware [1]. Originally I
>> thought it was the existing pulse-SCL routine but was quickly proven wrong; my
>> apologies to John Povey and Philby John for jumping to conclusions.
>>
>> A discussion was spawned on the e2e forums [2] where Brad Griffis was
>> instrumental in the development of the recovery routine proposed by this patch
>> series. It was pointed out by him that the da850's i2c controller has the
>> ability to control the SDA and SCL pins as GPIOS through its ICPFUNC registers.
>> The recovery routine implemented by the patch series toggles the SCL pin and
>> reads the SDA state using the ICPFUNC registers.
>
> [...]
>
>>
>> When creating this series I noticed that there are obvious similarities between
>> the existing recovery routine implemented by Philby John and John Povey and the
>> recovery routine proposed in this series. Testing was performed using the
>> ICPFUNC regsiters to toggle SCL as prescribed by the existing routine and it
>> was found that the recovery did not work. The method described initially by Brad
>> Griffis had the initial state of SCL high and delays of 5us with a maximum
> of
>> 8 pulses with a check of SDA each time as compared to the existing routine with
>> undetermined SCL initial state, 20us delays and a maximum of 8 pulses. I tested
>
> I wonder how these values (5us or 20us) are derived. The document[1]
> referred to in the existing code says that "the master should send
> 9 clock pulses". I think 5us suggested by Brad assumes a 100KHz
> bus. If that is so, this number should be derived out of the bus_freq
> platform data.
>
> [1] http://www.nxp.com/documents/user_manual/UM10204.pdf

I'm not sure how the 20us in the existing method was derived -- I
wonder if Philby John or John Povey could comment? I agree that the
5us suggested by Brad assumes a 100KHz bus -- I could certainly derive
this delay from the bus_freq platform data for the new recovery
procedure since board-da850-evm.c sets bus_freq to 100 kHz.

But I was apprehensive to do so for the existing recovery procedure
since the two (in-tree) boards that are assigning the scl_pin member
are dm355 and dm644x which assign bus_freq values of 400kHz and 20kHz,
respectively. These would require delays of 1.25us and 25us,
respectively; neither of which is the hard-coded 20us. To complicate
matters further, dm644x (20kHz) sets bus_delay to 100us with a comment
that "the msp430 uses a slow bitbanged i2c implementation [...] which
requires 100us of idle after i2c writes [...]." If I derived the
clock-pulse delays from bus_freq the dm644x would probably still work
but if the clock-pulse width was at all important for dm355 then the
recovery procedure could fail there. I don't want to introduce
instabilities into other platforms.

>> and found that if I changed the initial state of SCL or the number of pulses
>> (Bastian had success with 16) that the recovery did not occur as expected;
>
> Was this testing done with the original PinMuxed GPIO based approach or with
> the new internal GPIO based approach?

This testing was performed using the 'new' ICPFUNC-based GPIO toggling.

>> furthermore Brad pointed out that it was important to check the state of SDA
>> after each pulse to see if the slave had released the line. Indeed, adding the
>> check of SDA resulted in a quicker recovery. On my da850evm, at least, the
>> recovery took only one SCL pulse every time.
>
> Yes, the document referred to above suggests this too.
>
> I guess the existing recovery mechanism and the new ICPFUNC
> based approach shouldn't be functionally different - correct?

At the outset I had hoped so too; but what we have in the result is a
method proven to work on the problem hardware here that is now
different than the existing method.

> In that case, it would really be worthwhile to fix the existing
> recovery method as well.
>
>>
>> It would be nice to consolidate the two recovery routines and -- since they
>> are gpio-based -- put the shared recovery routine in i2c-algo-bit so it can
>> also be used by bitbanging i2c masters. I did not undertake the former because
>> I don't have access to the hardware on which the existing recovery routine was
>> tested.
>
> I think if you see issues with existing code, you should just
> go ahead and fix. AFAICT, the existing approach should have
> worked for you. You can test on your hardware and those with
> other hardware can test your patches as well and send their
> acks.

Ok -- I did not want to break behaviour on other hardware with my
changes --but I respect your expert opinion coming from the experience
you have in merging patches as a kernel mach maintainer. I will
re-spin (not immeadiately -- I don't expect to be able to return to
development of this patch series for another couple weeks) another
version, this time replacing the existing recovery procedure with the
tested method here. Also, allowing the individual da8xx boards to
specify whether they enable the recovery procedure as you requested in
reviewing 6/6 of this series. I would still prefer to do the
extraction to i2c-algo-bit in a separate series.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

RE: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Jon Povey]

Ben Gardiner wrote:

>>> When creating this series I noticed that there are obvious
>>> similarities between the existing recovery routine implemented by
>>> Philby John and John Povey

> I'm not sure how the 20us in the existing method was derived -- I
> wonder if Philby John or John Povey could comment?

I've been a little bemused about why I am getting credited for I2C
bus recovery work. I don't remember doing any work on that.

I did a couple of patches to fix a race when setting up a TX, but
those are, afaik, unrelated.

All I know about the bus recovery stuff is looking at it a while back
and thinking hmm, that seems to wiggle gpio without changing the
pinmuxing, so it can't possibly work.

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

RE: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Jon Povey]

Ben Gardiner wrote:

>>> When creating this series I noticed that there are obvious
>>> similarities between the existing recovery routine implemented by
>>> Philby John and John Povey

> I'm not sure how the 20us in the existing method was derived -- I
> wonder if Philby John or John Povey could comment?

I've been a little bemused about why I am getting credited for I2C
bus recovery work. I don't remember doing any work on that.

I did a couple of patches to fix a race when setting up a TX, but
those are, afaik, unrelated.

All I know about the bus recovery stuff is looking at it a while back
and thinking hmm, that seems to wiggle gpio without changing the
pinmuxing, so it can't possibly work.

--
Jon Povey
jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Jon Povey]

Ben Gardiner wrote:

>>> When creating this series I noticed that there are obvious
>>> similarities between the existing recovery routine implemented by
>>> Philby John and John Povey

> I'm not sure how the 20us in the existing method was derived -- I
> wonder if Philby John or John Povey could comment?

I've been a little bemused about why I am getting credited for I2C
bus recovery work. I don't remember doing any work on that.

I did a couple of patches to fix a race when setting up a TX, but
those are, afaik, unrelated.

All I know about the bus recovery stuff is looking at it a while back
and thinking hmm, that seems to wiggle gpio without changing the
pinmuxing, so it can't possibly work.

--
Jon Povey
jon.povey at racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

Re: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

Hi Jon,

On Mon, Apr 18, 2011 at 9:47 PM, Jon Povey <Jon.Povey@racelogic.co.uk> wrote:
> Ben Gardiner wrote:
>
>>>> When creating this series I noticed that there are obvious
>>>> similarities between the existing recovery routine implemented by
>>>> Philby John and John Povey
>
>> I'm not sure how the 20us in the existing method was derived -- I
>> wonder if Philby John or John Povey could comment?
>
> I've been a little bemused about why I am getting credited for I2C
> bus recovery work. I don't remember doing any work on that.

My mistake -- sorry. Also sorry for getting your name wrong repeatedly.

> I did a couple of patches to fix a race when setting up a TX, but
> those are, afaik, unrelated.

In "i2c-davinci: Fix race when setting up for TX" and "i2c-davinci:
Fix TX setup for more SoCs" you mention testing on DM355 -- is there
any you have hardware on which the recovery procedure is executed on
occasion and that you would be available to test modifications to the
current implementation?

> All I know about the bus recovery stuff is looking at it a while back
> and thinking hmm, that seems to wiggle gpio without changing the
> pinmuxing, so it can't possibly work.

:) Probably not then.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

Re: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

Hi Jon,

On Mon, Apr 18, 2011 at 9:47 PM, Jon Povey <Jon.Povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org> wrote:
> Ben Gardiner wrote:
>
>>>> When creating this series I noticed that there are obvious
>>>> similarities between the existing recovery routine implemented by
>>>> Philby John and John Povey
>
>> I'm not sure how the 20us in the existing method was derived -- I
>> wonder if Philby John or John Povey could comment?
>
> I've been a little bemused about why I am getting credited for I2C
> bus recovery work. I don't remember doing any work on that.

My mistake -- sorry. Also sorry for getting your name wrong repeatedly.

> I did a couple of patches to fix a race when setting up a TX, but
> those are, afaik, unrelated.

In "i2c-davinci: Fix race when setting up for TX" and "i2c-davinci:
Fix TX setup for more SoCs" you mention testing on DM355 -- is there
any you have hardware on which the recovery procedure is executed on
occasion and that you would be available to test modifications to the
current implementation?

> All I know about the bus recovery stuff is looking at it a while back
> and thinking hmm, that seems to wiggle gpio without changing the
> pinmuxing, so it can't possibly work.

:) Probably not then.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Ben Gardiner]

Hi Jon,

On Mon, Apr 18, 2011 at 9:47 PM, Jon Povey <Jon.Povey@racelogic.co.uk> wrote:
> Ben Gardiner wrote:
>
>>>> When creating this series I noticed that there are obvious
>>>> similarities between the existing recovery routine implemented by
>>>> Philby John and John Povey
>
>> I'm not sure how the 20us in the existing method was derived -- I
>> wonder if Philby John or John Povey could comment?
>
> I've been a little bemused about why I am getting credited for I2C
> bus recovery work. I don't remember doing any work on that.

My mistake -- sorry. Also sorry for getting your name wrong repeatedly.

> I did a couple of patches to fix a race when setting up a TX, but
> those are, afaik, unrelated.

In "i2c-davinci: Fix race when setting up for TX" and "i2c-davinci:
Fix TX setup for more SoCs" you mention testing on DM355 -- is there
any you have hardware on which the recovery procedure is executed on
occasion and that you would be available to test modifications to the
current implementation?

> All I know about the bus recovery stuff is looking at it a while back
> and thinking hmm, that seems to wiggle gpio without changing the
> pinmuxing, so it can't possibly work.

:) Probably not then.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

RE: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Jon Povey]

Ben Gardiner wrote:
> In "i2c-davinci: Fix race when setting up for TX" and "i2c-davinci:
> Fix TX setup for more SoCs" you mention testing on DM355 -- is there
> any you have hardware on which the recovery procedure is executed on
> occasion and that you would be available to test modifications to the
> current implementation?

Nope, sorry. I've never seen the bus lock up.

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

RE: [PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Jon Povey]

Ben Gardiner wrote:
> In "i2c-davinci: Fix race when setting up for TX" and "i2c-davinci:
> Fix TX setup for more SoCs" you mention testing on DM355 -- is there
> any you have hardware on which the recovery procedure is executed on
> occasion and that you would be available to test modifications to the
> current implementation?

Nope, sorry. I've never seen the bus lock up.

--
Jon Povey
jon.povey-Ean/AyPsLtfkYMGBc/C6ZA@public.gmane.org

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC

[Jon Povey]

Ben Gardiner wrote:
> In "i2c-davinci: Fix race when setting up for TX" and "i2c-davinci:
> Fix TX setup for more SoCs" you mention testing on DM355 -- is there
> any you have hardware on which the recovery procedure is executed on
> occasion and that you would be available to test modifications to the
> current implementation?

Nope, sorry. I've never seen the bus lock up.

--
Jon Povey
jon.povey at racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network