[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

This patch configures the I2C bus timing registers according
to information passed via DT. Currently, 100kHz and 400kHz
modes are supported.

The TIMING2 register value is wrong in the documentation for
i.MX28! This was found and fixed by:
  Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |   56 +++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index 1bfc02d..d2bf750 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Should be "fsl,<chip>-i2c"
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
+- clock-frequency: desired I2C bus clock frequency in Hz.
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index ee3778a..832d30a 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -419,6 +419,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
+				clock-frequency = <400000>;
 				status = "disabled";
 			};
 
@@ -428,6 +429,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
+				clock-frequency = <400000>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 04eb441..cdcfd3f 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -46,6 +46,10 @@
 #define MXS_I2C_CTRL0_DIRECTION			0x00010000
 #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
 
+#define MXS_I2C_TIMING0		(0x10)
+#define MXS_I2C_TIMING1		(0x20)
+#define MXS_I2C_TIMING2		(0x30)
+
 #define MXS_I2C_CTRL1		(0x40)
 #define MXS_I2C_CTRL1_SET	(0x44)
 #define MXS_I2C_CTRL1_CLR	(0x48)
@@ -97,6 +101,25 @@
 #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
 				 MXS_I2C_CTRL0_MASTER_MODE)
 
+struct mxs_i2c_speed_config {
+	uint32_t	timing0;
+	uint32_t	timing1;
+	uint32_t	timing2;
+};
+
+/* Timing values for the default 24MHz clock supplied into the i2c block. */
+const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
+	.timing0	= 0x00780030,
+	.timing1	= 0x00800030,
+	.timing2	= 0x00300030,
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
+	.timing0	= 0x000f0007,
+	.timing1	= 0x001f000f,
+	.timing2	= 0x00300030,
+};
+
 /**
  * struct mxs_i2c_dev - per device, private MXS-I2C data
  *
@@ -112,11 +135,17 @@ struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+	const struct mxs_i2c_speed_config *speed;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
 	stmp_reset_block(i2c->regs);
+
+	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
+	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
+	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
+
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
@@ -319,6 +348,28 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
+{
+	uint32_t speed;
+	struct device *dev = i2c->dev;
+	struct device_node *node = dev->of_node;
+	int ret;
+
+	if (!node)
+		return -EINVAL;
+
+	i2c->speed = &mxs_i2c_95kHz_config;
+	ret = of_property_read_u32(node, "clock-frequency", &speed);
+	if (ret)
+		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
+	else if (speed == 400000)
+		i2c->speed = &mxs_i2c_400kHz_config;
+	else if (speed != 100000)
+		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
+
+	return 0;
+}
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -358,6 +409,11 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 		return err;
 
 	i2c->dev = dev;
+
+	err = mxs_i2c_get_ofdata(i2c);
+	if (err)
+		return err;
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */
-- 
1.7.10

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

This patch configures the I2C bus timing registers according
to information passed via DT. Currently, 100kHz and 400kHz
modes are supported.

The TIMING2 register value is wrong in the documentation for
i.MX28! This was found and fixed by:
  Shawn Guo <shawn.guo@linaro.org>

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
CC: Dong Aisheng <b29396@freescale.com>
CC: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
Cc: linux-i2c at vger.kernel.org
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |   56 +++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index 1bfc02d..d2bf750 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Should be "fsl,<chip>-i2c"
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
+- clock-frequency: desired I2C bus clock frequency in Hz.
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index ee3778a..832d30a 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -419,6 +419,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
+				clock-frequency = <400000>;
 				status = "disabled";
 			};
 
@@ -428,6 +429,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
+				clock-frequency = <400000>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 04eb441..cdcfd3f 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -46,6 +46,10 @@
 #define MXS_I2C_CTRL0_DIRECTION			0x00010000
 #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
 
+#define MXS_I2C_TIMING0		(0x10)
+#define MXS_I2C_TIMING1		(0x20)
+#define MXS_I2C_TIMING2		(0x30)
+
 #define MXS_I2C_CTRL1		(0x40)
 #define MXS_I2C_CTRL1_SET	(0x44)
 #define MXS_I2C_CTRL1_CLR	(0x48)
@@ -97,6 +101,25 @@
 #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
 				 MXS_I2C_CTRL0_MASTER_MODE)
 
+struct mxs_i2c_speed_config {
+	uint32_t	timing0;
+	uint32_t	timing1;
+	uint32_t	timing2;
+};
+
+/* Timing values for the default 24MHz clock supplied into the i2c block. */
+const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
+	.timing0	= 0x00780030,
+	.timing1	= 0x00800030,
+	.timing2	= 0x00300030,
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
+	.timing0	= 0x000f0007,
+	.timing1	= 0x001f000f,
+	.timing2	= 0x00300030,
+};
+
 /**
  * struct mxs_i2c_dev - per device, private MXS-I2C data
  *
@@ -112,11 +135,17 @@ struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+	const struct mxs_i2c_speed_config *speed;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
 	stmp_reset_block(i2c->regs);
+
+	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
+	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
+	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
+
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
@@ -319,6 +348,28 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
+{
+	uint32_t speed;
+	struct device *dev = i2c->dev;
+	struct device_node *node = dev->of_node;
+	int ret;
+
+	if (!node)
+		return -EINVAL;
+
+	i2c->speed = &mxs_i2c_95kHz_config;
+	ret = of_property_read_u32(node, "clock-frequency", &speed);
+	if (ret)
+		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
+	else if (speed == 400000)
+		i2c->speed = &mxs_i2c_400kHz_config;
+	else if (speed != 100000)
+		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
+
+	return 0;
+}
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -358,6 +409,11 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 		return err;
 
 	i2c->dev = dev;
+
+	err = mxs_i2c_get_ofdata(i2c);
+	if (err)
+		return err;
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */
-- 
1.7.10

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Marek Vasut]

This patch implements DMA support into mxs-i2c. DMA transfers are now enabled
via DT. The DMA operation is enabled by default.

Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |  267 +++++++++++++++++++--
 3 files changed, 251 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index d2bf750..9497ee0 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -5,6 +5,10 @@ Required properties:
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
 - clock-frequency: desired I2C bus clock frequency in Hz.
+- fsl,i2c-dma-channel: APBX DMA channel for the I2C
+
+Optional properties:
+- fsl,use-pio: Use PIO transfers instead of DMA, useful for debug
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 832d30a..e4a4842 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -420,6 +420,7 @@
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
 				clock-frequency = <400000>;
+				fsl,i2c-dma-channel = <6>;
 				status = "disabled";
 			};
 
@@ -430,6 +431,7 @@
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
 				clock-frequency = <400000>;
+				fsl,i2c-dma-channel = <7>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index cdcfd3f..b8efc8b 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -7,8 +7,6 @@
  *
  * Copyright (C) 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
  *
- * TODO: add dma-support if platform-support for it is available
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -31,6 +29,9 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_i2c.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/fsl/mxs-dma.h>
 
 #define DRIVER_NAME "mxs-i2c"
 
@@ -136,6 +137,16 @@ struct mxs_i2c_dev {
 	u32 cmd_err;
 	struct i2c_adapter adapter;
 	const struct mxs_i2c_speed_config *speed;
+
+	/* DMA support components */
+	bool				dma_mode;
+	int				dma_channel;
+	struct dma_chan         	*dmach;
+	struct mxs_dma_data		dma_data;
+	uint32_t			pio_data[2];
+	uint32_t			addr_data;
+	struct scatterlist		sg_io[2];
+	bool				dma_read;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
@@ -147,7 +158,11 @@ static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
 
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
-	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+	if (i2c->dma_mode)
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
+	else
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
 }
 
@@ -238,6 +253,151 @@ static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 	return 0;
 }
 
+static void mxs_i2c_dma_finish(struct mxs_i2c_dev *i2c)
+{
+	if (i2c->dma_read) {
+		dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+	} else {
+		dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+	}
+}
+
+static void mxs_i2c_dma_irq_callback(void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	complete(&i2c->cmd_complete);
+	mxs_i2c_dma_finish(i2c);
+}
+
+static int mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap,
+			struct i2c_msg *msg, uint32_t flags)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
+
+	if (msg->flags & I2C_M_RD) {
+		i2c->dma_read = 1;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_READ;
+
+		/*
+		 * SELECT command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = MXS_CMD_I2C_SELECT;
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1);
+		dma_map_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[0], 1,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/*
+		 * READ command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[1] = flags | MXS_CMD_I2C_READ |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[1],
+					1, DMA_TRANS_NONE, DMA_PREP_INTERRUPT);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[1], 1,
+					DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto read_init_dma_fail;
+		}
+	} else {
+		i2c->dma_read = 0;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_WRITE;
+
+		/*
+		 * WRITE command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = flags | MXS_CMD_I2C_WRITE |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto write_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_table(i2c->sg_io, 2);
+		sg_set_buf(&i2c->sg_io[0], &i2c->addr_data, 1);
+		sg_set_buf(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, i2c->sg_io, 2,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto write_init_dma_fail;
+		}
+	}
+
+	/*
+	 * The last descriptor must have this callback,
+	 * to finish the DMA transaction.
+	 */
+	desc->callback = mxs_i2c_dma_irq_callback;
+	desc->callback_param = i2c;
+
+	/* Start the transfer. */
+	dmaengine_submit(desc);
+	dma_async_issue_pending(i2c->dmach);
+	return 0;
+
+/* Read failpath. */
+read_init_dma_fail:
+	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+select_init_dma_fail:
+	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+select_init_pio_fail:
+	return 1;
+
+
+/* Write failpath. */
+write_init_dma_fail:
+	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+write_init_pio_fail:
+	return 1;
+}
+
 /*
  * Low level master read/write transaction.
  */
@@ -248,6 +408,8 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	int ret;
 	int flags;
 
+	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
+
 	dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
 		msg->addr, msg->len, msg->flags, stop);
 
@@ -257,23 +419,29 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	init_completion(&i2c->cmd_complete);
 	i2c->cmd_err = 0;
 
-	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
-
-	if (msg->flags & I2C_M_RD)
-		mxs_i2c_pioq_setup_read(i2c, msg->addr, msg->len, flags);
-	else
-		mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, msg->len,
-					flags);
+	if (i2c->dma_mode) {
+		ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
+		if (ret)
+			return -EINVAL;
+	} else {
+		if (msg->flags & I2C_M_RD) {
+			mxs_i2c_pioq_setup_read(i2c, msg->addr,
+						msg->len, flags);
+		} else {
+			mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf,
+						msg->len, flags);
+		}
 
-	writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
+	}
 
 	ret = wait_for_completion_timeout(&i2c->cmd_complete,
 						msecs_to_jiffies(1000));
 	if (ret == 0)
 		goto timeout;
 
-	if ((!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
+	if (!i2c->dma_mode && !i2c->cmd_err && (msg->flags & I2C_M_RD)) {
 		ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len);
 		if (ret)
 			goto timeout;
@@ -291,6 +459,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 
 timeout:
 	dev_dbg(i2c->dev, "Timeout!\n");
+	mxs_i2c_dma_finish(i2c);
 	mxs_i2c_reset(i2c);
 	return -ETIMEDOUT;
 }
@@ -332,11 +501,13 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id)
 		/* MXS_I2C_CTRL1_OVERSIZE_XFER_TERM_IRQ is only for slaves */
 		i2c->cmd_err = -EIO;
 
-	is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
-		MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
+	if (!i2c->dma_mode) {
+		is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
+			MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
 
-	if (is_last_cmd || i2c->cmd_err)
-		complete(&i2c->cmd_complete);
+		if (is_last_cmd || i2c->cmd_err)
+			complete(&i2c->cmd_complete);
+	}
 
 	writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR);
 
