linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 0/6] Add I2C support for Allwinner SoCs
@ 2013-05-27 10:00 Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver Maxime Ripard
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

This patchset adds support for the I2C controller found on most of the
Allwinner SoCs, especially the already supported A10 and A13, and the
yet to come A31.

It has been tested on a A13-Olinuxino.

Thanks,
Maxime

Changes from v2:
  * Slightly modified the switch comments again
  * Removed the of_* calls in favor of platform_get_* functions

Changes from v1:
  * Added comments to the switch statements to clarify when the fall through to
    the next case is made on purpose
  * Use devm_ioremap_resource instead of of_iomap
  * Moved the reset after enabling the clocks
  * Added Emilio Lopez' patch to add the available i2c controllers to the
    cubieboard

Emilio L?pez (1):
  ARM: sun4i: cubieboard: Enable the i2c controllers

Maxime Ripard (5):
  i2c: sunxi: Add Allwinner A1X i2c driver
  ARM: sunxi: dt: Add i2c controller nodes to the DTSI
  ARM: sun4i: dt: Add i2c muxing options
  ARM: sun5i: dt: Add i2c muxing options
  ARM: sun5i: olinuxino: Enable the i2c controllers

 .../devicetree/bindings/i2c/i2c-sunxi.txt          |  19 +
 arch/arm/boot/dts/sun4i-a10-cubieboard.dts         |  12 +
 arch/arm/boot/dts/sun4i-a10.dtsi                   |  48 +++
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts          |  18 +
 arch/arm/boot/dts/sun5i-a13.dtsi                   |  48 +++
 drivers/i2c/busses/Kconfig                         |  10 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-sunxi.c                     | 444 +++++++++++++++++++++
 8 files changed, 600 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi.txt
 create mode 100644 drivers/i2c/busses/i2c-sunxi.c

