Skip to contentSkip to conversation

i2c-designware setup & hold time fix

PATCH In review Kuppuswamy Sathyanarayanan 2 participants 5 messages Apr 7, 2015 – Jun 3, 2015 2 messages not in the archive

Subsystems (3)
I2C subsystem host drivers Intel LPSS (Low Power SubSystem) drivers Synopsys DesignWare I2C driver
lore
Share & tools
lore

Kuppuswamy Sathyanarayanan

author
#0 lore raw

cover i2c-designware setup & hold time fix

This patch fixes hold & setup time failures in dw i2c driver.

Please let me know your review comments.

v1:

Added the fix.

Kuppuswamy Sathyanarayanan (1):
i2c: designware: Fix hold and setup time issue

 drivers/i2c/busses/i2c-designware-core.c    | 37 +++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  2 ++
 drivers/i2c/busses/i2c-designware-platdrv.c |  4 ++++
 3 files changed, 43 insertions(+)
Signature · 1 line
1.9.1

A message that is not in the archive (sent off-list or not yet received): cover.1428374207.git.sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/[email protected]

author
#2 lore raw

PATCH 1/1 i2c: designware: Fix hold and setup time issue

If the device configuration is updated by firmware and the information is not passed to driver via firmware interfaces like ACPI or DT parameters, then that information will be lost once the device either resets or suspends. This patch checks the boot value of hold and setup time during device probe and updates the driver hold and setup time params. This fix will ensure that the updates to hold and setup time values are not lost after device suspend or resets.

Signed-off-by Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/[email protected]>
---
 drivers/i2c/busses/i2c-designware-core.c    | 37 +++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  2 ++
 drivers/i2c/busses/i2c-designware-platdrv.c |  4 ++++
 3 files changed, 43 insertions(+)
drivers/i2c/busses/i2c-designware-core.c +37 -0
index 6e25c01..2598207 100644
@@ -64,6 +64,7 @@
 #define DW_IC_RXFLR		0x78
 #define DW_IC_SDA_HOLD		0x7c
 #define DW_IC_TX_ABRT_SOURCE	0x80
+#define DW_IC_SDA_SETUP		0x94
 #define DW_IC_ENABLE_STATUS	0x9c
 #define DW_IC_COMP_PARAM_1	0xf4
 #define DW_IC_COMP_VERSION	0xf8

@@ -89,6 +90,9 @@
 					 DW_IC_INTR_TX_ABRT | \
 					 DW_IC_INTR_STOP_DET)
 
+#define DW_IC_DEFAULT_SDA_SETUP	0x64
+#define DW_IC_DEFAULT_SDA_HOLD	0x1
+
 #define DW_IC_STATUS_ACTIVITY	0x1
 
 #define DW_IC_ERR_TX_ABRT	0x1

@@ -271,6 +275,35 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
 		 enable ? "en" : "dis");
 }
 
+/*
+ * Update device default param values if its changed by
+ * firmware
+ */
+void dw_i2c_update_defaults(struct dw_i2c_dev *dev)
+{
+	int ret;
+
+	/*
+	 * make sure default is not updated by firmware
+	 * interfaces like OF or ACPI
+	 */
+
+	/* SDA hold time update */
+	if (!dev->sda_hold_time) {
+		ret = dw_readl(dev, DW_IC_SDA_HOLD);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_HOLD))
+			dev->sda_hold_time = ret;
+	}
+
+	/* SDA setup time update */
+	if (!dev->sda_setup_time) {
+		ret = dw_readl(dev, DW_IC_SDA_SETUP);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_SETUP))
+			dev->sda_setup_time = ret;
+	}
+}
+EXPORT_SYMBOL_GPL(dw_i2c_update_defaults);
+
 /**
  * i2c_dw_init() - initialize the designware i2c master hardware
  * @dev: device private data

@@ -368,6 +401,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
 				"Hardware too old to adjust SDA hold time.");
 	}
 
+	/* Configure SDA setup time if required */
+	if (dev->sda_setup_time)
+		dw_writel(dev, dev->sda_setup_time, DW_IC_SDA_SETUP);
+
 	/* Configure Tx/Rx FIFO threshold levels */
 	dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
 	dw_writel(dev, 0, DW_IC_RX_TL);