@@ -348,6 +519,21 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static bool mxs_i2c_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	if (!mxs_dma_is_apbx(chan))
+		return false;
+
+	if (chan->chan_id != i2c->dma_channel)
+		return false;
+
+	chan->private = &i2c->dma_data;
+
+	return true;
+}
+
 static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
 {
 	uint32_t speed;
@@ -358,6 +544,28 @@ static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
 	if (!node)
 		return -EINVAL;
 
+	/*
+	 * The MXS I2C DMA mode is prefered and enabled by default.
+	 * The PIO mode is still supported, but should be used only
+	 * for debuging purposes etc.
+	 */
+	i2c->dma_mode = 1;
+	if (of_find_property(node, "fsl,use-pio", NULL)) {
+		i2c->dma_mode = 0;
+		dev_info(dev, "Using PIO mode for I2C transfers!\n");
+	}
+
+	/*
+	 * TODO: This is a temporary solution and should be changed
+	 * to use generic DMA binding later when the helpers get in.
+	 */
+	ret = of_property_read_u32(node, "fsl,i2c-dma-channel",
+				   &i2c->dma_channel);
+	if (ret) {
+		dev_warn(dev, "Failed to get DMA channel!\n");
+		i2c->dma_mode = 0;
+	}
+
 	i2c->speed = &mxs_i2c_95kHz_config;
 	ret = of_property_read_u32(node, "clock-frequency", &speed);
 	if (ret)
@@ -378,7 +586,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	struct pinctrl *pinctrl;
 	struct resource *res;
 	resource_size_t res_size;
-	int err, irq;
+	int err, irq, dmairq;
+	dma_cap_mask_t mask;
 
 	pinctrl = devm_pinctrl_get_select_default(dev);
 	if (IS_ERR(pinctrl))
@@ -389,7 +598,10 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
+	irq = platform_get_irq(pdev, 0);
+	dmairq = platform_get_irq(pdev, 1);
+
+	if (!res || irq < 0 || dmairq < 0)
 		return -ENOENT;
 
 	res_size = resource_size(res);
@@ -400,10 +612,6 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (!i2c->regs)
 		return -EBUSY;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c);
 	if (err)
 		return err;
@@ -414,6 +622,18 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/* Setup the DMA */
+	if (i2c->dma_mode) {
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+		i2c->dma_data.chan_irq = dmairq;
+		i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c);
+		if (!i2c->dmach) {
+			dev_err(dev, "Failed to request dma\n");
+			return -ENODEV;
+		}
+	}
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */
@@ -449,6 +669,9 @@ static int __devexit mxs_i2c_remove(struct platform_device *pdev)
 	if (ret)
 		return -EBUSY;
 
+	if (i2c->dmach)
+		dma_release_channel(i2c->dmach);
+
 	writel(MXS_I2C_CTRL0_SFTRST, i2c->regs + MXS_I2C_CTRL0_SET);
 
 	platform_set_drvdata(pdev, NULL);
-- 
1.7.10

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Marek Vasut]

This patch implements DMA support into mxs-i2c. DMA transfers are now enabled
via DT. The DMA operation is enabled by default.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
CC: Dong Aisheng <b29396@freescale.com>
CC: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
Cc: linux-i2c at vger.kernel.org
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |  267 +++++++++++++++++++--
 3 files changed, 251 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index d2bf750..9497ee0 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -5,6 +5,10 @@ Required properties:
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
 - clock-frequency: desired I2C bus clock frequency in Hz.
+- fsl,i2c-dma-channel: APBX DMA channel for the I2C
+
+Optional properties:
+- fsl,use-pio: Use PIO transfers instead of DMA, useful for debug
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 832d30a..e4a4842 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -420,6 +420,7 @@
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
 				clock-frequency = <400000>;
+				fsl,i2c-dma-channel = <6>;
 				status = "disabled";
 			};
 
@@ -430,6 +431,7 @@
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
 				clock-frequency = <400000>;
+				fsl,i2c-dma-channel = <7>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index cdcfd3f..b8efc8b 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -7,8 +7,6 @@
  *
  * Copyright (C) 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
  *
- * TODO: add dma-support if platform-support for it is available
- *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -31,6 +29,9 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_i2c.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/fsl/mxs-dma.h>
 
 #define DRIVER_NAME "mxs-i2c"
 
@@ -136,6 +137,16 @@ struct mxs_i2c_dev {
 	u32 cmd_err;
 	struct i2c_adapter adapter;
 	const struct mxs_i2c_speed_config *speed;
+
+	/* DMA support components */
+	bool				dma_mode;
+	int				dma_channel;
+	struct dma_chan         	*dmach;
+	struct mxs_dma_data		dma_data;
+	uint32_t			pio_data[2];
+	uint32_t			addr_data;
+	struct scatterlist		sg_io[2];
+	bool				dma_read;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
@@ -147,7 +158,11 @@ static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
 
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
-	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+	if (i2c->dma_mode)
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
+			i2c->regs + MXS_I2C_QUEUECTRL_CLR);
+	else
+		writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
 }
 
@@ -238,6 +253,151 @@ static int mxs_i2c_finish_read(struct mxs_i2c_dev *i2c, u8 *buf, int len)
 	return 0;
 }
 
+static void mxs_i2c_dma_finish(struct mxs_i2c_dev *i2c)
+{
+	if (i2c->dma_read) {
+		dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+	} else {
+		dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+	}
+}
+
+static void mxs_i2c_dma_irq_callback(void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	complete(&i2c->cmd_complete);
+	mxs_i2c_dma_finish(i2c);
+}
+
+static int mxs_i2c_dma_setup_xfer(struct i2c_adapter *adap,
+			struct i2c_msg *msg, uint32_t flags)
+{
+	struct dma_async_tx_descriptor *desc;
+	struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
+
+	if (msg->flags & I2C_M_RD) {
+		i2c->dma_read = 1;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_READ;
+
+		/*
+		 * SELECT command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = MXS_CMD_I2C_SELECT;
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[0], &i2c->addr_data, 1);
+		dma_map_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[0], 1,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/*
+		 * READ command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[1] = flags | MXS_CMD_I2C_READ |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[1],
+					1, DMA_TRANS_NONE, DMA_PREP_INTERRUPT);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto select_init_dma_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_one(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, &i2c->sg_io[1], 1,
+					DMA_DEV_TO_MEM,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto read_init_dma_fail;
+		}
+	} else {
+		i2c->dma_read = 0;
+		i2c->addr_data = (msg->addr << 1) | I2C_SMBUS_WRITE;
+
+		/*
+		 * WRITE command.
+		 */
+
+		/* Queue the PIO register write transfer. */
+		i2c->pio_data[0] = flags | MXS_CMD_I2C_WRITE |
+				MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1);
+		desc = dmaengine_prep_slave_sg(i2c->dmach,
+					(struct scatterlist *)&i2c->pio_data[0],
+					1, DMA_TRANS_NONE, 0);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get PIO reg. write descriptor.\n");
+			goto write_init_pio_fail;
+		}
+
+		/* Queue the DMA data transfer. */
+		sg_init_table(i2c->sg_io, 2);
+		sg_set_buf(&i2c->sg_io[0], &i2c->addr_data, 1);
+		sg_set_buf(&i2c->sg_io[1], msg->buf, msg->len);
+		dma_map_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+		desc = dmaengine_prep_slave_sg(i2c->dmach, i2c->sg_io, 2,
+					DMA_MEM_TO_DEV,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+		if (!desc) {
+			dev_err(i2c->dev,
+				"Failed to get DMA data write descriptor.\n");
+			goto write_init_dma_fail;
+		}
+	}
+
+	/*
+	 * The last descriptor must have this callback,
+	 * to finish the DMA transaction.
+	 */
+	desc->callback = mxs_i2c_dma_irq_callback;
+	desc->callback_param = i2c;
+
+	/* Start the transfer. */
+	dmaengine_submit(desc);
+	dma_async_issue_pending(i2c->dmach);
+	return 0;
+
+/* Read failpath. */
+read_init_dma_fail:
+	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
+select_init_dma_fail:
+	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
+select_init_pio_fail:
+	return 1;
+
+
+/* Write failpath. */
+write_init_dma_fail:
+	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
+write_init_pio_fail:
+	return 1;
+}
+
 /*
  * Low level master read/write transaction.
  */
@@ -248,6 +408,8 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	int ret;
 	int flags;
 
+	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
+
 	dev_dbg(i2c->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
 		msg->addr, msg->len, msg->flags, stop);
 
@@ -257,23 +419,29 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	init_completion(&i2c->cmd_complete);
 	i2c->cmd_err = 0;
 
-	flags = stop ? MXS_I2C_CTRL0_POST_SEND_STOP : 0;
-
-	if (msg->flags & I2C_M_RD)
-		mxs_i2c_pioq_setup_read(i2c, msg->addr, msg->len, flags);
-	else
-		mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf, msg->len,
-					flags);
+	if (i2c->dma_mode) {
+		ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
+		if (ret)
+			return -EINVAL;
+	} else {
+		if (msg->flags & I2C_M_RD) {
+			mxs_i2c_pioq_setup_read(i2c, msg->addr,
+						msg->len, flags);
+		} else {
+			mxs_i2c_pioq_setup_write(i2c, msg->addr, msg->buf,
+						msg->len, flags);
+		}
 
-	writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
+		writel(MXS_I2C_QUEUECTRL_QUEUE_RUN,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
+	}
 
 	ret = wait_for_completion_timeout(&i2c->cmd_complete,
 						msecs_to_jiffies(1000));
 	if (ret == 0)
 		goto timeout;
 
-	if ((!i2c->cmd_err) && (msg->flags & I2C_M_RD)) {
+	if (!i2c->dma_mode && !i2c->cmd_err && (msg->flags & I2C_M_RD)) {
 		ret = mxs_i2c_finish_read(i2c, msg->buf, msg->len);
 		if (ret)
 			goto timeout;
@@ -291,6 +459,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 
 timeout:
 	dev_dbg(i2c->dev, "Timeout!\n");
+	mxs_i2c_dma_finish(i2c);
 	mxs_i2c_reset(i2c);
 	return -ETIMEDOUT;
 }
@@ -332,11 +501,13 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id)
 		/* MXS_I2C_CTRL1_OVERSIZE_XFER_TERM_IRQ is only for slaves */
 		i2c->cmd_err = -EIO;
 
-	is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
-		MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
+	if (!i2c->dma_mode) {
+		is_last_cmd = (readl(i2c->regs + MXS_I2C_QUEUESTAT) &
+			MXS_I2C_QUEUESTAT_WRITE_QUEUE_CNT_MASK) == 0;
 
-	if (is_last_cmd || i2c->cmd_err)
-		complete(&i2c->cmd_complete);
+		if (is_last_cmd || i2c->cmd_err)
+			complete(&i2c->cmd_complete);
+	}
 
 	writel(stat, i2c->regs + MXS_I2C_CTRL1_CLR);
 
@@ -348,6 +519,21 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+static bool mxs_i2c_dma_filter(struct dma_chan *chan, void *param)
+{
+	struct mxs_i2c_dev *i2c = param;
+
+	if (!mxs_dma_is_apbx(chan))
+		return false;
+
+	if (chan->chan_id != i2c->dma_channel)
+		return false;
+
+	chan->private = &i2c->dma_data;
+
+	return true;
+}
+
 static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
 {
 	uint32_t speed;
@@ -358,6 +544,28 @@ static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
 	if (!node)
 		return -EINVAL;
 
+	/*
+	 * The MXS I2C DMA mode is prefered and enabled by default.
+	 * The PIO mode is still supported, but should be used only
+	 * for debuging purposes etc.
+	 */
+	i2c->dma_mode = 1;
+	if (of_find_property(node, "fsl,use-pio", NULL)) {
+		i2c->dma_mode = 0;
+		dev_info(dev, "Using PIO mode for I2C transfers!\n");
+	}
+
+	/*
+	 * TODO: This is a temporary solution and should be changed
+	 * to use generic DMA binding later when the helpers get in.
+	 */
+	ret = of_property_read_u32(node, "fsl,i2c-dma-channel",
+				   &i2c->dma_channel);
+	if (ret) {
+		dev_warn(dev, "Failed to get DMA channel!\n");
+		i2c->dma_mode = 0;
+	}
+
 	i2c->speed = &mxs_i2c_95kHz_config;
 	ret = of_property_read_u32(node, "clock-frequency", &speed);
 	if (ret)
@@ -378,7 +586,8 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	struct pinctrl *pinctrl;
 	struct resource *res;
 	resource_size_t res_size;
-	int err, irq;
+	int err, irq, dmairq;
+	dma_cap_mask_t mask;
 
 	pinctrl = devm_pinctrl_get_select_default(dev);
 	if (IS_ERR(pinctrl))
@@ -389,7 +598,10 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
+	irq = platform_get_irq(pdev, 0);
+	dmairq = platform_get_irq(pdev, 1);
+
+	if (!res || irq < 0 || dmairq < 0)
 		return -ENOENT;
 
 	res_size = resource_size(res);
@@ -400,10 +612,6 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (!i2c->regs)
 		return -EBUSY;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
-
 	err = devm_request_irq(dev, irq, mxs_i2c_isr, 0, dev_name(dev), i2c);
 	if (err)
 		return err;
@@ -414,6 +622,18 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/* Setup the DMA */
+	if (i2c->dma_mode) {
+		dma_cap_zero(mask);
+		dma_cap_set(DMA_SLAVE, mask);
+		i2c->dma_data.chan_irq = dmairq;
+		i2c->dmach = dma_request_channel(mask, mxs_i2c_dma_filter, i2c);
+		if (!i2c->dmach) {
+			dev_err(dev, "Failed to request dma\n");
+			return -ENODEV;
+		}
+	}
+
 	platform_set_drvdata(pdev, i2c);
 
 	/* Do reset to enforce correct startup after pinmuxing */
@@ -449,6 +669,9 @@ static int __devexit mxs_i2c_remove(struct platform_device *pdev)
 	if (ret)
 		return -EBUSY;
 
+	if (i2c->dmach)
+		dma_release_channel(i2c->dmach);
+
 	writel(MXS_I2C_CTRL0_SFTRST, i2c->regs + MXS_I2C_CTRL0_SET);
 
 	platform_set_drvdata(pdev, NULL);
-- 
1.7.10

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Dong Aisheng]

Hi Marek,

On Fri, Jun 29, 2012 at 04:24:02PM +0800, Marek Vasut wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
> 
> The TIMING2 register value is wrong in the documentation for
> i.MX28! This was found and fixed by:
>   Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |   56 +++++++++++++++++++++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..d2bf750 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Should be "fsl,<chip>-i2c"
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
> +- clock-frequency: desired I2C bus clock frequency in Hz.
Is this required properties?
If yes, it may be good to also update examples.

>  
>  Examples:
>  
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index ee3778a..832d30a 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -419,6 +419,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x80058000 2000>;
>  				interrupts = <111 68>;
> +				clock-frequency = <400000>;
>  				status = "disabled";
>  			};
>  
> @@ -428,6 +429,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x8005a000 2000>;
>  				interrupts = <110 69>;
> +				clock-frequency = <400000>;
>  				status = "disabled";
>  			};
>  
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 04eb441..cdcfd3f 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -46,6 +46,10 @@
>  #define MXS_I2C_CTRL0_DIRECTION			0x00010000
>  #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
>  
> +#define MXS_I2C_TIMING0		(0x10)
> +#define MXS_I2C_TIMING1		(0x20)
> +#define MXS_I2C_TIMING2		(0x30)
> +
>  #define MXS_I2C_CTRL1		(0x40)
>  #define MXS_I2C_CTRL1_SET	(0x44)
>  #define MXS_I2C_CTRL1_CLR	(0x48)
> @@ -97,6 +101,25 @@
>  #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
>  				 MXS_I2C_CTRL0_MASTER_MODE)
>  
> +struct mxs_i2c_speed_config {
> +	uint32_t	timing0;
> +	uint32_t	timing1;
> +	uint32_t	timing2;
> +};
> +
> +/* Timing values for the default 24MHz clock supplied into the i2c block. */
I did not check spec,
is it possible the supplied clock changed?