-- 
1.8.2.3

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
@ 2013-05-27 10:00 ` Maxime Ripard
  2013-06-05 13:39   ` Wolfram Sang
  2013-05-27 10:00 ` [PATCHv3 2/6] ARM: sunxi: dt: Add i2c controller nodes to the DTSI Maxime Ripard
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

This patch implements a basic driver for the I2C host driver found on
the Allwinner A10, A13 and A31 SoCs.

Notable missing feature is 10-bit addressing.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/i2c/i2c-sunxi.txt          |  19 +
 drivers/i2c/busses/Kconfig                         |  10 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-sunxi.c                     | 444 +++++++++++++++++++++
 4 files changed, 474 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi.txt
 create mode 100644 drivers/i2c/busses/i2c-sunxi.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-sunxi.txt b/Documentation/devicetree/bindings/i2c/i2c-sunxi.txt
new file mode 100644
index 0000000..40c16d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-sunxi.txt
@@ -0,0 +1,19 @@
+Allwinner SoC I2C controller
+
+Required properties:
+- compatible : Should be "allwinner,sun4i-i2c".
+- reg: Should contain register location and length.
+- interrupts: Should contain interrupt.
+- clocks : The parent clock feeding the I2C controller.
+
+Recommended properties:
+- clock-frequency : desired I2C bus clock frequency in Hz.
+
+Example:
+i2c0: i2c at 01c2ac00 {
+	compatible = "allwinner,sun4i-i2c";
+	reg = <0x01c2ac00 0x400>;
+	interrupts = <7>;
+	clocks = <&apb1_gates 0>;
+	clock-frequency = <100000>;
+};
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 631736e..1df8ba1 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -706,6 +706,16 @@ config I2C_STU300
 	  This driver can also be built as a module. If so, the module
 	  will be called i2c-stu300.
 
+config I2C_SUNXI
+	tristate "Allwinner A1X I2C controller"
+	depends on ARCH_SUNXI
+	help
+	  If you say yes to this option, support will be included for the
+	  I2C controller embedded in Allwinner A1X SoCs.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called i2c-sunxi.
+
 config I2C_TEGRA
 	tristate "NVIDIA Tegra internal I2C controller"
 	depends on ARCH_TEGRA
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 8f4fc23..7225818 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_I2C_SH_MOBILE)	+= i2c-sh_mobile.o
 obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
 obj-$(CONFIG_I2C_SIRF)		+= i2c-sirf.o
 obj-$(CONFIG_I2C_STU300)	+= i2c-stu300.o
+obj-$(CONFIG_I2C_SUNXI)		+= i2c-sunxi.o
 obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
 obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
 obj-$(CONFIG_I2C_OCTEON)	+= i2c-octeon.o
diff --git a/drivers/i2c/busses/i2c-sunxi.c b/drivers/i2c/busses/i2c-sunxi.c
new file mode 100644
index 0000000..09e086a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-sunxi.c
@@ -0,0 +1,444 @@
+/*
+ * Allwinner A1X SoCs i2c controller driver.
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define SUNXI_I2C_ADDR_REG		(0x00)
+#define SUNXI_I2C_ADDR_ADDR(v)			((v & 0x7f) << 1)
+#define SUNXI_I2C_XADDR_REG		(0x04)
+#define SUNXI_I2C_DATA_REG		(0x08)
+#define SUNXI_I2C_CNTR_REG		(0x0c)
+#define SUNXI_I2C_CNTR_ASSERT_ACK		BIT(2)
+#define SUNXI_I2C_CNTR_INT_FLAG			BIT(3)
+#define SUNXI_I2C_CNTR_MASTER_STOP		BIT(4)
+#define SUNXI_I2C_CNTR_MASTER_START		BIT(5)
+#define SUNXI_I2C_CNTR_BUS_ENABLE		BIT(6)
+#define SUNXI_I2C_CNTR_INT_ENABLE		BIT(7)
+#define SUNXI_I2C_STA_REG		(0x10)
+#define SUNXI_I2C_STA_BUS_ERROR			(0x00)
+#define SUNXI_I2C_STA_START			(0x08)
+#define SUNXI_I2C_STA_START_REPEAT		(0x10)
+#define SUNXI_I2C_STA_MASTER_WADDR_ACK		(0x18)
+#define SUNXI_I2C_STA_MASTER_WADDR_NAK		(0x20)
+#define SUNXI_I2C_STA_MASTER_DATA_SENT_ACK	(0x28)
+#define SUNXI_I2C_STA_MASTER_DATA_SENT_NAK	(0x30)
+#define SUNXI_I2C_STA_MASTER_RADDR_ACK		(0x40)
+#define SUNXI_I2C_STA_MASTER_RADDR_NAK		(0x48)
+#define SUNXI_I2C_STA_MASTER_DATA_RECV_ACK	(0x50)
+#define SUNXI_I2C_STA_MASTER_DATA_RECV_NAK	(0x58)
+#define SUNXI_I2C_CCR_REG		(0x14)
+#define SUNXI_I2C_CCR_DIV_N(val)		(val & 0x3)
+#define SUNXI_I2C_CCR_DIV_M(val)		((val & 0xf) << 3)
+#define SUNXI_I2C_SRST_REG		(0x18)
+#define SUNXI_I2C_SRST_RESET			BIT(0)
+#define SUNXI_I2C_EFR_REG		(0x1c)
+#define SUNXI_I2C_LCR_REG		(0x20)
+
+#define SUNXI_I2C_DONE			BIT(0)
+#define SUNXI_I2C_ERROR			BIT(1)
+#define SUNXI_I2C_NAK			BIT(2)
+#define SUNXI_I2C_BUS_ERROR		BIT(3)
+
+struct sunxi_i2c_dev {
+	struct i2c_adapter	adapter;
+	struct clk		*clk;
+	struct device		*dev;
+	struct completion	completion;
+	int			irq;
+	void __iomem		*membase;
+
+	struct i2c_msg		*msg_cur;
+	u8			*msg_buf;
+	size_t			msg_buf_remaining;
+	unsigned int		msg_err;
+};
+
+static void sunxi_i2c_write(struct sunxi_i2c_dev *i2c_dev, u16 reg, u32 value)
+{
+	writel(value, i2c_dev->membase + reg);
+}
+
+static u32 sunxi_i2c_read(struct sunxi_i2c_dev *i2c_dev, u16 reg)
+{
+	return readl(i2c_dev->membase + reg);
+}
+
+/*
+ * This is where all the magic happens. The I2C controller works as a
+ * state machine, each state being a step in the i2c protocol, with
+ * the controller sending an interrupt at each state transition.
+ *
+ * The state we're in is stored in a register, which leads to a pretty
+ * huge switch statement, all of this in the interrupt handler...
+ */
+static irqreturn_t sunxi_i2c_handler(int irq, void *data)
+{
+	struct sunxi_i2c_dev *i2c_dev = (struct sunxi_i2c_dev *)data;
+	u32 status = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+	u32 addr, val;
+
+	if (!(status & SUNXI_I2C_CNTR_INT_FLAG))
+		return IRQ_NONE;
+
+	/* Read the current state we're in */
+	status = sunxi_i2c_read(i2c_dev, SUNXI_I2C_STA_REG);
+
+	switch (status & 0xff) {
+	/* Start condition has been transmitted */
+	case SUNXI_I2C_STA_START:
+	/* A repeated start condition has been transmitted */
+	case SUNXI_I2C_STA_START_REPEAT:
+		addr = SUNXI_I2C_ADDR_ADDR(i2c_dev->msg_cur->addr);
+
+		if (i2c_dev->msg_cur->flags & I2C_M_RD)
+			addr |= 1;
+
+		sunxi_i2c_write(i2c_dev, SUNXI_I2C_DATA_REG, addr);
+		break;
+
+	/*
+	 * Address + Write bit have been transmitted, ACK has not been
+	 * received.
+	 * Intentional fall through.
+	 */
+	case SUNXI_I2C_STA_MASTER_WADDR_NAK:
+	/*
+	 * Data byte has been transmitted, ACK has not been
+	 * received.
+	 * Intentional fall through.
+	 */
+	case SUNXI_I2C_STA_MASTER_DATA_SENT_NAK:
+		if (!(i2c_dev->msg_cur->flags & I2C_M_IGNORE_NAK)) {
+			i2c_dev->msg_err = SUNXI_I2C_NAK;
+			goto out;
+		}
+
+	/*
+	 * Address + Write bit have been transmitted, ACK has been
+	 * received.
+	 * Intentional fall through.
+	 */
+	case SUNXI_I2C_STA_MASTER_WADDR_ACK:
+	/* Data byte has been transmitted, ACK has been received */
+	case SUNXI_I2C_STA_MASTER_DATA_SENT_ACK:
+		if (i2c_dev->msg_buf_remaining) {
+			sunxi_i2c_write(i2c_dev, SUNXI_I2C_DATA_REG,
+					*i2c_dev->msg_buf);
+			i2c_dev->msg_buf++;
+			i2c_dev->msg_buf_remaining--;
+			break;
+		}
+
+		if (i2c_dev->msg_buf_remaining == 0) {
+			i2c_dev->msg_err = SUNXI_I2C_DONE;
+			goto out;
+		}
+
+		break;
+
+	/*
+	 * Address + Read bit have been transmitted, ACK has not been
+	 * received.
+	 * Intentional fall through.
+	 */
+	case SUNXI_I2C_STA_MASTER_RADDR_NAK:
+		if (!(i2c_dev->msg_cur->flags & I2C_M_IGNORE_NAK)) {
+			i2c_dev->msg_err = SUNXI_I2C_NAK;
+			goto out;
+		}
+
+	/*
+	 * Address + Read bit have been transmitted, ACK has been
+	 * received.
+	 */
+	case SUNXI_I2C_STA_MASTER_RADDR_ACK:
+		/*
+		 * We only need to send the ACK for the all the bytes
+		 * but the last one
+		 */
+		if (i2c_dev->msg_buf_remaining > 1) {
+			val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+			sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG,
+					val | SUNXI_I2C_CNTR_ASSERT_ACK);
+		}
+
+		break;
+
+	/*
+	 * Data byte has been received, ACK has not been
+	 * transmitted.
+	 * Intentional fall through.
+	 */
+	case SUNXI_I2C_STA_MASTER_DATA_RECV_NAK:
+		if (i2c_dev->msg_buf_remaining == 1) {
+			val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_DATA_REG);
+			*i2c_dev->msg_buf = val & 0xff;
+			i2c_dev->msg_buf_remaining--;
+			i2c_dev->msg_err = SUNXI_I2C_DONE;
+			goto out;
+		}
+
+		if (!(i2c_dev->msg_cur->flags & I2C_M_IGNORE_NAK)) {
+			i2c_dev->msg_err = SUNXI_I2C_NAK;
+			goto out;
+		}
+
+	/* Data byte has been received, ACK has been transmitted */
+	case SUNXI_I2C_STA_MASTER_DATA_RECV_ACK:
+		val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_DATA_REG) & 0xff;
+		*i2c_dev->msg_buf = val;
+		i2c_dev->msg_buf++;
+		i2c_dev->msg_buf_remaining--;
+
+		/* If there's only one byte left, disable the ACK */
+		if (i2c_dev->msg_buf_remaining == 1) {
+			val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+			sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG,
+					val & ~SUNXI_I2C_CNTR_ASSERT_ACK);
+
+		};
+
+		break;
+
+	case SUNXI_I2C_STA_BUS_ERROR:
+		i2c_dev->msg_err = SUNXI_I2C_BUS_ERROR;
+		goto out;
+
+	default:
+		i2c_dev->msg_err = SUNXI_I2C_ERROR;
+		goto out;
+	}
+
+	val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG,
+			val & ~SUNXI_I2C_CNTR_INT_FLAG);
+
+	return IRQ_HANDLED;
+
+out:
+	val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG,
+			val | SUNXI_I2C_CNTR_MASTER_STOP);
+
+	val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG,
+			val & ~SUNXI_I2C_CNTR_INT_FLAG);
+
+	complete(&i2c_dev->completion);
+	return IRQ_HANDLED;
+}
+
+static int sunxi_i2c_xfer_msg(struct sunxi_i2c_dev *i2c_dev,
+			      struct i2c_msg *msg)
+{
+	int time_left;
+	u32 val;
+
+	i2c_dev->msg_cur = msg;
+	i2c_dev->msg_buf = msg->buf;
+	i2c_dev->msg_buf_remaining = msg->len;
+	i2c_dev->msg_err = 0;
+	INIT_COMPLETION(i2c_dev->completion);
+
+	val = sunxi_i2c_read(i2c_dev, SUNXI_I2C_CNTR_REG);
+	val |= SUNXI_I2C_CNTR_MASTER_START;
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG, val);
+
+	time_left = wait_for_completion_timeout(&i2c_dev->completion,
+						i2c_dev->adapter.timeout);
+	if (!time_left) {
+		dev_err(i2c_dev->dev, "i2c transfer timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	if (likely(i2c_dev->msg_err == SUNXI_I2C_DONE))
+		return 0;
+
+	dev_dbg(i2c_dev->dev, "i2c transfer failed: %x\n", i2c_dev->msg_err);
+
+	return -EIO;
+}
+
+static int sunxi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			  int num)
+{
+	struct sunxi_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
+	int i, ret;
+
+	for (i = 0; i < num; i++) {
+		ret = sunxi_i2c_xfer_msg(i2c_dev, &msgs[i]);
+		if (ret)
+			return ret;
+	}
+
+	return i;
+}
+
+static u32 sunxi_i2c_func(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm sunxi_i2c_algo = {
+	.master_xfer	= sunxi_i2c_xfer,
+	.functionality	= sunxi_i2c_func,
+};
+
+static int sunxi_i2c_probe(struct platform_device *pdev)
+{
+	struct sunxi_i2c_dev *i2c_dev;
+	struct device_node *np;
+	u32 freq, div_m, div_n;
+	struct resource *res;
+	int ret;
+
+	np = pdev->dev.of_node;
+	if (!np)
+		return -EINVAL;
+
+	i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
+	if (!i2c_dev)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, i2c_dev);
+	i2c_dev->dev = &pdev->dev;
+
+	init_completion(&i2c_dev->completion);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	i2c_dev->membase = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(i2c_dev->membase))
+		return PTR_ERR(i2c_dev->membase);
+
+	i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(i2c_dev->clk))
+		return PTR_ERR(i2c_dev->clk);
+
+	clk_prepare_enable(i2c_dev->clk);
+
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_SRST_REG, SUNXI_I2C_SRST_RESET);
+
+	ret = of_property_read_u32(np, "clock-frequency", &freq);
+	if (ret < 0) {
+		dev_warn(&pdev->dev, "Could not read clock-frequency property\n");
+		freq = 100000;
+	}
+
+	/*
+	 * Set the clock dividers. we don't need to be super smart
+	 * here, the datasheet defines the value of the factors for
+	 * the two supported frequencies, and only the M factor
+	 * changes between 100kHz and 400kHz.
+	 *
+	 * The bus clock is generated from the parent clock with two
+	 * different dividers. It is generated as such:
+	 *     f0 = fclk / (2 ^ DIV_N)
+	 *     fbus = f0 / (10 * (DIV_M + 1))
+	 *
+	 * With DIV_N being on 3 bits, and DIV_M on 4 bits.
+	 * So DIV_N < 8, and DIV_M < 16.
+	 *
+	 * However, we can generate both the supported frequencies
+	 * with f0 = 12MHz, and only change M to get back on our
+	 * feet.
+	 */
+	div_n = ilog2(clk_get_rate(i2c_dev->clk) / 12000000);
+	if (freq == 100000)
+		div_m = 11;
+	else if (freq == 400000)
+		div_m = 2;
+	else {
+		dev_err(&pdev->dev, "Unsupported bus frequency\n");
+		ret = -EINVAL;
+		goto out_clk_dis;
+	}
+
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_CCR_REG,
+			SUNXI_I2C_CCR_DIV_N(div_n) | SUNXI_I2C_CCR_DIV_M(div_m));
+
+	i2c_dev->irq = platform_get_irq(pdev, 0);
+	if (i2c_dev->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ resource\n");
+		ret = i2c_dev->irq;
+		goto out_clk_dis;
+	}
+
+	ret = devm_request_irq(&pdev->dev, i2c_dev->irq, sunxi_i2c_handler,
+			       IRQF_SHARED, dev_name(&pdev->dev), i2c_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not request IRQ\n");
+		goto out_clk_dis;
+	}
+
+	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
+
+	i2c_dev->adapter.owner = THIS_MODULE;
+	strlcpy(i2c_dev->adapter.name, "sunxi I2C adapter",
+		sizeof(i2c_dev->adapter.name));
+	i2c_dev->adapter.algo = &sunxi_i2c_algo;
+	i2c_dev->adapter.dev.parent = &pdev->dev;
+
+	sunxi_i2c_write(i2c_dev, SUNXI_I2C_CNTR_REG,
+			SUNXI_I2C_CNTR_BUS_ENABLE | SUNXI_I2C_CNTR_INT_ENABLE);
+
+	ret = i2c_add_adapter(&i2c_dev->adapter);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register i2c adapter\n");
+		goto out_clk_dis;
+	}
+
+	return 0;
+
+out_clk_dis:
+	clk_disable_unprepare(i2c_dev->clk);
+	return ret;
+}
+
+
+static int sunxi_i2c_remove(struct platform_device *pdev)
+{
+	struct sunxi_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&i2c_dev->adapter);
+	clk_disable_unprepare(i2c_dev->clk);
+
+	return 0;
+}
+
+static const struct of_device_id sunxi_i2c_of_match[] = {
+	{ .compatible = "allwinner,sun4i-i2c" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sunxi_i2c_of_match);
+
+static struct platform_driver sunxi_i2c_driver = {
+	.probe		= sunxi_i2c_probe,
+	.remove		= sunxi_i2c_remove,
+	.driver		= {
+		.name	= "i2c-sunxi",
+		.owner	= THIS_MODULE,
+		.of_match_table = sunxi_i2c_of_match,
+	},
+};
+module_platform_driver(sunxi_i2c_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com");
+MODULE_DESCRIPTION("Allwinner A1X I2C bus adapter");
+MODULE_LICENSE("GPL");
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 2/6] ARM: sunxi: dt: Add i2c controller nodes to the DTSI
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver Maxime Ripard
@ 2013-05-27 10:00 ` Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 3/6] ARM: sun4i: dt: Add i2c muxing options Maxime Ripard
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

