All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
@ 2021-06-03  5:25 ` Chris Packham
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Packham @ 2021-06-03  5:25 UTC (permalink / raw)
  To: wsa, rjui, sbranden, bcm-kernel-feedback-list
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, Richard Laing, Chris Packham

From: Richard Laing <richard.laing@alliedtelesis.co.nz>

The bcm-iproc controller can put the SDA/SCL lines into bit-bang mode,
make use of this to support i2c bus recovery.

Signed-off-by: Richard Laing <richard.laing@alliedtelesis.co.nz>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Richard did most of the work on this. I'm just cleaning it up to get it
upstream.

Changes in v3:
- drop const from bcm_iproc_recovery_info due to build error from the
  kernel test bot

Changes in v2:
- Incorporate feedback from Ray Jui
- Move bcm_iproc_i2c_resume so it can be re-used to return the i2c bus
  to normal operation at the correct speed after a recovery.
- Add iproc_i2c_lockup_recover() helper to only trigger recovery if sda
  is actually stuck
- Use usleep_range() instead of udelay()
- Cosmetic changes to register bit definitions

 drivers/i2c/busses/i2c-bcm-iproc.c | 176 +++++++++++++++++++++++++----
 1 file changed, 151 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index cceaf69279a9..fdbb98a848e3 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -25,6 +25,7 @@
 #define CFG_OFFSET                   0x00
 #define CFG_RESET_SHIFT              31
 #define CFG_EN_SHIFT                 30
+#define CFG_BIT_BANG_SHIFT           29
 #define CFG_SLAVE_ADDR_0_SHIFT       28
 #define CFG_M_RETRY_CNT_SHIFT        16
 #define CFG_M_RETRY_CNT_MASK         0x0f
@@ -66,6 +67,12 @@
 #define S_FIFO_RX_THLD_SHIFT         8
 #define S_FIFO_RX_THLD_MASK          0x3f
 
+#define M_BB_CTRL_OFFSET             0x14
+#define M_BB_SMBCLK_IN_SHIFT         31
+#define M_BB_SMBCLK_OUT_EN_SHIFT     30
+#define M_BB_SMBDAT_IN_SHIFT         29
+#define M_BB_SMBDAT_OUT_EN_SHIFT     28
+
 #define M_CMD_OFFSET                 0x30
 #define M_CMD_START_BUSY_SHIFT       31
 #define M_CMD_STATUS_SHIFT           25
@@ -713,6 +720,147 @@ static void bcm_iproc_i2c_enable_disable(struct bcm_iproc_i2c_dev *iproc_i2c,
 	iproc_i2c_wr_reg(iproc_i2c, CFG_OFFSET, val);
 }
 
+static int bcm_iproc_i2c_resume(struct device *dev)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
+	int ret;
+	u32 val;
+
+	/*
+	 * Power domain could have been shut off completely in system deep
+	 * sleep, so re-initialize the block here
+	 */
+	ret = bcm_iproc_i2c_init(iproc_i2c);
+	if (ret)
+		return ret;
+
+	/* configure to the desired bus speed */
+	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
+	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
+	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
+	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
+
+	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
+
+	return 0;
+}
+
+static void bcm_iproc_i2c_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	dev_dbg(iproc_i2c->device, "Prepare recovery\n");
+
+	/* Disable interrupts */
+	writel(0, iproc_i2c->base + IE_OFFSET);
+	readl(iproc_i2c->base + IE_OFFSET);
+	synchronize_irq(iproc_i2c->irq);
+
+	/* Place controller in reset */
+	tmp = readl(iproc_i2c->base + CFG_OFFSET);
+	tmp |= BIT(CFG_RESET_SHIFT);
+	writel(tmp, iproc_i2c->base + CFG_OFFSET);
+	usleep_range(100, 200);
+
+	/* Switch to bit-bang mode */
+	tmp = readl(iproc_i2c->base + CFG_OFFSET);
+	tmp |= BIT(CFG_BIT_BANG_SHIFT);
+	writel(tmp, iproc_i2c->base + CFG_OFFSET);
+	usleep_range(100, 200);
+}
+
+static void bcm_iproc_i2c_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	/* Switch to normal mode */
+	tmp = readl(iproc_i2c->base + CFG_OFFSET);
+	tmp &= ~BIT(CFG_BIT_BANG_SHIFT);
+	writel(tmp, iproc_i2c->base + CFG_OFFSET);
+	usleep_range(100, 200);
+
+	bcm_iproc_i2c_resume(iproc_i2c->device);
+
+	dev_dbg(iproc_i2c->device, "Recovery complete\n");
+}
+
+static int bcm_iproc_i2c_get_scl(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+
+	return !!(tmp & BIT(M_BB_SMBCLK_IN_SHIFT));
+}
+
+static void bcm_iproc_i2c_set_scl(struct i2c_adapter *adap, int val)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+	if (val)
+		tmp |= BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
+	else
+		tmp &= ~BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
+
+	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
+}
+
+static void bcm_iproc_i2c_set_sda(struct i2c_adapter *adap, int val)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+	if (val)
+		tmp |= BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
+	else
+		tmp &= ~BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
+
+	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
+}
+
+static int bcm_iproc_i2c_get_sda(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+
+	return !!(tmp & BIT(M_BB_SMBDAT_IN_SHIFT));
+}
+
+/* Check if bus lockup occurred, and invoke recovery if so. */
+static void iproc_i2c_lockup_recover(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+	/*
+	 * assume bus lockup if SDA line is low;
+	 * note that there is no need to switch to
+	 * bit-bang mode for this check.
+	 */
+	if (!bcm_iproc_i2c_get_sda(&iproc_i2c->adapter)) {
+		/* locked up - invoke i2c bus recovery. */
+		int ret = i2c_recover_bus(&iproc_i2c->adapter);
+
+		if (ret)
+			dev_err(iproc_i2c->device, "bus recovery: error %d\n", ret);
+	}
+}
+
+static struct i2c_bus_recovery_info bcm_iproc_recovery_info = {
+	.recover_bus = i2c_generic_scl_recovery,
+	.prepare_recovery = bcm_iproc_i2c_prepare_recovery,
+	.unprepare_recovery = bcm_iproc_i2c_unprepare_recovery,
+	.set_scl = bcm_iproc_i2c_set_scl,
+	.get_scl = bcm_iproc_i2c_get_scl,
+	.set_sda = bcm_iproc_i2c_set_sda,
+	.get_sda = bcm_iproc_i2c_get_sda,
+};
+
 static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
 				      struct i2c_msg *msg)
 {
@@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* flush both TX/RX FIFOs */
 		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
 		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
+		iproc_i2c_lockup_recover(iproc_i2c);
 		return -ETIMEDOUT;
 	}
 
@@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* flush both TX/RX FIFOs */
 		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
 		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
+		iproc_i2c_lockup_recover(iproc_i2c);
 		return ret;
 	}
 
@@ -1111,6 +1261,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
 		of_node_full_name(iproc_i2c->device->of_node));
 	adap->algo = &bcm_iproc_algo;
 	adap->quirks = &bcm_iproc_i2c_quirks;
+	adap->bus_recovery_info = &bcm_iproc_recovery_info;
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
@@ -1159,31 +1310,6 @@ static int bcm_iproc_i2c_suspend(struct device *dev)
 	return 0;
 }
 
-static int bcm_iproc_i2c_resume(struct device *dev)
-{
-	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
-	int ret;
-	u32 val;
-
-	/*
-	 * Power domain could have been shut off completely in system deep
-	 * sleep, so re-initialize the block here
-	 */
-	ret = bcm_iproc_i2c_init(iproc_i2c);
-	if (ret)
-		return ret;
-
-	/* configure to the desired bus speed */
-	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
-	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
-	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
-	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
-
-	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
-
-	return 0;
-}
-
 static const struct dev_pm_ops bcm_iproc_i2c_pm_ops = {
 	.suspend_late = &bcm_iproc_i2c_suspend,
 	.resume_early = &bcm_iproc_i2c_resume
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
@ 2021-06-03  5:25 ` Chris Packham
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Packham @ 2021-06-03  5:25 UTC (permalink / raw)
  To: wsa, rjui, sbranden, bcm-kernel-feedback-list
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, Richard Laing, Chris Packham