> +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
Do we need static for it?

> +	.timing0	= 0x00780030,
> +	.timing1	= 0x00800030,
> +	.timing2	= 0x00300030,
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
ditto

> +	.timing0	= 0x000f0007,
> +	.timing1	= 0x001f000f,
> +	.timing2	= 0x00300030,
> +};
> +
>  /**
>   * struct mxs_i2c_dev - per device, private MXS-I2C data
>   *
> @@ -112,11 +135,17 @@ struct mxs_i2c_dev {
>  	struct completion cmd_complete;
>  	u32 cmd_err;
>  	struct i2c_adapter adapter;
> +	const struct mxs_i2c_speed_config *speed;
>  };
>  
>  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
>  {
>  	stmp_reset_block(i2c->regs);
> +
> +	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
> +	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
> +	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
> +
>  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
>  	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
>  			i2c->regs + MXS_I2C_QUEUECTRL_SET);
> @@ -319,6 +348,28 @@ static const struct i2c_algorithm mxs_i2c_algo = {
>  	.functionality = mxs_i2c_func,
>  };
>  
> +static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
> +{
> +	uint32_t speed;
> +	struct device *dev = i2c->dev;
> +	struct device_node *node = dev->of_node;
> +	int ret;
> +
> +	if (!node)
> +		return -EINVAL;
> +
> +	i2c->speed = &mxs_i2c_95kHz_config;
Can you explain why here is 95khz rather than 100kh?
It seems this is for 100khz below.

> +	ret = of_property_read_u32(node, "clock-frequency", &speed);
> +	if (ret)
> +		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
> +	else if (speed == 400000)
> +		i2c->speed = &mxs_i2c_400kHz_config;
> +	else if (speed != 100000)
> +		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
> +
> +	return 0;
> +}
> +
>  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -358,6 +409,11 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  		return err;
>  
>  	i2c->dev = dev;
> +
> +	err = mxs_i2c_get_ofdata(i2c);
> +	if (err)
> +		return err;
> +
>  	platform_set_drvdata(pdev, i2c);
>  
>  	/* Do reset to enforce correct startup after pinmuxing */

Regards
Dong Aisheng

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Dong Aisheng]

Hi Marek,

On Fri, Jun 29, 2012 at 04:24:02PM +0800, Marek Vasut wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
> 
> The TIMING2 register value is wrong in the documentation for
> i.MX28! This was found and fixed by:
>   Shawn Guo <shawn.guo@linaro.org>
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> CC: Dong Aisheng <b29396@freescale.com>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> Cc: linux-i2c at vger.kernel.org
> CC: Sascha Hauer <s.hauer@pengutronix.de>
> CC: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |   56 +++++++++++++++++++++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..d2bf750 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Should be "fsl,<chip>-i2c"
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
> +- clock-frequency: desired I2C bus clock frequency in Hz.
Is this required properties?
If yes, it may be good to also update examples.

>  
>  Examples:
>  
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index ee3778a..832d30a 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -419,6 +419,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x80058000 2000>;
>  				interrupts = <111 68>;
> +				clock-frequency = <400000>;
>  				status = "disabled";
>  			};
>  
> @@ -428,6 +429,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x8005a000 2000>;
>  				interrupts = <110 69>;
> +				clock-frequency = <400000>;
>  				status = "disabled";
>  			};
>  
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 04eb441..cdcfd3f 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -46,6 +46,10 @@
>  #define MXS_I2C_CTRL0_DIRECTION			0x00010000
>  #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
>  
> +#define MXS_I2C_TIMING0		(0x10)
> +#define MXS_I2C_TIMING1		(0x20)
> +#define MXS_I2C_TIMING2		(0x30)
> +
>  #define MXS_I2C_CTRL1		(0x40)
>  #define MXS_I2C_CTRL1_SET	(0x44)
>  #define MXS_I2C_CTRL1_CLR	(0x48)
> @@ -97,6 +101,25 @@
>  #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
>  				 MXS_I2C_CTRL0_MASTER_MODE)
>  
> +struct mxs_i2c_speed_config {
> +	uint32_t	timing0;
> +	uint32_t	timing1;
> +	uint32_t	timing2;
> +};
> +
> +/* Timing values for the default 24MHz clock supplied into the i2c block. */
I did not check spec,
is it possible the supplied clock changed?

> +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
Do we need static for it?

> +	.timing0	= 0x00780030,
> +	.timing1	= 0x00800030,
> +	.timing2	= 0x00300030,
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
ditto

> +	.timing0	= 0x000f0007,
> +	.timing1	= 0x001f000f,
> +	.timing2	= 0x00300030,
> +};
> +
>  /**
>   * struct mxs_i2c_dev - per device, private MXS-I2C data
>   *
> @@ -112,11 +135,17 @@ struct mxs_i2c_dev {
>  	struct completion cmd_complete;
>  	u32 cmd_err;
>  	struct i2c_adapter adapter;
> +	const struct mxs_i2c_speed_config *speed;
>  };
>  
>  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
>  {
>  	stmp_reset_block(i2c->regs);
> +
> +	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
> +	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
> +	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
> +
>  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
>  	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
>  			i2c->regs + MXS_I2C_QUEUECTRL_SET);
> @@ -319,6 +348,28 @@ static const struct i2c_algorithm mxs_i2c_algo = {
>  	.functionality = mxs_i2c_func,
>  };
>  
> +static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
> +{
> +	uint32_t speed;
> +	struct device *dev = i2c->dev;
> +	struct device_node *node = dev->of_node;
> +	int ret;
> +
> +	if (!node)
> +		return -EINVAL;
> +
> +	i2c->speed = &mxs_i2c_95kHz_config;
Can you explain why here is 95khz rather than 100kh?
It seems this is for 100khz below.

> +	ret = of_property_read_u32(node, "clock-frequency", &speed);
> +	if (ret)
> +		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
> +	else if (speed == 400000)
> +		i2c->speed = &mxs_i2c_400kHz_config;
> +	else if (speed != 100000)
> +		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
> +
> +	return 0;
> +}
> +
>  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -358,6 +409,11 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  		return err;
>  
>  	i2c->dev = dev;
> +
> +	err = mxs_i2c_get_ofdata(i2c);
> +	if (err)
> +		return err;
> +
>  	platform_set_drvdata(pdev, i2c);
>  
>  	/* Do reset to enforce correct startup after pinmuxing */

Regards
Dong Aisheng

Re: [PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> This patch implements DMA support into mxs-i2c. DMA transfers are now enabled
> via DT. The DMA operation is enabled by default.
> 
> Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |  267 +++++++++++++++++++--
>  3 files changed, 251 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index d2bf750..9497ee0 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -5,6 +5,10 @@ Required properties:
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
>  - clock-frequency: desired I2C bus clock frequency in Hz.
> +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
Shoundn't this be optional?

> +	/*
> +	 * The last descriptor must have this callback,
> +	 * to finish the DMA transaction.
> +	 */
> +	desc->callback = mxs_i2c_dma_irq_callback;
> +	desc->callback_param = i2c;
> +
> +	/* Start the transfer. */
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(i2c->dmach);
> +	return 0;
> +
> +/* Read failpath. */
> +read_init_dma_fail:
> +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> +select_init_dma_fail:
> +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> +select_init_pio_fail:
> +	return 1;
look strange why return 1;
> +
> +
One more unnecessary line?

> +/* Write failpath. */
> +write_init_dma_fail:
> +	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
> +write_init_pio_fail:
> +	return 1;
> +}
> +
..
> @@ -291,6 +459,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
>  
>  timeout:
>  	dev_dbg(i2c->dev, "Timeout!\n");
> +	mxs_i2c_dma_finish(i2c);
Shared with pio?

>  	mxs_i2c_reset(i2c);
>  	return -ETIMEDOUT;
>  }

Regards
Dong Aisheng

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> This patch implements DMA support into mxs-i2c. DMA transfers are now enabled
> via DT. The DMA operation is enabled by default.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> CC: Dong Aisheng <b29396@freescale.com>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> Cc: linux-i2c at vger.kernel.org
> CC: Sascha Hauer <s.hauer@pengutronix.de>
> CC: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |  267 +++++++++++++++++++--
>  3 files changed, 251 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index d2bf750..9497ee0 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -5,6 +5,10 @@ Required properties:
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
>  - clock-frequency: desired I2C bus clock frequency in Hz.
> +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
Shoundn't this be optional?