drivers/i2c/busses/i2c-designware-core.h +2 -0
index 9630222..23ae2e8 100644
@@ -98,6 +98,7 @@ struct dw_i2c_dev {
 	unsigned int		rx_fifo_depth;
 	int			rx_outstanding;
 	u32			sda_hold_time;
+	u32			sda_setup_time;
 	u32			sda_falling_time;
 	u32			scl_falling_time;
 	u16			ss_hcnt;

@@ -131,3 +132,4 @@ extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
 #else
 static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) { return 0; }
 #endif
+extern void dw_i2c_update_defaults(struct dw_i2c_dev *dev);
drivers/i2c/busses/i2c-designware-platdrv.c +4 -0
index c270f5f..d8995d8 100644
@@ -233,6 +233,10 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
 		dev->adapter.nr = pdev->id;
 	}
+
+	/* call after firmware param update */
+	dw_i2c_update_defaults(dev);
+
 	r = i2c_dw_init(dev);
 	if (r)
 		return r;
Signature · 1 line
1.9.1

A message that is not in the archive (sent off-list or not yet received): b3d42396c046e4ab4a14edec84f3484b0f27d192.1428374207.git.sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/[email protected]

author
#4 lore raw

Re: [PATCH v1 1/1] i2c: designware: Fix hold and setup time issue

ping.

Quoted 117 lines · Kuppuswamy Sathyanarayanan

If the device configuration is updated by firmware and the information is not passed to driver via firmware interfaces like ACPI or DT parameters, then that information will be lost once the device either resets or suspends. This patch checks the boot value of hold and setup time during device probe and updates the driver hold and setup time params. This fix will ensure that the updates to hold and setup time values are not lost after device suspend or resets.

Signed-off-by Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/[email protected]>
---
  drivers/i2c/busses/i2c-designware-core.c    | 37 +++++++++++++++++++++++++++++
  drivers/i2c/busses/i2c-designware-core.h    |  2 ++
  drivers/i2c/busses/i2c-designware-platdrv.c |  4 ++++
  3 files changed, 43 insertions(+)
drivers/i2c/busses/i2c-designware-core.c +37 -0
index 6e25c01..2598207 100644
@@ -64,6 +64,7 @@
  #define DW_IC_RXFLR		0x78
  #define DW_IC_SDA_HOLD		0x7c
  #define DW_IC_TX_ABRT_SOURCE	0x80
+#define DW_IC_SDA_SETUP		0x94
  #define DW_IC_ENABLE_STATUS	0x9c
  #define DW_IC_COMP_PARAM_1	0xf4
  #define DW_IC_COMP_VERSION	0xf8

@@ -89,6 +90,9 @@
  					 DW_IC_INTR_TX_ABRT | \
  					 DW_IC_INTR_STOP_DET)
 
+#define DW_IC_DEFAULT_SDA_SETUP	0x64
+#define DW_IC_DEFAULT_SDA_HOLD	0x1
+
  #define DW_IC_STATUS_ACTIVITY	0x1
 
  #define DW_IC_ERR_TX_ABRT	0x1

@@ -271,6 +275,35 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
  		 enable ? "en" : "dis");
  }
 