From: Richard Laing <richard.laing@alliedtelesis.co.nz>

The bcm-iproc controller can put the SDA/SCL lines into bit-bang mode,
make use of this to support i2c bus recovery.

Signed-off-by: Richard Laing <richard.laing@alliedtelesis.co.nz>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Richard did most of the work on this. I'm just cleaning it up to get it
upstream.

Changes in v3:
- drop const from bcm_iproc_recovery_info due to build error from the
  kernel test bot

Changes in v2:
- Incorporate feedback from Ray Jui
- Move bcm_iproc_i2c_resume so it can be re-used to return the i2c bus
  to normal operation at the correct speed after a recovery.
- Add iproc_i2c_lockup_recover() helper to only trigger recovery if sda
  is actually stuck
- Use usleep_range() instead of udelay()
- Cosmetic changes to register bit definitions

 drivers/i2c/busses/i2c-bcm-iproc.c | 176 +++++++++++++++++++++++++----
 1 file changed, 151 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index cceaf69279a9..fdbb98a848e3 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -25,6 +25,7 @@
 #define CFG_OFFSET                   0x00
 #define CFG_RESET_SHIFT              31
 #define CFG_EN_SHIFT                 30
+#define CFG_BIT_BANG_SHIFT           29
 #define CFG_SLAVE_ADDR_0_SHIFT       28
 #define CFG_M_RETRY_CNT_SHIFT        16
 #define CFG_M_RETRY_CNT_MASK         0x0f