> +	/*
> +	 * The last descriptor must have this callback,
> +	 * to finish the DMA transaction.
> +	 */
> +	desc->callback = mxs_i2c_dma_irq_callback;
> +	desc->callback_param = i2c;
> +
> +	/* Start the transfer. */
> +	dmaengine_submit(desc);
> +	dma_async_issue_pending(i2c->dmach);
> +	return 0;
> +
> +/* Read failpath. */
> +read_init_dma_fail:
> +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> +select_init_dma_fail:
> +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> +select_init_pio_fail:
> +	return 1;
look strange why return 1;
> +
> +
One more unnecessary line?

> +/* Write failpath. */
> +write_init_dma_fail:
> +	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
> +write_init_pio_fail:
> +	return 1;
> +}
> +
..
> @@ -291,6 +459,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
>  
>  timeout:
>  	dev_dbg(i2c->dev, "Timeout!\n");
> +	mxs_i2c_dma_finish(i2c);
Shared with pio?

>  	mxs_i2c_reset(i2c);
>  	return -ETIMEDOUT;
>  }

Regards
Dong Aisheng

Re: [PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > enabled via DT. The DMA operation is enabled by default.
> > 
> > Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> > Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> > CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> > CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> > Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > 
> >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >  drivers/i2c/busses/i2c-mxs.c                      |  267
> >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > d2bf750..9497ee0 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > 
> > @@ -5,6 +5,10 @@ Required properties:
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain ERROR and DMA interrupts
> >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > 
> > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> 
> Shoundn't this be optional?

DMA channel? No, why?

> 
> > +	/*
> > +	 * The last descriptor must have this callback,
> > +	 * to finish the DMA transaction.
> > +	 */
> > +	desc->callback = mxs_i2c_dma_irq_callback;
> > +	desc->callback_param = i2c;
> > +
> > +	/* Start the transfer. */
> > +	dmaengine_submit(desc);
> > +	dma_async_issue_pending(i2c->dmach);
> > +	return 0;
> > +
> > +/* Read failpath. */
> > +read_init_dma_fail:
> > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > +select_init_dma_fail:
> > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > +select_init_pio_fail:
> > +	return 1;
> 
> look strange why return 1;

Because it failed. -Esomething might be better ?

> > +
> > +
> 
> One more unnecessary line?
> 
> > +/* Write failpath. */
> > +write_init_dma_fail:
> > +	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
> > +write_init_pio_fail:
> > +	return 1;
> > +}
> > +
> 
> ..
> 
> > @@ -291,6 +459,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap,
> > struct i2c_msg *msg,
> > 
> >  timeout:
> >  	dev_dbg(i2c->dev, "Timeout!\n");
> > 
> > +	mxs_i2c_dma_finish(i2c);
> 
> Shared with pio?

Doesn't dma_unmap_sg() call check if the buffer is there or not? Therefore it 
shouldn't have any effect on PIO ops. But it can be indeed if-ed.

> >  	mxs_i2c_reset(i2c);
> >  	return -ETIMEDOUT;
> >  
> >  }
> 
> Regards
> Dong Aisheng

Best regards,
Marek Vasut

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > enabled via DT. The DMA operation is enabled by default.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Detlev Zundel <dzu@denx.de>
> > CC: Dong Aisheng <b29396@freescale.com>
> > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> > Cc: linux-i2c at vger.kernel.org
> > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > CC: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Stefano Babic <sbabic@denx.de>
> > CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > ---
> > 
> >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >  drivers/i2c/busses/i2c-mxs.c                      |  267
> >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > d2bf750..9497ee0 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > 
> > @@ -5,6 +5,10 @@ Required properties:
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain ERROR and DMA interrupts
> >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > 
> > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> 
> Shoundn't this be optional?

DMA channel? No, why?

> 
> > +	/*
> > +	 * The last descriptor must have this callback,
> > +	 * to finish the DMA transaction.
> > +	 */
> > +	desc->callback = mxs_i2c_dma_irq_callback;
> > +	desc->callback_param = i2c;
> > +
> > +	/* Start the transfer. */
> > +	dmaengine_submit(desc);
> > +	dma_async_issue_pending(i2c->dmach);
> > +	return 0;
> > +
> > +/* Read failpath. */
> > +read_init_dma_fail:
> > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > +select_init_dma_fail:
> > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > +select_init_pio_fail:
> > +	return 1;
> 
> look strange why return 1;

Because it failed. -Esomething might be better ?

> > +
> > +
> 
> One more unnecessary line?
> 
> > +/* Write failpath. */
> > +write_init_dma_fail:
> > +	dma_unmap_sg(i2c->dev, i2c->sg_io, 2, DMA_TO_DEVICE);
> > +write_init_pio_fail:
> > +	return 1;
> > +}
> > +
> 
> ..
> 
> > @@ -291,6 +459,7 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap,
> > struct i2c_msg *msg,
> > 
> >  timeout:
> >  	dev_dbg(i2c->dev, "Timeout!\n");
> > 
> > +	mxs_i2c_dma_finish(i2c);
> 
> Shared with pio?

Doesn't dma_unmap_sg() call check if the buffer is there or not? Therefore it 
shouldn't have any effect on PIO ops. But it can be indeed if-ed.

> >  	mxs_i2c_reset(i2c);
> >  	return -ETIMEDOUT;
> >  
> >  }
> 
> Regards
> Dong Aisheng

Best regards,
Marek Vasut

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> Hi Marek,
> 
> On Fri, Jun 29, 2012 at 04:24:02PM +0800, Marek Vasut wrote:
> > This patch configures the I2C bus timing registers according
> > to information passed via DT. Currently, 100kHz and 400kHz
> > modes are supported.
> > 
> > The TIMING2 register value is wrong in the documentation for
> > 
> > i.MX28! This was found and fixed by:
> >   Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > 
> > Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> > Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> > CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> > CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> > Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > 
> >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
> >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >  drivers/i2c/busses/i2c-mxs.c                      |   56
> >  +++++++++++++++++++++ 3 files changed, 59 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > 1bfc02d..d2bf750 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > 
> > @@ -4,6 +4,7 @@ Required properties:
> >  - compatible: Should be "fsl,<chip>-i2c"
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain ERROR and DMA interrupts
> > 
> > +- clock-frequency: desired I2C bus clock frequency in Hz.
> 
> Is this required properties?
> If yes, it may be good to also update examples.

Good point

> 
> >  Examples:
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index ee3778a..832d30a 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -419,6 +419,7 @@
> > 
> >  				compatible = "fsl,imx28-i2c";
> >  				reg = <0x80058000 2000>;
> >  				interrupts = <111 68>;
> > 
> > +				clock-frequency = <400000>;
> > 
> >  				status = "disabled";
> >  			
> >  			};
> > 
> > @@ -428,6 +429,7 @@
> > 
> >  				compatible = "fsl,imx28-i2c";
> >  				reg = <0x8005a000 2000>;
> >  				interrupts = <110 69>;
> > 
> > +				clock-frequency = <400000>;
> > 
> >  				status = "disabled";
> >  			
> >  			};
> > 
> > diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> > index 04eb441..cdcfd3f 100644
> > --- a/drivers/i2c/busses/i2c-mxs.c
> > +++ b/drivers/i2c/busses/i2c-mxs.c
> > @@ -46,6 +46,10 @@
> > 
> >  #define MXS_I2C_CTRL0_DIRECTION			0x00010000
> >  #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
> > 
> > +#define MXS_I2C_TIMING0		(0x10)
> > +#define MXS_I2C_TIMING1		(0x20)
> > +#define MXS_I2C_TIMING2		(0x30)
> > +
> > 
> >  #define MXS_I2C_CTRL1		(0x40)
> >  #define MXS_I2C_CTRL1_SET	(0x44)
> >  #define MXS_I2C_CTRL1_CLR	(0x48)
> > 
> > @@ -97,6 +101,25 @@
> > 
> >  #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
> >  
> >  				 MXS_I2C_CTRL0_MASTER_MODE)
> > 
> > +struct mxs_i2c_speed_config {
> > +	uint32_t	timing0;
> > +	uint32_t	timing1;
> > +	uint32_t	timing2;
> > +};
> > +
> > +/* Timing values for the default 24MHz clock supplied into the i2c
> > block. */
> 
> I did not check spec,
> is it possible the supplied clock changed?

It's possible indeed ... Shawn?

> > +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
> 
> Do we need static for it?

MX28 datasheet 27.5.2 - 27.5.4 ... there was a discussion about these in the 
thread under older patches. Those shall explain your questions below.

> > +	.timing0	= 0x00780030,
> > +	.timing1	= 0x00800030,
> > +	.timing2	= 0x00300030,
> > +};
> > +
> > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> 
> ditto
> 
> > +	.timing0	= 0x000f0007,
> > +	.timing1	= 0x001f000f,
> > +	.timing2	= 0x00300030,
> > +};
> > +
> > 
> >  /**
> >  
> >   * struct mxs_i2c_dev - per device, private MXS-I2C data
> >   *
> > 
> > @@ -112,11 +135,17 @@ struct mxs_i2c_dev {
> > 
> >  	struct completion cmd_complete;
> >  	u32 cmd_err;
> >  	struct i2c_adapter adapter;
> > 
> > +	const struct mxs_i2c_speed_config *speed;
> > 
> >  };
> >  
> >  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
> >  {
> >  
> >  	stmp_reset_block(i2c->regs);
> > 
> > +
> > +	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
> > +	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
> > +	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
> > +
> > 
> >  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
> >  	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
> >  	
> >  			i2c->regs + MXS_I2C_QUEUECTRL_SET);
> > 
> > @@ -319,6 +348,28 @@ static const struct i2c_algorithm mxs_i2c_algo = {
> > 
> >  	.functionality = mxs_i2c_func,
> >  
> >  };
> > 
> > +static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
> > +{
> > +	uint32_t speed;
> > +	struct device *dev = i2c->dev;
> > +	struct device_node *node = dev->of_node;
> > +	int ret;
> > +
> > +	if (!node)
> > +		return -EINVAL;
> > +
> > +	i2c->speed = &mxs_i2c_95kHz_config;
> 
> Can you explain why here is 95khz rather than 100kh?
> It seems this is for 100khz below.
> 
> > +	ret = of_property_read_u32(node, "clock-frequency", &speed);
> > +	if (ret)
> > +		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
> > +	else if (speed == 400000)
> > +		i2c->speed = &mxs_i2c_400kHz_config;
> > +	else if (speed != 100000)
> > +		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
> > +
> > +	return 0;
> > +}
> > +
> > 
> >  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct device *dev = &pdev->dev;
> > 
> > @@ -358,6 +409,11 @@ static int __devinit mxs_i2c_probe(struct
> > platform_device *pdev)
> > 
> >  		return err;
> >  	
> >  	i2c->dev = dev;
> > 
> > +
> > +	err = mxs_i2c_get_ofdata(i2c);
> > +	if (err)
> > +		return err;
> > +
> > 
> >  	platform_set_drvdata(pdev, i2c);
> >  	
> >  	/* Do reset to enforce correct startup after pinmuxing */
> 
> Regards
> Dong Aisheng

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> Hi Marek,
> 
> On Fri, Jun 29, 2012 at 04:24:02PM +0800, Marek Vasut wrote:
> > This patch configures the I2C bus timing registers according
> > to information passed via DT. Currently, 100kHz and 400kHz
> > modes are supported.
> > 
> > The TIMING2 register value is wrong in the documentation for
> > 
> > i.MX28! This was found and fixed by:
> >   Shawn Guo <shawn.guo@linaro.org>
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Detlev Zundel <dzu@denx.de>
> > CC: Dong Aisheng <b29396@freescale.com>
> > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> > Cc: linux-i2c at vger.kernel.org
> > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > CC: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Stefano Babic <sbabic@denx.de>
> > CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > ---
> > 
> >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
> >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >  drivers/i2c/busses/i2c-mxs.c                      |   56
> >  +++++++++++++++++++++ 3 files changed, 59 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > 1bfc02d..d2bf750 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > 
> > @@ -4,6 +4,7 @@ Required properties:
> >  - compatible: Should be "fsl,<chip>-i2c"
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain ERROR and DMA interrupts
> > 
> > +- clock-frequency: desired I2C bus clock frequency in Hz.
> 
> Is this required properties?
> If yes, it may be good to also update examples.

Good point

