This series contains some fixes and improvements for Davinci I2C:
patch 1 - small improvement
patch 2 - fixes "Bus busy" state for some case (was reported long time ago:()
patch 3-5 - converts driver to use I2C bus recovery infrastructure and
adds Davinci I2C bus recovery mechanizm based on using ICPFUNC registers.
These patches are combination of two patches from Ben Gardiner [1] and
Mike Looijmans [2] which i've reworked to use I2C bus recovery infrastructure
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-April/047305.html
"[PATCH 0/6] i2c-davinci gpio pulsed SCL recovery with ICPFUNC"
[2] https://lkml.org/lkml/2014/2/28/133
"[PATCH] i2c-davinci: Implement a bus recovery that actually works"
Changes in v3:
- minor comments applied
Changes in v2:
- patch 1: error handling improved
- patch 2: commit message updated
- patch 4: minor comments applied
- patch 5: added new optional DT property "ti,has-pfunc"
Link on v2:
https://lkml.org/lkml/2014/11/26/252
Link on v1:
http://www.spinics.net/lists/linux-i2c/msg17601.html
CC: Sekhar Nori <[email protected]>
CC: Kevin Hilman <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Murali Karicheri <[email protected]>
Grygorii Strashko (5):
i2c: i2c-davinci: switch to use platform_get_irq
i2c: davinci: generate STP always when NACK is received
i2c: recovery: change input parameter to i2c_adapter for
prepare/unprepare_recovery
i2c: davinci: use bus recovery infrastructure
i2c: davinci: use ICPFUNC to toggle I2C as gpio for bus recovery
.../devicetree/bindings/i2c/i2c-davinci.txt | 3 +
drivers/i2c/busses/i2c-davinci.c | 205 +++++++++++++++------
drivers/i2c/i2c-core.c | 4 +-
include/linux/i2c.h | 4 +-
include/linux/platform_data/i2c-davinci.h | 1 +
5 files changed, 159 insertions(+), 58 deletions(-)
--
1.9.1
This patch converts Davinci I2C driver to use I2C bus recovery
infrastructure, introduced by commit 5f9296ba21b3 ("i2c: Add
bus recovery infrastructure").
The i2c_bus_recovery_info is configured for Davinci I2C adapter
only in case scl_pin is provided in platform data.
As the controller must be held in reset while doing so, the
recovery routine must re-init the controller. Since this was already
being done after each call to i2c_recover_bus, move those calls into
the recovery_prepare/unprepare routines and as well.
CC: Sekhar Nori <[email protected]>
CC: Kevin Hilman <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Murali Karicheri <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/i2c/busses/i2c-davinci.c | 77 +++++++++++++++++++---------------------
1 file changed, 36 insertions(+), 41 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 17e1203..00aed63 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -133,43 +133,6 @@ static inline u16 davinci_i2c_read_reg(struct davinci_i2c_dev *i2c_dev, int reg)
return readw_relaxed(i2c_dev->base + reg);
}
-/* Generate a pulse on the i2c clock pin. */
-static void davinci_i2c_clock_pulse(unsigned int scl_pin)
-{
- u16 i;
-
- if (scl_pin) {
- /* Send high and low on the SCL line */
- for (i = 0; i < 9; i++) {
- gpio_set_value(scl_pin, 0);
- udelay(20);
- gpio_set_value(scl_pin, 1);
- udelay(20);
- }
- }
-}
-
-/* This routine does i2c bus recovery as specified in the
- * i2c protocol Rev. 03 section 3.16 titled "Bus clear"
- */
-static void davinci_i2c_recover_bus(struct davinci_i2c_dev *dev)
-{
- u32 flag = 0;
- struct davinci_i2c_platform_data *pdata = dev->pdata;
-
- dev_err(dev->dev, "initiating i2c bus recovery\n");
- /* Send NACK to the slave */
- flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
- flag |= DAVINCI_I2C_MDR_NACK;
- /* write the data into mode register */
- davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
- davinci_i2c_clock_pulse(pdata->scl_pin);
- /* Send STOP */
- flag = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
- flag |= DAVINCI_I2C_MDR_STP;
- davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, flag);
-}
-
static inline void davinci_i2c_reset_ctrl(struct davinci_i2c_dev *i2c_dev,
int val)
{
@@ -267,6 +230,34 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
}
/*
+ * This routine does i2c bus recovery by using i2c_generic_gpio_recovery
+ * which is provided by I2C Bus recovery infrastructure.
+ */
+static void davinci_i2c_prepare_recovery(struct i2c_adapter *adap)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ /* Disable interrupts */
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_IMR_REG, 0);
+
+ /* put I2C into reset */
+ davinci_i2c_reset_ctrl(dev, 0);
+}
+
+static void davinci_i2c_unprepare_recovery(struct i2c_adapter *adap)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ i2c_davinci_init(dev);
+}
+
+static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
+ .recover_bus = i2c_generic_gpio_recovery,
+ .prepare_recovery = davinci_i2c_prepare_recovery,
+ .unprepare_recovery = davinci_i2c_unprepare_recovery,
+};
+
+/*
* Waiting for bus not busy
*/
static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
@@ -286,8 +277,7 @@ static int i2c_davinci_wait_bus_not_busy(struct davinci_i2c_dev *dev,
return -ETIMEDOUT;
} else {
to_cnt = 0;
- davinci_i2c_recover_bus(dev);
- i2c_davinci_init(dev);
+ i2c_recover_bus(&dev->adapter);
}
}
if (allow_sleep)
@@ -376,8 +366,7 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
dev->adapter.timeout);
if (r == 0) {
dev_err(dev->dev, "controller timed out\n");
- davinci_i2c_recover_bus(dev);
- i2c_davinci_init(dev);
+ i2c_recover_bus(adap);
dev->buf_len = 0;
return -ETIMEDOUT;
}
@@ -721,6 +710,12 @@ static int davinci_i2c_probe(struct platform_device *pdev)
adap->timeout = DAVINCI_I2C_TIMEOUT;
adap->dev.of_node = pdev->dev.of_node;
+ if (dev->pdata->scl_pin) {
+ adap->bus_recovery_info = &davinci_i2c_gpio_recovery_info;
+ adap->bus_recovery_info->scl_gpio = dev->pdata->scl_pin;
+ adap->bus_recovery_info->sda_gpio = dev->pdata->sda_pin;
+ }
+
adap->nr = pdev->id;
r = i2c_add_numbered_adapter(adap);
if (r) {
--
1.9.1
Having a board where the I2C bus locks up occasionally made it clear
that the bus recovery in the i2c-davinci driver will only work on
some boards, because on regular boards, this will only toggle GPIO
lines that aren't muxed to the actual pins.
The I2C controller on SoCs like da850 (and da830), Keystone 2 has the
built-in capability to bit-bang its lines by using the ICPFUNC registers
of the i2c controller.
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 and add optional DT property "ti,has-pfunc" to indicate
the same in DT.
CC: Sekhar Nori <[email protected]>
CC: Kevin Hilman <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Murali Karicheri <[email protected]>
CC: Mike Looijmans <[email protected]>
CC: <[email protected]>
Reviewed-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Ben Gardiner <[email protected]>
Signed-off-by: Mike Looijmans <[email protected]>
[[email protected]: combined patches from Ben Gardiner and
Mike Looijmans and reimplemented ICPFUNC bus recovery using I2C
bus recovery infrastructure]
Signed-off-by: Grygorii Strashko <[email protected]>
---
.../devicetree/bindings/i2c/i2c-davinci.txt | 3 +
drivers/i2c/busses/i2c-davinci.c | 102 ++++++++++++++++++++-
include/linux/platform_data/i2c-davinci.h | 1 +
3 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-davinci.txt b/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
index 2dc935b..a4e1cbc 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
@@ -10,6 +10,9 @@ Required properties:
Recommended properties :
- interrupts : standard interrupt property.
- clock-frequency : desired I2C bus clock frequency in Hz.
+- ti,has-pfunc: boolean; if defined, it indicates that SoC supports PFUNC
+ registers. PFUNC registers allow to switch I2C pins to function as
+ GPIOs, so they can by toggled manually.
Example (enbw_cmc board):
i2c@1c22000 {
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 00aed63..a1bb587 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -64,6 +64,12 @@
#define DAVINCI_I2C_IVR_REG 0x28
#define DAVINCI_I2C_EMDR_REG 0x2c
#define DAVINCI_I2C_PSC_REG 0x30
+#define DAVINCI_I2C_FUNC_REG 0x48
+#define DAVINCI_I2C_DIR_REG 0x4c
+#define DAVINCI_I2C_DIN_REG 0x50
+#define DAVINCI_I2C_DOUT_REG 0x54
+#define DAVINCI_I2C_DSET_REG 0x58
+#define DAVINCI_I2C_DCLR_REG 0x5c
#define DAVINCI_I2C_IVR_AAS 0x07
#define DAVINCI_I2C_IVR_SCD 0x06
@@ -97,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_FUNC_PFUNC0 BIT(0)
+
+/* set SCL as output when used as GPIO*/
+#define DAVINCI_I2C_DIR_PDIR0 BIT(0)
+/* set SDA as output when used as GPIO*/
+#define DAVINCI_I2C_DIR_PDIR1 BIT(1)
+
+/* read SCL GPIO level */
+#define DAVINCI_I2C_DIN_PDIN0 BIT(0)
+/* read SDA GPIO level */
+#define DAVINCI_I2C_DIN_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;
@@ -257,6 +286,71 @@ static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
.unprepare_recovery = davinci_i2c_unprepare_recovery,
};
+static void davinci_i2c_set_scl(struct i2c_adapter *adap, int val)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ if (val)
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
+ DAVINCI_I2C_DSET_PDSET0);
+ else
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,
+ DAVINCI_I2C_DCLR_PDCLR0);
+}
+
+static int davinci_i2c_get_scl(struct i2c_adapter *adap)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+ int val;
+
+ /* read the state of SCL */
+ val = davinci_i2c_read_reg(dev, DAVINCI_I2C_DIN_REG);
+ return val & DAVINCI_I2C_DIN_PDIN0;
+}
+
+static int davinci_i2c_get_sda(struct i2c_adapter *adap)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+ int val;
+
+ /* read the state of SDA */
+ val = davinci_i2c_read_reg(dev, DAVINCI_I2C_DIN_REG);
+ return val & DAVINCI_I2C_DIN_PDIN1;
+}
+
+static void davinci_i2c_scl_prepare_recovery(struct i2c_adapter *adap)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ davinci_i2c_prepare_recovery(adap);
+
+ /* SCL output, SDA input */
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_DIR_REG, DAVINCI_I2C_DIR_PDIR0);
+
+ /* change to GPIO mode */
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_FUNC_REG,
+ DAVINCI_I2C_FUNC_PFUNC0);
+}
+
+static void davinci_i2c_scl_unprepare_recovery(struct i2c_adapter *adap)
+{
+ struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
+
+ /* change back to I2C mode */
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_FUNC_REG, 0);
+
+ davinci_i2c_unprepare_recovery(adap);
+}
+
+static struct i2c_bus_recovery_info davinci_i2c_scl_recovery_info = {
+ .recover_bus = i2c_generic_scl_recovery,
+ .set_scl = davinci_i2c_set_scl,
+ .get_scl = davinci_i2c_get_scl,
+ .get_sda = davinci_i2c_get_sda,
+ .prepare_recovery = davinci_i2c_scl_prepare_recovery,
+ .unprepare_recovery = davinci_i2c_scl_unprepare_recovery,
+};
+
/*
* Waiting for bus not busy
*/
@@ -669,6 +763,10 @@ static int davinci_i2c_probe(struct platform_device *pdev)
if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",
&prop))
dev->pdata->bus_freq = prop / 1000;
+
+ dev->pdata->has_pfunc =
+ of_property_read_bool(pdev->dev.of_node,
+ "ti,has-pfunc");
} else if (!dev->pdata) {
dev->pdata = &davinci_i2c_platform_data_default;
}
@@ -710,7 +808,9 @@ static int davinci_i2c_probe(struct platform_device *pdev)
adap->timeout = DAVINCI_I2C_TIMEOUT;
adap->dev.of_node = pdev->dev.of_node;
- if (dev->pdata->scl_pin) {
+ if (dev->pdata->has_pfunc)
+ adap->bus_recovery_info = &davinci_i2c_scl_recovery_info;
+ else if (dev->pdata->scl_pin) {
adap->bus_recovery_info = &davinci_i2c_gpio_recovery_info;
adap->bus_recovery_info->scl_gpio = dev->pdata->scl_pin;
adap->bus_recovery_info->sda_gpio = dev->pdata->sda_pin;
diff --git a/include/linux/platform_data/i2c-davinci.h b/include/linux/platform_data/i2c-davinci.h
index 2312d19..89fd347 100644
--- a/include/linux/platform_data/i2c-davinci.h
+++ b/include/linux/platform_data/i2c-davinci.h
@@ -18,6 +18,7 @@ struct davinci_i2c_platform_data {
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 */
+ bool has_pfunc; /*chip has a ICPFUNC register */
};
/* for board setup code */
--
1.9.1
This patch changes type of input parameter for .prepare/unprepare_recovery()
callbacks from struct i2c_bus_recovery_info * to struct i2c_adapter *.
This allows to simplify implementation of these callbacks and avoid
type conversations from i2c_bus_recovery_info to i2c_adapter.
The i2c_bus_recovery_info can be simply retrieved from struct i2c_adapter
which contains pointer on it.
CC: Sekhar Nori <[email protected]>
CC: Kevin Hilman <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Murali Karicheri <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/i2c/i2c-core.c | 4 ++--
include/linux/i2c.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 2f90ac6..72b6e34 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -563,7 +563,7 @@ static int i2c_generic_recovery(struct i2c_adapter *adap)
int i = 0, val = 1, ret = 0;
if (bri->prepare_recovery)
- bri->prepare_recovery(bri);
+ bri->prepare_recovery(adap);
/*
* By this time SCL is high, as we need to give 9 falling-rising edges
@@ -588,7 +588,7 @@ static int i2c_generic_recovery(struct i2c_adapter *adap)
}
if (bri->unprepare_recovery)
- bri->unprepare_recovery(bri);
+ bri->unprepare_recovery(adap);
return ret;
}
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b556e0a..cf9380f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -404,8 +404,8 @@ struct i2c_bus_recovery_info {
void (*set_scl)(struct i2c_adapter *, int val);
int (*get_sda)(struct i2c_adapter *);
- void (*prepare_recovery)(struct i2c_bus_recovery_info *bri);
- void (*unprepare_recovery)(struct i2c_bus_recovery_info *bri);
+ void (*prepare_recovery)(struct i2c_adapter *);
+ void (*unprepare_recovery)(struct i2c_adapter *);
/* gpio recovery */
int scl_gpio;
--
1.9.1
According to I2C specification the NACK should be handled as follows:
"When SDA remains HIGH during this ninth clock pulse, this is defined as the Not
Acknowledge signal. The master can then generate either a STOP condition to
abort the transfer, or a repeated START condition to start a new transfer."
[I2C spec Rev. 6, 3.1.6: http://www.nxp.com/documents/user_manual/UM10204.pdf]
Currently the Davinci i2c driver interrupts the transfer on receipt of a
NACK but fails to send a STOP in some situations and so makes the bus
stuck until next I2C IP reset (idle/enable).
For example, the issue will happen during SMBus read transfer which
consists from two i2c messages write command/address and read data:
S Slave Address Wr A Command Code A Sr Slave Address Rd A D1..Dn A P
<--- write -----------------------> <--- read --------------------->
The I2C client device will send NACK if it can't recognize "Command Code"
and it's expected from I2C master to generate STP in this case.
But now, Davinci i2C driver will just exit with -EREMOTEIO and STP will
not be generated.
Hence, fix it by generating Stop condition (STP) always when NACK is received.
This patch fixes Davinci I2C in the same way it was done for OMAP I2C
commit cda2109a26eb ("i2c: omap: query STP always when NACK is received").
CC: Sekhar Nori <[email protected]>
CC: Kevin Hilman <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Murali Karicheri <[email protected]>
Reviewed-by: Uwe Kleine-König <[email protected]>
Reported-by: Hein Tibosch <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/i2c/busses/i2c-davinci.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 25e8e25..17e1203 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -411,11 +411,9 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
if (dev->cmd_err & DAVINCI_I2C_STR_NACK) {
if (msg->flags & I2C_M_IGNORE_NAK)
return msg->len;
- if (stop) {
- w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
- w |= DAVINCI_I2C_MDR_STP;
- davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
- }
+ w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
+ w |= DAVINCI_I2C_MDR_STP;
+ davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
return -EREMOTEIO;
}
return -EIO;
--
1.9.1
Switch Davinci I2C driver to use platform_get_irq(), because
it is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ, ..)
for requesting IRQ resources any more, as they can be not ready yet
in case of DT-boot.
CC: Sekhar Nori <[email protected]>
CC: Kevin Hilman <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Murali Karicheri <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/i2c/busses/i2c-davinci.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 4d96147..25e8e25 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -640,13 +640,17 @@ static int davinci_i2c_probe(struct platform_device *pdev)
{
struct davinci_i2c_dev *dev;
struct i2c_adapter *adap;
- struct resource *mem, *irq;
- int r;
-
- irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!irq) {
- dev_err(&pdev->dev, "no irq resource?\n");
- return -ENODEV;
+ struct resource *mem;
+ int r, irq;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq <= 0) {
+ if (!irq)
+ irq = -ENXIO;
+ if (irq != -EPROBE_DEFER)
+ dev_err(&pdev->dev,
+ "can't get irq resource ret=%d\n", irq);
+ return irq;
}
dev = devm_kzalloc(&pdev->dev, sizeof(struct davinci_i2c_dev),
@@ -661,7 +665,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
init_completion(&dev->xfr_complete);
#endif
dev->dev = &pdev->dev;
- dev->irq = irq->start;
+ dev->irq = irq;
dev->pdata = dev_get_platdata(&pdev->dev);
platform_set_drvdata(pdev, dev);
--
1.9.1
On Mon, Dec 01, 2014 at 05:34:03PM +0200, Grygorii Strashko wrote:
> Switch Davinci I2C driver to use platform_get_irq(), because
> it is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ, ..)
> for requesting IRQ resources any more, as they can be not ready yet
> in case of DT-boot.
>
> CC: Sekhar Nori <[email protected]>
> CC: Kevin Hilman <[email protected]>
> CC: Santosh Shilimkar <[email protected]>
> CC: Murali Karicheri <[email protected]>
> Acked-by: Uwe Kleine-König <[email protected]>
> Signed-off-by: Grygorii Strashko <[email protected]>
Applied to for-next, thanks!
On Mon, Dec 01, 2014 at 05:34:04PM +0200, Grygorii Strashko wrote:
> According to I2C specification the NACK should be handled as follows:
> "When SDA remains HIGH during this ninth clock pulse, this is defined as the Not
> Acknowledge signal. The master can then generate either a STOP condition to
> abort the transfer, or a repeated START condition to start a new transfer."
> [I2C spec Rev. 6, 3.1.6: http://www.nxp.com/documents/user_manual/UM10204.pdf]
>
> Currently the Davinci i2c driver interrupts the transfer on receipt of a
> NACK but fails to send a STOP in some situations and so makes the bus
> stuck until next I2C IP reset (idle/enable).
>
> For example, the issue will happen during SMBus read transfer which
> consists from two i2c messages write command/address and read data:
>
> S Slave Address Wr A Command Code A Sr Slave Address Rd A D1..Dn A P
Applied to for-current, thanks!
On Mon, Dec 01, 2014 at 05:34:05PM +0200, Grygorii Strashko wrote:
> This patch changes type of input parameter for .prepare/unprepare_recovery()
> callbacks from struct i2c_bus_recovery_info * to struct i2c_adapter *.
> This allows to simplify implementation of these callbacks and avoid
> type conversations from i2c_bus_recovery_info to i2c_adapter.
> The i2c_bus_recovery_info can be simply retrieved from struct i2c_adapter
> which contains pointer on it.
>
> CC: Sekhar Nori <[email protected]>
> CC: Kevin Hilman <[email protected]>
> CC: Santosh Shilimkar <[email protected]>
> CC: Murali Karicheri <[email protected]>
> Acked-by: Uwe Kleine-König <[email protected]>
> Signed-off-by: Grygorii Strashko <[email protected]>
Ah, bus recovery, I need to have a closer look on this one. Scheduled
for 3.20.
Hello Grygorii,
On 01/12/14 16:34, Grygorii Strashko wrote:
> Having a board where the I2C bus locks up occasionally made it clear
> that the bus recovery in the i2c-davinci driver will only work on
> some boards, because on regular boards, this will only toggle GPIO
> lines that aren't muxed to the actual pins.
>
> The I2C controller on SoCs like da850 (and da830), Keystone 2 has the
> built-in capability to bit-bang its lines by using the ICPFUNC registers
> of the i2c controller.
> 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 and add optional DT property "ti,has-pfunc" to indicate
> the same in DT.
>
> CC: Sekhar Nori <[email protected]>
> CC: Kevin Hilman <[email protected]>
> CC: Santosh Shilimkar <[email protected]>
> CC: Murali Karicheri <[email protected]>
> CC: Mike Looijmans <[email protected]>
> CC: <[email protected]>
> Reviewed-by: Uwe Kleine-König <[email protected]>
> Signed-off-by: Ben Gardiner <[email protected]>
> Signed-off-by: Mike Looijmans <[email protected]>
> [[email protected]: combined patches from Ben Gardiner and
> Mike Looijmans and reimplemented ICPFUNC bus recovery using I2C
> bus recovery infrastructure]
> Signed-off-by: Grygorii Strashko <[email protected]>
We have tested it on a custom Keystone2-based board, recovery seems to work
when SDA is held low externally.
Acked-by: Alexander Sverdlin <[email protected]>
Tested-by: Michael Lawnick <[email protected]>
> ---
> .../devicetree/bindings/i2c/i2c-davinci.txt | 3 +
> drivers/i2c/busses/i2c-davinci.c | 102 ++++++++++++++++++++-
> include/linux/platform_data/i2c-davinci.h | 1 +
> 3 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-davinci.txt b/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
> index 2dc935b..a4e1cbc 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
> @@ -10,6 +10,9 @@ Required properties:
> Recommended properties :
> - interrupts : standard interrupt property.
> - clock-frequency : desired I2C bus clock frequency in Hz.
> +- ti,has-pfunc: boolean; if defined, it indicates that SoC supports PFUNC
> + registers. PFUNC registers allow to switch I2C pins to function as
> + GPIOs, so they can by toggled manually.
>
> Example (enbw_cmc board):
> i2c@1c22000 {
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index 00aed63..a1bb587 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -64,6 +64,12 @@
> #define DAVINCI_I2C_IVR_REG 0x28
> #define DAVINCI_I2C_EMDR_REG 0x2c
> #define DAVINCI_I2C_PSC_REG 0x30
> +#define DAVINCI_I2C_FUNC_REG 0x48
> +#define DAVINCI_I2C_DIR_REG 0x4c
> +#define DAVINCI_I2C_DIN_REG 0x50
> +#define DAVINCI_I2C_DOUT_REG 0x54
> +#define DAVINCI_I2C_DSET_REG 0x58
> +#define DAVINCI_I2C_DCLR_REG 0x5c
>
> #define DAVINCI_I2C_IVR_AAS 0x07
> #define DAVINCI_I2C_IVR_SCD 0x06
> @@ -97,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_FUNC_PFUNC0 BIT(0)
> +
> +/* set SCL as output when used as GPIO*/
> +#define DAVINCI_I2C_DIR_PDIR0 BIT(0)
> +/* set SDA as output when used as GPIO*/
> +#define DAVINCI_I2C_DIR_PDIR1 BIT(1)
> +
> +/* read SCL GPIO level */
> +#define DAVINCI_I2C_DIN_PDIN0 BIT(0)
> +/* read SDA GPIO level */
> +#define DAVINCI_I2C_DIN_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;
> @@ -257,6 +286,71 @@ static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
> .unprepare_recovery = davinci_i2c_unprepare_recovery,
> };
>
> +static void davinci_i2c_set_scl(struct i2c_adapter *adap, int val)
> +{
> + struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
> +
> + if (val)
> + davinci_i2c_write_reg(dev, DAVINCI_I2C_DSET_REG,
> + DAVINCI_I2C_DSET_PDSET0);
> + else
> + davinci_i2c_write_reg(dev, DAVINCI_I2C_DCLR_REG,
> + DAVINCI_I2C_DCLR_PDCLR0);
> +}
> +
> +static int davinci_i2c_get_scl(struct i2c_adapter *adap)
> +{
> + struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
> + int val;
> +
> + /* read the state of SCL */
> + val = davinci_i2c_read_reg(dev, DAVINCI_I2C_DIN_REG);
> + return val & DAVINCI_I2C_DIN_PDIN0;
> +}
> +
> +static int davinci_i2c_get_sda(struct i2c_adapter *adap)
> +{
> + struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
> + int val;
> +
> + /* read the state of SDA */
> + val = davinci_i2c_read_reg(dev, DAVINCI_I2C_DIN_REG);
> + return val & DAVINCI_I2C_DIN_PDIN1;
> +}
> +
> +static void davinci_i2c_scl_prepare_recovery(struct i2c_adapter *adap)
> +{
> + struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
> +
> + davinci_i2c_prepare_recovery(adap);
> +
> + /* SCL output, SDA input */
> + davinci_i2c_write_reg(dev, DAVINCI_I2C_DIR_REG, DAVINCI_I2C_DIR_PDIR0);
> +
> + /* change to GPIO mode */
> + davinci_i2c_write_reg(dev, DAVINCI_I2C_FUNC_REG,
> + DAVINCI_I2C_FUNC_PFUNC0);
> +}
> +
> +static void davinci_i2c_scl_unprepare_recovery(struct i2c_adapter *adap)
> +{
> + struct davinci_i2c_dev *dev = i2c_get_adapdata(adap);
> +
> + /* change back to I2C mode */
> + davinci_i2c_write_reg(dev, DAVINCI_I2C_FUNC_REG, 0);
> +
> + davinci_i2c_unprepare_recovery(adap);
> +}
> +
> +static struct i2c_bus_recovery_info davinci_i2c_scl_recovery_info = {
> + .recover_bus = i2c_generic_scl_recovery,
> + .set_scl = davinci_i2c_set_scl,
> + .get_scl = davinci_i2c_get_scl,
> + .get_sda = davinci_i2c_get_sda,
> + .prepare_recovery = davinci_i2c_scl_prepare_recovery,
> + .unprepare_recovery = davinci_i2c_scl_unprepare_recovery,
> +};
> +
> /*
> * Waiting for bus not busy
> */
> @@ -669,6 +763,10 @@ static int davinci_i2c_probe(struct platform_device *pdev)
> if (!of_property_read_u32(pdev->dev.of_node, "clock-frequency",
> &prop))
> dev->pdata->bus_freq = prop / 1000;
> +
> + dev->pdata->has_pfunc =
> + of_property_read_bool(pdev->dev.of_node,
> + "ti,has-pfunc");
> } else if (!dev->pdata) {
> dev->pdata = &davinci_i2c_platform_data_default;
> }
> @@ -710,7 +808,9 @@ static int davinci_i2c_probe(struct platform_device *pdev)
> adap->timeout = DAVINCI_I2C_TIMEOUT;
> adap->dev.of_node = pdev->dev.of_node;
>
> - if (dev->pdata->scl_pin) {
> + if (dev->pdata->has_pfunc)
> + adap->bus_recovery_info = &davinci_i2c_scl_recovery_info;
> + else if (dev->pdata->scl_pin) {
> adap->bus_recovery_info = &davinci_i2c_gpio_recovery_info;
> adap->bus_recovery_info->scl_gpio = dev->pdata->scl_pin;
> adap->bus_recovery_info->sda_gpio = dev->pdata->sda_pin;
> diff --git a/include/linux/platform_data/i2c-davinci.h b/include/linux/platform_data/i2c-davinci.h
> index 2312d19..89fd347 100644
> --- a/include/linux/platform_data/i2c-davinci.h
> +++ b/include/linux/platform_data/i2c-davinci.h
> @@ -18,6 +18,7 @@ struct davinci_i2c_platform_data {
> 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 */
> + bool has_pfunc; /*chip has a ICPFUNC register */
> };
>
> /* for board setup code */
--
Best regards,
Alexander Sverdlin.
> > The I2C specs say in 3.1.16 that the recovery procedure should be used
> > when SDA is stuck low. So, I do wonder if we should apply the recovery
> > after a timeout. Stuck SDA might be one reason for timeout, but there
> > may be others...
>
> This is ancient code. And regarding your question -
> Might be it would be reasonable to add call of
> i2c_davinci_wait_bus_not_busy() at the end of i2c_davinci_xfer()?
> This way we will wait for Bus Free before performing recovery.
That might be an improvement, but the generic question still remains:
Is a timeout a reason for recovery? SDA stuck low is one reason for a
timeout. I have problems making up my mind here between being pragmatic
and being in accordance with the specs.
> Of course, i2c_davinci_wait_bus_not_busy() has to be fixed first
> as proposed by Alexander Sverdlin here:
> https://patchwork.ozlabs.org/patch/448994/.
Okay, good that you said it. So I'll give his patch series priority over
this one.
On 04/03/2015 11:18 PM, Wolfram Sang wrote:
>
>>> The I2C specs say in 3.1.16 that the recovery procedure should be used
>>> when SDA is stuck low. So, I do wonder if we should apply the recovery
>>> after a timeout. Stuck SDA might be one reason for timeout, but there
>>> may be others...
>>
>> This is ancient code. And regarding your question -
>> Might be it would be reasonable to add call of
>> i2c_davinci_wait_bus_not_busy() at the end of i2c_davinci_xfer()?
>> This way we will wait for Bus Free before performing recovery.
>
> That might be an improvement, but the generic question still remains:
> Is a timeout a reason for recovery? SDA stuck low is one reason for a
> timeout. I have problems making up my mind here between being pragmatic
> and being in accordance with the specs.
The timeout here means there were no responses from I2C controller within some
reasonable time period (default - 1 sec). Which in turn
means that Bus/HW state is "unknown" and init&recovery seems reasonable here, but
yes - "init&recovery" could be optimized more, but, in my opinion, only
as subsequent patches.
Actually, i2c_generic_recovery() will just exit if SDA is high already.
>
>> Of course, i2c_davinci_wait_bus_not_busy() has to be fixed first
>> as proposed by Alexander Sverdlin here:
>> https://patchwork.ozlabs.org/patch/448994/.
>
> Okay, good that you said it. So I'll give his patch series priority over
> this one.
Sorry, but this series already mises few merge windows and it has a lot
of revied-by and tested-by, so could we proceed please?
Re-based & re-sent http://www.spinics.net/lists/arm-kernel/msg410810.html
--
regards,
-grygorii
> >> Of course, i2c_davinci_wait_bus_not_busy() has to be fixed first
> >> as proposed by Alexander Sverdlin here:
> >> https://patchwork.ozlabs.org/patch/448994/.
> >
> > Okay, good that you said it. So I'll give his patch series priority over
> > this one.
>
>
> Sorry, but this series already mises few merge windows and it has a lot
> of revied-by and tested-by, so could we proceed please?
>
> Re-based & re-sent http://www.spinics.net/lists/arm-kernel/msg410810.html
??? Didn't you say above that Alexaders's patch is needed first?
On 04/06/2015 07:09 PM, Wolfram Sang wrote:
>
>>>> Of course, i2c_davinci_wait_bus_not_busy() has to be fixed first
>>>> as proposed by Alexander Sverdlin here:
>>>> https://patchwork.ozlabs.org/patch/448994/.
>>>
>>> Okay, good that you said it. So I'll give his patch series priority over
>>> this one.
>>
>>
>> Sorry, but this series already mises few merge windows and it has a lot
>> of revied-by and tested-by, so could we proceed please?
>>
>> Re-based & re-sent http://www.spinics.net/lists/arm-kernel/msg410810.html
>
> ??? Didn't you say above that Alexaders's patch is needed first?
>
Sorry for misunderstanding.
I said that if We'd like to continue and optimize more recovery path
then yes - Alexaders's patch will be needed (patch 2 from his series
[PATCH 2/3] i2c: davinci: Refactor i2c_davinci_wait_bus_not_busy(),
which, in turn need to be rebased as the first one in his series and
re-send). And in my opinion all such improvements could be done by
subsequent patches.
--
regards,
-grygorii