+/*
+ * Update device default param values if its changed by
+ * firmware
+ */
+void dw_i2c_update_defaults(struct dw_i2c_dev *dev)
+{
+	int ret;
+
+	/*
+	 * make sure default is not updated by firmware
+	 * interfaces like OF or ACPI
+	 */
+
+	/* SDA hold time update */
+	if (!dev->sda_hold_time) {
+		ret = dw_readl(dev, DW_IC_SDA_HOLD);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_HOLD))
+			dev->sda_hold_time = ret;
+	}
+
+	/* SDA setup time update */
+	if (!dev->sda_setup_time) {
+		ret = dw_readl(dev, DW_IC_SDA_SETUP);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_SETUP))
+			dev->sda_setup_time = ret;
+	}
+}
+EXPORT_SYMBOL_GPL(dw_i2c_update_defaults);
+
  /**
   * i2c_dw_init() - initialize the designware i2c master hardware
   * @dev: device private data

@@ -368,6 +401,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
  				"Hardware too old to adjust SDA hold time.");
  	}
 
+	/* Configure SDA setup time if required */
+	if (dev->sda_setup_time)
+		dw_writel(dev, dev->sda_setup_time, DW_IC_SDA_SETUP);
+
  	/* Configure Tx/Rx FIFO threshold levels */
  	dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
  	dw_writel(dev, 0, DW_IC_RX_TL);
drivers/i2c/busses/i2c-designware-core.h +2 -0
index 9630222..23ae2e8 100644
@@ -98,6 +98,7 @@ struct dw_i2c_dev {
  	unsigned int		rx_fifo_depth;
  	int			rx_outstanding;
  	u32			sda_hold_time;
+	u32			sda_setup_time;
  	u32			sda_falling_time;
  	u32			scl_falling_time;
  	u16			ss_hcnt;

@@ -131,3 +132,4 @@ extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
  #else
  static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) { return 0; }
  #endif
+extern void dw_i2c_update_defaults(struct dw_i2c_dev *dev);
drivers/i2c/busses/i2c-designware-platdrv.c +4 -0
index c270f5f..d8995d8 100644
@@ -233,6 +233,10 @@ static int dw_i2c_probe(struct platform_device *pdev)
  		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
  		dev->adapter.nr = pdev->id;
  	}
+
+	/* call after firmware param update */
+	dw_i2c_update_defaults(dev);
+
  	r = i2c_dw_init(dev);
  	if (r)
  		return r;
Signature · 2 lines
Sathyanarayanan Kuppuswamy
Android kernel developer
#5 lore raw

Re: [PATCH v1 1/1] i2c: designware: Fix hold and setup time issue

In reply to a message not in the archive
Quoted 10 lines · Kuppuswamy Sathyanarayanan

If the device configuration is updated by firmware and the information is not passed to driver via firmware interfaces like ACPI or DT parameters, then that information will be lost once the device either resets or suspends. This patch checks the boot value of hold and setup time during device probe and updates the driver hold and setup time params. This fix will ensure that the updates to hold and setup time values are not lost after device suspend or resets.

Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>

Adding Mika to CC.

Quoted 97 lines
---
 drivers/i2c/busses/i2c-designware-core.c    | 37 +++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  2 ++
 drivers/i2c/busses/i2c-designware-platdrv.c |  4 ++++
 3 files changed, 43 insertions(+)
drivers/i2c/busses/i2c-designware-core.c +37 -0
index 6e25c01..2598207 100644
@@ -64,6 +64,7 @@
 #define DW_IC_RXFLR		0x78
 #define DW_IC_SDA_HOLD		0x7c
 #define DW_IC_TX_ABRT_SOURCE	0x80
+#define DW_IC_SDA_SETUP		0x94
 #define DW_IC_ENABLE_STATUS	0x9c
 #define DW_IC_COMP_PARAM_1	0xf4
 #define DW_IC_COMP_VERSION	0xf8

@@ -89,6 +90,9 @@
 					 DW_IC_INTR_TX_ABRT | \
 					 DW_IC_INTR_STOP_DET)
 
+#define DW_IC_DEFAULT_SDA_SETUP	0x64
+#define DW_IC_DEFAULT_SDA_HOLD	0x1
+
 #define DW_IC_STATUS_ACTIVITY	0x1
 
 #define DW_IC_ERR_TX_ABRT	0x1