> 
> >  Examples:
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index ee3778a..832d30a 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -419,6 +419,7 @@
> > 
> >  				compatible = "fsl,imx28-i2c";
> >  				reg = <0x80058000 2000>;
> >  				interrupts = <111 68>;
> > 
> > +				clock-frequency = <400000>;
> > 
> >  				status = "disabled";
> >  			
> >  			};
> > 
> > @@ -428,6 +429,7 @@
> > 
> >  				compatible = "fsl,imx28-i2c";
> >  				reg = <0x8005a000 2000>;
> >  				interrupts = <110 69>;
> > 
> > +				clock-frequency = <400000>;
> > 
> >  				status = "disabled";
> >  			
> >  			};
> > 
> > diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> > index 04eb441..cdcfd3f 100644
> > --- a/drivers/i2c/busses/i2c-mxs.c
> > +++ b/drivers/i2c/busses/i2c-mxs.c
> > @@ -46,6 +46,10 @@
> > 
> >  #define MXS_I2C_CTRL0_DIRECTION			0x00010000
> >  #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
> > 
> > +#define MXS_I2C_TIMING0		(0x10)
> > +#define MXS_I2C_TIMING1		(0x20)
> > +#define MXS_I2C_TIMING2		(0x30)
> > +
> > 
> >  #define MXS_I2C_CTRL1		(0x40)
> >  #define MXS_I2C_CTRL1_SET	(0x44)
> >  #define MXS_I2C_CTRL1_CLR	(0x48)
> > 
> > @@ -97,6 +101,25 @@
> > 
> >  #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
> >  
> >  				 MXS_I2C_CTRL0_MASTER_MODE)
> > 
> > +struct mxs_i2c_speed_config {
> > +	uint32_t	timing0;
> > +	uint32_t	timing1;
> > +	uint32_t	timing2;
> > +};
> > +
> > +/* Timing values for the default 24MHz clock supplied into the i2c
> > block. */
> 
> I did not check spec,
> is it possible the supplied clock changed?

It's possible indeed ... Shawn?

> > +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
> 
> Do we need static for it?

MX28 datasheet 27.5.2 - 27.5.4 ... there was a discussion about these in the 
thread under older patches. Those shall explain your questions below.

> > +	.timing0	= 0x00780030,
> > +	.timing1	= 0x00800030,
> > +	.timing2	= 0x00300030,
> > +};
> > +
> > +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> 
> ditto
> 
> > +	.timing0	= 0x000f0007,
> > +	.timing1	= 0x001f000f,
> > +	.timing2	= 0x00300030,
> > +};
> > +
> > 
> >  /**
> >  
> >   * struct mxs_i2c_dev - per device, private MXS-I2C data
> >   *
> > 
> > @@ -112,11 +135,17 @@ struct mxs_i2c_dev {
> > 
> >  	struct completion cmd_complete;
> >  	u32 cmd_err;
> >  	struct i2c_adapter adapter;
> > 
> > +	const struct mxs_i2c_speed_config *speed;
> > 
> >  };
> >  
> >  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
> >  {
> >  
> >  	stmp_reset_block(i2c->regs);
> > 
> > +
> > +	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
> > +	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
> > +	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
> > +
> > 
> >  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
> >  	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
> >  	
> >  			i2c->regs + MXS_I2C_QUEUECTRL_SET);
> > 
> > @@ -319,6 +348,28 @@ static const struct i2c_algorithm mxs_i2c_algo = {
> > 
> >  	.functionality = mxs_i2c_func,
> >  
> >  };
> > 
> > +static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
> > +{
> > +	uint32_t speed;
> > +	struct device *dev = i2c->dev;
> > +	struct device_node *node = dev->of_node;
> > +	int ret;
> > +
> > +	if (!node)
> > +		return -EINVAL;
> > +
> > +	i2c->speed = &mxs_i2c_95kHz_config;
> 
> Can you explain why here is 95khz rather than 100kh?
> It seems this is for 100khz below.
> 
> > +	ret = of_property_read_u32(node, "clock-frequency", &speed);
> > +	if (ret)
> > +		dev_warn(dev, "No I2C speed selected, using 100kHz\n");
> > +	else if (speed == 400000)
> > +		i2c->speed = &mxs_i2c_400kHz_config;
> > +	else if (speed != 100000)
> > +		dev_warn(dev, "Invalid I2C speed selected, using 100kHz\n");
> > +
> > +	return 0;
> > +}
> > +
> > 
> >  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct device *dev = &pdev->dev;
> > 
> > @@ -358,6 +409,11 @@ static int __devinit mxs_i2c_probe(struct
> > platform_device *pdev)
> > 
> >  		return err;
> >  	
> >  	i2c->dev = dev;
> > 
> > +
> > +	err = mxs_i2c_get_ofdata(i2c);
> > +	if (err)
> > +		return err;
> > +
> > 
> >  	platform_set_drvdata(pdev, i2c);
> >  	
> >  	/* Do reset to enforce correct startup after pinmuxing */
> 
> Regards
> Dong Aisheng

Re: [PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 05:30:16PM +0800, Marek Vasut wrote:
> Dear Dong Aisheng,
> 
> > On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > > enabled via DT. The DMA operation is enabled by default.
> > > 
> > > Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> > > Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> > > CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> > > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> > > CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> > > Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> > >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> > >  drivers/i2c/busses/i2c-mxs.c                      |  267
> > >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> > >  deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > > d2bf750..9497ee0 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > 
> > > @@ -5,6 +5,10 @@ Required properties:
> > >  - reg: Should contain registers location and length
> > >  - interrupts: Should contain ERROR and DMA interrupts
> > >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > > 
> > > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> > 
> > Shoundn't this be optional?
> 
> DMA channel? No, why?
> 
User can use pio mode, so actually it's optional.
Is it reasonable?

> > 
> > > +	/*
> > > +	 * The last descriptor must have this callback,
> > > +	 * to finish the DMA transaction.
> > > +	 */
> > > +	desc->callback = mxs_i2c_dma_irq_callback;
> > > +	desc->callback_param = i2c;
> > > +
> > > +	/* Start the transfer. */
> > > +	dmaengine_submit(desc);
> > > +	dma_async_issue_pending(i2c->dmach);
> > > +	return 0;
> > > +
> > > +/* Read failpath. */
> > > +read_init_dma_fail:
> > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > > +select_init_dma_fail:
> > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > > +select_init_pio_fail:
> > > +	return 1;
> > 
> > look strange why return 1;
> 
> Because it failed. -Esomething might be better ?
> 
i think yes.

Regards
Dong Aisheng

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 05:30:16PM +0800, Marek Vasut wrote:
> Dear Dong Aisheng,
> 
> > On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > > enabled via DT. The DMA operation is enabled by default.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Detlev Zundel <dzu@denx.de>
> > > CC: Dong Aisheng <b29396@freescale.com>
> > > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > > Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> > > Cc: linux-i2c at vger.kernel.org
> > > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > > CC: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Stefano Babic <sbabic@denx.de>
> > > CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > Cc: Wolfgang Denk <wd@denx.de>
> > > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> > >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> > >  drivers/i2c/busses/i2c-mxs.c                      |  267
> > >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> > >  deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > > d2bf750..9497ee0 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > 
> > > @@ -5,6 +5,10 @@ Required properties:
> > >  - reg: Should contain registers location and length
> > >  - interrupts: Should contain ERROR and DMA interrupts
> > >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > > 
> > > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> > 
> > Shoundn't this be optional?
> 
> DMA channel? No, why?
> 
User can use pio mode, so actually it's optional.
Is it reasonable?

> > 
> > > +	/*
> > > +	 * The last descriptor must have this callback,
> > > +	 * to finish the DMA transaction.
> > > +	 */
> > > +	desc->callback = mxs_i2c_dma_irq_callback;
> > > +	desc->callback_param = i2c;
> > > +
> > > +	/* Start the transfer. */
> > > +	dmaengine_submit(desc);
> > > +	dma_async_issue_pending(i2c->dmach);
> > > +	return 0;
> > > +
> > > +/* Read failpath. */
> > > +read_init_dma_fail:
> > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > > +select_init_dma_fail:
> > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > > +select_init_pio_fail:
> > > +	return 1;
> > 
> > look strange why return 1;
> 
> Because it failed. -Esomething might be better ?
> 
i think yes.

Regards
Dong Aisheng

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 05:33:09PM +0800, Marek Vasut wrote:
> MX28 datasheet 27.5.2 - 27.5.4 ... there was a discussion about these in the 
> thread under older patches. Those shall explain your questions below.
> 
Thanks for the info.

Regards
Dong Aisheng

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 05:33:09PM +0800, Marek Vasut wrote:
> MX28 datasheet 27.5.2 - 27.5.4 ... there was a discussion about these in the 
> thread under older patches. Those shall explain your questions below.
> 
Thanks for the info.

Regards
Dong Aisheng

Re: [PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> On Fri, Jun 29, 2012 at 05:30:16PM +0800, Marek Vasut wrote:
> > Dear Dong Aisheng,
> > 
> > > On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > > > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > > > enabled via DT. The DMA operation is enabled by default.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> > > > Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> > > > CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > > Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> > > > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > > Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> > > > CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> > > > Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > ---
> > > > 
> > > >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> > > >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> > > >  drivers/i2c/busses/i2c-mxs.c                      |  267
> > > >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> > > >  deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > > > d2bf750..9497ee0 100644
> > > > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > 
> > > > @@ -5,6 +5,10 @@ Required properties:
> > > >  - reg: Should contain registers location and length
> > > >  - interrupts: Should contain ERROR and DMA interrupts
> > > >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > > > 
> > > > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> > > 
> > > Shoundn't this be optional?
> > 
> > DMA channel? No, why?
> 
> User can use pio mode, so actually it's optional.
> Is it reasonable?

PIO mode is something that should not be used unless it's to be used for 
debugging purposes. The ultimate goal then is to use PIO for small transfers and 
DMA for large transfers. But that's not implemented yet and DMA is default.

> > > > +	/*
> > > > +	 * The last descriptor must have this callback,
> > > > +	 * to finish the DMA transaction.
> > > > +	 */
> > > > +	desc->callback = mxs_i2c_dma_irq_callback;
> > > > +	desc->callback_param = i2c;
> > > > +
> > > > +	/* Start the transfer. */
> > > > +	dmaengine_submit(desc);
> > > > +	dma_async_issue_pending(i2c->dmach);
> > > > +	return 0;
> > > > +
> > > > +/* Read failpath. */
> > > > +read_init_dma_fail:
> > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > > > +select_init_dma_fail:
> > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > > > +select_init_pio_fail:
> > > > +	return 1;
> > > 
> > > look strange why return 1;
> > 
> > Because it failed. -Esomething might be better ?
> 
> i think yes.

And -Ewhat do you suggest ? :)

> Regards
> Dong Aisheng

Best regards,
Marek Vasut

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> On Fri, Jun 29, 2012 at 05:30:16PM +0800, Marek Vasut wrote:
> > Dear Dong Aisheng,
> > 
> > > On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > > > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > > > enabled via DT. The DMA operation is enabled by default.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > Cc: Detlev Zundel <dzu@denx.de>
> > > > CC: Dong Aisheng <b29396@freescale.com>
> > > > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > > > Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> > > > Cc: linux-i2c at vger.kernel.org
> > > > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > > > CC: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Stefano Babic <sbabic@denx.de>
> > > > CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > > Cc: Wolfgang Denk <wd@denx.de>
> > > > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > > > ---
> > > > 
> > > >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> > > >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> > > >  drivers/i2c/busses/i2c-mxs.c                      |  267
> > > >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> > > >  deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > > > d2bf750..9497ee0 100644
> > > > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > 
> > > > @@ -5,6 +5,10 @@ Required properties:
> > > >  - reg: Should contain registers location and length
> > > >  - interrupts: Should contain ERROR and DMA interrupts
> > > >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > > > 
> > > > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> > > 
> > > Shoundn't this be optional?
> > 
> > DMA channel? No, why?
> 
> User can use pio mode, so actually it's optional.
> Is it reasonable?

PIO mode is something that should not be used unless it's to be used for 
debugging purposes. The ultimate goal then is to use PIO for small transfers and 
DMA for large transfers. But that's not implemented yet and DMA is default.

> > > > +	/*
> > > > +	 * The last descriptor must have this callback,
> > > > +	 * to finish the DMA transaction.
> > > > +	 */
> > > > +	desc->callback = mxs_i2c_dma_irq_callback;
> > > > +	desc->callback_param = i2c;
> > > > +
> > > > +	/* Start the transfer. */
> > > > +	dmaengine_submit(desc);
> > > > +	dma_async_issue_pending(i2c->dmach);
> > > > +	return 0;
> > > > +
> > > > +/* Read failpath. */
> > > > +read_init_dma_fail:
> > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > > > +select_init_dma_fail:
> > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > > > +select_init_pio_fail:
> > > > +	return 1;
> > > 
> > > look strange why return 1;
> > 
> > Because it failed. -Esomething might be better ?
> 
> i think yes.

And -Ewhat do you suggest ? :)