The Allwinner A10 and A13 both have 3 i2c controller embedded.
Add those to the common sunxi dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 27 +++++++++++++++++++++++++++
 arch/arm/boot/dts/sun5i-a13.dtsi | 27 +++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index e7ef619..311dcd4 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -292,5 +292,32 @@
 			clocks = <&apb1_gates 23>;
 			status = "disabled";
 		};
+
+		i2c0: i2c at 01c2ac00 {
+			compatible = "allwinner,sun4i-i2c";
+			reg = <0x01c2ac00 0x400>;
+			interrupts = <7>;
+			clocks = <&apb1_gates 0>;
+			clock-frequency = <100000>;
+			status = "disabled";
+		};
+
+		i2c1: i2c at 01c2b000 {
+			compatible = "allwinner,sun4i-i2c";
+			reg = <0x01c2b000 0x400>;
+			interrupts = <8>;
+			clocks = <&apb1_gates 1>;
+			clock-frequency = <100000>;
+			status = "disabled";
+		};
+
+		i2c2: i2c at 01c2b400 {
+			compatible = "allwinner,sun4i-i2c";
+			reg = <0x01c2b400 0x400>;
+			interrupts = <9>;
+			clocks = <&apb1_gates 2>;
+			clock-frequency = <100000>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 8ba65c1..16df784 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -215,5 +215,32 @@
 			clocks = <&apb1_gates 19>;
 			status = "disabled";
 		};