@@ -271,6 +275,35 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
 		 enable ? "en" : "dis");
 }
 
+/*
+ * Update device default param values if its changed by
+ * firmware
+ */
+void dw_i2c_update_defaults(struct dw_i2c_dev *dev)
+{
+	int ret;
+
+	/*
+	 * make sure default is not updated by firmware
+	 * interfaces like OF or ACPI
+	 */
+
+	/* SDA hold time update */
+	if (!dev->sda_hold_time) {
+		ret = dw_readl(dev, DW_IC_SDA_HOLD);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_HOLD))
+			dev->sda_hold_time = ret;
+	}
+
+	/* SDA setup time update */
+	if (!dev->sda_setup_time) {
+		ret = dw_readl(dev, DW_IC_SDA_SETUP);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_SETUP))
+			dev->sda_setup_time = ret;
+	}
+}
+EXPORT_SYMBOL_GPL(dw_i2c_update_defaults);
+
 /**
  * i2c_dw_init() - initialize the designware i2c master hardware
  * @dev: device private data

@@ -368,6 +401,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
 				"Hardware too old to adjust SDA hold time.");
 	}
 
+	/* Configure SDA setup time if required */
+	if (dev->sda_setup_time)
+		dw_writel(dev, dev->sda_setup_time, DW_IC_SDA_SETUP);
+
 	/* Configure Tx/Rx FIFO threshold levels */
 	dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
 	dw_writel(dev, 0, DW_IC_RX_TL);
drivers/i2c/busses/i2c-designware-core.h +2 -0
index 9630222..23ae2e8 100644
@@ -98,6 +98,7 @@ struct dw_i2c_dev {
 	unsigned int		rx_fifo_depth;
 	int			rx_outstanding;
 	u32			sda_hold_time;
+	u32			sda_setup_time;
 	u32			sda_falling_time;
 	u32			scl_falling_time;
 	u16			ss_hcnt;

@@ -131,3 +132,4 @@ extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
 #else
 static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) { return 0; }
 #endif
+extern void dw_i2c_update_defaults(struct dw_i2c_dev *dev);
drivers/i2c/busses/i2c-designware-platdrv.c +0 -0
index c270f5f..d8995d8 100644

drivers/i2c/busses/i2c-designware-platdrv.c static int dw_i2c_probe(struct platform_device *pdev) · line 238

Quoted 6 lines
drivers/i2c/busses/i2c-designware-platdrv.c +3 -0
@@ -233,6 +233,10 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
 		dev->adapter.nr = pdev->id;
 	}
+
+	/* call after firmware param update */
+	dw_i2c_update_defaults(dev);

What about calling this from the pci driver?

author
#6 lore raw

Re: [PATCH v1 1/1] i2c: designware: Fix hold and setup time issue

Quoted 109 lines · Wolfram Sang
Quoted 10 lines · Kuppuswamy Sathyanarayanan

If the device configuration is updated by firmware and the information is not passed to driver via firmware interfaces like ACPI or DT parameters, then that information will be lost once the device either resets or suspends. This patch checks the boot value of hold and setup time during device probe and updates the driver hold and setup time params. This fix will ensure that the updates to hold and setup time values are not lost after device suspend or resets.

Signed-off-by Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/[email protected]>

Adding Mika to CC.

Quoted 96 lines
---
  drivers/i2c/busses/i2c-designware-core.c    | 37 +++++++++++++++++++++++++++++
  drivers/i2c/busses/i2c-designware-core.h    |  2 ++
  drivers/i2c/busses/i2c-designware-platdrv.c |  4 ++++
  3 files changed, 43 insertions(+)
drivers/i2c/busses/i2c-designware-core.c +37 -0
index 6e25c01..2598207 100644
@@ -64,6 +64,7 @@
  #define DW_IC_RXFLR		0x78
  #define DW_IC_SDA_HOLD		0x7c
  #define DW_IC_TX_ABRT_SOURCE	0x80
