[PATCH] add I2C adapter driver for the netX family of processors

[PATCH] add I2C adapter driver for the netX family of processors

[Juergen Beisert]

Signed-off-by: Juergen Beisert <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d2c5095..6c01f17 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -425,6 +425,16 @@ config I2C_IMX
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-imx.
 
+config I2C_NETX
+	tristate "NETX I2C interface"
+	depends on ARCH_NETX
+	help
+	  Say Y here if you want to use the IIC bus controller on
+	  the Hilscher netX processors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-netx.
+
 config I2C_INTEL_MID
 	tristate "Intel Moorestown/Medfield Platform I2C controller"
 	depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 569567b..acbe444 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_I2C_SH7760)	+= i2c-sh7760.o
 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_NETX)		+= i2c-netx.o
 obj-$(CONFIG_I2C_STU300)	+= i2c-stu300.o
 obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
 obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o
diff --git a/drivers/i2c/busses/i2c-netx.c b/drivers/i2c/busses/i2c-netx.c
new file mode 100644
index 0000000..4a18996
--- /dev/null
+++ b/drivers/i2c/busses/i2c-netx.c
@@ -0,0 +1,422 @@
+/*
+ * Copyright (c) 2012 Juergen Beisert <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>, Pengutronix
+ * On behalf of Hilscher GmbH, http://www.hilscher.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Note: the I2C master hardware on these chips is broken. The silicon cannot
+ * detect any ACK signals.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/i2c.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/netx_iic.h>
+
+#define NETX_I2C_CTRL 0x00
+# define NETX_I2C_CTRL_ENABLE (1 << 0)
+# define NETX_I2C_CTRL_SPEED(x) ((x) << 1)
+# define NETX_I2C_CTRL_SLAVEID(x) (((x) & 0x7f) << 4)
+# define NETX_I2C_CTRL_SLAVEMASK (0x7f << 4)
+
+#define NETX_I2C_DATA 0x04
+# define NETX_I2C_DATA_SEND_STOP (1 << 8)
+# define NETX_I2C_DATA_READ (1 << 9)
+# define NETX_I2C_DATA_WRITE (0 << 9)
+# define NETX_I2C_DATA_SEND_START (1 << 10)
+# define NETX_I2C_DATA_BUSY (1 << 11)
+# define NETX_I2C_DATA_EXECUTE (1 << 11)
+# define NETX_I2C_DATA_RDF (1 << 12)
+
+struct netx_i2c {
+	struct i2c_adapter adapter;
+	unsigned time_idx; /* index into the bt table */
+	void __iomem *regs;
+};
+
+/* time per byte in [µs] at different clock speeds */
+static const unsigned bt[8] = {400, 200, 100, 50, 25, 16, 12, 10};
+
+static int netx_iic_is_busy(void __iomem *regs)
+{
+	u32 val = readl(regs + NETX_I2C_DATA);
+	if (val & NETX_I2C_DATA_BUSY)
+		return 1; /* still busy */
+	return 0;
+}
+
+static void netx_iic_set_slaveid(void __iomem *regs, unsigned sid)
+{
+	u32 reg = readl(regs + NETX_I2C_CTRL) & ~NETX_I2C_CTRL_SLAVEMASK;
+
+	reg |= NETX_I2C_CTRL_SLAVEID(sid);
+	writel(reg, regs + NETX_I2C_CTRL);
+}
+
+static void netx_iic_read_cycle(void __iomem *regs, unsigned iic_flags)
+{
+	u32 val = iic_flags | NETX_I2C_DATA_READ | NETX_I2C_DATA_EXECUTE;
+	writel(val, regs + NETX_I2C_DATA);
+}
+
+static u8 netx_iic_read_byte(void __iomem *regs)
+{
+	return readl(regs + NETX_I2C_DATA) & 0xff;
+}
+
+static void netx_iic_write_byte(void __iomem *regs, unsigned byte,
+							unsigned iic_flags)
+{
+	u32 val = (byte & 0xff) | iic_flags | NETX_I2C_DATA_WRITE |
+							NETX_I2C_DATA_EXECUTE;
+	writel(val, regs + NETX_I2C_DATA);
+}
+
+static void netx_iic_reset_controller(struct netx_i2c *this)
+{
+	writel(0, this->regs + NETX_I2C_CTRL);
+	udelay(5);
+	/* re-enable the controller and clear the bus */
+	writel(NETX_I2C_CTRL_SPEED(this->time_idx) | NETX_I2C_CTRL_ENABLE,
+			this->regs + NETX_I2C_CTRL);
+	/* send some empty clocks to terminate all external statemachines */
+	netx_iic_write_byte(this->regs, 0xff, 0);
+	udelay(bt[this->time_idx]); /* wait, until this byte was sent */
+}
+
+static int netx_iic_wait(struct netx_i2c *this, unsigned cnt)
+{
+	/* we have no interrupt :( */
+	udelay(bt[this->time_idx] * cnt); /* wait the time for 'n' bytes */
+	if (netx_iic_is_busy(this->regs)) { /* still busy? */
+		/* wait a little bit more... */
+		udelay((bt[this->time_idx] * cnt) >> 1);
+		if (netx_iic_is_busy(this->regs)) { /* still busy? */
+			/* no luck */
+			netx_iic_reset_controller(this);
+			return -ETIMEDOUT;
+		}
+	}
+	return 0;
+}
+
+/*
+ * This I2C master controller cannot handle sending the slave address without
+ * a payload byte. Thus, we we need a special handling here to get the address
+ * out.
+ */
+static int netx_iic_start_transfer(struct netx_i2c *this, struct i2c_msg *msg,
+							unsigned iic_flags)
+{
+	int rc;
+
+	iic_flags |= NETX_I2C_DATA_SEND_START;
+
+	netx_iic_set_slaveid(this->regs, msg->addr);
+
+	if (msg->flags & I2C_M_RD) {
+		dev_dbg(&this->adapter.dev,
+			"Send address 0x%02X for reading\n", msg->addr);
+		netx_iic_read_cycle(this->regs, iic_flags);
+	} else {
+		dev_dbg(&this->adapter.dev,
+			"Send address 0x%02X and byte 0x%02X for writing\n",
+			msg->addr, msg->buf[0]);
+		netx_iic_write_byte(this->regs, msg->buf[0], iic_flags);
+	}
+
+	rc = netx_iic_wait(this, 2);	/* addr + data */
+	if (rc < 0) {
+		dev_err(&this->adapter.dev, "Device addressing timeout\n");
+		return rc;
+	}
+
+	if (msg->flags & I2C_M_RD) {
+		msg->buf[0] = netx_iic_read_byte(this->regs);
+		dev_dbg(&this->adapter.dev,
+				"Receiving byte 0x%02X\n", msg->buf[0]);
+	}
+
+	return 0;
+}
+
+/* set 'iic_flags' to 'NETX_I2C_DATA_SEND_STOP' if this is the last message */
+static int netx_iic_read_data_stream(struct netx_i2c *this,
+			struct i2c_msg *msg, int rec, unsigned iic_flags)
+{
+	unsigned u;
+	int rc;
+
+	for (u = rec; u < msg->len; u++) {
+		if (u == (msg->len - 1)) { /* last byte? */
+			netx_iic_read_cycle(this->regs, iic_flags);
+			dev_dbg(&this->adapter.dev, "Last Byte read\n");
+		} else {
+			netx_iic_read_cycle(this->regs, 0);
+		}
+
+		rc = netx_iic_wait(this, 1);
+		if (rc < 0) {
+			dev_dbg(&this->adapter.dev, "Read timeout\n");
+			return rc;
+		}
+
+		msg->buf[u] = netx_iic_read_byte(this->regs);
+		dev_dbg(&this->adapter.dev, "Read Byte %02X\n", msg->buf[u]);
+	}
+
+	return 0;
+}
+
+/* set 'iic_flags' to 'NETX_I2C_DATA_SEND_STOP' if this is the last message */
+static int netx_iic_write_data_stream(struct netx_i2c *this,
+			struct i2c_msg *msg, int sent, unsigned iic_flags)
+{
+	unsigned u;
+	int rc;
+
+	for (u = sent; u < msg->len; u++) {
+		if (u == (msg->len - 1)) { /* last byte? */
+			netx_iic_write_byte(this->regs, msg->buf[u], iic_flags);
+			dev_dbg(&this->adapter.dev,
+				"Last byte '%02X' written\n", msg->buf[u]);
+		} else {
+			netx_iic_write_byte(this->regs, msg->buf[u], 0);
+			dev_dbg(&this->adapter.dev,
+				"byte '%02X' written\n", msg->buf[u]);
+			rc = netx_iic_wait(this, 1);
+			if (rc != 0) {
+				dev_dbg(&this->adapter.dev, "Write timeout\n");
+				return rc;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int netx_iic_data_stream(struct netx_i2c *this, struct i2c_msg *msg,
+					int sent, int last_msg)
+{
+	unsigned iic_flags = last_msg ? NETX_I2C_DATA_SEND_STOP : 0;
+
+	if (msg->flags & I2C_M_RD)
+		return netx_iic_read_data_stream(this, msg, sent, iic_flags);
+
+	return netx_iic_write_data_stream(this, msg, sent, iic_flags);
+}
+
+static int netx_iic_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
+				int msg_cnt)
+{
+	unsigned flags = 0;
+	int i, rc, sent;
+	struct netx_i2c *this = i2c_get_adapdata(adapter);
+
+	dev_dbg(&this->adapter.dev, "Called to transfer %d messages\n",
+								msg_cnt);
+
+	/* walk through the messages list */
+	for (i = 0; i < msg_cnt; msgs++, i++) {
+		/* read/write data in one message */
+		sent = 0;
+		if (!(msgs->flags & I2C_M_NOSTART)) {
+			if (msgs->len < 1) {
+				dev_err(&this->adapter.dev,
+					"Cannot handle messages without data\n");
+				return -EINVAL;
+			}
+
+			if ((msgs->len == 1) && ((i + 1) == msg_cnt))
+				flags |= NETX_I2C_DATA_SEND_STOP;
+
+			rc = netx_iic_start_transfer(this, msgs, flags);
+			if (rc != 0)
+				return rc;
+			sent = 1;
+			/*
+			 * the regular job here would be to wait for the ACK of
+			 * the addressed device. But the ACK detection is broken
+			 * in this I2C master hardware and cannot be used. So,
+			 * we must continue blindly.
+			 */
+		} else
+			dev_dbg(&this->adapter.dev, "Message without START\n");
+
+		if (msgs->len > sent) { /* still remaining data? */
+			rc = netx_iic_data_stream(this, msgs, sent,
+							(i + 1) == msg_cnt);
+			if (rc != 0)
+				return rc;
+		}
+	}
+
+	return i;
+}
+
+static u32 netx_iic_func(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm netx_iic_algo = {
+	.master_xfer = netx_iic_xfer,
+	.functionality = netx_iic_func,
+};
+
+static void __init netx_iic_set_speed(struct netx_i2c *this,
+					struct netx_i2c_pdata *pdata)
+{
+	unsigned speed = ARRAY_SIZE(bt) - 1; /* start with 1 MHz (maximum) */
+
+	if (pdata->speed < 1000)
+		speed--; /* 800 kHz */
+	if (pdata->speed < 800)
+		speed--; /* 600 kHz */
+	if (pdata->speed < 600)
+		speed--; /* 400 kHz */
+	if (pdata->speed < 400)
+		speed--; /* 200 kHz */
+	if (pdata->speed < 200)
+		speed--; /* 100 kHz */
+	if (pdata->speed < 100)
+		speed--; /* 50 kHz */
+	if (pdata->speed < 50)
+		speed--; /* 25 kHz (minimum) */
+
+	dev_dbg(&this->adapter.dev, "Setting up speed to index %u\n", speed);
+
+	writel(NETX_I2C_CTRL_SPEED(speed), this->regs + NETX_I2C_CTRL);
+	this->time_idx = speed;
+	netx_iic_reset_controller(this);
+}
+
+static int __init netx_iic_probe(struct platform_device *pdev)
+{
+	struct netx_i2c *this;
+	struct netx_i2c_pdata *pdata = pdev->dev.platform_data;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	dev_dbg(&pdev->dev, "Probing IIC master device\n");
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "can't get device resources\n");
+		return -ENOENT;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res),
+						NETX_IIC_DRIVER_NAME)) {
+		dev_err(&pdev->dev, "request_mem_region failed\n");
+		return -EBUSY;
+	}
+
+	base = ioremap(res->start, resource_size(res));
+	if (!base) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		ret = -EIO;
+		goto fail1;
+	}
+
+	this = kzalloc(sizeof(struct netx_i2c), GFP_KERNEL);
+	if (!this) {
+		dev_err(&pdev->dev, "can't allocate interface\n");
+		ret = -ENOMEM;
+		goto fail2;
+	}
+
+	/* accumulate all available info */
+	this->regs = base;
+	strcpy(this->adapter.name, pdev->name);
+	this->adapter.owner = THIS_MODULE;
+	this->adapter.algo = &netx_iic_algo;
+	this->adapter.dev.parent  = &pdev->dev;
+	this->adapter.nr = pdev->id;
+	this->adapter.dev.of_node = pdev->dev.of_node; /* TODO */
+
+	i2c_set_adapdata(&this->adapter, this);
+
+	ret = i2c_add_numbered_adapter(&this->adapter);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "registration failed\n");
+		goto fail3;
+	}
+
+	netx_iic_set_speed(this, pdata);
+
+	platform_set_drvdata(pdev, this);
+
+	return 0;
+
+fail3:
+	kfree(this);
+fail2:
+	iounmap(base);
+fail1:
+	release_mem_region(res->start, resource_size(res));
+	return ret;
+}
+
+static int __devexit netx_iic_remove(struct platform_device *pdev)
+{
+	struct netx_i2c *this = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	/* remove adapter */
+	i2c_del_adapter(&this->adapter);
+	platform_set_drvdata(pdev, NULL);
+
+	/* disable the I2C unit */
+	writel(0, this->regs + NETX_I2C_CTRL);
+
+	iounmap(this->regs);
+	release_mem_region(res->start, resource_size(res));
+	kfree(this);
+
+	return 0;
+}
+
+static const struct of_device_id netx_iic_dt_ids[] = {
+	{ .compatible = "hilscher,netx-i2c", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver netx_iic_driver = {
+	.remove = __exit_p(netx_iic_remove),
+	.driver = {
+		.name = NETX_IIC_DRIVER_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = netx_iic_dt_ids,
+	}
+};
+
+static int __init netx_i2c_adap_init(void)
+{
+	return platform_driver_probe(&netx_iic_driver, netx_iic_probe);
+}
+subsys_initcall(netx_i2c_adap_init);
+
+static void __exit netx_i2c_adap_exit(void)
+{
+	platform_driver_unregister(&netx_iic_driver);
+}
+module_exit(netx_i2c_adap_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Juergen Beisert");
+MODULE_DESCRIPTION("I2C adapter driver for netX I2C bus");
+MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/include/linux/platform_data/netx_iic.h b/include/linux/platform_data/netx_iic.h
new file mode 100644
index 0000000..a3e9536
--- /dev/null
+++ b/include/linux/platform_data/netx_iic.h
@@ -0,0 +1,10 @@
+#ifndef PLATFORM_DATA_NETX_IIC_H
+# define PLATFORM_DATA_NETX_IIC_H
+
+struct netx_i2c_pdata {
+	unsigned speed;	/* in [kHz] */
+};
+
+#define NETX_IIC_DRIVER_NAME "netx-iic"
+
+#endif

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

Re: [PATCH] add I2C adapter driver for the netX family of processors

[Wolfram Sang]

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

On Sat, May 26, 2012 at 01:04:38PM +0200, Juergen Beisert wrote:
> Signed-off-by: Juergen Beisert <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index d2c5095..6c01f17 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -425,6 +425,16 @@ config I2C_IMX
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-imx.
>  
> +config I2C_NETX
> +	tristate "NETX I2C interface"
> +	depends on ARCH_NETX
> +	help
> +	  Say Y here if you want to use the IIC bus controller on
> +	  the Hilscher netX processors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-netx.
> +
>  config I2C_INTEL_MID
>  	tristate "Intel Moorestown/Medfield Platform I2C controller"
>  	depends on PCI
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 569567b..acbe444 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_I2C_SH7760)	+= i2c-sh7760.o
>  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_NETX)		+= i2c-netx.o
>  obj-$(CONFIG_I2C_STU300)	+= i2c-stu300.o
>  obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
>  obj-$(CONFIG_I2C_VERSATILE)	+= i2c-versatile.o

Kconfig and Makefile are sorted, please stick to that.

> diff --git a/drivers/i2c/busses/i2c-netx.c b/drivers/i2c/busses/i2c-netx.c
> new file mode 100644
> index 0000000..4a18996
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-netx.c
> @@ -0,0 +1,422 @@
> +/*
> + * Copyright (c) 2012 Juergen Beisert <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>, Pengutronix
> + * On behalf of Hilscher GmbH, http://www.hilscher.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Note: the I2C master hardware on these chips is broken. The silicon cannot
> + * detect any ACK signals.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/i2c.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/netx_iic.h>

I think I'd prefer "linux/i2c/..."

> +
> +#define NETX_I2C_CTRL 0x00
> +# define NETX_I2C_CTRL_ENABLE (1 << 0)
> +# define NETX_I2C_CTRL_SPEED(x) ((x) << 1)
> +# define NETX_I2C_CTRL_SLAVEID(x) (((x) & 0x7f) << 4)
> +# define NETX_I2C_CTRL_SLAVEMASK (0x7f << 4)
> +
> +#define NETX_I2C_DATA 0x04
> +# define NETX_I2C_DATA_SEND_STOP (1 << 8)
> +# define NETX_I2C_DATA_READ (1 << 9)
> +# define NETX_I2C_DATA_WRITE (0 << 9)
> +# define NETX_I2C_DATA_SEND_START (1 << 10)
> +# define NETX_I2C_DATA_BUSY (1 << 11)
> +# define NETX_I2C_DATA_EXECUTE (1 << 11)
> +# define NETX_I2C_DATA_RDF (1 << 12)
> +
> +struct netx_i2c {
> +	struct i2c_adapter adapter;
> +	unsigned time_idx; /* index into the bt table */
> +	void __iomem *regs;
> +};
> +
> +/* time per byte in [µs] at different clock speeds */