+
+		i2c0: i2c at 01c2ac00 {
+			compatible = "allwinner,sun4i-i2c";
+			reg = <0x01c2ac00 0x400>;
+			interrupts = <7>;
+			clocks = <&apb1_gates 0>;
+			clock-frequency = <100000>;
+			status = "disabled";
+		};
+
+		i2c1: i2c at 01c2b000 {
+			compatible = "allwinner,sun4i-i2c";
+			reg = <0x01c2b000 0x400>;
+			interrupts = <8>;
+			clocks = <&apb1_gates 1>;
+			clock-frequency = <100000>;
+			status = "disabled";
+		};
+
+		i2c2: i2c at 01c2b400 {
+			compatible = "allwinner,sun4i-i2c";
+			reg = <0x01c2b400 0x400>;
+			interrupts = <9>;
+			clocks = <&apb1_gates 2>;
+			clock-frequency = <100000>;
+			status = "disabled";
+		};
 	};
 };
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 3/6] ARM: sun4i: dt: Add i2c muxing options
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 2/6] ARM: sunxi: dt: Add i2c controller nodes to the DTSI Maxime Ripard
@ 2013-05-27 10:00 ` Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 4/6] ARM: sun5i: " Maxime Ripard
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

The i2c controller found on the Allwinner A10 has only one muxing option
available for each controller. Add them to the dtsi

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 311dcd4..63f6716 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -199,6 +199,27 @@
 				allwinner,drive = <0>;
 				allwinner,pull = <0>;
 			};
+
+			i2c0_pins_a: i2c0 at 0 {
+				allwinner,pins = "PB0", "PB1";
+				allwinner,function = "i2c0";
+				allwinner,drive = <0>;
+				allwinner,pull = <1>;
+			};
+
+			i2c1_pins_a: i2c1 at 0 {
+				allwinner,pins = "PB18", "PB19";
+				allwinner,function = "i2c1";
+				allwinner,drive = <0>;
+				allwinner,pull = <1>;
+			};
+
+			i2c2_pins_a: i2c2 at 0 {
+				allwinner,pins = "PB20", "PB21";
+				allwinner,function = "i2c2";
+				allwinner,drive = <0>;
+				allwinner,pull = <1>;
+			};
 		};
 
 		timer at 01c20c00 {
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 4/6] ARM: sun5i: dt: Add i2c muxing options
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
                   ` (2 preceding siblings ...)
  2013-05-27 10:00 ` [PATCHv3 3/6] ARM: sun4i: dt: Add i2c muxing options Maxime Ripard
@ 2013-05-27 10:00 ` Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 5/6] ARM: sun5i: olinuxino: Enable the i2c controllers Maxime Ripard
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