> Regards
> Dong Aisheng

Best regards,
Marek Vasut

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> On Fri, Jun 29, 2012 at 05:33:09PM +0800, Marek Vasut wrote:
> > MX28 datasheet 27.5.2 - 27.5.4 ... there was a discussion about these in
> > the thread under older patches. Those shall explain your questions
> > below.
> 
> Thanks for the info.

You're welcome. Hope it helps ;-)

> Regards
> Dong Aisheng

Best regards,
Marek Vasut

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

Dear Dong Aisheng,

> On Fri, Jun 29, 2012 at 05:33:09PM +0800, Marek Vasut wrote:
> > MX28 datasheet 27.5.2 - 27.5.4 ... there was a discussion about these in
> > the thread under older patches. Those shall explain your questions
> > below.
> 
> Thanks for the info.

You're welcome. Hope it helps ;-)

> Regards
> Dong Aisheng

Best regards,
Marek Vasut

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Shawn Guo]

On Fri, Jun 29, 2012 at 11:33:09AM +0200, Marek Vasut wrote:
> > I did not check spec,
> > is it possible the supplied clock changed?
> 
> It's possible indeed ... Shawn?
> 
It's technically possible, but probably not practically.  The imx28 RM
does not mention i2c clock at all, and I was told by designer that i2c
always runs at the same frequency as apbx, which sources from ref_xtal
(always 24 MHz).  Though there is a divider for apbx clock, it seems
every single running system sets the divider as 1 all the time.  That
said, what I have seen is the i2c clock is always 24 MHz.

-- 
Regards,
Shawn

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Shawn Guo]

On Fri, Jun 29, 2012 at 11:33:09AM +0200, Marek Vasut wrote:
> > I did not check spec,
> > is it possible the supplied clock changed?
> 
> It's possible indeed ... Shawn?
> 
It's technically possible, but probably not practically.  The imx28 RM
does not mention i2c clock at all, and I was told by designer that i2c
always runs at the same frequency as apbx, which sources from ref_xtal
(always 24 MHz).  Though there is a divider for apbx clock, it seems
every single running system sets the divider as 1 all the time.  That
said, what I have seen is the i2c clock is always 24 MHz.

-- 
Regards,
Shawn

Re: [PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 06:02:46PM +0800, Marek Vasut wrote:
> Dear Dong Aisheng,
> 
> > On Fri, Jun 29, 2012 at 05:30:16PM +0800, Marek Vasut wrote:
> > > Dear Dong Aisheng,
> > > 
> > > > On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > > > > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > > > > enabled via DT. The DMA operation is enabled by default.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> > > > > Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> > > > > CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > > > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > > > Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> > > > > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > > CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > > CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > > > Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> > > > > CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > > Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> > > > > Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > > > ---
> > > > > 
> > > > >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> > > > >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> > > > >  drivers/i2c/busses/i2c-mxs.c                      |  267
> > > > >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> > > > >  deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > > > > d2bf750..9497ee0 100644
> > > > > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > > 
> > > > > @@ -5,6 +5,10 @@ Required properties:
> > > > >  - reg: Should contain registers location and length
> > > > >  - interrupts: Should contain ERROR and DMA interrupts
> > > > >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > > > > 
> > > > > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> > > > 
> > > > Shoundn't this be optional?
> > > 
> > > DMA channel? No, why?
> > 
> > User can use pio mode, so actually it's optional.
> > Is it reasonable?
> 
> PIO mode is something that should not be used unless it's to be used for 
> debugging purposes. The ultimate goal then is to use PIO for small transfers and 
> DMA for large transfers. But that's not implemented yet and DMA is default.
> 
Because DMA channel is not the minimum required properties to make HW work,
so i had that question.
Anyway, i think it's not a big issue.
If you really like it to be mandatory required for default dma mode, i'm ok with it.

> > > > > +	/*
> > > > > +	 * The last descriptor must have this callback,
> > > > > +	 * to finish the DMA transaction.
> > > > > +	 */
> > > > > +	desc->callback = mxs_i2c_dma_irq_callback;
> > > > > +	desc->callback_param = i2c;
> > > > > +
> > > > > +	/* Start the transfer. */
> > > > > +	dmaengine_submit(desc);
> > > > > +	dma_async_issue_pending(i2c->dmach);
> > > > > +	return 0;
> > > > > +
> > > > > +/* Read failpath. */
> > > > > +read_init_dma_fail:
> > > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > > > > +select_init_dma_fail:
> > > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > > > > +select_init_pio_fail:
> > > > > +	return 1;
> > > > 
> > > > look strange why return 1;
> > > 
> > > Because it failed. -Esomething might be better ?
> > 
> > i think yes.
> 
> And -Ewhat do you suggest ? :)
> 
I checked mxs-dma driver, it seems both EBUSY or EINVAL are ok.
You can choose one. :)

Regards
Dong Aisheng

[PATCH 2/2] MXS: Implement DMA support into mxs-i2c

[Dong Aisheng]

On Fri, Jun 29, 2012 at 06:02:46PM +0800, Marek Vasut wrote:
> Dear Dong Aisheng,
> 
> > On Fri, Jun 29, 2012 at 05:30:16PM +0800, Marek Vasut wrote:
> > > Dear Dong Aisheng,
> > > 
> > > > On Fri, Jun 29, 2012 at 04:24:03PM +0800, Marek Vasut wrote:
> > > > > This patch implements DMA support into mxs-i2c. DMA transfers are now
> > > > > enabled via DT. The DMA operation is enabled by default.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > > Cc: Detlev Zundel <dzu@denx.de>
> > > > > CC: Dong Aisheng <b29396@freescale.com>
> > > > > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > > > > Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> > > > > Cc: linux-i2c at vger.kernel.org
> > > > > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > CC: Shawn Guo <shawn.guo@linaro.org>
> > > > > Cc: Stefano Babic <sbabic@denx.de>
> > > > > CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > > > Cc: Wolfgang Denk <wd@denx.de>
> > > > > Cc: Wolfram Sang <w.sang@pengutronix.de>
> > > > > ---
> > > > > 
> > > > >  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    4 +
> > > > >  arch/arm/boot/dts/imx28.dtsi                      |    2 +
> > > > >  drivers/i2c/busses/i2c-mxs.c                      |  267
> > > > >  +++++++++++++++++++-- 3 files changed, 251 insertions(+), 22
> > > > >  deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > > > > d2bf750..9497ee0 100644
> > > > > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > > > > 
> > > > > @@ -5,6 +5,10 @@ Required properties:
> > > > >  - reg: Should contain registers location and length
> > > > >  - interrupts: Should contain ERROR and DMA interrupts
> > > > >  - clock-frequency: desired I2C bus clock frequency in Hz.
> > > > > 
> > > > > +- fsl,i2c-dma-channel: APBX DMA channel for the I2C
> > > > 
> > > > Shoundn't this be optional?
> > > 
> > > DMA channel? No, why?
> > 
> > User can use pio mode, so actually it's optional.
> > Is it reasonable?
> 
> PIO mode is something that should not be used unless it's to be used for 
> debugging purposes. The ultimate goal then is to use PIO for small transfers and 
> DMA for large transfers. But that's not implemented yet and DMA is default.
> 
Because DMA channel is not the minimum required properties to make HW work,
so i had that question.
Anyway, i think it's not a big issue.
If you really like it to be mandatory required for default dma mode, i'm ok with it.

> > > > > +	/*
> > > > > +	 * The last descriptor must have this callback,
> > > > > +	 * to finish the DMA transaction.
> > > > > +	 */
> > > > > +	desc->callback = mxs_i2c_dma_irq_callback;
> > > > > +	desc->callback_param = i2c;
> > > > > +
> > > > > +	/* Start the transfer. */
> > > > > +	dmaengine_submit(desc);
> > > > > +	dma_async_issue_pending(i2c->dmach);
> > > > > +	return 0;
> > > > > +
> > > > > +/* Read failpath. */
> > > > > +read_init_dma_fail:
> > > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[1], 1, DMA_FROM_DEVICE);
> > > > > +select_init_dma_fail:
> > > > > +	dma_unmap_sg(i2c->dev, &i2c->sg_io[0], 1, DMA_TO_DEVICE);
> > > > > +select_init_pio_fail:
> > > > > +	return 1;
> > > > 
> > > > look strange why return 1;
> > > 
> > > Because it failed. -Esomething might be better ?
> > 
> > i think yes.
> 
> And -Ewhat do you suggest ? :)
> 
I checked mxs-dma driver, it seems both EBUSY or EINVAL are ok.
You can choose one. :)

Regards
Dong Aisheng

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Shawn Guo]

Welcome to mxs device tree, Marek :)

On Sun, May 27, 2012 at 04:10:55AM +0200, Marek Vasut wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
> 
> Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
> CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
> Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |   64 +++++++++++++++++++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..790b5c6 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Should be "fsl,<chip>-i2c"
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
> +- speed: Speed of the bus in kHz (400 or 100 are supported)
>  
>  Examples:
>  
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 4634cb8..d927155 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -381,6 +381,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x80058000 2000>;
>  				interrupts = <111 68>;
> +				fsl,speed = <400>;
>  				status = "disabled";
>  			};
>  
> @@ -390,6 +391,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x8005a000 2000>;
>  				interrupts = <110 69>;
> +				fsl,speed = <400>;
>  				status = "disabled";
>  			};
>  
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 04eb441..fff0a09 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -46,6 +46,10 @@
>  #define MXS_I2C_CTRL0_DIRECTION			0x00010000
>  #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
>  
> +#define MXS_I2C_TIMING0		(0x10)
> +#define MXS_I2C_TIMING1		(0x20)
> +#define MXS_I2C_TIMING2		(0x30)
> +
>  #define MXS_I2C_CTRL1		(0x40)
>  #define MXS_I2C_CTRL1_SET	(0x44)
>  #define MXS_I2C_CTRL1_CLR	(0x48)
> @@ -97,6 +101,24 @@
>  #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
>  				 MXS_I2C_CTRL0_MASTER_MODE)
>  
> +struct mxs_i2c_speed_config {
> +	uint32_t	timing0;
> +	uint32_t	timing1;
> +	uint32_t	timing2;
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
> +	.timing0	= 0x00780030,
> +	.timing1	= 0x00800030,
> +	.timing2	= 0x0015000d,
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> +	.timing0	= 0x000f0007,
> +	.timing1	= 0x001f000f,
> +	.timing2	= 0x0015000d,
> +};
> +
>  /**
>   * struct mxs_i2c_dev - per device, private MXS-I2C data
>   *
> @@ -112,11 +134,17 @@ struct mxs_i2c_dev {
>  	struct completion cmd_complete;
>  	u32 cmd_err;
>  	struct i2c_adapter adapter;
> +	const struct mxs_i2c_speed_config *speed;
>  };
>  
>  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
>  {
>  	stmp_reset_block(i2c->regs);
> +
> +	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
> +	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
> +	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
> +
>  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
>  	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
>  			i2c->regs + MXS_I2C_QUEUECTRL_SET);
> @@ -319,6 +347,38 @@ static const struct i2c_algorithm mxs_i2c_algo = {
>  	.functionality = mxs_i2c_func,
>  };
>  
> +#ifdef CONFIG_OF

We already force ARCH_MXS to select USE_OF while I2C_MXS only depends
on SOC_IMX28, so this #ifdef is not really needed?

> +static int mxs_i2c_get_ofdata(struct platform_device *pdev,
> +				struct mxs_i2c_dev *i2c)
> +{
> +	const __be32 *speed;
> +	uint32_t speed_khz;
> +	struct device_node *node = pdev->dev.of_node;
> +
> +	if (!node)
> +		return -EINVAL;
> +
> +	i2c->speed = &mxs_i2c_95kHz_config;
> +	speed = of_get_property(node, "fsl,speed", NULL);
> +	if (!speed)
> +		dev_warn(&pdev->dev,
> +			"No I2C speed selected, using 100kHz\n");
> +
> +	speed_khz = be32_to_cpup(speed);

You may want to try of_property_read_u32().

> +	if (speed_khz == 400)
> +		i2c->speed = &mxs_i2c_400kHz_config;
> +	else if (speed_khz != 100)
> +		dev_warn(&pdev->dev,
> +			"Invalid I2C speed selected, using 100kHz\n");
> +
> +	return 0;
> +}
> +#else
> +static int mxs_i2c_get_ofdata(struct platform_device *pdev,
> +				struct mxs_i2c_dev *i2c)
> +{ }
> +#endif /* CONFIG_OF */
> +
>  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -357,6 +417,10 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> +	err = mxs_i2c_get_ofdata(pdev, i2c);
> +	if (err)
> +		return err;
> +

