2013-05-03 09:18:13

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH 1/5] i2c: sunxi: Add Allwinner A1X i2c driver

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 <[email protected]>
---
.../devicetree/bindings/i2c/i2c-sunxi.txt | 19 +
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-sunxi.c | 441 +++++++++++++++++++++
4 files changed, 471 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@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 adfee98..327a49b 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..f9f8bd4
--- /dev/null
+++ b/drivers/i2c/busses/i2c-sunxi.c
@@ -0,0 +1,441 @@
+/*
+ * Allwinner A1X SoCs i2c controller driver.
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * 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/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_address.h>
+#include <linux/of_irq.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;
+ unsigned 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, u8 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.
+ */
+ case SUNXI_I2C_STA_MASTER_WADDR_NAK:
+ /*
+ * Data byte has been transmitted, ACK has not been
+ * received
+ */
+ 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
+ */
+ 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
+ */
+ 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
+ */
+ 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;
+ 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);
+
+ i2c_dev->membase = of_iomap(np, 0);
+ if (!i2c_dev->membase)
+ return -EADDRNOTAVAIL;
+
+ sunxi_i2c_write(i2c_dev, SUNXI_I2C_SRST_REG, SUNXI_I2C_SRST_RESET);
+
+ i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(i2c_dev->clk)) {
+ ret = PTR_ERR(i2c_dev->clk);
+ goto out_iounmap;
+ }
+ clk_prepare_enable(i2c_dev->clk);
+
+ 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 = irq_of_parse_and_map(np, 0);
+ if (!i2c_dev->irq) {
+ dev_err(&pdev->dev, "No IRQ resource\n");
+ ret = -ENODEV;
+ 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);
+out_iounmap:
+ iounmap(i2c_dev->membase);
+ 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);
+ iounmap(i2c_dev->membase);
+
+ 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 <[email protected]");
+MODULE_DESCRIPTION("Allwinner A1X I2C bus adapter");
+MODULE_LICENSE("GPL");
--
1.8.1.2


2013-05-25 12:07:00

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 1/5] i2c: sunxi: Add Allwinner A1X i2c driver

Hi Maxime,

Overall the driver looks good, just some minor nitpicks inline.

On Friday 03 of May 2013 11:17:45 Maxime Ripard wrote:
> 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 <[email protected]>
> ---
> .../devicetree/bindings/i2c/i2c-sunxi.txt | 19 +
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-sunxi.c | 441
> +++++++++++++++++++++ 4 files changed, 471 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@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 adfee98..327a49b 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..f9f8bd4
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-sunxi.c
> @@ -0,0 +1,441 @@
> +/*
> + * Allwinner A1X SoCs i2c controller driver.
> + *
> + * Copyright (C) 2013 Maxime Ripard
> + *
> + * Maxime Ripard <[email protected]>
> + *
> + * 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/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_address.h>
> +#include <linux/of_irq.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;
> + unsigned 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, u8
> 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.
> + */
> + case SUNXI_I2C_STA_MASTER_WADDR_NAK:
> + /*
> + * Data byte has been transmitted, ACK has not been
> + * received
> + */
> + 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;
> + }

A comment here that the fall-through is intentional would be nice.

> +
> + /*
> + * Address + Write bit have been transmitted, ACK has been
> + * received
> + */
> + 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
> + */
> + 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;
> + }

Here as well.

> +
> + /*
> + * 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
> + */
> + 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;
> + }

Ditto.

> +
> + /* 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;
> + 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);
> +
> + i2c_dev->membase = of_iomap(np, 0);

What about calling platform_get_resource() and devm_ioremap_resource()
here? This would make the mapping managed and eliminate the need to unmap
it manually.

> + if (!i2c_dev->membase)
> + return -EADDRNOTAVAIL;
> +
> + sunxi_i2c_write(i2c_dev, SUNXI_I2C_SRST_REG,
SUNXI_I2C_SRST_RESET);

Hmm, is it really correct to write to device registers before enabling its
clock?

> +
> + i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(i2c_dev->clk)) {
> + ret = PTR_ERR(i2c_dev->clk);
> + goto out_iounmap;
> + }
> + clk_prepare_enable(i2c_dev->clk);
> +
> + 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 = irq_of_parse_and_map(np, 0);

IMHO platform_get_irq() would be enough here.

Best regards,
Tomasz

> + if (!i2c_dev->irq) {
> + dev_err(&pdev->dev, "No IRQ resource\n");
> + ret = -ENODEV;
> + 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);
> +out_iounmap:
> + iounmap(i2c_dev->membase);
> + 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);
> + iounmap(i2c_dev->membase);
> +
> + 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 <[email protected]");
> +MODULE_DESCRIPTION("Allwinner A1X I2C bus adapter");
> +MODULE_LICENSE("GPL");

2013-05-25 15:22:37

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/5] i2c: sunxi: Add Allwinner A1X i2c driver

Hi Tomasz,

On Sat, May 25, 2013 at 02:06:51PM +0200, Tomasz Figa wrote:
> Hi Maxime,
>
> Overall the driver looks good, just some minor nitpicks inline.

Thanks for the review!

I'll fix the comments you had and send a new version.

Maxime

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