The i2c controller found on the Allwinner A13 has only one muxing option
available for each controller. Add them to the dtsi

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun5i-a13.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 16df784..450b410 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -182,6 +182,27 @@
 				allwinner,drive = <0>;
 				allwinner,pull = <0>;
 			};
+
+			i2c0_pins_a: i2c0 at 0 {
+				allwinner,pins = "PB0", "PB1";
+				allwinner,function = "i2c0";
+				allwinner,drive = <0>;
+				allwinner,pull = <0>;
+			};
+
+			i2c1_pins_a: i2c1 at 0 {
+				allwinner,pins = "PB15", "PB16";
+				allwinner,function = "i2c1";
+				allwinner,drive = <0>;
+				allwinner,pull = <1>;
+			};
+
+			i2c2_pins_a: i2c2 at 0 {
+				allwinner,pins = "PB17", "PB18";
+				allwinner,function = "i2c2";
+				allwinner,drive = <0>;
+				allwinner,pull = <1>;
+			};
 		};
 
 		timer at 01c20c00 {
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 5/6] ARM: sun5i: olinuxino: Enable the i2c controllers
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
                   ` (3 preceding siblings ...)
  2013-05-27 10:00 ` [PATCHv3 4/6] ARM: sun5i: " Maxime Ripard
@ 2013-05-27 10:00 ` Maxime Ripard
  2013-05-27 10:00 ` [PATCHv3 6/6] ARM: sun4i: cubieboard: " Maxime Ripard
  2013-05-27 19:17 ` [PATCHv3 0/6] Add I2C support for Allwinner SoCs Tomasz Figa
  6 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

The A13-Olinuxino makes use of the 3 i2c controllers found on the Allwinner
A13. Enable them in the device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
index 3ca5506..80497e3 100644
--- a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
+++ b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
@@ -37,6 +37,24 @@
 			pinctrl-0 = <&uart1_pins_b>;
 			status = "okay";
 		};
+
+		i2c0: i2c at 01c2ac00 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c0_pins_a>;
+			status = "okay";
+		};
+
+		i2c1: i2c at 01c2b000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c1_pins_a>;
+			status = "okay";
+		};
+
+		i2c2: i2c at 01c2b400 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c2_pins_a>;
+			status = "okay";
+		};
 	};
 
 	leds {
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 6/6] ARM: sun4i: cubieboard: Enable the i2c controllers
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
                   ` (4 preceding siblings ...)
  2013-05-27 10:00 ` [PATCHv3 5/6] ARM: sun5i: olinuxino: Enable the i2c controllers Maxime Ripard
@ 2013-05-27 10:00 ` Maxime Ripard
  2013-05-27 19:17 ` [PATCHv3 0/6] Add I2C support for Allwinner SoCs Tomasz Figa
  6 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-27 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

From: Emilio L?pez <emilio@elopez.com.ar>

The Cubieboard makes use of the first two i2c controllers found on the
Allwinner A10; i2c-0 is used internally for the PMIC, while i2c-1
is exposed on the board headers. This patch enables them in the device
tree.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
index b70fe0d..0e22a28 100644
--- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
+++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
@@ -41,6 +41,18 @@
 			pinctrl-0 = <&uart0_pins_a>;
 			status = "okay";
 		};
+
+		i2c0: i2c at 01c2ac00 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c0_pins_a>;
+			status = "okay";
+		};
+
+		i2c1: i2c at 01c2b000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c1_pins_a>;
+			status = "okay";
+		};
 	};
 
 	leds {
-- 
1.8.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv3 0/6] Add I2C support for Allwinner SoCs
  2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
                   ` (5 preceding siblings ...)
  2013-05-27 10:00 ` [PATCHv3 6/6] ARM: sun4i: cubieboard: " Maxime Ripard
@ 2013-05-27 19:17 ` Tomasz Figa
  2013-05-29  7:34   ` Maxime Ripard
  6 siblings, 1 reply; 15+ messages in thread
From: Tomasz Figa @ 2013-05-27 19:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Maxime,

On Monday 27 of May 2013 12:00:37 Maxime Ripard wrote:
> Hi,
> 
> This patchset adds support for the I2C controller found on most of the
> Allwinner SoCs, especially the already supported A10 and A13, and the
> yet to come A31.
> 
> It has been tested on a A13-Olinuxino.
> 
> Thanks,
> Maxime
> 
> Changes from v2:
>   * Slightly modified the switch comments again
>   * Removed the of_* calls in favor of platform_get_* functions
> 
> Changes from v1:
>   * Added comments to the switch statements to clarify when the fall
> through to the next case is made on purpose
>   * Use devm_ioremap_resource instead of of_iomap
>   * Moved the reset after enabling the clocks
>   * Added Emilio Lopez' patch to add the available i2c controllers to
> the cubieboard
> 
> Emilio L?pez (1):
>   ARM: sun4i: cubieboard: Enable the i2c controllers
> 
> Maxime Ripard (5):
>   i2c: sunxi: Add Allwinner A1X i2c driver
>   ARM: sunxi: dt: Add i2c controller nodes to the DTSI
>   ARM: sun4i: dt: Add i2c muxing options
>   ARM: sun5i: dt: Add i2c muxing options
>   ARM: sun5i: olinuxino: Enable the i2c controllers
> 
>  .../devicetree/bindings/i2c/i2c-sunxi.txt          |  19 +
>  arch/arm/boot/dts/sun4i-a10-cubieboard.dts         |  12 +
>  arch/arm/boot/dts/sun4i-a10.dtsi                   |  48 +++
>  arch/arm/boot/dts/sun5i-a13-olinuxino.dts          |  18 +
>  arch/arm/boot/dts/sun5i-a13.dtsi                   |  48 +++
>  drivers/i2c/busses/Kconfig                         |  10 +
>  drivers/i2c/busses/Makefile                        |   1 +
>  drivers/i2c/busses/i2c-sunxi.c                     | 444
> +++++++++++++++++++++ 8 files changed, 600 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi.txt
>  create mode 100644 drivers/i2c/busses/i2c-sunxi.c

All the patches look good to me.

Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com>

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 0/6] Add I2C support for Allwinner SoCs
  2013-05-27 19:17 ` [PATCHv3 0/6] Add I2C support for Allwinner SoCs Tomasz Figa