@@ -66,6 +67,12 @@
 #define S_FIFO_RX_THLD_SHIFT         8
 #define S_FIFO_RX_THLD_MASK          0x3f
 
+#define M_BB_CTRL_OFFSET             0x14
+#define M_BB_SMBCLK_IN_SHIFT         31
+#define M_BB_SMBCLK_OUT_EN_SHIFT     30
+#define M_BB_SMBDAT_IN_SHIFT         29
+#define M_BB_SMBDAT_OUT_EN_SHIFT     28
+
 #define M_CMD_OFFSET                 0x30
 #define M_CMD_START_BUSY_SHIFT       31
 #define M_CMD_STATUS_SHIFT           25
@@ -713,6 +720,147 @@ static void bcm_iproc_i2c_enable_disable(struct bcm_iproc_i2c_dev *iproc_i2c,
 	iproc_i2c_wr_reg(iproc_i2c, CFG_OFFSET, val);
 }
 
+static int bcm_iproc_i2c_resume(struct device *dev)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
+	int ret;
+	u32 val;
+
+	/*
+	 * Power domain could have been shut off completely in system deep
+	 * sleep, so re-initialize the block here
+	 */
+	ret = bcm_iproc_i2c_init(iproc_i2c);
+	if (ret)
+		return ret;
+
+	/* configure to the desired bus speed */
+	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
+	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
+	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
+	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
+
+	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
+
+	return 0;
+}
+
+static void bcm_iproc_i2c_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	dev_dbg(iproc_i2c->device, "Prepare recovery\n");
+
+	/* Disable interrupts */
+	writel(0, iproc_i2c->base + IE_OFFSET);
+	readl(iproc_i2c->base + IE_OFFSET);
+	synchronize_irq(iproc_i2c->irq);
+
+	/* Place controller in reset */
+	tmp = readl(iproc_i2c->base + CFG_OFFSET);
+	tmp |= BIT(CFG_RESET_SHIFT);
+	writel(tmp, iproc_i2c->base + CFG_OFFSET);
+	usleep_range(100, 200);
+
+	/* Switch to bit-bang mode */
+	tmp = readl(iproc_i2c->base + CFG_OFFSET);
+	tmp |= BIT(CFG_BIT_BANG_SHIFT);
+	writel(tmp, iproc_i2c->base + CFG_OFFSET);
+	usleep_range(100, 200);
+}
+
+static void bcm_iproc_i2c_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	/* Switch to normal mode */
+	tmp = readl(iproc_i2c->base + CFG_OFFSET);
+	tmp &= ~BIT(CFG_BIT_BANG_SHIFT);
+	writel(tmp, iproc_i2c->base + CFG_OFFSET);
+	usleep_range(100, 200);
+
+	bcm_iproc_i2c_resume(iproc_i2c->device);
+
+	dev_dbg(iproc_i2c->device, "Recovery complete\n");
+}
+
+static int bcm_iproc_i2c_get_scl(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+
+	return !!(tmp & BIT(M_BB_SMBCLK_IN_SHIFT));
+}
+
+static void bcm_iproc_i2c_set_scl(struct i2c_adapter *adap, int val)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+	if (val)
+		tmp |= BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
+	else
+		tmp &= ~BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
+
+	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
+}
+
+static void bcm_iproc_i2c_set_sda(struct i2c_adapter *adap, int val)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+	if (val)
+		tmp |= BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
+	else
+		tmp &= ~BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
+
+	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
+}
+
+static int bcm_iproc_i2c_get_sda(struct i2c_adapter *adap)
+{
+	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
+	u32 tmp;
+
+	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
+
+	return !!(tmp & BIT(M_BB_SMBDAT_IN_SHIFT));
+}
+
+/* Check if bus lockup occurred, and invoke recovery if so. */
+static void iproc_i2c_lockup_recover(struct bcm_iproc_i2c_dev *iproc_i2c)
+{
+	/*
+	 * assume bus lockup if SDA line is low;
+	 * note that there is no need to switch to
+	 * bit-bang mode for this check.
+	 */
+	if (!bcm_iproc_i2c_get_sda(&iproc_i2c->adapter)) {
+		/* locked up - invoke i2c bus recovery. */
+		int ret = i2c_recover_bus(&iproc_i2c->adapter);
+
+		if (ret)
+			dev_err(iproc_i2c->device, "bus recovery: error %d\n", ret);
+	}
+}
+
+static struct i2c_bus_recovery_info bcm_iproc_recovery_info = {
+	.recover_bus = i2c_generic_scl_recovery,
+	.prepare_recovery = bcm_iproc_i2c_prepare_recovery,
+	.unprepare_recovery = bcm_iproc_i2c_unprepare_recovery,
+	.set_scl = bcm_iproc_i2c_set_scl,
+	.get_scl = bcm_iproc_i2c_get_scl,
+	.set_sda = bcm_iproc_i2c_set_sda,
+	.get_sda = bcm_iproc_i2c_get_sda,
+};
+
 static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
 				      struct i2c_msg *msg)
 {
@@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* flush both TX/RX FIFOs */
 		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
 		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
+		iproc_i2c_lockup_recover(iproc_i2c);
 		return -ETIMEDOUT;
 	}
 