If we have this block put after the following line, we can save pdev
parameter of mxs_i2c_get_ofdata()?

>  	i2c->dev = dev;
>  	platform_set_drvdata(pdev, i2c);
>  
> -- 
> 1.7.10
> 

-- 
Regards,
Shawn

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Shawn Guo]

Welcome to mxs device tree, Marek :)

On Sun, May 27, 2012 at 04:10:55AM +0200, Marek Vasut wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> CC: Dong Aisheng <b29396@freescale.com>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> Cc: linux-i2c at vger.kernel.org
> CC: Sascha Hauer <s.hauer@pengutronix.de>
> CC: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>  arch/arm/boot/dts/imx28.dtsi                      |    2 +
>  drivers/i2c/busses/i2c-mxs.c                      |   64 +++++++++++++++++++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..790b5c6 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>  - compatible: Should be "fsl,<chip>-i2c"
>  - reg: Should contain registers location and length
>  - interrupts: Should contain ERROR and DMA interrupts
> +- speed: Speed of the bus in kHz (400 or 100 are supported)
>  
>  Examples:
>  
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 4634cb8..d927155 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -381,6 +381,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x80058000 2000>;
>  				interrupts = <111 68>;
> +				fsl,speed = <400>;
>  				status = "disabled";
>  			};
>  
> @@ -390,6 +391,7 @@
>  				compatible = "fsl,imx28-i2c";
>  				reg = <0x8005a000 2000>;
>  				interrupts = <110 69>;
> +				fsl,speed = <400>;
>  				status = "disabled";
>  			};
>  
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 04eb441..fff0a09 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -46,6 +46,10 @@
>  #define MXS_I2C_CTRL0_DIRECTION			0x00010000
>  #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
>  
> +#define MXS_I2C_TIMING0		(0x10)
> +#define MXS_I2C_TIMING1		(0x20)
> +#define MXS_I2C_TIMING2		(0x30)
> +
>  #define MXS_I2C_CTRL1		(0x40)
>  #define MXS_I2C_CTRL1_SET	(0x44)
>  #define MXS_I2C_CTRL1_CLR	(0x48)
> @@ -97,6 +101,24 @@
>  #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
>  				 MXS_I2C_CTRL0_MASTER_MODE)
>  
> +struct mxs_i2c_speed_config {
> +	uint32_t	timing0;
> +	uint32_t	timing1;
> +	uint32_t	timing2;
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
> +	.timing0	= 0x00780030,
> +	.timing1	= 0x00800030,
> +	.timing2	= 0x0015000d,
> +};
> +
> +const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
> +	.timing0	= 0x000f0007,
> +	.timing1	= 0x001f000f,
> +	.timing2	= 0x0015000d,
> +};
> +
>  /**
>   * struct mxs_i2c_dev - per device, private MXS-I2C data
>   *
> @@ -112,11 +134,17 @@ struct mxs_i2c_dev {
>  	struct completion cmd_complete;
>  	u32 cmd_err;
>  	struct i2c_adapter adapter;
> +	const struct mxs_i2c_speed_config *speed;
>  };
>  
>  static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
>  {
>  	stmp_reset_block(i2c->regs);
> +
> +	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
> +	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
> +	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
> +
>  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
>  	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
>  			i2c->regs + MXS_I2C_QUEUECTRL_SET);
> @@ -319,6 +347,38 @@ static const struct i2c_algorithm mxs_i2c_algo = {
>  	.functionality = mxs_i2c_func,
>  };
>  
> +#ifdef CONFIG_OF

We already force ARCH_MXS to select USE_OF while I2C_MXS only depends
on SOC_IMX28, so this #ifdef is not really needed?

> +static int mxs_i2c_get_ofdata(struct platform_device *pdev,
> +				struct mxs_i2c_dev *i2c)
> +{
> +	const __be32 *speed;
> +	uint32_t speed_khz;
> +	struct device_node *node = pdev->dev.of_node;
> +
> +	if (!node)
> +		return -EINVAL;
> +
> +	i2c->speed = &mxs_i2c_95kHz_config;
> +	speed = of_get_property(node, "fsl,speed", NULL);
> +	if (!speed)
> +		dev_warn(&pdev->dev,
> +			"No I2C speed selected, using 100kHz\n");
> +
> +	speed_khz = be32_to_cpup(speed);

You may want to try of_property_read_u32().

> +	if (speed_khz == 400)
> +		i2c->speed = &mxs_i2c_400kHz_config;
> +	else if (speed_khz != 100)
> +		dev_warn(&pdev->dev,
> +			"Invalid I2C speed selected, using 100kHz\n");
> +
> +	return 0;
> +}
> +#else
> +static int mxs_i2c_get_ofdata(struct platform_device *pdev,
> +				struct mxs_i2c_dev *i2c)
> +{ }
> +#endif /* CONFIG_OF */
> +
>  static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -357,6 +417,10 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> +	err = mxs_i2c_get_ofdata(pdev, i2c);
> +	if (err)
> +		return err;
> +

If we have this block put after the following line, we can save pdev
parameter of mxs_i2c_get_ofdata()?

>  	i2c->dev = dev;
>  	platform_set_drvdata(pdev, i2c);
>  
> -- 
> 1.7.10
> 

-- 
Regards,
Shawn

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Wolfram Sang]

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


> > Is there not a standard speed setting in the i2c binding?
> 
> Not that I know of, but if someone hinted one to me, I'd gladly rework the 
> patches :)

It is "clock-frequency". Check 'Documentation/devicetree/bindings/i2c'.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Wolfram Sang]

> > Is there not a standard speed setting in the i2c binding?
> 
> Not that I know of, but if someone hinted one to me, I'd gladly rework the 
> patches :)

It is "clock-frequency". Check 'Documentation/devicetree/bindings/i2c'.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120602/b19a34e9/attachment.sig>

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

Dear Ben Dooks,

> On 27/05/12 03:10, Marek Vasut wrote:
> > This patch configures the I2C bus timing registers according
> > to information passed via DT. Currently, 100kHz and 400kHz
> > modes are supported.
> > 
> > Signed-off-by: Marek Vasut<marex-ynQEQJNshbs@public.gmane.org>
> > Cc: Detlev Zundel<dzu-ynQEQJNshbs@public.gmane.org>
> > CC: Dong Aisheng<b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > CC: Fabio Estevam<fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > Cc: Linux ARM kernel<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
> > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > CC: Sascha Hauer<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > CC: Shawn Guo<shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Stefano Babic<sbabic-ynQEQJNshbs@public.gmane.org>
> > CC: Uwe Kleine-König<u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Wolfgang Denk<wd-ynQEQJNshbs@public.gmane.org>
> > Cc: Wolfram Sang<w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > 
> >   Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
> >   arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >   drivers/i2c/busses/i2c-mxs.c                      |   64
> >   +++++++++++++++++++++ 3 files changed, 67 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > 1bfc02d..790b5c6 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > 
> > @@ -4,6 +4,7 @@ Required properties:
> >   - compatible: Should be "fsl,<chip>-i2c"
> >   - reg: Should contain registers location and length
> >   - interrupts: Should contain ERROR and DMA interrupts
> > 
> > +- speed: Speed of the bus in kHz (400 or 100 are supported)
> > 
> >   Examples:
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index 4634cb8..d927155 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -381,6 +381,7 @@
> > 
> >   				compatible = "fsl,imx28-i2c";
> >   				reg =<0x80058000 2000>;
> >   				interrupts =<111 68>;
> > 
> > +				fsl,speed =<400>;
> > 
> >   				status = "disabled";
> >   			
> >   			};
> > 
> > @@ -390,6 +391,7 @@
> > 
> >   				compatible = "fsl,imx28-i2c";
> >   				reg =<0x8005a000 2000>;
> >   				interrupts =<110 69>;
> > 
> > +				fsl,speed =<400>;
> > 
> >   				status = "disabled";
> >   			
> >   			};
> 
> Is there not a standard speed setting in the i2c binding?

Not that I know of, but if someone hinted one to me, I'd gladly rework the 
patches :)

Best regards,
Marek Vasut

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

Dear Ben Dooks,

> On 27/05/12 03:10, Marek Vasut wrote:
> > This patch configures the I2C bus timing registers according
> > to information passed via DT. Currently, 100kHz and 400kHz
> > modes are supported.
> > 
> > Signed-off-by: Marek Vasut<marex@denx.de>
> > Cc: Detlev Zundel<dzu@denx.de>
> > CC: Dong Aisheng<b29396@freescale.com>
> > CC: Fabio Estevam<fabio.estevam@freescale.com>
> > Cc: Linux ARM kernel<linux-arm-kernel@lists.infradead.org>
> > Cc: linux-i2c at vger.kernel.org
> > CC: Sascha Hauer<s.hauer@pengutronix.de>
> > CC: Shawn Guo<shawn.guo@linaro.org>
> > Cc: Stefano Babic<sbabic@denx.de>
> > CC: Uwe Kleine-K?nig<u.kleine-koenig@pengutronix.de>
> > Cc: Wolfgang Denk<wd@denx.de>
> > Cc: Wolfram Sang<w.sang@pengutronix.de>
> > ---
> > 
> >   Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
> >   arch/arm/boot/dts/imx28.dtsi                      |    2 +
> >   drivers/i2c/busses/i2c-mxs.c                      |   64
> >   +++++++++++++++++++++ 3 files changed, 67 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt index
> > 1bfc02d..790b5c6 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> > 
> > @@ -4,6 +4,7 @@ Required properties:
> >   - compatible: Should be "fsl,<chip>-i2c"
> >   - reg: Should contain registers location and length
> >   - interrupts: Should contain ERROR and DMA interrupts
> > 
> > +- speed: Speed of the bus in kHz (400 or 100 are supported)
> > 
> >   Examples:
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index 4634cb8..d927155 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -381,6 +381,7 @@
> > 
> >   				compatible = "fsl,imx28-i2c";
> >   				reg =<0x80058000 2000>;
> >   				interrupts =<111 68>;
> > 
> > +				fsl,speed =<400>;
> > 
> >   				status = "disabled";
> >   			
> >   			};
> > 
> > @@ -390,6 +391,7 @@
> > 
> >   				compatible = "fsl,imx28-i2c";
> >   				reg =<0x8005a000 2000>;
> >   				interrupts =<110 69>;
> > 
> > +				fsl,speed =<400>;
> > 
> >   				status = "disabled";
> >   			
> >   			};
> 
> Is there not a standard speed setting in the i2c binding?

Not that I know of, but if someone hinted one to me, I'd gladly rework the 
patches :)

Best regards,
Marek Vasut

Re: [PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Ben Dooks]