@ 2013-05-29  7:34   ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-05-29  7:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas,

On Mon, May 27, 2013 at 09:17:27PM +0200, Tomasz Figa wrote:
> All the patches look good to me.
> 
> Reviewed-by: Tomasz Figa <tomasz.figa@gmail.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-05-27 10:00 ` [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver Maxime Ripard
@ 2013-06-05 13:39   ` Wolfram Sang
  2013-06-05 21:31     ` Maxime Ripard
  0 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2013-06-05 13:39 UTC (permalink / raw)
  To: linux-arm-kernel


> +#define SUNXI_I2C_ADDR_REG		(0x00)
> +#define SUNXI_I2C_ADDR_ADDR(v)			((v & 0x7f) << 1)
> +#define SUNXI_I2C_XADDR_REG		(0x04)
> +#define SUNXI_I2C_DATA_REG		(0x08)
> +#define SUNXI_I2C_CNTR_REG		(0x0c)
> +#define SUNXI_I2C_CNTR_ASSERT_ACK		BIT(2)
> +#define SUNXI_I2C_CNTR_INT_FLAG			BIT(3)
> +#define SUNXI_I2C_CNTR_MASTER_STOP		BIT(4)
> +#define SUNXI_I2C_CNTR_MASTER_START		BIT(5)
> +#define SUNXI_I2C_CNTR_BUS_ENABLE		BIT(6)
> +#define SUNXI_I2C_CNTR_INT_ENABLE		BIT(7)
> +#define SUNXI_I2C_STA_REG		(0x10)
> +#define SUNXI_I2C_STA_BUS_ERROR			(0x00)
> +#define SUNXI_I2C_STA_START			(0x08)
> +#define SUNXI_I2C_STA_START_REPEAT		(0x10)
> +#define SUNXI_I2C_STA_MASTER_WADDR_ACK		(0x18)
> +#define SUNXI_I2C_STA_MASTER_WADDR_NAK		(0x20)
> +#define SUNXI_I2C_STA_MASTER_DATA_SENT_ACK	(0x28)
> +#define SUNXI_I2C_STA_MASTER_DATA_SENT_NAK	(0x30)
> +#define SUNXI_I2C_STA_MASTER_RADDR_ACK		(0x40)
> +#define SUNXI_I2C_STA_MASTER_RADDR_NAK		(0x48)
> +#define SUNXI_I2C_STA_MASTER_DATA_RECV_ACK	(0x50)
> +#define SUNXI_I2C_STA_MASTER_DATA_RECV_NAK	(0x58)
> +#define SUNXI_I2C_CCR_REG		(0x14)
> +#define SUNXI_I2C_CCR_DIV_N(val)		(val & 0x3)
> +#define SUNXI_I2C_CCR_DIV_M(val)		((val & 0xf) << 3)
> +#define SUNXI_I2C_SRST_REG		(0x18)
> +#define SUNXI_I2C_SRST_RESET			BIT(0)
> +#define SUNXI_I2C_EFR_REG		(0x1c)
> +#define SUNXI_I2C_LCR_REG		(0x20)
> +
> +#define SUNXI_I2C_DONE			BIT(0)
> +#define SUNXI_I2C_ERROR			BIT(1)
> +#define SUNXI_I2C_NAK			BIT(2)
> +#define SUNXI_I2C_BUS_ERROR		BIT(3)

The register set looks similar to i2c-mv64xxx.c. Has it been considered
to merge the two drivers?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130605/cbf31644/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-06-05 13:39   ` Wolfram Sang
@ 2013-06-05 21:31     ` Maxime Ripard
  2013-06-10 10:08       ` Maxime Ripard
  2013-06-11 18:21       ` Wolfram Sang
  0 siblings, 2 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-06-05 21:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Wolfram,

On Wed, Jun 05, 2013 at 03:39:47PM +0200, Wolfram Sang wrote:
> 
> > +#define SUNXI_I2C_ADDR_REG		(0x00)
> > +#define SUNXI_I2C_ADDR_ADDR(v)			((v & 0x7f) << 1)
> > +#define SUNXI_I2C_XADDR_REG		(0x04)
> > +#define SUNXI_I2C_DATA_REG		(0x08)
> > +#define SUNXI_I2C_CNTR_REG		(0x0c)
> > +#define SUNXI_I2C_CNTR_ASSERT_ACK		BIT(2)
> > +#define SUNXI_I2C_CNTR_INT_FLAG			BIT(3)
> > +#define SUNXI_I2C_CNTR_MASTER_STOP		BIT(4)
> > +#define SUNXI_I2C_CNTR_MASTER_START		BIT(5)
> > +#define SUNXI_I2C_CNTR_BUS_ENABLE		BIT(6)
> > +#define SUNXI_I2C_CNTR_INT_ENABLE		BIT(7)
> > +#define SUNXI_I2C_STA_REG		(0x10)
> > +#define SUNXI_I2C_STA_BUS_ERROR			(0x00)
> > +#define SUNXI_I2C_STA_START			(0x08)
> > +#define SUNXI_I2C_STA_START_REPEAT		(0x10)
> > +#define SUNXI_I2C_STA_MASTER_WADDR_ACK		(0x18)
> > +#define SUNXI_I2C_STA_MASTER_WADDR_NAK		(0x20)
> > +#define SUNXI_I2C_STA_MASTER_DATA_SENT_ACK	(0x28)
> > +#define SUNXI_I2C_STA_MASTER_DATA_SENT_NAK	(0x30)
> > +#define SUNXI_I2C_STA_MASTER_RADDR_ACK		(0x40)
> > +#define SUNXI_I2C_STA_MASTER_RADDR_NAK		(0x48)
> > +#define SUNXI_I2C_STA_MASTER_DATA_RECV_ACK	(0x50)
> > +#define SUNXI_I2C_STA_MASTER_DATA_RECV_NAK	(0x58)
> > +#define SUNXI_I2C_CCR_REG		(0x14)
> > +#define SUNXI_I2C_CCR_DIV_N(val)		(val & 0x3)
> > +#define SUNXI_I2C_CCR_DIV_M(val)		((val & 0xf) << 3)
> > +#define SUNXI_I2C_SRST_REG		(0x18)
> > +#define SUNXI_I2C_SRST_RESET			BIT(0)
> > +#define SUNXI_I2C_EFR_REG		(0x1c)
> > +#define SUNXI_I2C_LCR_REG		(0x20)
> > +
> > +#define SUNXI_I2C_DONE			BIT(0)
> > +#define SUNXI_I2C_ERROR			BIT(1)
> > +#define SUNXI_I2C_NAK			BIT(2)
> > +#define SUNXI_I2C_BUS_ERROR		BIT(3)
> 
> The register set looks similar to i2c-mv64xxx.c. Has it been considered
> to merge the two drivers?

Hmm, right, the logic seems surprisingly similar. I wasn't aware of such
a driver, so I didn't put much thought into merging them :)

However, the register layout is quite different enough to be quite
painful to support: the XADDR register here seems to be equivalent to
the Marvell's EXT_SLAVE_ADDR register, yet in the allwinner case, the
offset is 0x4, while it's 0x1c in Marvell's case, which offsets all the
other registers obviously. The status and clock registers in marvell
case looks to be merged together, while they are separate registers
here, etc.

I guess it would make the code much more complicated for such simple
drivers, so I wouldn't push too much for merging, but I guess it's your
call.

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-06-05 21:31     ` Maxime Ripard
@ 2013-06-10 10:08       ` Maxime Ripard
  2013-06-11 18:22         ` Wolfram Sang
  2013-06-11 18:21       ` Wolfram Sang
  1 sibling, 1 reply; 15+ messages in thread
From: Maxime Ripard @ 2013-06-10 10:08 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Wolfram,

On Wed, Jun 05, 2013 at 11:31:44PM +0200, Maxime Ripard wrote:
> I guess it would make the code much more complicated for such simple
> drivers, so I wouldn't push too much for merging, but I guess it's your
> call.

I'd like this to be in 3.11, would merging the driver separately for
this release and merging it with mv64xxx for 3.12 be acceptable to you ?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-06-05 21:31     ` Maxime Ripard
  2013-06-10 10:08       ` Maxime Ripard
@ 2013-06-11 18:21       ` Wolfram Sang
  2013-06-12  8:11         ` Maxime Ripard
  1 sibling, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2013-06-11 18:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jun 05, 2013 at 11:31:44PM +0200, Maxime Ripard wrote:
> Hi Wolfram,
> 
> On Wed, Jun 05, 2013 at 03:39:47PM +0200, Wolfram Sang wrote:
> > 
> > > +#define SUNXI_I2C_ADDR_REG		(0x00)
> > > +#define SUNXI_I2C_ADDR_ADDR(v)			((v & 0x7f) << 1)
> > > +#define SUNXI_I2C_XADDR_REG		(0x04)
> > > +#define SUNXI_I2C_DATA_REG		(0x08)
> > > +#define SUNXI_I2C_CNTR_REG		(0x0c)
> > > +#define SUNXI_I2C_CNTR_ASSERT_ACK		BIT(2)
> > > +#define SUNXI_I2C_CNTR_INT_FLAG			BIT(3)
> > > +#define SUNXI_I2C_CNTR_MASTER_STOP		BIT(4)
> > > +#define SUNXI_I2C_CNTR_MASTER_START		BIT(5)
> > > +#define SUNXI_I2C_CNTR_BUS_ENABLE		BIT(6)
> > > +#define SUNXI_I2C_CNTR_INT_ENABLE		BIT(7)
> > > +#define SUNXI_I2C_STA_REG		(0x10)
> > > +#define SUNXI_I2C_STA_BUS_ERROR			(0x00)
> > > +#define SUNXI_I2C_STA_START			(0x08)
> > > +#define SUNXI_I2C_STA_START_REPEAT		(0x10)
> > > +#define SUNXI_I2C_STA_MASTER_WADDR_ACK		(0x18)
> > > +#define SUNXI_I2C_STA_MASTER_WADDR_NAK		(0x20)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_SENT_ACK	(0x28)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_SENT_NAK	(0x30)
> > > +#define SUNXI_I2C_STA_MASTER_RADDR_ACK		(0x40)
> > > +#define SUNXI_I2C_STA_MASTER_RADDR_NAK		(0x48)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_RECV_ACK	(0x50)
> > > +#define SUNXI_I2C_STA_MASTER_DATA_RECV_NAK	(0x58)
> > > +#define SUNXI_I2C_CCR_REG		(0x14)
> > > +#define SUNXI_I2C_CCR_DIV_N(val)		(val & 0x3)
> > > +#define SUNXI_I2C_CCR_DIV_M(val)		((val & 0xf) << 3)
> > > +#define SUNXI_I2C_SRST_REG		(0x18)
> > > +#define SUNXI_I2C_SRST_RESET			BIT(0)
> > > +#define SUNXI_I2C_EFR_REG		(0x1c)
> > > +#define SUNXI_I2C_LCR_REG		(0x20)
> > > +
> > > +#define SUNXI_I2C_DONE			BIT(0)
> > > +#define SUNXI_I2C_ERROR			BIT(1)
> > > +#define SUNXI_I2C_NAK			BIT(2)
> > > +#define SUNXI_I2C_BUS_ERROR		BIT(3)
> > 
> > The register set looks similar to i2c-mv64xxx.c. Has it been considered
> > to merge the two drivers?
> 
> Hmm, right, the logic seems surprisingly similar. I wasn't aware of such
> a driver, so I didn't put much thought into merging them :)

Too late now, yet grepping for specific register names is a good thing
to do before writing a new driber.

> However, the register layout is quite different enough to be quite
> painful to support: the XADDR register here seems to be equivalent to
> the Marvell's EXT_SLAVE_ADDR register, yet in the allwinner case, the
> offset is 0x4, while it's 0x1c in Marvell's case, which offsets all the
> other registers obviously. The status and clock registers in marvell
> case looks to be merged together, while they are separate registers
> here, etc.
> 
> I guess it would make the code much more complicated for such simple
> drivers, so I wouldn't push too much for merging, but I guess it's your
> call.

The thing is: If there is a bug/errata found in one driver, it goes
unnoticed that it probably should be fixed in the other, too. This is
why sharing logic is a good thing from a maintenance view.

Register offsets are annoying, but one can handle them. There are
examples in the kernel tree. If some registers are totally different
they often can be encapsulated in specific functions processing them.

So, I'd like to ask you if you can prepare a list of what would be
needed to merge this support into mv64xxx? Then I get a better picture
of what way to go. Can you do this?

Thanks,

   Wolfram

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130611/d60fee9d/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-06-10 10:08       ` Maxime Ripard
@ 2013-06-11 18:22         ` Wolfram Sang
  0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2013-06-11 18:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 10, 2013 at 12:08:47PM +0200, Maxime Ripard wrote:
> Hi Wolfram,
> 
> On Wed, Jun 05, 2013 at 11:31:44PM +0200, Maxime Ripard wrote:
> > I guess it would make the code much more complicated for such simple
> > drivers, so I wouldn't push too much for merging, but I guess it's your
> > call.
> 
> I'd like this to be in 3.11, would merging the driver separately for
> this release and merging it with mv64xxx for 3.12 be acceptable to you ?

Thanks for the offer, but the chance of a regression is too high when
switching drivers. Also, configs would need updates then, etc...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130611/e476ea31/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver
  2013-06-11 18:21       ` Wolfram Sang
