[PATCH 0/4] i2c-designware: add Intel Lynxpoint support

[PATCH 0/4] i2c-designware: add Intel Lynxpoint support

[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.

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(-)

-- 
1.7.10.4

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

[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@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index f5258c2..94fd818 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -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--;
 		}
 
-- 
1.7.10.4

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

[Felipe Balbi]

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

Hi,

On Thu, Jan 17, 2013 at 12:31:05PM +0200, Mika Westerberg wrote:
> 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@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index f5258c2..94fd818 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -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);

also need to send STP bit if I2C_M_STOP is set.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Mika Westerberg]

On Thu, Jan 17, 2013 at 12:32:48PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Jan 17, 2013 at 12:31:05PM +0200, Mika Westerberg wrote:
> > 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@linux.intel.com>
> > ---
> >  drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> > index f5258c2..94fd818 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.c
> > +++ b/drivers/i2c/busses/i2c-designware-core.c
> > @@ -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);
> 
> 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.

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

[Felipe Balbi]

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

On Thu, Jan 17, 2013 at 12:42:47PM +0200, Mika Westerberg wrote:
> On Thu, Jan 17, 2013 at 12:32:48PM +0200, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Jan 17, 2013 at 12:31:05PM +0200, Mika Westerberg wrote:
> > > 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/CA@public.gmane.org>
> > > ---
> > >  drivers/i2c/busses/i2c-designware-core.c |   16 ++++++++++++++--
> > >  1 file changed, 14 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> > > index f5258c2..94fd818 100644
> > > --- a/drivers/i2c/busses/i2c-designware-core.c
> > > +++ b/drivers/i2c/busses/i2c-designware-core.c
> > > @@ -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);
> > 
> > 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.

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

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[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 <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 343357a..d8afc85 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -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;
-- 
1.7.10.4

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

[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 <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |   49 +++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d8afc85..d2a33e9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -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,
 	},
 };
-- 
1.7.10.4

[PATCH 1/4] i2c-designware: add missing MODULE_LICENSE

[Mika Westerberg]

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 <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index cbba7db..f5258c2 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -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");
-- 
1.7.10.4

Re: [PATCH 0/4] i2c-designware: add Intel Lynxpoint support

[Wolfram Sang]

On Thu, Jan 17, 2013 at 12:31:03PM +0200, Mika Westerberg wrote:
> 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.

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

Re: [PATCH 0/4] i2c-designware: add Intel Lynxpoint support

[Mika Westerberg]

On Thu, Jan 24, 2013 at 08:28:46AM +0100, Wolfram Sang wrote:
> I can't tell much about ACPI usage, but rest looks good to me. Applied
> to next.

Thanks!