i2c-designware: add Intel Lynxpoint support

PATCH In review Mika Westerberg 3 participants 10 messages Jan 17, 2013 – Jan 24, 2013

Subsystems (3)

I2C subsystem host drivers Intel LPSS (Low Power SubSystem) drivers Synopsys DesignWare I2C driver

lore

Share & tools

lore

Mika Westerberg

author maintainer

2013-01-17 10:31 UTC #0 lore raw

cover i2c-designware: add Intel Lynxpoint support

Hi all,

This series adds support for the Intel Lynxpoint Low Power Subsystem I2C controllers. They are compatible with the DesignWare I2C controller.

Patches [1/4] and [2/4] are fixes that are necessary to get the driver working on Lynxpoint.

Patch [3/4] brings minimal runtime PM support for the designware platform driver. This is useful if the PM subsystem/domain implements runtime PM, like in case of ACPI.

Patch [4/4] finally implements ACPI enumeration support for the designware platform driver and adds Lynxpoint ACPI IDs.

Please review.

Mika Westerberg (4):
i2c-designware: add missing MODULE_LICENSE
i2c-designware: always set the STOP bit after last byte
i2c-designware: add minimal support for runtime PM
i2c-designware: add support for Intel Lynxpoint

 drivers/i2c/busses/i2c-designware-core.c    |   20 ++++++++-
 drivers/i2c/busses/i2c-designware-platdrv.c |   59 +++++++++++++++++++++++++--
 2 files changed, 74 insertions(+), 5 deletions(-)

Signature · 1 line

1.7.10.4

Mika Westerberg

author maintainer

2013-01-17 10:30 UTC #1 lore raw

PATCH 1/4 i2c-designware: add missing MODULE_LICENSE

The driver can also be built as a module so add MODULE_LICENSE for it. In addition add MODULE_DESCRIPTION as well.

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/i2c/busses/i2c-designware-core.c |    4 ++++
 1 file changed, 4 insertions(+)

drivers/i2c/busses/i2c-designware-core.c +4 -0

index cbba7db..f5258c2 100644
@@ -34,6 +34,7 @@
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
+#include <linux/module.h>
 #include "i2c-designware-core.h"
 
 /*

@@ -725,3 +726,6 @@ u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
 	return dw_readl(dev, DW_IC_COMP_PARAM_1);
 }
 EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
+
+MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
+MODULE_LICENSE("GPL");

Signature · 1 line

1.7.10.4

Mika Westerberg

author maintainer

2013-01-17 10:30 UTC #2 lore raw

PATCH 2/4 i2c-designware: always set the STOP bit after last byte

In reply to Mika Westerberg

If IC_EMPTYFIFO_HOLD_MASTER_EN is set to one, the DesignWare I2C controller doesn't generate STOP on the bus when the FIFO is empty. This violates the rules of Linux I2C stack as it requires that the STOP is issued once the i2c_transfer() is finished.

However, there is no way to detect this from the hardware registers, so we must make sure that the STOP bit is always set once the last byte of the last message is transferred.

This patch is based on the work of Dirk Brandewie.

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/i2c/busses/i2c-designware-core.c +14 -2

index f5258c2..94fd818 100644
@@ -413,11 +413,23 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
+			u32 cmd = 0;
+
+			/*
+			 * If IC_EMPTYFIFO_HOLD_MASTER_EN is set we must
+			 * manually set the stop bit. However, it cannot be
+			 * detected from the registers so we set it always
+			 * when writing/reading the last byte.
+			 */
+			if (dev->msg_write_idx == dev->msgs_num - 1 &&
+			    buf_len == 1)
+				cmd |= BIT(9);
+
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
-				dw_writel(dev, 0x100, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				dw_writel(dev, *buf++, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);
 			tx_limit--; buf_len--;
 		}

Signature · 1 line

1.7.10.4

Felipe Balbi

2013-01-17 10:32 UTC #3 lore raw

Re: [PATCH 2/4] i2c-designware: always set the STOP bit after last byte

Hi,

Quoted 33 lines · Mika Westerberg

If IC_EMPTYFIFO_HOLD_MASTER_EN is set to one, the DesignWare I2C controller doesn't generate STOP on the bus when the FIFO is empty. This violates the rules of Linux I2C stack as it requires that the STOP is issued once the i2c_transfer() is finished.
However, there is no way to detect this from the hardware registers, so we must make sure that the STOP bit is always set once the last byte of the last message is transferred.
This patch is based on the work of Dirk Brandewie.
Signed-off-by Mika Westerberg <[email protected]>
---
 drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-designware-core.c +9 -0