Please '[us]' to avoid unnecessary encoding issues.

> +static const unsigned bt[8] = {400, 200, 100, 50, 25, 16, 12, 10};
> +
> +static int netx_iic_is_busy(void __iomem *regs)
> +{
> +	u32 val = readl(regs + NETX_I2C_DATA);

Newline please.

> +	if (val & NETX_I2C_DATA_BUSY)
> +		return 1; /* still busy */
> +	return 0;

return !!(val & NETX_I2C_DATA_BUSY)?

> +}
> +
> +static void netx_iic_set_slaveid(void __iomem *regs, unsigned sid)
> +{
> +	u32 reg = readl(regs + NETX_I2C_CTRL) & ~NETX_I2C_CTRL_SLAVEMASK;
> +
> +	reg |= NETX_I2C_CTRL_SLAVEID(sid);
> +	writel(reg, regs + NETX_I2C_CTRL);
> +}
> +
> +static void netx_iic_read_cycle(void __iomem *regs, unsigned iic_flags)
> +{
> +	u32 val = iic_flags | NETX_I2C_DATA_READ | NETX_I2C_DATA_EXECUTE;

newline

> +	writel(val, regs + NETX_I2C_DATA);
> +}
> +
> +static u8 netx_iic_read_byte(void __iomem *regs)
> +{
> +	return readl(regs + NETX_I2C_DATA) & 0xff;
> +}
> +
> +static void netx_iic_write_byte(void __iomem *regs, unsigned byte,
> +							unsigned iic_flags)
> +{

Couldn't 'byte' be u8 instead of unsigned?

> +	u32 val = (byte & 0xff) | iic_flags | NETX_I2C_DATA_WRITE |
> +							NETX_I2C_DATA_EXECUTE;
> +	writel(val, regs + NETX_I2C_DATA);
> +}
> +
> +static void netx_iic_reset_controller(struct netx_i2c *this)
> +{
> +	writel(0, this->regs + NETX_I2C_CTRL);
> +	udelay(5);
> +	/* re-enable the controller and clear the bus */
> +	writel(NETX_I2C_CTRL_SPEED(this->time_idx) | NETX_I2C_CTRL_ENABLE,
> +			this->regs + NETX_I2C_CTRL);
> +	/* send some empty clocks to terminate all external statemachines */
> +	netx_iic_write_byte(this->regs, 0xff, 0);
> +	udelay(bt[this->time_idx]); /* wait, until this byte was sent */
> +}
> +
> +static int netx_iic_wait(struct netx_i2c *this, unsigned cnt)
> +{
> +	/* we have no interrupt :( */
> +	udelay(bt[this->time_idx] * cnt); /* wait the time for 'n' bytes */
> +	if (netx_iic_is_busy(this->regs)) { /* still busy? */
> +		/* wait a little bit more... */
> +		udelay((bt[this->time_idx] * cnt) >> 1);
> +		if (netx_iic_is_busy(this->regs)) { /* still busy? */
> +			/* no luck */
> +			netx_iic_reset_controller(this);
> +			return -ETIMEDOUT;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/*
> + * This I2C master controller cannot handle sending the slave address without
> + * a payload byte. Thus, we we need a special handling here to get the address
> + * out.
> + */
> +static int netx_iic_start_transfer(struct netx_i2c *this, struct i2c_msg *msg,
> +							unsigned iic_flags)
> +{
> +	int rc;
> +
> +	iic_flags |= NETX_I2C_DATA_SEND_START;
> +
> +	netx_iic_set_slaveid(this->regs, msg->addr);
> +
> +	if (msg->flags & I2C_M_RD) {
> +		dev_dbg(&this->adapter.dev,
> +			"Send address 0x%02X for reading\n", msg->addr);
> +		netx_iic_read_cycle(this->regs, iic_flags);
> +	} else {
> +		dev_dbg(&this->adapter.dev,
> +			"Send address 0x%02X and byte 0x%02X for writing\n",
> +			msg->addr, msg->buf[0]);
> +		netx_iic_write_byte(this->regs, msg->buf[0], iic_flags);
> +	}
> +
> +	rc = netx_iic_wait(this, 2);	/* addr + data */
> +	if (rc < 0) {
> +		dev_err(&this->adapter.dev, "Device addressing timeout\n");
> +		return rc;
> +	}
> +
> +	if (msg->flags & I2C_M_RD) {
> +		msg->buf[0] = netx_iic_read_byte(this->regs);
> +		dev_dbg(&this->adapter.dev,
> +				"Receiving byte 0x%02X\n", msg->buf[0]);
> +	}
> +
> +	return 0;
> +}
> +
> +/* set 'iic_flags' to 'NETX_I2C_DATA_SEND_STOP' if this is the last message */
> +static int netx_iic_read_data_stream(struct netx_i2c *this,
> +			struct i2c_msg *msg, int rec, unsigned iic_flags)
> +{
> +	unsigned u;
> +	int rc;
> +
> +	for (u = rec; u < msg->len; u++) {
> +		if (u == (msg->len - 1)) { /* last byte? */
> +			netx_iic_read_cycle(this->regs, iic_flags);
> +			dev_dbg(&this->adapter.dev, "Last Byte read\n");
> +		} else {
> +			netx_iic_read_cycle(this->regs, 0);
> +		}
> +
> +		rc = netx_iic_wait(this, 1);
> +		if (rc < 0) {
> +			dev_dbg(&this->adapter.dev, "Read timeout\n");
> +			return rc;
> +		}
> +
> +		msg->buf[u] = netx_iic_read_byte(this->regs);
> +		dev_dbg(&this->adapter.dev, "Read Byte %02X\n", msg->buf[u]);
> +	}
> +
> +	return 0;
> +}
> +
> +/* set 'iic_flags' to 'NETX_I2C_DATA_SEND_STOP' if this is the last message */
> +static int netx_iic_write_data_stream(struct netx_i2c *this,
> +			struct i2c_msg *msg, int sent, unsigned iic_flags)
> +{
> +	unsigned u;
> +	int rc;
> +
> +	for (u = sent; u < msg->len; u++) {
> +		if (u == (msg->len - 1)) { /* last byte? */
> +			netx_iic_write_byte(this->regs, msg->buf[u], iic_flags);
> +			dev_dbg(&this->adapter.dev,
> +				"Last byte '%02X' written\n", msg->buf[u]);
> +		} else {
> +			netx_iic_write_byte(this->regs, msg->buf[u], 0);
> +			dev_dbg(&this->adapter.dev,
> +				"byte '%02X' written\n", msg->buf[u]);
> +			rc = netx_iic_wait(this, 1);
> +			if (rc != 0) {

if (netx_iic_wait(this, 1)) ?
Would save the rc-variable and still be readable.

> +				dev_dbg(&this->adapter.dev, "Write timeout\n");
> +				return rc;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int netx_iic_data_stream(struct netx_i2c *this, struct i2c_msg *msg,
> +					int sent, int last_msg)
> +{
> +	unsigned iic_flags = last_msg ? NETX_I2C_DATA_SEND_STOP : 0;
> +
> +	if (msg->flags & I2C_M_RD)
> +		return netx_iic_read_data_stream(this, msg, sent, iic_flags);
> +
> +	return netx_iic_write_data_stream(this, msg, sent, iic_flags);
> +}
> +
> +static int netx_iic_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
> +				int msg_cnt)
> +{
> +	unsigned flags = 0;
> +	int i, rc, sent;
> +	struct netx_i2c *this = i2c_get_adapdata(adapter);
> +
> +	dev_dbg(&this->adapter.dev, "Called to transfer %d messages\n",
> +								msg_cnt);

Drop this. There is a similar debug message in the core which can be
activated via Kconfig.

> +
> +	/* walk through the messages list */
> +	for (i = 0; i < msg_cnt; msgs++, i++) {
> +		/* read/write data in one message */
> +		sent = 0;
> +		if (!(msgs->flags & I2C_M_NOSTART)) {
> +			if (msgs->len < 1) {
> +				dev_err(&this->adapter.dev,
> +					"Cannot handle messages without data\n");
> +				return -EINVAL;
> +			}
> +
> +			if ((msgs->len == 1) && ((i + 1) == msg_cnt))
> +				flags |= NETX_I2C_DATA_SEND_STOP;
> +
> +			rc = netx_iic_start_transfer(this, msgs, flags);
> +			if (rc != 0)
> +				return rc;
> +			sent = 1;
> +			/*
> +			 * the regular job here would be to wait for the ACK of
> +			 * the addressed device. But the ACK detection is broken
> +			 * in this I2C master hardware and cannot be used. So,
> +			 * we must continue blindly.
> +			 */
> +		} else
> +			dev_dbg(&this->adapter.dev, "Message without START\n");
> +
> +		if (msgs->len > sent) { /* still remaining data? */
> +			rc = netx_iic_data_stream(this, msgs, sent,
> +							(i + 1) == msg_cnt);
> +			if (rc != 0)
> +				return rc;
> +		}
> +	}
> +
> +	return i;
> +}
> +
> +static u32 netx_iic_func(struct i2c_adapter *adapter)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static struct i2c_algorithm netx_iic_algo = {
> +	.master_xfer = netx_iic_xfer,
> +	.functionality = netx_iic_func,
> +};
> +
> +static void __init netx_iic_set_speed(struct netx_i2c *this,
> +					struct netx_i2c_pdata *pdata)
> +{
> +	unsigned speed = ARRAY_SIZE(bt) - 1; /* start with 1 MHz (maximum) */
> +
> +	if (pdata->speed < 1000)
> +		speed--; /* 800 kHz */
> +	if (pdata->speed < 800)
> +		speed--; /* 600 kHz */
> +	if (pdata->speed < 600)
> +		speed--; /* 400 kHz */
> +	if (pdata->speed < 400)
> +		speed--; /* 200 kHz */
> +	if (pdata->speed < 200)
> +		speed--; /* 100 kHz */
> +	if (pdata->speed < 100)
> +		speed--; /* 50 kHz */
> +	if (pdata->speed < 50)
> +		speed--; /* 25 kHz (minimum) */
> +
> +	dev_dbg(&this->adapter.dev, "Setting up speed to index %u\n", speed);
> +
> +	writel(NETX_I2C_CTRL_SPEED(speed), this->regs + NETX_I2C_CTRL);
> +	this->time_idx = speed;
> +	netx_iic_reset_controller(this);
> +}
> +
> +static int __init netx_iic_probe(struct platform_device *pdev)
> +{
> +	struct netx_i2c *this;
> +	struct netx_i2c_pdata *pdata = pdev->dev.platform_data;
> +	struct resource *res;
> +	void __iomem *base;
> +	int ret;
> +
> +	dev_dbg(&pdev->dev, "Probing IIC master device\n");

There is similar debug output in the driver core.

> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "can't get device resources\n");
> +		return -ENOENT;
> +	}
> +
> +	if (!request_mem_region(res->start, resource_size(res),
> +						NETX_IIC_DRIVER_NAME)) {
> +		dev_err(&pdev->dev, "request_mem_region failed\n");
> +		return -EBUSY;
> +	}
> +
> +	base = ioremap(res->start, resource_size(res));
> +	if (!base) {
> +		dev_err(&pdev->dev, "ioremap failed\n");
> +		ret = -EIO;
> +		goto fail1;
> +	}

If you use devm_request_and_ioremap() here...

> +
> +	this = kzalloc(sizeof(struct netx_i2c), GFP_KERNEL);
> +	if (!this) {
> +		dev_err(&pdev->dev, "can't allocate interface\n");
> +		ret = -ENOMEM;
> +		goto fail2;
> +	}

... and devm_kzalloc here, you can remove all error paths from this
function and heavily simplify remove().

> +
> +	/* accumulate all available info */
> +	this->regs = base;
> +	strcpy(this->adapter.name, pdev->name);
> +	this->adapter.owner = THIS_MODULE;
> +	this->adapter.algo = &netx_iic_algo;
> +	this->adapter.dev.parent  = &pdev->dev;
> +	this->adapter.nr = pdev->id;
> +	this->adapter.dev.of_node = pdev->dev.of_node; /* TODO */
> +
> +	i2c_set_adapdata(&this->adapter, this);
> +
> +	ret = i2c_add_numbered_adapter(&this->adapter);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "registration failed\n");
> +		goto fail3;
> +	}
> +
> +	netx_iic_set_speed(this, pdata);
> +
> +	platform_set_drvdata(pdev, this);
> +
> +	return 0;
> +
> +fail3:
> +	kfree(this);
> +fail2:
> +	iounmap(base);
> +fail1:
> +	release_mem_region(res->start, resource_size(res));
> +	return ret;
> +}
> +
> +static int __devexit netx_iic_remove(struct platform_device *pdev)
> +{
> +	struct netx_i2c *this = platform_get_drvdata(pdev);
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	/* remove adapter */
> +	i2c_del_adapter(&this->adapter);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	/* disable the I2C unit */
> +	writel(0, this->regs + NETX_I2C_CTRL);
> +
> +	iounmap(this->regs);
> +	release_mem_region(res->start, resource_size(res));
> +	kfree(this);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id netx_iic_dt_ids[] = {
> +	{ .compatible = "hilscher,netx-i2c", },
> +	{ /* sentinel */ }
> +};

Please skip this. This platform has no DT support yet, and I prefer to
add it later when it gets added, so it has been tested then.

> +
> +static struct platform_driver netx_iic_driver = {
> +	.remove = __exit_p(netx_iic_remove),

__devexit_p!

> +	.driver = {
> +		.name = NETX_IIC_DRIVER_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = netx_iic_dt_ids,
> +	}
> +};
> +
> +static int __init netx_i2c_adap_init(void)
> +{
> +	return platform_driver_probe(&netx_iic_driver, netx_iic_probe);
> +}
> +subsys_initcall(netx_i2c_adap_init);
> +
> +static void __exit netx_i2c_adap_exit(void)
> +{
> +	platform_driver_unregister(&netx_iic_driver);
> +}
> +module_exit(netx_i2c_adap_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Juergen Beisert");
> +MODULE_DESCRIPTION("I2C adapter driver for netX I2C bus");
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/include/linux/platform_data/netx_iic.h b/include/linux/platform_data/netx_iic.h
> new file mode 100644
> index 0000000..a3e9536
> --- /dev/null
> +++ b/include/linux/platform_data/netx_iic.h
> @@ -0,0 +1,10 @@
> +#ifndef PLATFORM_DATA_NETX_IIC_H
> +# define PLATFORM_DATA_NETX_IIC_H
> +
> +struct netx_i2c_pdata {
> +	unsigned speed;	/* in [kHz] */
> +};
> +
> +#define NETX_IIC_DRIVER_NAME "netx-iic"
> +
> +#endif

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

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