Patchwork [1/5] i2c: sunxi: Add Allwinner A1X i2c driver

login
register
mail settings
Submitter Maxime Ripard
Date May 3, 2013, 9:17 a.m.
Message ID <1367572669-24354-2-git-send-email-maxime.ripard@free-electrons.com>
Download mbox | patch
Permalink /patch/241253/
State Superseded
Headers show

Comments

Maxime Ripard - May 3, 2013, 9:17 a.m.
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                     | 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
Tomasz Figa - May 25, 2013, 12:06 p.m.
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 <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                     | 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 <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/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 <maxime.ripard@free-electrons.com");
> +MODULE_DESCRIPTION("Allwinner A1X I2C bus adapter");
> +MODULE_LICENSE("GPL");
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard - May 25, 2013, 3:22 p.m.
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

Patch

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 <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/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 <maxime.ripard@free-electrons.com");
+MODULE_DESCRIPTION("Allwinner A1X I2C bus adapter");
+MODULE_LICENSE("GPL");