index f5258c2..94fd818 100644
@@ -413,11 +413,23 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
+			u32 cmd = 0;
+
+			/*
+			 * If IC_EMPTYFIFO_HOLD_MASTER_EN is set we must
+			 * manually set the stop bit. However, it cannot be
+			 * detected from the registers so we set it always
+			 * when writing/reading the last byte.
+			 */
+			if (dev->msg_write_idx == dev->msgs_num - 1 &&

drivers/i2c/busses/i2c-designware-core.c i2c_dw_xfer_msg(struct dw_i2c_dev *dev) · line 432

Quoted 10 lines

drivers/i2c/busses/i2c-designware-core.c +5 -2

+			    buf_len == 1)
+				cmd |= BIT(9);
+
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
-				dw_writel(dev, 0x100, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				dw_writel(dev, *buf++, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);

also need to send STP bit if I2C_M_STOP is set.

Signature · 1 line

balbi

Mika Westerberg

author maintainer

2013-01-17 10:38 UTC #4 lore raw

Re: [PATCH 2/4] i2c-designware: always set the STOP bit after last byte

Quoted 40 lines · Felipe Balbi

Hi,
Quoted 37 lines · Mika Westerberg
If IC_EMPTYFIFO_HOLD_MASTER_EN is set to one, the DesignWare I2C controller doesn't generate STOP on the bus when the FIFO is empty. This violates the rules of Linux I2C stack as it requires that the STOP is issued once the i2c_transfer() is finished.
However, there is no way to detect this from the hardware registers, so we must make sure that the STOP bit is always set once the last byte of the last message is transferred.
This patch is based on the work of Dirk Brandewie.
Signed-off-by Mika Westerberg <[email protected]>
---
 drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-designware-core.c +12 -0
index f5258c2..94fd818 100644
@@ -413,11 +413,23 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
+			u32 cmd = 0;
+
+			/*
+			 * If IC_EMPTYFIFO_HOLD_MASTER_EN is set we must
+			 * manually set the stop bit. However, it cannot be
+			 * detected from the registers so we set it always
+			 * when writing/reading the last byte.
+			 */
+			if (dev->msg_write_idx == dev->msgs_num - 1 &&
+			    buf_len == 1)
+				cmd |= BIT(9);
+
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {

Quoted 8 lines

drivers/i2c/busses/i2c-designware-core.c i2c_dw_xfer_msg(struct dw_i2c_dev *dev) · line 432
Quoted 6 lines
drivers/i2c/busses/i2c-designware-core.c +2 -2
-				dw_writel(dev, 0x100, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				dw_writel(dev, *buf++, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);
also need to send STP bit if I2C_M_STOP is set.

True but only if the driver exposes I2C_FUNC_PROTOCOL_MANGLING, which is not done in this driver.

Felipe Balbi

2013-01-17 10:43 UTC #5 lore raw

Re: [PATCH 2/4] i2c-designware: always set the STOP bit after last byte

Quoted 50 lines · Mika Westerberg

Quoted 40 lines · Felipe Balbi
Hi,
Quoted 37 lines · Mika Westerberg
If IC_EMPTYFIFO_HOLD_MASTER_EN is set to one, the DesignWare I2C controller doesn't generate STOP on the bus when the FIFO is empty. This violates the rules of Linux I2C stack as it requires that the STOP is issued once the i2c_transfer() is finished.
However, there is no way to detect this from the hardware registers, so we must make sure that the STOP bit is always set once the last byte of the last message is transferred.
This patch is based on the work of Dirk Brandewie.
Signed-off-by Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/[email protected]>
---
 drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-designware-core.c +12 -0
index f5258c2..94fd818 100644
@@ -413,11 +413,23 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
+			u32 cmd = 0;
+
+			/*
+			 * If IC_EMPTYFIFO_HOLD_MASTER_EN is set we must
+			 * manually set the stop bit. However, it cannot be
+			 * detected from the registers so we set it always
+			 * when writing/reading the last byte.
+			 */
+			if (dev->msg_write_idx == dev->msgs_num - 1 &&
+			    buf_len == 1)
+				cmd |= BIT(9);
+
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
Quoted 8 lines
drivers/i2c/busses/i2c-designware-core.c i2c_dw_xfer_msg(struct dw_i2c_dev *dev) · line 432
Quoted 6 lines
drivers/i2c/busses/i2c-designware-core.c +2 -2
-				dw_writel(dev, 0x100, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				dw_writel(dev, *buf++, DW_IC_DATA_CMD);
+				dw_writel(dev, cmd | *buf++, DW_IC_DATA_CMD);
also need to send STP bit if I2C_M_STOP is set.

Quoted 2 lines

True but only if the driver exposes I2C_FUNC_PROTOCOL_MANGLING, which is not done in this driver.

right, didn't check the driver, my bad.

Signature · 1 line

balbi

Mika Westerberg

author maintainer

2013-01-17 10:31 UTC #6 lore raw

PATCH 3/4 i2c-designware: add minimal support for runtime PM

In reply to Mika Westerberg

In order to save power the device should be put to low power states whenever it is not being used. We implement this by enabling minimal runtime PM support.

There isn't much to do for the device itself as it is disabled once the last transfer is completed but subsystem/domain runtime PM hooks can save more power by power gating the device etc.

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/i2c/busses/i2c-designware-platdrv.c +10 -0

index 343357a..d8afc85 100644
@@ -37,6 +37,7 @@
 #include <linux/of_i2c.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/io.h>
 #include <linux/slab.h>
 #include "i2c-designware-core.h"

@@ -149,6 +150,10 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	}
 	of_i2c_register_devices(adap);
 
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_put(&pdev->dev);
+
 	return 0;
 
 err_free_irq:

@@ -175,6 +180,8 @@ static int dw_i2c_remove(struct platform_device *pdev)
 	struct resource *mem;
 
 	platform_set_drvdata(pdev, NULL);
+	pm_runtime_get_sync(&pdev->dev);
+
 	i2c_del_adapter(&dev->adapter);
 	put_device(&pdev->dev);

@@ -186,6 +193,9 @@ static int dw_i2c_remove(struct platform_device *pdev)
 	free_irq(dev->irq, dev);
 	kfree(dev);
 
+	pm_runtime_put(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 	return 0;

Signature · 1 line

1.7.10.4

Mika Westerberg

author maintainer

2013-01-17 10:30 UTC #7 lore raw

PATCH 4/4 i2c-designware: add support for Intel Lynxpoint

In reply to Mika Westerberg

Intel Lynxpoint has two I2C controllers. These controllers are enumerated from ACPI namespace with IDs INT33C2 and INT33C3. Add support for these to the I2C DesignWare platform driver.

This is based on the work of Dirk Brandewie.

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/i2c/busses/i2c-designware-platdrv.c |   49 +++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

drivers/i2c/busses/i2c-designware-platdrv.c +46 -3

index d8afc85..d2a33e9 100644
@@ -40,6 +40,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
 #include "i2c-designware-core.h"
 
 static struct i2c_algorithm i2c_dw_algo = {

@@ -51,6 +52,42 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 	return clk_get_rate(dev->clk)/1000;
 }
 
+#ifdef CONFIG_ACPI
+static int dw_i2c_acpi_configure(struct platform_device *pdev)
+{
+	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+	struct acpi_device *adev;
+	int busno, ret;
+
+	if (!ACPI_HANDLE(&pdev->dev))
+		return -ENODEV;
+
+	ret = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
+	if (ret)
+		return -ENODEV;
+
+	dev->adapter.nr = -1;
+	if (adev->pnp.unique_id && !kstrtoint(adev->pnp.unique_id, 0, &busno))
+		dev->adapter.nr = busno;
+
+	dev->tx_fifo_depth = 32;
+	dev->rx_fifo_depth = 32;
+	return 0;
+}
+
+static const struct acpi_device_id dw_i2c_acpi_match[] = {
+	{ "INT33C2", 0 },
+	{ "INT33C3", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
+#else
+static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
+{
+	return -ENODEV;
+}
+#endif
+
 static int dw_i2c_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev;

@@ -115,18 +152,22 @@ static int dw_i2c_probe(struct platform_device *pdev)
 		r = -EBUSY;
 		goto err_unuse_clocks;
 	}
-	{
+
+	/* Try first if we can configure the device from ACPI */
+	r = dw_i2c_acpi_configure(pdev);
+	if (r) {
 		u32 param1 = i2c_dw_read_comp_param(dev);
 
 		dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
 		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;
+		dev->adapter.nr = pdev->id;
 	}
 	r = i2c_dw_init(dev);
 	if (r)
 		goto err_iounmap;
 
 	i2c_dw_disable_int(dev);
-	r = request_irq(dev->irq, i2c_dw_isr, IRQF_DISABLED, pdev->name, dev);
+	r = request_irq(dev->irq, i2c_dw_isr, IRQF_SHARED, pdev->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
 		goto err_iounmap;

@@ -141,14 +182,15 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	adap->algo = &i2c_dw_algo;
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
+	ACPI_HANDLE_SET(&adap->dev, ACPI_HANDLE(&pdev->dev));
 
-	adap->nr = pdev->id;
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
 		dev_err(&pdev->dev, "failure adding adapter\n");
 		goto err_free_irq;
 	}
 	of_i2c_register_devices(adap);
+	acpi_i2c_register_devices(adap);
 
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);

@@ -243,6 +285,7 @@ static struct platform_driver dw_i2c_driver = {
 		.name	= "i2c_designware",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(dw_i2c_of_match),
+		.acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
 		.pm	= &dw_i2c_dev_pm_ops,
 	},
 };

Signature · 1 line

1.7.10.4

Wolfram Sang

2013-01-24 07:28 UTC #8 lore raw

In reply to Mika Westerberg

Quoted 8 lines · Mika Westerberg

Hi all,
This series adds support for the Intel Lynxpoint Low Power Subsystem I2C controllers. They are compatible with the DesignWare I2C controller.
Patches [1/4] and [2/4] are fixes that are necessary to get the driver working on Lynxpoint.

Quoted 6 lines

Patch [3/4] brings minimal runtime PM support for the designware platform driver. This is useful if the PM subsystem/domain implements runtime PM, like in case of ACPI.
Patch [4/4] finally implements ACPI enumeration support for the designware platform driver and adds Lynxpoint ACPI IDs.

I can't tell much about ACPI usage, but rest looks good to me. Applied to next.

Mika Westerberg

author maintainer

2013-01-24 09:14 UTC #9 lore raw

Quoted 2 lines · Wolfram Sang

I can't tell much about ACPI usage, but rest looks good to me. Applied to next.

Thanks!