@@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* flush both TX/RX FIFOs */
 		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
 		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
+		iproc_i2c_lockup_recover(iproc_i2c);
 		return ret;
 	}
 
@@ -1111,6 +1261,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
 		of_node_full_name(iproc_i2c->device->of_node));
 	adap->algo = &bcm_iproc_algo;
 	adap->quirks = &bcm_iproc_i2c_quirks;
+	adap->bus_recovery_info = &bcm_iproc_recovery_info;
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
@@ -1159,31 +1310,6 @@ static int bcm_iproc_i2c_suspend(struct device *dev)
 	return 0;
 }
 
-static int bcm_iproc_i2c_resume(struct device *dev)
-{
-	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
-	int ret;
-	u32 val;
-
-	/*
-	 * Power domain could have been shut off completely in system deep
-	 * sleep, so re-initialize the block here
-	 */
-	ret = bcm_iproc_i2c_init(iproc_i2c);
-	if (ret)
-		return ret;
-
-	/* configure to the desired bus speed */
-	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
-	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
-	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
-	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
-
-	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
-
-	return 0;
-}
-
 static const struct dev_pm_ops bcm_iproc_i2c_pm_ops = {
 	.suspend_late = &bcm_iproc_i2c_suspend,
 	.resume_early = &bcm_iproc_i2c_resume
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
  2021-06-03  5:25 ` Chris Packham
@ 2021-06-03 17:06   ` Ray Jui
  -1 siblings, 0 replies; 6+ messages in thread
From: Ray Jui @ 2021-06-03 17:06 UTC (permalink / raw)
  To: Chris Packham, wsa, rjui, sbranden, bcm-kernel-feedback-list
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, Richard Laing

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



On 6/2/2021 10:25 PM, Chris Packham wrote:
> From: Richard Laing <richard.laing@alliedtelesis.co.nz>
> 
> The bcm-iproc controller can put the SDA/SCL lines into bit-bang mode,
> make use of this to support i2c bus recovery.
> 
> Signed-off-by: Richard Laing <richard.laing@alliedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Richard did most of the work on this. I'm just cleaning it up to get it
> upstream.
> 
> Changes in v3:
> - drop const from bcm_iproc_recovery_info due to build error from the
>   kernel test bot
> 
> Changes in v2:
> - Incorporate feedback from Ray Jui
> - Move bcm_iproc_i2c_resume so it can be re-used to return the i2c bus
>   to normal operation at the correct speed after a recovery.
> - Add iproc_i2c_lockup_recover() helper to only trigger recovery if sda
>   is actually stuck
> - Use usleep_range() instead of udelay()
> - Cosmetic changes to register bit definitions
> 
>  drivers/i2c/busses/i2c-bcm-iproc.c | 176 +++++++++++++++++++++++++----
>  1 file changed, 151 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index cceaf69279a9..fdbb98a848e3 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -25,6 +25,7 @@
>  #define CFG_OFFSET                   0x00
>  #define CFG_RESET_SHIFT              31
>  #define CFG_EN_SHIFT                 30
> +#define CFG_BIT_BANG_SHIFT           29
>  #define CFG_SLAVE_ADDR_0_SHIFT       28
>  #define CFG_M_RETRY_CNT_SHIFT        16
>  #define CFG_M_RETRY_CNT_MASK         0x0f
> @@ -66,6 +67,12 @@
>  #define S_FIFO_RX_THLD_SHIFT         8
>  #define S_FIFO_RX_THLD_MASK          0x3f
>  
> +#define M_BB_CTRL_OFFSET             0x14
> +#define M_BB_SMBCLK_IN_SHIFT         31
> +#define M_BB_SMBCLK_OUT_EN_SHIFT     30
> +#define M_BB_SMBDAT_IN_SHIFT         29
> +#define M_BB_SMBDAT_OUT_EN_SHIFT     28
> +
>  #define M_CMD_OFFSET                 0x30
>  #define M_CMD_START_BUSY_SHIFT       31
>  #define M_CMD_STATUS_SHIFT           25
> @@ -713,6 +720,147 @@ static void bcm_iproc_i2c_enable_disable(struct bcm_iproc_i2c_dev *iproc_i2c,
>  	iproc_i2c_wr_reg(iproc_i2c, CFG_OFFSET, val);
>  }
>  
> +static int bcm_iproc_i2c_resume(struct device *dev)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
> +	int ret;
> +	u32 val;
> +
> +	/*
> +	 * Power domain could have been shut off completely in system deep
> +	 * sleep, so re-initialize the block here
> +	 */
> +	ret = bcm_iproc_i2c_init(iproc_i2c);
> +	if (ret)
> +		return ret;
> +
> +	/* configure to the desired bus speed */
> +	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
> +	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
> +	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
> +	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
> +
> +	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> +
> +	return 0;
> +}
> +
> +static void bcm_iproc_i2c_prepare_recovery(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	dev_dbg(iproc_i2c->device, "Prepare recovery\n");
> +
> +	/* Disable interrupts */
> +	writel(0, iproc_i2c->base + IE_OFFSET);
> +	readl(iproc_i2c->base + IE_OFFSET);
> +	synchronize_irq(iproc_i2c->irq);
> +
> +	/* Place controller in reset */
> +	tmp = readl(iproc_i2c->base + CFG_OFFSET);
> +	tmp |= BIT(CFG_RESET_SHIFT);
> +	writel(tmp, iproc_i2c->base + CFG_OFFSET);
> +	usleep_range(100, 200);
> +
> +	/* Switch to bit-bang mode */
> +	tmp = readl(iproc_i2c->base + CFG_OFFSET);
> +	tmp |= BIT(CFG_BIT_BANG_SHIFT);
> +	writel(tmp, iproc_i2c->base + CFG_OFFSET);
> +	usleep_range(100, 200);
> +}
> +
> +static void bcm_iproc_i2c_unprepare_recovery(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	/* Switch to normal mode */
> +	tmp = readl(iproc_i2c->base + CFG_OFFSET);
> +	tmp &= ~BIT(CFG_BIT_BANG_SHIFT);
> +	writel(tmp, iproc_i2c->base + CFG_OFFSET);
> +	usleep_range(100, 200);
> +
> +	bcm_iproc_i2c_resume(iproc_i2c->device);
> +
> +	dev_dbg(iproc_i2c->device, "Recovery complete\n");
> +}
> +
> +static int bcm_iproc_i2c_get_scl(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +
> +	return !!(tmp & BIT(M_BB_SMBCLK_IN_SHIFT));
> +}
> +
> +static void bcm_iproc_i2c_set_scl(struct i2c_adapter *adap, int val)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +	if (val)
> +		tmp |= BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
> +	else
> +		tmp &= ~BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
> +
> +	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
> +}
> +
> +static void bcm_iproc_i2c_set_sda(struct i2c_adapter *adap, int val)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +	if (val)
> +		tmp |= BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
> +	else
> +		tmp &= ~BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
> +
> +	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
> +}
> +
> +static int bcm_iproc_i2c_get_sda(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +
> +	return !!(tmp & BIT(M_BB_SMBDAT_IN_SHIFT));
> +}
> +
> +/* Check if bus lockup occurred, and invoke recovery if so. */
> +static void iproc_i2c_lockup_recover(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> +	/*
> +	 * assume bus lockup if SDA line is low;
> +	 * note that there is no need to switch to
> +	 * bit-bang mode for this check.
> +	 */
> +	if (!bcm_iproc_i2c_get_sda(&iproc_i2c->adapter)) {
> +		/* locked up - invoke i2c bus recovery. */
> +		int ret = i2c_recover_bus(&iproc_i2c->adapter);
> +
> +		if (ret)
> +			dev_err(iproc_i2c->device, "bus recovery: error %d\n", ret);
> +	}
> +}
> +
> +static struct i2c_bus_recovery_info bcm_iproc_recovery_info = {
> +	.recover_bus = i2c_generic_scl_recovery,
> +	.prepare_recovery = bcm_iproc_i2c_prepare_recovery,
> +	.unprepare_recovery = bcm_iproc_i2c_unprepare_recovery,
> +	.set_scl = bcm_iproc_i2c_set_scl,
> +	.get_scl = bcm_iproc_i2c_get_scl,
> +	.set_sda = bcm_iproc_i2c_set_sda,
> +	.get_sda = bcm_iproc_i2c_get_sda,
> +};
> +
>  static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
>  				      struct i2c_msg *msg)
>  {
> @@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return -ETIMEDOUT;
>  	}
>  
> @@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return ret;
>  	}
>  
> @@ -1111,6 +1261,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
>  		of_node_full_name(iproc_i2c->device->of_node));
>  	adap->algo = &bcm_iproc_algo;
>  	adap->quirks = &bcm_iproc_i2c_quirks;
> +	adap->bus_recovery_info = &bcm_iproc_recovery_info;
>  	adap->dev.parent = &pdev->dev;
>  	adap->dev.of_node = pdev->dev.of_node;
>  
> @@ -1159,31 +1310,6 @@ static int bcm_iproc_i2c_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int bcm_iproc_i2c_resume(struct device *dev)
> -{
> -	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
> -	int ret;
> -	u32 val;
> -
> -	/*
> -	 * Power domain could have been shut off completely in system deep
> -	 * sleep, so re-initialize the block here
> -	 */
> -	ret = bcm_iproc_i2c_init(iproc_i2c);
> -	if (ret)
> -		return ret;
> -
> -	/* configure to the desired bus speed */
> -	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
> -	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
> -	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
> -	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
> -
> -	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> -
> -	return 0;
> -}
> -
>  static const struct dev_pm_ops bcm_iproc_i2c_pm_ops = {
>  	.suspend_late = &bcm_iproc_i2c_suspend,
>  	.resume_early = &bcm_iproc_i2c_resume
> 

This latest change looks good to me. Thanks a lot!

Acked-by: Ray Jui <ray.jui@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4194 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
@ 2021-06-03 17:06   ` Ray Jui
  0 siblings, 0 replies; 6+ messages in thread
From: Ray Jui @ 2021-06-03 17:06 UTC (permalink / raw)
  To: Chris Packham, wsa, rjui, sbranden, bcm-kernel-feedback-list
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, Richard Laing


[-- Attachment #1.1: Type: text/plain, Size: 8646 bytes --]



On 6/2/2021 10:25 PM, Chris Packham wrote:
> From: Richard Laing <richard.laing@alliedtelesis.co.nz>
> 
> The bcm-iproc controller can put the SDA/SCL lines into bit-bang mode,
> make use of this to support i2c bus recovery.
> 
> Signed-off-by: Richard Laing <richard.laing@alliedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Richard did most of the work on this. I'm just cleaning it up to get it
> upstream.
> 
> Changes in v3:
> - drop const from bcm_iproc_recovery_info due to build error from the
>   kernel test bot
> 
> Changes in v2:
> - Incorporate feedback from Ray Jui
> - Move bcm_iproc_i2c_resume so it can be re-used to return the i2c bus
>   to normal operation at the correct speed after a recovery.
> - Add iproc_i2c_lockup_recover() helper to only trigger recovery if sda
>   is actually stuck
> - Use usleep_range() instead of udelay()
> - Cosmetic changes to register bit definitions
> 
>  drivers/i2c/busses/i2c-bcm-iproc.c | 176 +++++++++++++++++++++++++----
>  1 file changed, 151 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index cceaf69279a9..fdbb98a848e3 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -25,6 +25,7 @@
>  #define CFG_OFFSET                   0x00
>  #define CFG_RESET_SHIFT              31
>  #define CFG_EN_SHIFT                 30
> +#define CFG_BIT_BANG_SHIFT           29
>  #define CFG_SLAVE_ADDR_0_SHIFT       28
>  #define CFG_M_RETRY_CNT_SHIFT        16
>  #define CFG_M_RETRY_CNT_MASK         0x0f
> @@ -66,6 +67,12 @@
>  #define S_FIFO_RX_THLD_SHIFT         8
>  #define S_FIFO_RX_THLD_MASK          0x3f
>  
> +#define M_BB_CTRL_OFFSET             0x14
> +#define M_BB_SMBCLK_IN_SHIFT         31
> +#define M_BB_SMBCLK_OUT_EN_SHIFT     30
> +#define M_BB_SMBDAT_IN_SHIFT         29
> +#define M_BB_SMBDAT_OUT_EN_SHIFT     28
> +
>  #define M_CMD_OFFSET                 0x30
>  #define M_CMD_START_BUSY_SHIFT       31
>  #define M_CMD_STATUS_SHIFT           25
> @@ -713,6 +720,147 @@ static void bcm_iproc_i2c_enable_disable(struct bcm_iproc_i2c_dev *iproc_i2c,
>  	iproc_i2c_wr_reg(iproc_i2c, CFG_OFFSET, val);
>  }
>  
> +static int bcm_iproc_i2c_resume(struct device *dev)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
> +	int ret;
> +	u32 val;
> +
> +	/*
> +	 * Power domain could have been shut off completely in system deep
> +	 * sleep, so re-initialize the block here
> +	 */
> +	ret = bcm_iproc_i2c_init(iproc_i2c);
> +	if (ret)
> +		return ret;
> +
> +	/* configure to the desired bus speed */
> +	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
> +	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
> +	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
> +	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
> +
> +	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> +
> +	return 0;
> +}
> +
> +static void bcm_iproc_i2c_prepare_recovery(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	dev_dbg(iproc_i2c->device, "Prepare recovery\n");
> +
> +	/* Disable interrupts */
> +	writel(0, iproc_i2c->base + IE_OFFSET);
> +	readl(iproc_i2c->base + IE_OFFSET);
> +	synchronize_irq(iproc_i2c->irq);
> +
> +	/* Place controller in reset */
> +	tmp = readl(iproc_i2c->base + CFG_OFFSET);
> +	tmp |= BIT(CFG_RESET_SHIFT);
> +	writel(tmp, iproc_i2c->base + CFG_OFFSET);
> +	usleep_range(100, 200);
> +
> +	/* Switch to bit-bang mode */
> +	tmp = readl(iproc_i2c->base + CFG_OFFSET);
> +	tmp |= BIT(CFG_BIT_BANG_SHIFT);
> +	writel(tmp, iproc_i2c->base + CFG_OFFSET);
> +	usleep_range(100, 200);
> +}
> +
> +static void bcm_iproc_i2c_unprepare_recovery(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	/* Switch to normal mode */
> +	tmp = readl(iproc_i2c->base + CFG_OFFSET);
> +	tmp &= ~BIT(CFG_BIT_BANG_SHIFT);
> +	writel(tmp, iproc_i2c->base + CFG_OFFSET);
> +	usleep_range(100, 200);
> +
> +	bcm_iproc_i2c_resume(iproc_i2c->device);
> +
> +	dev_dbg(iproc_i2c->device, "Recovery complete\n");
> +}
> +
> +static int bcm_iproc_i2c_get_scl(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +
> +	return !!(tmp & BIT(M_BB_SMBCLK_IN_SHIFT));
> +}
> +
> +static void bcm_iproc_i2c_set_scl(struct i2c_adapter *adap, int val)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +	if (val)
> +		tmp |= BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
> +	else
> +		tmp &= ~BIT(M_BB_SMBCLK_OUT_EN_SHIFT);
> +
> +	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
> +}
> +
> +static void bcm_iproc_i2c_set_sda(struct i2c_adapter *adap, int val)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +	if (val)
> +		tmp |= BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
> +	else
> +		tmp &= ~BIT(M_BB_SMBDAT_OUT_EN_SHIFT);
> +
> +	writel(tmp, iproc_i2c->base + M_BB_CTRL_OFFSET);
> +}
> +
> +static int bcm_iproc_i2c_get_sda(struct i2c_adapter *adap)
> +{
> +	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adap);
> +	u32 tmp;
> +
> +	tmp = readl(iproc_i2c->base + M_BB_CTRL_OFFSET);
> +
> +	return !!(tmp & BIT(M_BB_SMBDAT_IN_SHIFT));
> +}
> +
> +/* Check if bus lockup occurred, and invoke recovery if so. */
> +static void iproc_i2c_lockup_recover(struct bcm_iproc_i2c_dev *iproc_i2c)
> +{
> +	/*
> +	 * assume bus lockup if SDA line is low;
> +	 * note that there is no need to switch to
> +	 * bit-bang mode for this check.
> +	 */
> +	if (!bcm_iproc_i2c_get_sda(&iproc_i2c->adapter)) {
> +		/* locked up - invoke i2c bus recovery. */
> +		int ret = i2c_recover_bus(&iproc_i2c->adapter);
> +
> +		if (ret)
> +			dev_err(iproc_i2c->device, "bus recovery: error %d\n", ret);
> +	}
> +}
> +
> +static struct i2c_bus_recovery_info bcm_iproc_recovery_info = {
> +	.recover_bus = i2c_generic_scl_recovery,
> +	.prepare_recovery = bcm_iproc_i2c_prepare_recovery,
> +	.unprepare_recovery = bcm_iproc_i2c_unprepare_recovery,
> +	.set_scl = bcm_iproc_i2c_set_scl,
> +	.get_scl = bcm_iproc_i2c_get_scl,
> +	.set_sda = bcm_iproc_i2c_set_sda,
> +	.get_sda = bcm_iproc_i2c_get_sda,
> +};
> +
>  static int bcm_iproc_i2c_check_status(struct bcm_iproc_i2c_dev *iproc_i2c,
>  				      struct i2c_msg *msg)
>  {
> @@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return -ETIMEDOUT;
>  	}
>  
> @@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return ret;
>  	}
>  
> @@ -1111,6 +1261,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
>  		of_node_full_name(iproc_i2c->device->of_node));
>  	adap->algo = &bcm_iproc_algo;
>  	adap->quirks = &bcm_iproc_i2c_quirks;
> +	adap->bus_recovery_info = &bcm_iproc_recovery_info;
>  	adap->dev.parent = &pdev->dev;
>  	adap->dev.of_node = pdev->dev.of_node;
>  
> @@ -1159,31 +1310,6 @@ static int bcm_iproc_i2c_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int bcm_iproc_i2c_resume(struct device *dev)
> -{
> -	struct bcm_iproc_i2c_dev *iproc_i2c = dev_get_drvdata(dev);
> -	int ret;
> -	u32 val;
> -
> -	/*
> -	 * Power domain could have been shut off completely in system deep
> -	 * sleep, so re-initialize the block here
> -	 */
> -	ret = bcm_iproc_i2c_init(iproc_i2c);
> -	if (ret)
> -		return ret;
> -
> -	/* configure to the desired bus speed */
> -	val = iproc_i2c_rd_reg(iproc_i2c, TIM_CFG_OFFSET);
> -	val &= ~BIT(TIM_CFG_MODE_400_SHIFT);
> -	val |= (iproc_i2c->bus_speed == I2C_MAX_FAST_MODE_FREQ) << TIM_CFG_MODE_400_SHIFT;
> -	iproc_i2c_wr_reg(iproc_i2c, TIM_CFG_OFFSET, val);
> -
> -	bcm_iproc_i2c_enable_disable(iproc_i2c, true);
> -
> -	return 0;
> -}
> -
>  static const struct dev_pm_ops bcm_iproc_i2c_pm_ops = {
>  	.suspend_late = &bcm_iproc_i2c_suspend,
>  	.resume_early = &bcm_iproc_i2c_resume
> 

This latest change looks good to me. Thanks a lot!

Acked-by: Ray Jui <ray.jui@broadcom.com>

[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4194 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
  2021-06-03  5:25 ` Chris Packham
@ 2021-11-29 19:01   ` Wolfram Sang
  -1 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-11-29 19:01 UTC (permalink / raw)
  To: Chris Packham
  Cc: rjui, sbranden, bcm-kernel-feedback-list, linux-i2c,
	linux-arm-kernel, linux-kernel, Richard Laing

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

Hi Chris,

sorry for the long delay.

All looks good except for this:

> @@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return -ETIMEDOUT;
>  	}
>  
> @@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return ret;
>  	}

I2C specs suggest recovery at the beginning of a transfer when SDA is
detected low. Not at the end of a transfer.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] i2c: bcm-iproc: Add i2c recovery support
@ 2021-11-29 19:01   ` Wolfram Sang
  0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2021-11-29 19:01 UTC (permalink / raw)
  To: Chris Packham
  Cc: rjui, sbranden, bcm-kernel-feedback-list, linux-i2c,
	linux-arm-kernel, linux-kernel, Richard Laing


[-- Attachment #1.1: Type: text/plain, Size: 895 bytes --]

Hi Chris,

sorry for the long delay.

All looks good except for this:

> @@ -806,6 +954,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return -ETIMEDOUT;
>  	}
>  
> @@ -814,6 +963,7 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>  		/* flush both TX/RX FIFOs */
>  		val = BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT);
>  		iproc_i2c_wr_reg(iproc_i2c, M_FIFO_CTRL_OFFSET, val);
> +		iproc_i2c_lockup_recover(iproc_i2c);
>  		return ret;
>  	}

I2C specs suggest recovery at the beginning of a transfer when SDA is
detected low. Not at the end of a transfer.

Happy hacking,

   Wolfram


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-29 19:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03  5:25 [PATCH v3] i2c: bcm-iproc: Add i2c recovery support Chris Packham
2021-06-03  5:25 ` Chris Packham
2021-06-03 17:06 ` Ray Jui
2021-06-03 17:06   ` Ray Jui
2021-11-29 19:01 ` Wolfram Sang
2021-11-29 19:01   ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.