+#define DW_IC_SDA_SETUP		0x94
  #define DW_IC_ENABLE_STATUS	0x9c
  #define DW_IC_COMP_PARAM_1	0xf4
  #define DW_IC_COMP_VERSION	0xf8

@@ -89,6 +90,9 @@
  					 DW_IC_INTR_TX_ABRT | \
  					 DW_IC_INTR_STOP_DET)
 
+#define DW_IC_DEFAULT_SDA_SETUP	0x64
+#define DW_IC_DEFAULT_SDA_HOLD	0x1
+
  #define DW_IC_STATUS_ACTIVITY	0x1
 
  #define DW_IC_ERR_TX_ABRT	0x1

@@ -271,6 +275,35 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
  		 enable ? "en" : "dis");
  }
 
+/*
+ * Update device default param values if its changed by
+ * firmware
+ */
+void dw_i2c_update_defaults(struct dw_i2c_dev *dev)
+{
+	int ret;
+
+	/*
+	 * make sure default is not updated by firmware
+	 * interfaces like OF or ACPI
+	 */
+
+	/* SDA hold time update */
+	if (!dev->sda_hold_time) {
+		ret = dw_readl(dev, DW_IC_SDA_HOLD);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_HOLD))
+			dev->sda_hold_time = ret;
+	}
+
+	/* SDA setup time update */
+	if (!dev->sda_setup_time) {
+		ret = dw_readl(dev, DW_IC_SDA_SETUP);
+		if ((ret > 0) && (ret != DW_IC_DEFAULT_SDA_SETUP))
+			dev->sda_setup_time = ret;
+	}
+}
+EXPORT_SYMBOL_GPL(dw_i2c_update_defaults);
+
  /**
   * i2c_dw_init() - initialize the designware i2c master hardware
   * @dev: device private data

@@ -368,6 +401,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
  				"Hardware too old to adjust SDA hold time.");
  	}
 
+	/* Configure SDA setup time if required */
+	if (dev->sda_setup_time)
+		dw_writel(dev, dev->sda_setup_time, DW_IC_SDA_SETUP);
+
  	/* Configure Tx/Rx FIFO threshold levels */
  	dw_writel(dev, dev->tx_fifo_depth / 2, DW_IC_TX_TL);
  	dw_writel(dev, 0, DW_IC_RX_TL);
drivers/i2c/busses/i2c-designware-core.h +2 -0
index 9630222..23ae2e8 100644
@@ -98,6 +98,7 @@ struct dw_i2c_dev {
  	unsigned int		rx_fifo_depth;
  	int			rx_outstanding;
  	u32			sda_hold_time;
+	u32			sda_setup_time;
  	u32			sda_falling_time;
  	u32			scl_falling_time;
  	u16			ss_hcnt;

@@ -131,3 +132,4 @@ extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
  #else
  static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) { return 0; }
  #endif
+extern void dw_i2c_update_defaults(struct dw_i2c_dev *dev);
drivers/i2c/busses/i2c-designware-platdrv.c +0 -0
index c270f5f..d8995d8 100644
Quoted 8 lines

drivers/i2c/busses/i2c-designware-platdrv.c static int dw_i2c_probe(struct platform_device *pdev) · line 238

Quoted 7 lines
drivers/i2c/busses/i2c-designware-platdrv.c +3 -0
@@ -233,6 +233,10 @@ static int dw_i2c_probe(struct platform_device *pdev)
  		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
  		dev->adapter.nr = pdev->id;
  	}
+
+	/* call after firmware param update */
+	dw_i2c_update_defaults(dev);

What about calling this from the pci driver?

Agreed. I will fix it in next version.

Quoted 1 line
Signature · 2 lines
Sathyanarayanan Kuppuswamy
Android kernel developer