@ 2013-06-12  8:11         ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-06-12  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Wolfram,

On Tue, Jun 11, 2013 at 08:21:22PM +0200, Wolfram Sang wrote:
> The thing is: If there is a bug/errata found in one driver, it goes
> unnoticed that it probably should be fixed in the other, too. This is
> why sharing logic is a good thing from a maintenance view.

Yes, I definitely understand that.

> Register offsets are annoying, but one can handle them. There are
> examples in the kernel tree. If some registers are totally different
> they often can be encapsulated in specific functions processing them.
> 
> So, I'd like to ask you if you can prepare a list of what would be
> needed to merge this support into mv64xxx? Then I get a better picture
> of what way to go. Can you do this?

I had some time to work on this, I sent you the patches. Except for the
register offset parts, the logic of the driver itself needed no
modifications, so the only "big" change is to pass the register
structure somehow.

Anyway, I just sent to you the patches I had.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-06-12  8:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-27 10:00 [PATCHv3 0/6] Add I2C support for Allwinner SoCs Maxime Ripard
2013-05-27 10:00 ` [PATCHv3 1/6] i2c: sunxi: Add Allwinner A1X i2c driver Maxime Ripard
2013-06-05 13:39   ` Wolfram Sang
2013-06-05 21:31     ` Maxime Ripard
2013-06-10 10:08       ` Maxime Ripard
2013-06-11 18:22         ` Wolfram Sang
2013-06-11 18:21       ` Wolfram Sang
2013-06-12  8:11         ` Maxime Ripard
2013-05-27 10:00 ` [PATCHv3 2/6] ARM: sunxi: dt: Add i2c controller nodes to the DTSI Maxime Ripard
2013-05-27 10:00 ` [PATCHv3 3/6] ARM: sun4i: dt: Add i2c muxing options Maxime Ripard
2013-05-27 10:00 ` [PATCHv3 4/6] ARM: sun5i: " Maxime Ripard
2013-05-27 10:00 ` [PATCHv3 5/6] ARM: sun5i: olinuxino: Enable the i2c controllers Maxime Ripard
2013-05-27 10:00 ` [PATCHv3 6/6] ARM: sun4i: cubieboard: " Maxime Ripard
2013-05-27 19:17 ` [PATCHv3 0/6] Add I2C support for Allwinner SoCs Tomasz Figa
2013-05-29  7:34   ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).