On 27/05/12 03:10, Marek Vasut wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
>
> Signed-off-by: Marek Vasut<marex@denx.de>
> Cc: Detlev Zundel<dzu@denx.de>
> CC: Dong Aisheng<b29396@freescale.com>
> CC: Fabio Estevam<fabio.estevam@freescale.com>
> Cc: Linux ARM kernel<linux-arm-kernel@lists.infradead.org>
> Cc: linux-i2c@vger.kernel.org
> CC: Sascha Hauer<s.hauer@pengutronix.de>
> CC: Shawn Guo<shawn.guo@linaro.org>
> Cc: Stefano Babic<sbabic@denx.de>
> CC: Uwe Kleine-König<u.kleine-koenig@pengutronix.de>
> Cc: Wolfgang Denk<wd@denx.de>
> Cc: Wolfram Sang<w.sang@pengutronix.de>
> ---
>   Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>   arch/arm/boot/dts/imx28.dtsi                      |    2 +
>   drivers/i2c/busses/i2c-mxs.c                      |   64 +++++++++++++++++++++
>   3 files changed, 67 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..790b5c6 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>   - compatible: Should be "fsl,<chip>-i2c"
>   - reg: Should contain registers location and length
>   - interrupts: Should contain ERROR and DMA interrupts
> +- speed: Speed of the bus in kHz (400 or 100 are supported)
>
>   Examples:
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 4634cb8..d927155 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -381,6 +381,7 @@
>   				compatible = "fsl,imx28-i2c";
>   				reg =<0x80058000 2000>;
>   				interrupts =<111 68>;
> +				fsl,speed =<400>;
>   				status = "disabled";
>   			};
>
> @@ -390,6 +391,7 @@
>   				compatible = "fsl,imx28-i2c";
>   				reg =<0x8005a000 2000>;
>   				interrupts =<110 69>;
> +				fsl,speed =<400>;
>   				status = "disabled";
>   			};

Is there not a standard speed setting in the i2c binding?

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

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

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Ben Dooks]

On 27/05/12 03:10, Marek Vasut wrote:
> This patch configures the I2C bus timing registers according
> to information passed via DT. Currently, 100kHz and 400kHz
> modes are supported.
>
> Signed-off-by: Marek Vasut<marex@denx.de>
> Cc: Detlev Zundel<dzu@denx.de>
> CC: Dong Aisheng<b29396@freescale.com>
> CC: Fabio Estevam<fabio.estevam@freescale.com>
> Cc: Linux ARM kernel<linux-arm-kernel@lists.infradead.org>
> Cc: linux-i2c at vger.kernel.org
> CC: Sascha Hauer<s.hauer@pengutronix.de>
> CC: Shawn Guo<shawn.guo@linaro.org>
> Cc: Stefano Babic<sbabic@denx.de>
> CC: Uwe Kleine-K?nig<u.kleine-koenig@pengutronix.de>
> Cc: Wolfgang Denk<wd@denx.de>
> Cc: Wolfram Sang<w.sang@pengutronix.de>
> ---
>   Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
>   arch/arm/boot/dts/imx28.dtsi                      |    2 +
>   drivers/i2c/busses/i2c-mxs.c                      |   64 +++++++++++++++++++++
>   3 files changed, 67 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> index 1bfc02d..790b5c6 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
> @@ -4,6 +4,7 @@ Required properties:
>   - compatible: Should be "fsl,<chip>-i2c"
>   - reg: Should contain registers location and length
>   - interrupts: Should contain ERROR and DMA interrupts
> +- speed: Speed of the bus in kHz (400 or 100 are supported)
>
>   Examples:
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 4634cb8..d927155 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -381,6 +381,7 @@
>   				compatible = "fsl,imx28-i2c";
>   				reg =<0x80058000 2000>;
>   				interrupts =<111 68>;
> +				fsl,speed =<400>;
>   				status = "disabled";
>   			};
>
> @@ -390,6 +391,7 @@
>   				compatible = "fsl,imx28-i2c";
>   				reg =<0x8005a000 2000>;
>   				interrupts =<110 69>;
> +				fsl,speed =<400>;
>   				status = "disabled";
>   			};

Is there not a standard speed setting in the i2c binding?

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

This patch configures the I2C bus timing registers according
to information passed via DT. Currently, 100kHz and 400kHz
modes are supported.

Signed-off-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
Cc: Detlev Zundel <dzu-ynQEQJNshbs@public.gmane.org>
CC: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
CC: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
CC: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Wolfgang Denk <wd-ynQEQJNshbs@public.gmane.org>
Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |   64 +++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index 1bfc02d..790b5c6 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Should be "fsl,<chip>-i2c"
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
+- speed: Speed of the bus in kHz (400 or 100 are supported)
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 4634cb8..d927155 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -381,6 +381,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
+				fsl,speed = <400>;
 				status = "disabled";
 			};
 
@@ -390,6 +391,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
+				fsl,speed = <400>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 04eb441..fff0a09 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -46,6 +46,10 @@
 #define MXS_I2C_CTRL0_DIRECTION			0x00010000
 #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
 
+#define MXS_I2C_TIMING0		(0x10)
+#define MXS_I2C_TIMING1		(0x20)
+#define MXS_I2C_TIMING2		(0x30)
+
 #define MXS_I2C_CTRL1		(0x40)
 #define MXS_I2C_CTRL1_SET	(0x44)
 #define MXS_I2C_CTRL1_CLR	(0x48)
@@ -97,6 +101,24 @@
 #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
 				 MXS_I2C_CTRL0_MASTER_MODE)
 
+struct mxs_i2c_speed_config {
+	uint32_t	timing0;
+	uint32_t	timing1;
+	uint32_t	timing2;
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
+	.timing0	= 0x00780030,
+	.timing1	= 0x00800030,
+	.timing2	= 0x0015000d,
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
+	.timing0	= 0x000f0007,
+	.timing1	= 0x001f000f,
+	.timing2	= 0x0015000d,
+};
+
 /**
  * struct mxs_i2c_dev - per device, private MXS-I2C data
  *
@@ -112,11 +134,17 @@ struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+	const struct mxs_i2c_speed_config *speed;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
 	stmp_reset_block(i2c->regs);
+
+	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
+	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
+	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
+
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
@@ -319,6 +347,38 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+#ifdef CONFIG_OF
+static int mxs_i2c_get_ofdata(struct platform_device *pdev,
+				struct mxs_i2c_dev *i2c)
+{
+	const __be32 *speed;
+	uint32_t speed_khz;
+	struct device_node *node = pdev->dev.of_node;
+
+	if (!node)
+		return -EINVAL;
+
+	i2c->speed = &mxs_i2c_95kHz_config;
+	speed = of_get_property(node, "fsl,speed", NULL);
+	if (!speed)
+		dev_warn(&pdev->dev,
+			"No I2C speed selected, using 100kHz\n");
+
+	speed_khz = be32_to_cpup(speed);
+	if (speed_khz == 400)
+		i2c->speed = &mxs_i2c_400kHz_config;
+	else if (speed_khz != 100)
+		dev_warn(&pdev->dev,
+			"Invalid I2C speed selected, using 100kHz\n");
+
+	return 0;
+}
+#else
+static int mxs_i2c_get_ofdata(struct platform_device *pdev,
+				struct mxs_i2c_dev *i2c)
+{ }
+#endif /* CONFIG_OF */
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -357,6 +417,10 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = mxs_i2c_get_ofdata(pdev, i2c);
+	if (err)
+		return err;
+
 	i2c->dev = dev;
 	platform_set_drvdata(pdev, i2c);
 
-- 
1.7.10

[PATCH 1/2] MXS: Set I2C timing registers for mxs-i2c

[Marek Vasut]

This patch configures the I2C bus timing registers according
to information passed via DT. Currently, 100kHz and 400kHz
modes are supported.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
CC: Dong Aisheng <b29396@freescale.com>
CC: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
Cc: linux-i2c at vger.kernel.org
CC: Sascha Hauer <s.hauer@pengutronix.de>
CC: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
CC: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 Documentation/devicetree/bindings/i2c/i2c-mxs.txt |    1 +
 arch/arm/boot/dts/imx28.dtsi                      |    2 +
 drivers/i2c/busses/i2c-mxs.c                      |   64 +++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
index 1bfc02d..790b5c6 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mxs.txt
@@ -4,6 +4,7 @@ Required properties:
 - compatible: Should be "fsl,<chip>-i2c"
 - reg: Should contain registers location and length
 - interrupts: Should contain ERROR and DMA interrupts
+- speed: Speed of the bus in kHz (400 or 100 are supported)
 
 Examples:
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 4634cb8..d927155 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -381,6 +381,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 2000>;
 				interrupts = <111 68>;
+				fsl,speed = <400>;
 				status = "disabled";
 			};
 
@@ -390,6 +391,7 @@
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 2000>;
 				interrupts = <110 69>;
+				fsl,speed = <400>;
 				status = "disabled";
 			};
 
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 04eb441..fff0a09 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -46,6 +46,10 @@
 #define MXS_I2C_CTRL0_DIRECTION			0x00010000
 #define MXS_I2C_CTRL0_XFER_COUNT(v)		((v) & 0x0000FFFF)
 
+#define MXS_I2C_TIMING0		(0x10)
+#define MXS_I2C_TIMING1		(0x20)
+#define MXS_I2C_TIMING2		(0x30)
+
 #define MXS_I2C_CTRL1		(0x40)
 #define MXS_I2C_CTRL1_SET	(0x44)
 #define MXS_I2C_CTRL1_CLR	(0x48)
@@ -97,6 +101,24 @@
 #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
 				 MXS_I2C_CTRL0_MASTER_MODE)
 
+struct mxs_i2c_speed_config {
+	uint32_t	timing0;
+	uint32_t	timing1;
+	uint32_t	timing2;
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_95kHz_config = {
+	.timing0	= 0x00780030,
+	.timing1	= 0x00800030,
+	.timing2	= 0x0015000d,
+};
+
+const struct mxs_i2c_speed_config mxs_i2c_400kHz_config = {
+	.timing0	= 0x000f0007,
+	.timing1	= 0x001f000f,
+	.timing2	= 0x0015000d,
+};
+
 /**
  * struct mxs_i2c_dev - per device, private MXS-I2C data
  *
@@ -112,11 +134,17 @@ struct mxs_i2c_dev {
 	struct completion cmd_complete;
 	u32 cmd_err;
 	struct i2c_adapter adapter;
+	const struct mxs_i2c_speed_config *speed;
 };
 
 static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
 {
 	stmp_reset_block(i2c->regs);
+
+	writel(i2c->speed->timing0, i2c->regs + MXS_I2C_TIMING0);
+	writel(i2c->speed->timing1, i2c->regs + MXS_I2C_TIMING1);
+	writel(i2c->speed->timing2, i2c->regs + MXS_I2C_TIMING2);
+
 	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
 	writel(MXS_I2C_QUEUECTRL_PIO_QUEUE_MODE,
 			i2c->regs + MXS_I2C_QUEUECTRL_SET);
@@ -319,6 +347,38 @@ static const struct i2c_algorithm mxs_i2c_algo = {
 	.functionality = mxs_i2c_func,
 };
 
+#ifdef CONFIG_OF
+static int mxs_i2c_get_ofdata(struct platform_device *pdev,
+				struct mxs_i2c_dev *i2c)
+{
+	const __be32 *speed;
+	uint32_t speed_khz;
+	struct device_node *node = pdev->dev.of_node;
+
+	if (!node)
+		return -EINVAL;
+
+	i2c->speed = &mxs_i2c_95kHz_config;
+	speed = of_get_property(node, "fsl,speed", NULL);
+	if (!speed)
+		dev_warn(&pdev->dev,
+			"No I2C speed selected, using 100kHz\n");
+
+	speed_khz = be32_to_cpup(speed);
+	if (speed_khz == 400)
+		i2c->speed = &mxs_i2c_400kHz_config;
+	else if (speed_khz != 100)
+		dev_warn(&pdev->dev,
+			"Invalid I2C speed selected, using 100kHz\n");
+
+	return 0;
+}
+#else
+static int mxs_i2c_get_ofdata(struct platform_device *pdev,
+				struct mxs_i2c_dev *i2c)
+{ }
+#endif /* CONFIG_OF */
+
 static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -357,6 +417,10 @@ static int __devinit mxs_i2c_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = mxs_i2c_get_ofdata(pdev, i2c);
+	if (err)
+		return err;
+
 	i2c->dev = dev;
 	platform_set_drvdata(pdev, i2c);
 
-- 
1.7.10