[PATCH v4 0/4] AT91: replace broken TWI driver i2c-at91.c

[PATCH v4 0/4] AT91: replace broken TWI driver i2c-at91.c

[Nikolaus Voss]

The old driver has two main deficencies:
i)  No repeated start (Sr) condiction is possible, this makes it unusable
    e.g. for most SMBus transfers.
ii) I/O was done with polling/busy waiting what caused over-/underruns
    even at light system loads and clock speeds.

The new driver overcomes these deficencies and in addition allows for
more than one TWI interface.

A remaining limitation is the fact, that only one repeated start is
possible (two concatenated messages). This limitation is imposed by
the hardware. However, this should not be a problem as all common
i2c-client communication does not rely on more than one repeated start.

v4: Integrated more review comments from Ryan Mallon and Felipe Balbi:
    Moved register include file to local include, code cleanups
v3: Integrated review comments from Ryan Mallon and Felipe Balbi
v2: Fixed whitespace issue

Nikolaus Voss (4):
  drivers/i2c/busses/i2c-at91.c: remove broken driver
  drivers/i2c/busses/i2c-at91.c: add new driver
  G45 TWI: remove open drain setting for twi function gpios
  Add lookup entries for twi_clk for devices with more than one TWI
    port

 arch/arm/mach-at91/at91sam9g45.c           |    2 +
 arch/arm/mach-at91/at91sam9g45_devices.c   |    6 -
 arch/arm/mach-at91/at91sam9rl.c            |    2 +
 arch/arm/mach-at91/include/mach/at91_twi.h |   68 ----
 drivers/i2c/busses/Kconfig                 |   11 +-
 drivers/i2c/busses/i2c-at91.c              |  476 +++++++++++++++++-----------
 drivers/i2c/busses/i2c-at91.h              |   80 +++++
 7 files changed, 371 insertions(+), 274 deletions(-)
 delete mode 100644 arch/arm/mach-at91/include/mach/at91_twi.h
 create mode 100644 drivers/i2c/busses/i2c-at91.h

-- 
1.7.5.4

[PATCH v4 0/4] AT91: replace broken TWI driver i2c-at91.c

[Nikolaus Voss]

The old driver has two main deficencies:
i)  No repeated start (Sr) condiction is possible, this makes it unusable
    e.g. for most SMBus transfers.
ii) I/O was done with polling/busy waiting what caused over-/underruns
    even at light system loads and clock speeds.

The new driver overcomes these deficencies and in addition allows for
more than one TWI interface.

A remaining limitation is the fact, that only one repeated start is
possible (two concatenated messages). This limitation is imposed by
the hardware. However, this should not be a problem as all common
i2c-client communication does not rely on more than one repeated start.

v4: Integrated more review comments from Ryan Mallon and Felipe Balbi:
    Moved register include file to local include, code cleanups
v3: Integrated review comments from Ryan Mallon and Felipe Balbi
v2: Fixed whitespace issue

Nikolaus Voss (4):
  drivers/i2c/busses/i2c-at91.c: remove broken driver
  drivers/i2c/busses/i2c-at91.c: add new driver
  G45 TWI: remove open drain setting for twi function gpios
  Add lookup entries for twi_clk for devices with more than one TWI
    port

 arch/arm/mach-at91/at91sam9g45.c           |    2 +
 arch/arm/mach-at91/at91sam9g45_devices.c   |    6 -
 arch/arm/mach-at91/at91sam9rl.c            |    2 +
 arch/arm/mach-at91/include/mach/at91_twi.h |   68 ----
 drivers/i2c/busses/Kconfig                 |   11 +-
 drivers/i2c/busses/i2c-at91.c              |  476 +++++++++++++++++-----------
 drivers/i2c/busses/i2c-at91.h              |   80 +++++
 7 files changed, 371 insertions(+), 274 deletions(-)
 delete mode 100644 arch/arm/mach-at91/include/mach/at91_twi.h
 create mode 100644 drivers/i2c/busses/i2c-at91.h

-- 
1.7.5.4

[PATCH v4 1/4] drivers/i2c/busses/i2c-at91.c: remove broken driver

[Nikolaus Voss]

Signed-off-by: Nikolaus Voss <n.voss-+umVssTZoCsb1SvskN2V4Q@public.gmane.org>
---
 arch/arm/mach-at91/include/mach/at91_twi.h |   68 ------
 drivers/i2c/busses/Makefile                |    1 -
 drivers/i2c/busses/i2c-at91.c              |  327 ----------------------------
 3 files changed, 0 insertions(+), 396 deletions(-)
 delete mode 100644 arch/arm/mach-at91/include/mach/at91_twi.h
 delete mode 100644 drivers/i2c/busses/i2c-at91.c

diff --git a/arch/arm/mach-at91/include/mach/at91_twi.h b/arch/arm/mach-at91/include/mach/at91_twi.h
deleted file mode 100644
index bb2880f..0000000
--- a/arch/arm/mach-at91/include/mach/at91_twi.h
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * arch/arm/mach-at91/include/mach/at91_twi.h
- *
- * Copyright (C) 2005 Ivan Kokshaysky
- * Copyright (C) SAN People
- *
- * Two-wire Interface (TWI) registers.
- * Based on AT91RM9200 datasheet revision E.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef AT91_TWI_H
-#define AT91_TWI_H
-
-#define	AT91_TWI_CR		0x00		/* Control Register */
-#define		AT91_TWI_START		(1 <<  0)	/* Send a Start Condition */
-#define		AT91_TWI_STOP		(1 <<  1)	/* Send a Stop Condition */
-#define		AT91_TWI_MSEN		(1 <<  2)	/* Master Transfer Enable */
-#define		AT91_TWI_MSDIS		(1 <<  3)	/* Master Transfer Disable */
-#define		AT91_TWI_SVEN		(1 <<  4)	/* Slave Transfer Enable [SAM9260 only] */
-#define		AT91_TWI_SVDIS		(1 <<  5)	/* Slave Transfer Disable [SAM9260 only] */
-#define		AT91_TWI_SWRST		(1 <<  7)	/* Software Reset */
-
-#define	AT91_TWI_MMR		0x04		/* Master Mode Register */
-#define		AT91_TWI_IADRSZ		(3    <<  8)	/* Internal Device Address Size */
-#define			AT91_TWI_IADRSZ_NO		(0 << 8)
-#define			AT91_TWI_IADRSZ_1		(1 << 8)
-#define			AT91_TWI_IADRSZ_2		(2 << 8)
-#define			AT91_TWI_IADRSZ_3		(3 << 8)
-#define		AT91_TWI_MREAD		(1    << 12)	/* Master Read Direction */
-#define		AT91_TWI_DADR		(0x7f << 16)	/* Device Address */
-
-#define	AT91_TWI_SMR		0x08		/* Slave Mode Register [SAM9260 only] */
-#define		AT91_TWI_SADR		(0x7f << 16)	/* Slave Address */
-
-#define	AT91_TWI_IADR		0x0c		/* Internal Address Register */
-
-#define	AT91_TWI_CWGR		0x10		/* Clock Waveform Generator Register */
-#define		AT91_TWI_CLDIV		(0xff <<  0)	/* Clock Low Divisor */
-#define		AT91_TWI_CHDIV		(0xff <<  8)	/* Clock High Divisor */
-#define		AT91_TWI_CKDIV		(7    << 16)	/* Clock Divider */
-
-#define	AT91_TWI_SR		0x20		/* Status Register */
-#define		AT91_TWI_TXCOMP		(1 <<  0)	/* Transmission Complete */
-#define		AT91_TWI_RXRDY		(1 <<  1)	/* Receive Holding Register Ready */
-#define		AT91_TWI_TXRDY		(1 <<  2)	/* Transmit Holding Register Ready */
-#define		AT91_TWI_SVREAD		(1 <<  3)	/* Slave Read [SAM9260 only] */
-#define		AT91_TWI_SVACC		(1 <<  4)	/* Slave Access [SAM9260 only] */
-#define		AT91_TWI_GACC		(1 <<  5)	/* General Call Access [SAM9260 only] */
-#define		AT91_TWI_OVRE		(1 <<  6)	/* Overrun Error [AT91RM9200 only] */
-#define		AT91_TWI_UNRE		(1 <<  7)	/* Underrun Error [AT91RM9200 only] */
-#define		AT91_TWI_NACK		(1 <<  8)	/* Not Acknowledged */
-#define		AT91_TWI_ARBLST		(1 <<  9)	/* Arbitration Lost [SAM9260 only] */
-#define		AT91_TWI_SCLWS		(1 << 10)	/* Clock Wait State [SAM9260 only] */
-#define		AT91_TWI_EOSACC		(1 << 11)	/* End of Slave Address [SAM9260 only] */
-
-#define	AT91_TWI_IER		0x24		/* Interrupt Enable Register */
-#define	AT91_TWI_IDR		0x28		/* Interrupt Disable Register */
-#define	AT91_TWI_IMR		0x2c		/* Interrupt Mask Register */
-#define	AT91_TWI_RHR		0x30		/* Receive Holding Register */
-#define	AT91_TWI_THR		0x34		/* Transmit Holding Register */
-
-#endif
-
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index fba6da6..e8a1852 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -28,7 +28,6 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 
 # Embedded system I2C/SMBus host controller drivers
-obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
 obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
deleted file mode 100644
index 305c075..0000000
--- a/drivers/i2c/busses/i2c-at91.c
+++ /dev/null
@@ -1,327 +0,0 @@
-/*
-    i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
-
-    Copyright (C) 2004 Rick Bronson
-    Converted to 2.6 by Andrew Victor <andrew-eS41wJS13H5l57MIdRCFDg@public.gmane.org>
-
-    Borrowed heavily from original work by:
-    Copyright (C) 2000 Philip Edelbrock <phil-LT64U7CwzWEZMC/lefXSXFaTQe2KTcn/@public.gmane.org>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-*/
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/err.h>
-#include <linux/slab.h>
-#include <linux/types.h>
-#include <linux/delay.h>
-#include <linux/i2c.h>
-#include <linux/init.h>
-#include <linux/clk.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-
-#include <mach/at91_twi.h>
-#include <mach/board.h>
-#include <mach/cpu.h>
-
-#define TWI_CLOCK		100000		/* Hz. max 400 Kbits/sec */
-
-
-static struct clk *twi_clk;
-static void __iomem *twi_base;
-
-#define at91_twi_read(reg)		__raw_readl(twi_base + (reg))
-#define at91_twi_write(reg, val)	__raw_writel((val), twi_base + (reg))
-
-
-/*
- * Initialize the TWI hardware registers.
- */
-static void __devinit at91_twi_hwinit(void)
-{
-	unsigned long cdiv, ckdiv;
-
-	at91_twi_write(AT91_TWI_IDR, 0xffffffff);	/* Disable all interrupts */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_SWRST);	/* Reset peripheral */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_MSEN);	/* Set Master mode */
-
-	/* Calcuate clock dividers */
-	cdiv = (clk_get_rate(twi_clk) / (2 * TWI_CLOCK)) - 3;
-	cdiv = cdiv + 1;	/* round up */
-	ckdiv = 0;
-	while (cdiv > 255) {
-		ckdiv++;
-		cdiv = cdiv >> 1;
-	}
-
-	if (cpu_is_at91rm9200()) {			/* AT91RM9200 Errata #22 */
-		if (ckdiv > 5) {
-			printk(KERN_ERR "AT91 I2C: Invalid TWI_CLOCK value!\n");
-			ckdiv = 5;
-		}
-	}
-
-	at91_twi_write(AT91_TWI_CWGR, (ckdiv << 16) | (cdiv << 8) | cdiv);
-}
-
-/*
- * Poll the i2c status register until the specified bit is set.
- * Returns 0 if timed out (100 msec).
- */
-static short at91_poll_status(unsigned long bit)
-{
-	int loop_cntr = 10000;
-
-	do {
-		udelay(10);
-	} while (!(at91_twi_read(AT91_TWI_SR) & bit) && (--loop_cntr > 0));
-
-	return (loop_cntr > 0);
-}
-
-static int xfer_read(struct i2c_adapter *adap, unsigned char *buf, int length)
-{
-	/* Send Start */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_START);
-
-	/* Read data */
-	while (length--) {
-		if (!length)	/* need to send Stop before reading last byte */
-			at91_twi_write(AT91_TWI_CR, AT91_TWI_STOP);
-		if (!at91_poll_status(AT91_TWI_RXRDY)) {
-			dev_dbg(&adap->dev, "RXRDY timeout\n");
-			return -ETIMEDOUT;
-		}
-		*buf++ = (at91_twi_read(AT91_TWI_RHR) & 0xff);
-	}
-
-	return 0;
-}
-
-static int xfer_write(struct i2c_adapter *adap, unsigned char *buf, int length)
-{
-	/* Load first byte into transmitter */
-	at91_twi_write(AT91_TWI_THR, *buf++);
-
-	/* Send Start */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_START);
-
-	do {
-		if (!at91_poll_status(AT91_TWI_TXRDY)) {
-			dev_dbg(&adap->dev, "TXRDY timeout\n");
-			return -ETIMEDOUT;
-		}
-
-		length--;	/* byte was transmitted */
-
-		if (length > 0)		/* more data to send? */
-			at91_twi_write(AT91_TWI_THR, *buf++);
-	} while (length);
-
-	/* Send Stop */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_STOP);
-
-	return 0;
-}
-
-/*
- * Generic i2c master transfer entrypoint.
- *
- * Note: We do not use Atmel's feature of storing the "internal device address".
- * Instead the "internal device address" has to be written using a separate
- * i2c message.
- * http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2004-September/024411.html
- */
-static int at91_xfer(struct i2c_adapter *adap, struct i2c_msg *pmsg, int num)
-{
-	int i, ret;
-
-	dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num);
-
-	for (i = 0; i < num; i++) {
-		dev_dbg(&adap->dev, " #%d: %sing %d byte%s %s 0x%02x\n", i,
-			pmsg->flags & I2C_M_RD ? "read" : "writ",
-			pmsg->len, pmsg->len > 1 ? "s" : "",
-			pmsg->flags & I2C_M_RD ? "from" : "to",	pmsg->addr);
-
-		at91_twi_write(AT91_TWI_MMR, (pmsg->addr << 16)
-			| ((pmsg->flags & I2C_M_RD) ? AT91_TWI_MREAD : 0));
-
-		if (pmsg->len && pmsg->buf) {	/* sanity check */
-			if (pmsg->flags & I2C_M_RD)
-				ret = xfer_read(adap, pmsg->buf, pmsg->len);
-			else
-				ret = xfer_write(adap, pmsg->buf, pmsg->len);
-
-			if (ret)
-				return ret;
-
-			/* Wait until transfer is finished */
-			if (!at91_poll_status(AT91_TWI_TXCOMP)) {
-				dev_dbg(&adap->dev, "TXCOMP timeout\n");
-				return -ETIMEDOUT;
-			}
-		}
-		dev_dbg(&adap->dev, "transfer complete\n");
-		pmsg++;		/* next message */
-	}
-	return i;
-}
-
-/*
- * Return list of supported functionality.
- */
-static u32 at91_func(struct i2c_adapter *adapter)
-{
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
-}
-
-static struct i2c_algorithm at91_algorithm = {
-	.master_xfer	= at91_xfer,
-	.functionality	= at91_func,
-};
-
-/*
- * Main initialization routine.
- */
-static int __devinit at91_i2c_probe(struct platform_device *pdev)
-{
-	struct i2c_adapter *adapter;
-	struct resource *res;
-	int rc;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENXIO;
-
-	if (!request_mem_region(res->start, resource_size(res), "at91_i2c"))
-		return -EBUSY;
-
-	twi_base = ioremap(res->start, resource_size(res));
-	if (!twi_base) {
-		rc = -ENOMEM;
-		goto fail0;
-	}
-
-	twi_clk = clk_get(NULL, "twi_clk");
-	if (IS_ERR(twi_clk)) {
-		dev_err(&pdev->dev, "no clock defined\n");
-		rc = -ENODEV;
-		goto fail1;
-	}
-
-	adapter = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (adapter == NULL) {
-		dev_err(&pdev->dev, "can't allocate inteface!\n");
-		rc = -ENOMEM;
-		goto fail2;
-	}
-	snprintf(adapter->name, sizeof(adapter->name), "AT91");
-	adapter->algo = &at91_algorithm;
-	adapter->class = I2C_CLASS_HWMON;
-	adapter->dev.parent = &pdev->dev;
-	/* adapter->id == 0 ... only one TWI controller for now */
-
-	platform_set_drvdata(pdev, adapter);
-
-	clk_enable(twi_clk);		/* enable peripheral clock */
-	at91_twi_hwinit();		/* initialize TWI controller */
-
-	rc = i2c_add_numbered_adapter(adapter);
-	if (rc) {
-		dev_err(&pdev->dev, "Adapter %s registration failed\n",
-				adapter->name);
-		goto fail3;
-	}
-
-	dev_info(&pdev->dev, "AT91 i2c bus driver.\n");
-	return 0;
-
-fail3:
-	platform_set_drvdata(pdev, NULL);
-	kfree(adapter);
-	clk_disable(twi_clk);
-fail2:
-	clk_put(twi_clk);
-fail1:
-	iounmap(twi_base);
-fail0:
-	release_mem_region(res->start, resource_size(res));
-
-	return rc;
-}
-
-static int __devexit at91_i2c_remove(struct platform_device *pdev)
-{
-	struct i2c_adapter *adapter = platform_get_drvdata(pdev);
-	struct resource *res;
-	int rc;
-
-	rc = i2c_del_adapter(adapter);
-	platform_set_drvdata(pdev, NULL);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	iounmap(twi_base);
-	release_mem_region(res->start, resource_size(res));
-
-	clk_disable(twi_clk);		/* disable peripheral clock */
-	clk_put(twi_clk);
-
-	return rc;
-}
-
-#ifdef CONFIG_PM
-
-/* NOTE: could save a few mA by keeping clock off outside of at91_xfer... */
-
-static int at91_i2c_suspend(struct platform_device *pdev, pm_message_t mesg)
-{
-	clk_disable(twi_clk);
-	return 0;
-}
-
-static int at91_i2c_resume(struct platform_device *pdev)
-{
-	return clk_enable(twi_clk);
-}
-
-#else
-#define at91_i2c_suspend	NULL
-#define at91_i2c_resume		NULL
-#endif
-
-/* work with "modprobe at91_i2c" from hotplugging or coldplugging */
-MODULE_ALIAS("platform:at91_i2c");
-
-static struct platform_driver at91_i2c_driver = {
-	.probe		= at91_i2c_probe,
-	.remove		= __devexit_p(at91_i2c_remove),
-	.suspend	= at91_i2c_suspend,
-	.resume		= at91_i2c_resume,
-	.driver		= {
-		.name	= "at91_i2c",
-		.owner	= THIS_MODULE,
-	},
-};
-
-static int __init at91_i2c_init(void)
-{
-	return platform_driver_register(&at91_i2c_driver);
-}
-
-static void __exit at91_i2c_exit(void)
-{
-	platform_driver_unregister(&at91_i2c_driver);
-}
-
-module_init(at91_i2c_init);
-module_exit(at91_i2c_exit);
-
-MODULE_AUTHOR("Rick Bronson");
-MODULE_DESCRIPTION("I2C (TWI) driver for Atmel AT91");
-MODULE_LICENSE("GPL");
-- 
1.7.5.4

[PATCH v4 1/4] drivers/i2c/busses/i2c-at91.c: remove broken driver

[Nikolaus Voss]

Signed-off-by: Nikolaus Voss <n.voss@weinmann.de>
---
 arch/arm/mach-at91/include/mach/at91_twi.h |   68 ------
 drivers/i2c/busses/Makefile                |    1 -
 drivers/i2c/busses/i2c-at91.c              |  327 ----------------------------
 3 files changed, 0 insertions(+), 396 deletions(-)
 delete mode 100644 arch/arm/mach-at91/include/mach/at91_twi.h
 delete mode 100644 drivers/i2c/busses/i2c-at91.c

diff --git a/arch/arm/mach-at91/include/mach/at91_twi.h b/arch/arm/mach-at91/include/mach/at91_twi.h
deleted file mode 100644
index bb2880f..0000000
--- a/arch/arm/mach-at91/include/mach/at91_twi.h
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * arch/arm/mach-at91/include/mach/at91_twi.h
- *
- * Copyright (C) 2005 Ivan Kokshaysky
- * Copyright (C) SAN People
- *
- * Two-wire Interface (TWI) registers.
- * Based on AT91RM9200 datasheet revision E.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef AT91_TWI_H
-#define AT91_TWI_H
-
-#define	AT91_TWI_CR		0x00		/* Control Register */
-#define		AT91_TWI_START		(1 <<  0)	/* Send a Start Condition */
-#define		AT91_TWI_STOP		(1 <<  1)	/* Send a Stop Condition */
-#define		AT91_TWI_MSEN		(1 <<  2)	/* Master Transfer Enable */
-#define		AT91_TWI_MSDIS		(1 <<  3)	/* Master Transfer Disable */
-#define		AT91_TWI_SVEN		(1 <<  4)	/* Slave Transfer Enable [SAM9260 only] */
-#define		AT91_TWI_SVDIS		(1 <<  5)	/* Slave Transfer Disable [SAM9260 only] */
-#define		AT91_TWI_SWRST		(1 <<  7)	/* Software Reset */
-
-#define	AT91_TWI_MMR		0x04		/* Master Mode Register */
-#define		AT91_TWI_IADRSZ		(3    <<  8)	/* Internal Device Address Size */
-#define			AT91_TWI_IADRSZ_NO		(0 << 8)
-#define			AT91_TWI_IADRSZ_1		(1 << 8)
-#define			AT91_TWI_IADRSZ_2		(2 << 8)
-#define			AT91_TWI_IADRSZ_3		(3 << 8)
-#define		AT91_TWI_MREAD		(1    << 12)	/* Master Read Direction */
-#define		AT91_TWI_DADR		(0x7f << 16)	/* Device Address */
-
-#define	AT91_TWI_SMR		0x08		/* Slave Mode Register [SAM9260 only] */
-#define		AT91_TWI_SADR		(0x7f << 16)	/* Slave Address */
-
-#define	AT91_TWI_IADR		0x0c		/* Internal Address Register */
-
-#define	AT91_TWI_CWGR		0x10		/* Clock Waveform Generator Register */
-#define		AT91_TWI_CLDIV		(0xff <<  0)	/* Clock Low Divisor */
-#define		AT91_TWI_CHDIV		(0xff <<  8)	/* Clock High Divisor */
-#define		AT91_TWI_CKDIV		(7    << 16)	/* Clock Divider */
-
-#define	AT91_TWI_SR		0x20		/* Status Register */
-#define		AT91_TWI_TXCOMP		(1 <<  0)	/* Transmission Complete */
-#define		AT91_TWI_RXRDY		(1 <<  1)	/* Receive Holding Register Ready */
-#define		AT91_TWI_TXRDY		(1 <<  2)	/* Transmit Holding Register Ready */
-#define		AT91_TWI_SVREAD		(1 <<  3)	/* Slave Read [SAM9260 only] */
-#define		AT91_TWI_SVACC		(1 <<  4)	/* Slave Access [SAM9260 only] */
-#define		AT91_TWI_GACC		(1 <<  5)	/* General Call Access [SAM9260 only] */
-#define		AT91_TWI_OVRE		(1 <<  6)	/* Overrun Error [AT91RM9200 only] */
-#define		AT91_TWI_UNRE		(1 <<  7)	/* Underrun Error [AT91RM9200 only] */
-#define		AT91_TWI_NACK		(1 <<  8)	/* Not Acknowledged */
-#define		AT91_TWI_ARBLST		(1 <<  9)	/* Arbitration Lost [SAM9260 only] */
-#define		AT91_TWI_SCLWS		(1 << 10)	/* Clock Wait State [SAM9260 only] */
-#define		AT91_TWI_EOSACC		(1 << 11)	/* End of Slave Address [SAM9260 only] */
-
-#define	AT91_TWI_IER		0x24		/* Interrupt Enable Register */
-#define	AT91_TWI_IDR		0x28		/* Interrupt Disable Register */
-#define	AT91_TWI_IMR		0x2c		/* Interrupt Mask Register */
-#define	AT91_TWI_RHR		0x30		/* Receive Holding Register */
-#define	AT91_TWI_THR		0x34		/* Transmit Holding Register */
-
-#endif
-
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index fba6da6..e8a1852 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -28,7 +28,6 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 
 # Embedded system I2C/SMBus host controller drivers
-obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
 obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
deleted file mode 100644
index 305c075..0000000
--- a/drivers/i2c/busses/i2c-at91.c
+++ /dev/null
@@ -1,327 +0,0 @@
-/*
-    i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
-
-    Copyright (C) 2004 Rick Bronson
-    Converted to 2.6 by Andrew Victor <andrew@sanpeople.com>
-
-    Borrowed heavily from original work by:
-    Copyright (C) 2000 Philip Edelbrock <phil@stimpy.netroedge.com>
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-*/
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/err.h>
-#include <linux/slab.h>
-#include <linux/types.h>
-#include <linux/delay.h>
-#include <linux/i2c.h>
-#include <linux/init.h>
-#include <linux/clk.h>
-#include <linux/platform_device.h>
-#include <linux/io.h>
-
-#include <mach/at91_twi.h>
-#include <mach/board.h>
-#include <mach/cpu.h>
-
-#define TWI_CLOCK		100000		/* Hz. max 400 Kbits/sec */
-
-
-static struct clk *twi_clk;
-static void __iomem *twi_base;
-
-#define at91_twi_read(reg)		__raw_readl(twi_base + (reg))
-#define at91_twi_write(reg, val)	__raw_writel((val), twi_base + (reg))
-
-
-/*
- * Initialize the TWI hardware registers.
- */
-static void __devinit at91_twi_hwinit(void)
-{
-	unsigned long cdiv, ckdiv;
-
-	at91_twi_write(AT91_TWI_IDR, 0xffffffff);	/* Disable all interrupts */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_SWRST);	/* Reset peripheral */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_MSEN);	/* Set Master mode */
-
-	/* Calcuate clock dividers */
-	cdiv = (clk_get_rate(twi_clk) / (2 * TWI_CLOCK)) - 3;
-	cdiv = cdiv + 1;	/* round up */
-	ckdiv = 0;
-	while (cdiv > 255) {
-		ckdiv++;
-		cdiv = cdiv >> 1;
-	}
-
-	if (cpu_is_at91rm9200()) {			/* AT91RM9200 Errata #22 */
-		if (ckdiv > 5) {
-			printk(KERN_ERR "AT91 I2C: Invalid TWI_CLOCK value!\n");
-			ckdiv = 5;
-		}
-	}
-
-	at91_twi_write(AT91_TWI_CWGR, (ckdiv << 16) | (cdiv << 8) | cdiv);
-}
-
-/*
- * Poll the i2c status register until the specified bit is set.
- * Returns 0 if timed out (100 msec).
- */
-static short at91_poll_status(unsigned long bit)
-{
-	int loop_cntr = 10000;
-
-	do {
-		udelay(10);
-	} while (!(at91_twi_read(AT91_TWI_SR) & bit) && (--loop_cntr > 0));
-
-	return (loop_cntr > 0);
-}
-
-static int xfer_read(struct i2c_adapter *adap, unsigned char *buf, int length)
-{
-	/* Send Start */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_START);
-
-	/* Read data */
-	while (length--) {
-		if (!length)	/* need to send Stop before reading last byte */
-			at91_twi_write(AT91_TWI_CR, AT91_TWI_STOP);
-		if (!at91_poll_status(AT91_TWI_RXRDY)) {
-			dev_dbg(&adap->dev, "RXRDY timeout\n");
-			return -ETIMEDOUT;
-		}
-		*buf++ = (at91_twi_read(AT91_TWI_RHR) & 0xff);
-	}
-
-	return 0;
-}
-
-static int xfer_write(struct i2c_adapter *adap, unsigned char *buf, int length)
-{
-	/* Load first byte into transmitter */
-	at91_twi_write(AT91_TWI_THR, *buf++);
-
-	/* Send Start */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_START);
-
-	do {
-		if (!at91_poll_status(AT91_TWI_TXRDY)) {
-			dev_dbg(&adap->dev, "TXRDY timeout\n");
-			return -ETIMEDOUT;
-		}
-
-		length--;	/* byte was transmitted */
-
-		if (length > 0)		/* more data to send? */
-			at91_twi_write(AT91_TWI_THR, *buf++);
-	} while (length);
-
-	/* Send Stop */
-	at91_twi_write(AT91_TWI_CR, AT91_TWI_STOP);
-
-	return 0;
-}
-
-/*
- * Generic i2c master transfer entrypoint.
- *
- * Note: We do not use Atmel's feature of storing the "internal device address".
- * Instead the "internal device address" has to be written using a separate
- * i2c message.
- * http://lists.arm.linux.org.uk/pipermail/linux-arm-kernel/2004-September/024411.html
- */
-static int at91_xfer(struct i2c_adapter *adap, struct i2c_msg *pmsg, int num)
-{
-	int i, ret;
-
-	dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num);
-
-	for (i = 0; i < num; i++) {
-		dev_dbg(&adap->dev, " #%d: %sing %d byte%s %s 0x%02x\n", i,
-			pmsg->flags & I2C_M_RD ? "read" : "writ",
-			pmsg->len, pmsg->len > 1 ? "s" : "",
-			pmsg->flags & I2C_M_RD ? "from" : "to",	pmsg->addr);
-
-		at91_twi_write(AT91_TWI_MMR, (pmsg->addr << 16)
-			| ((pmsg->flags & I2C_M_RD) ? AT91_TWI_MREAD : 0));
-
-		if (pmsg->len && pmsg->buf) {	/* sanity check */
-			if (pmsg->flags & I2C_M_RD)
-				ret = xfer_read(adap, pmsg->buf, pmsg->len);
-			else
-				ret = xfer_write(adap, pmsg->buf, pmsg->len);
-
-			if (ret)
-				return ret;
-
-			/* Wait until transfer is finished */
-			if (!at91_poll_status(AT91_TWI_TXCOMP)) {
-				dev_dbg(&adap->dev, "TXCOMP timeout\n");
-				return -ETIMEDOUT;
-			}
-		}
-		dev_dbg(&adap->dev, "transfer complete\n");
-		pmsg++;		/* next message */
-	}
-	return i;
-}
-
-/*
- * Return list of supported functionality.
- */
-static u32 at91_func(struct i2c_adapter *adapter)
-{
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
-}
-
-static struct i2c_algorithm at91_algorithm = {
-	.master_xfer	= at91_xfer,
-	.functionality	= at91_func,
-};
-
-/*
- * Main initialization routine.
- */
-static int __devinit at91_i2c_probe(struct platform_device *pdev)
-{
-	struct i2c_adapter *adapter;
-	struct resource *res;
-	int rc;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENXIO;
-
-	if (!request_mem_region(res->start, resource_size(res), "at91_i2c"))
-		return -EBUSY;
-
-	twi_base = ioremap(res->start, resource_size(res));
-	if (!twi_base) {
-		rc = -ENOMEM;
-		goto fail0;
-	}
-
-	twi_clk = clk_get(NULL, "twi_clk");
-	if (IS_ERR(twi_clk)) {
-		dev_err(&pdev->dev, "no clock defined\n");
-		rc = -ENODEV;
-		goto fail1;
-	}
-
-	adapter = kzalloc(sizeof(struct i2c_adapter), GFP_KERNEL);
-	if (adapter == NULL) {
-		dev_err(&pdev->dev, "can't allocate inteface!\n");
-		rc = -ENOMEM;
-		goto fail2;
-	}
-	snprintf(adapter->name, sizeof(adapter->name), "AT91");
-	adapter->algo = &at91_algorithm;
-	adapter->class = I2C_CLASS_HWMON;
-	adapter->dev.parent = &pdev->dev;
-	/* adapter->id == 0 ... only one TWI controller for now */
-
-	platform_set_drvdata(pdev, adapter);
-
-	clk_enable(twi_clk);		/* enable peripheral clock */
-	at91_twi_hwinit();		/* initialize TWI controller */
-
-	rc = i2c_add_numbered_adapter(adapter);
-	if (rc) {
-		dev_err(&pdev->dev, "Adapter %s registration failed\n",
-				adapter->name);
-		goto fail3;
-	}
-
-	dev_info(&pdev->dev, "AT91 i2c bus driver.\n");
-	return 0;
-
-fail3:
-	platform_set_drvdata(pdev, NULL);
-	kfree(adapter);
-	clk_disable(twi_clk);
-fail2:
-	clk_put(twi_clk);
-fail1:
-	iounmap(twi_base);
-fail0:
-	release_mem_region(res->start, resource_size(res));
-
-	return rc;
-}
-
-static int __devexit at91_i2c_remove(struct platform_device *pdev)
-{
-	struct i2c_adapter *adapter = platform_get_drvdata(pdev);
-	struct resource *res;
-	int rc;
-
-	rc = i2c_del_adapter(adapter);
-	platform_set_drvdata(pdev, NULL);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	iounmap(twi_base);
-	release_mem_region(res->start, resource_size(res));
-
-	clk_disable(twi_clk);		/* disable peripheral clock */
-	clk_put(twi_clk);
-
-	return rc;
-}
-
-#ifdef CONFIG_PM
-
-/* NOTE: could save a few mA by keeping clock off outside of at91_xfer... */
-
-static int at91_i2c_suspend(struct platform_device *pdev, pm_message_t mesg)
-{
-	clk_disable(twi_clk);
-	return 0;
-}
-
-static int at91_i2c_resume(struct platform_device *pdev)
-{
-	return clk_enable(twi_clk);
-}
-
-#else
-#define at91_i2c_suspend	NULL
-#define at91_i2c_resume		NULL
-#endif
-
-/* work with "modprobe at91_i2c" from hotplugging or coldplugging */
-MODULE_ALIAS("platform:at91_i2c");
-
-static struct platform_driver at91_i2c_driver = {
-	.probe		= at91_i2c_probe,
-	.remove		= __devexit_p(at91_i2c_remove),
-	.suspend	= at91_i2c_suspend,
-	.resume		= at91_i2c_resume,
-	.driver		= {
-		.name	= "at91_i2c",
-		.owner	= THIS_MODULE,
-	},
-};
-
-static int __init at91_i2c_init(void)
-{
-	return platform_driver_register(&at91_i2c_driver);
-}
-
-static void __exit at91_i2c_exit(void)
-{
-	platform_driver_unregister(&at91_i2c_driver);
-}
-
-module_init(at91_i2c_init);
-module_exit(at91_i2c_exit);
-
-MODULE_AUTHOR("Rick Bronson");
-MODULE_DESCRIPTION("I2C (TWI) driver for Atmel AT91");
-MODULE_LICENSE("GPL");
-- 
1.7.5.4

[PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Nikolaus Voss]

This driver has the following properties compared to the old driver:
1. Support for multiple interfaces.
2. Interrupt driven I/O as opposed to polling/busy waiting.
3. Support for _one_ repeated start (Sr) condition, which is enough
   for most real-world applications including all SMBus transfer types.
   (The hardware does not support issuing arbitrary Sr conditions on the
    bus.)

Tested on Atmel G45 with BQ20Z80 battery SMBus client.

Signed-off-by: Nikolaus Voss <n.voss@weinmann.de>
---
 drivers/i2c/busses/Kconfig    |   11 +-
 drivers/i2c/busses/Makefile   |    1 +
 drivers/i2c/busses/i2c-at91.c |  417 +++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-at91.h |   80 ++++++++
 4 files changed, 502 insertions(+), 7 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-at91.c
 create mode 100644 drivers/i2c/busses/i2c-at91.h

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index a3afac4..2ef618d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -285,18 +285,15 @@ comment "I2C system bus drivers (mostly embedded / system-on-chip)"
 
 config I2C_AT91
 	tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
-	depends on ARCH_AT91 && EXPERIMENTAL && BROKEN
+	depends on ARCH_AT91 && EXPERIMENTAL
 	help
 	  This supports the use of the I2C interface on Atmel AT91
 	  processors.
 
-	  This driver is BROKEN because the controller which it uses
-	  will easily trigger RX overrun and TX underrun errors.  Using
-	  low I2C clock rates may partially work around those issues
-	  on some systems.  Another serious problem is that there is no
-	  documented way to issue repeated START conditions, as needed
+	  A serious problem is that there is no documented way to issue
+	  repeated START conditions for more than two messages, as needed
 	  to support combined I2C messages.  Use the i2c-gpio driver
-	  unless your system can cope with those limitations.
+	  unless your system can cope with this limitation.
 
 config I2C_AU1550
 	tristate "Au1550/Au1200 SMBus interface"
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index e8a1852..fba6da6 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
 obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
 
 # Embedded system I2C/SMBus host controller drivers
+obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
 obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
 obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
 obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
new file mode 100644
index 0000000..5f4be34
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -0,0 +1,417 @@
+/*
+    i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
+
+    Copyright (C) 2011 Nikolaus Voss <n.voss@weinmann.de>
+
+    Evolved from original work by:
+    Copyright (C) 2004 Rick Bronson
+    Converted to 2.6 by Andrew Victor <andrew@sanpeople.com>
+
+    Borrowed heavily from original work by:
+    Copyright (C) 2000 Philip Edelbrock <phil@stimpy.netroedge.com>
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+*/
+
+#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/platform_device.h>
+#include <linux/slab.h>
+
+#include <mach/cpu.h>
+
+#include "i2c-at91.h"
+
+#define TWI_CLOCK_HZ		100000			/* max 400 Kbits/s */
+#define AT91_I2C_TIMEOUT	msecs_to_jiffies(10)	/* transfer timeout */
+
+struct at91_twi_dev {
+	struct device		*dev;
+	void __iomem		*base;
+	struct completion	cmd_complete;
+	struct clk		*clk;
+	u8			*buf;
+	size_t			buf_len;
+	int			irq;
+	unsigned		transfer_status;
+	struct i2c_adapter	adapter;
+};
+
+static unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg)
+{
+	return __raw_readl(dev->base + reg);
+}
+
+static void at91_twi_write(struct at91_twi_dev *dev, unsigned reg, unsigned val)
+{
+	__raw_writel(val, dev->base + reg);
+}
+
+static void at91_disable_twi_interrupts(struct at91_twi_dev *dev)
+{
+	at91_twi_write(dev, AT91_TWI_IDR,
+		       AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY);
+}
+
+static void at91_init_twi_bus(struct at91_twi_dev *dev)
+{
+	at91_disable_twi_interrupts(dev);
+	at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SWRST);
+	at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_MSEN);
+	at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_SVDIS);
+}
+
+static void at91_set_twi_clock(struct at91_twi_dev *dev)
+{
+	unsigned long cdiv, ckdiv;
+
+	/* Calcuate clock dividers and round up */
+	cdiv = (clk_get_rate(dev->clk) / (2 * TWI_CLOCK_HZ)) - 3 + 1;
+	ckdiv = 0;
+	while (cdiv > 255) {
+		ckdiv++;
+		cdiv = cdiv >> 1;
+	}
+
+	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
+		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");
+		ckdiv = 5;
+	}
+
+	at91_twi_write(dev, AT91_TWI_CWGR, (ckdiv << 16) | (cdiv << 8) | cdiv);
+}
+
+static void __devinit at91_twi_hwinit(struct at91_twi_dev *dev)
+{
+	at91_init_twi_bus(dev);
+	at91_set_twi_clock(dev);
+}
+
+static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
+{
+	if (dev->buf_len <= 0)
+		return;
+
+	at91_twi_write(dev, AT91_TWI_THR, *dev->buf);
+
+	/* send stop when last byte has been written */
+	if (--dev->buf_len == 0)
+		at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
+
+	dev_dbg(dev->dev, "wrote 0x%x, to go %d\n", *dev->buf, dev->buf_len);
+
+	++dev->buf;
+}
+
+static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
+{
+	*dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff;
+
+	/* send stop if second but last byte has been read */
+	if (--dev->buf_len == 1)
+		at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
+
+	dev_dbg(dev->dev, "read 0x%x, to go %d\n", *dev->buf, dev->buf_len);
+
+	++dev->buf;
+}
+
+static irqreturn_t atmel_twi_interrupt(int irq, void *dev_id)
+{
+	struct at91_twi_dev *dev = dev_id;
+	const unsigned status = at91_twi_read(dev, AT91_TWI_SR);
+	const unsigned irqstatus = status & at91_twi_read(dev, AT91_TWI_IMR);
+
+	if (irqstatus & AT91_TWI_TXCOMP) {
+		at91_disable_twi_interrupts(dev);
+		dev->transfer_status = status;
+		complete(&dev->cmd_complete);
+	} else if (irqstatus & AT91_TWI_RXRDY) {
+		at91_twi_read_next_byte(dev);
+	} else if (irqstatus & AT91_TWI_TXRDY) {
+		at91_twi_write_next_byte(dev);
+	} else {
+		return IRQ_NONE;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
+{
+	int ret;
+
+	INIT_COMPLETION(dev->cmd_complete);
+	if (is_read) {
+		if (!dev->buf_len)
+			at91_twi_write(dev, AT91_TWI_CR,
+				       AT91_TWI_START | AT91_TWI_STOP);
+		else
+			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
+		at91_twi_write(dev, AT91_TWI_IER,
+			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
+	} else {
+		at91_twi_write_next_byte(dev);
+		at91_twi_write(dev, AT91_TWI_IER,
+			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
+	}
+
+	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
+							dev->adapter.timeout);
+	if (ret == 0) {
+		dev_err(dev->dev, "controller timed out\n");
+		at91_init_twi_bus(dev);
+		return -ETIMEDOUT;
+	}
+	if (dev->transfer_status & AT91_TWI_NACK) {
+		dev_dbg(dev->dev, "received nack\n");
+		return -ENODEV;
+	}
+	if (dev->transfer_status & AT91_TWI_OVRE) {
+		dev_err(dev->dev, "overrun while reading\n");
+		return -EIO;
+	}
+	dev_dbg(dev->dev, "transfer complete\n");
+
+	return 0;
+}
+
+static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
+{
+	struct at91_twi_dev *dev = i2c_get_adapdata(adap);
+	int ret;
+	unsigned int_addr_flag = 0;
+	struct i2c_msg *m_start = msg;
+
+	dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num);
+
+	/*
+	 * The hardware can handle at most two messages concatenated by a
+	 * repeated start via it's internal address feature.
+	 */
+	if (num > 2) {
+		dev_err(dev->dev,
+			"cannot handle more than two concatenated messages.\n");
+		return 0;
+	} else if (num == 2) {
+		int internal_address = 0;
+		int i;
+
+		if (msg->flags & I2C_M_RD) {
+			dev_err(dev->dev, "first transfer must be write.\n");
+			return -EINVAL;
+		}
+		if (msg->len > 3) {
+			dev_err(dev->dev, "first message size must be <= 3.\n");
+			return -EINVAL;
+		}
+
+		/* 1st msg is put into the internal address, start with 2nd */
+		m_start = &msg[1];
+		for (i = 0; i < msg->len; ++i) {
+			internal_address |= ((unsigned)msg->buf[i]) << (8 * i);
+			int_addr_flag += AT91_TWI_IADRSZ_1;
+		}
+		at91_twi_write(dev, AT91_TWI_IADR, internal_address);
+	}
+
+	at91_twi_write(dev, AT91_TWI_MMR, (m_start->addr << 16) | int_addr_flag
+		       | ((m_start->flags & I2C_M_RD) ? AT91_TWI_MREAD : 0));
+
+	dev->buf_len = m_start->len;
+	dev->buf = m_start->buf;
+
+	ret = at91_do_twi_transfer(dev, m_start->flags & I2C_M_RD);
+	if (ret < 0)
+		return ret;
+
+	return num;
+}
+
+static u32 at91_twi_func(struct i2c_adapter *adapter)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static struct i2c_algorithm at91_twi_algorithm = {
+	.master_xfer	= at91_twi_xfer,
+	.functionality	= at91_twi_func,
+};
+
+static int __devinit at91_twi_probe(struct platform_device *pdev)
+{
+	struct at91_twi_dev *dev;
+	struct resource *mem, *irq, *ioarea;
+	int rc;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem)
+		return -ENODEV;
+
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!irq)
+		return -ENODEV;
+
+	ioarea = request_mem_region(mem->start, resource_size(mem), pdev->name);
+	if (!ioarea)
+		return -EBUSY;
+
+	dev = kzalloc(sizeof(struct at91_twi_dev), GFP_KERNEL);
+	if (!dev) {
+		rc = -ENOMEM;
+		goto err_release_region;
+	}
+
+	init_completion(&dev->cmd_complete);
+
+	dev->dev = &pdev->dev;
+	dev->irq = irq->start;
+	platform_set_drvdata(pdev, dev);
+
+	dev->clk = clk_get(dev->dev, "twi_clk");
+	if (IS_ERR(dev->clk)) {
+		dev_err(dev->dev, "no clock defined\n");
+		rc = -ENODEV;
+		goto err_free_mem;
+	}
+	clk_prepare(dev->clk);
+	clk_enable(dev->clk);
+
+	dev->base = ioremap(mem->start, resource_size(mem));
+	if (!dev->base) {
+		rc = -EBUSY;
+		goto err_mem_ioremap;
+	}
+
+	at91_twi_hwinit(dev);
+
+	rc = request_irq(dev->irq, atmel_twi_interrupt, 0,
+			 dev_name(dev->dev), dev);
+	if (rc) {
+		dev_err(dev->dev, "Cannot get irq %d: %d\n", dev->irq, rc);
+		goto err_unuse_clocks;
+	}
+
+	snprintf(dev->adapter.name, sizeof(dev->adapter.name), "AT91");
+	i2c_set_adapdata(&dev->adapter, dev);
+	dev->adapter.owner = THIS_MODULE;
+	dev->adapter.class = I2C_CLASS_HWMON;
+	dev->adapter.algo = &at91_twi_algorithm;
+	dev->adapter.dev.parent = dev->dev;
+	dev->adapter.nr = pdev->id;
+	dev->adapter.timeout = AT91_I2C_TIMEOUT;
+
+	rc = i2c_add_numbered_adapter(&dev->adapter);
+	if (rc) {
+		dev_err(dev->dev, "Adapter %s registration failed\n",
+			dev->adapter.name);
+		goto err_free_irq;
+	}
+
+	dev_info(dev->dev, "AT91 i2c bus driver.\n");
+	return 0;
+
+err_free_irq:
+	free_irq(dev->irq, dev);
+err_unuse_clocks:
+	iounmap(dev->base);
+err_mem_ioremap:
+	clk_disable(dev->clk);
+	clk_put(dev->clk);
+err_free_mem:
+	kfree(dev);
+err_release_region:
+	release_mem_region(mem->start, resource_size(mem));
+
+	return rc;
+}
+
+static int __devexit at91_twi_remove(struct platform_device *pdev)
+{
+	struct at91_twi_dev *dev = platform_get_drvdata(pdev);
+	struct resource *mem;
+	int rc;
+
+	platform_set_drvdata(pdev, NULL);
+	rc = i2c_del_adapter(&dev->adapter);
+	put_device(dev->dev);
+
+	clk_disable(dev->clk);
+	clk_put(dev->clk);
+	dev->clk = NULL;
+
+	free_irq(dev->irq, dev);
+
+	iounmap(dev->base);
+	kfree(dev);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+
+	return rc;
+}
+
+#ifdef CONFIG_PM
+
+static int at91_twi_suspend(struct device *dev)
+{
+	struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
+
+	clk_disable(twi_dev->clk);
+
+	return 0;
+}
+
+static int at91_twi_resume(struct device *dev)
+{
+	struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
+
+	return clk_enable(twi_dev->clk);
+}
+
+static const struct dev_pm_ops at91_twi_pm = {
+	.suspend	= at91_twi_suspend,
+	.resume		= at91_twi_resume,
+};
+
+#define at91_twi_pm_ops (&at91_twi_pm)
+#else
+#define at91_twi_pm_ops NULL
+#endif
+
+MODULE_ALIAS("platform:at91_i2c");
+
+static struct platform_driver at91_twi_driver = {
+	.probe		= at91_twi_probe,
+	.remove		= __devexit_p(at91_twi_remove),
+	.driver		= {
+		.name	= "at91_i2c",
+		.owner	= THIS_MODULE,
+		.pm	= at91_twi_pm_ops,
+	},
+};
+
+static int __init at91_twi_init(void)
+{
+	return platform_driver_register(&at91_twi_driver);
+}
+
+static void __exit at91_twi_exit(void)
+{
+	platform_driver_unregister(&at91_twi_driver);
+}
+
+module_init(at91_twi_init);
+module_exit(at91_twi_exit);
+
+MODULE_AUTHOR("Nikolaus Voss");
+MODULE_DESCRIPTION("I2C (TWI) driver for Atmel AT91");
+MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h
new file mode 100644
index 0000000..a898159
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91.h
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2005 Ivan Kokshaysky
+ * Copyright (C) SAN People
+ *
+ * Two-wire Interface (TWI) registers.
+ * Based on AT91RM9200 datasheet revision E.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef AT91_TWI_H
+#define AT91_TWI_H
+
+#define	AT91_TWI_CR		0x00		/* Control Register */
+#define	AT91_TWI_START		(1 <<  0)	/* Send a Start Condition */
+#define	AT91_TWI_STOP		(1 <<  1)	/* Send a Stop Condition */
+#define	AT91_TWI_MSEN		(1 <<  2)	/* Master Transfer Enable */
+#define	AT91_TWI_MSDIS		(1 <<  3)	/* Master Transfer Disable */
+#define	AT91_TWI_SVEN		(1 <<  4)	/* Slave Transfer Enable
+						 *  [SAM9260 only] */
+#define	AT91_TWI_SVDIS		(1 <<  5)	/* Slave Transfer Disable
+						 *  [SAM9260 only] */
+#define	AT91_TWI_SWRST		(1 <<  7)	/* Software Reset */
+
+#define	AT91_TWI_MMR		0x04		/* Master Mode Register */
+#define	AT91_TWI_IADRSZ		(3    <<  8)	/* Internal Device Address
+						 *  Size */
+#define	AT91_TWI_IADRSZ_NO	(0 << 8)
+#define	AT91_TWI_IADRSZ_1	(1 << 8)
+#define	AT91_TWI_IADRSZ_2	(2 << 8)
+#define	AT91_TWI_IADRSZ_3	(3 << 8)
+#define	AT91_TWI_MREAD		(1    << 12)	/* Master Read Direction */
+#define	AT91_TWI_DADR		(0x7f << 16)	/* Device Address */
+
+#define	AT91_TWI_SMR		0x08		/* Slave Mode Register
+						 *  [SAM9260 only] */
+#define	AT91_TWI_SADR		(0x7f << 16)	/* Slave Address */
+
+#define	AT91_TWI_IADR		0x0c		/* Internal Address Register */
+
+#define	AT91_TWI_CWGR		0x10		/* Clock Waveform Generator
+						 *  Register */
+#define	AT91_TWI_CLDIV		(0xff <<  0)	/* Clock Low Divisor */
+#define	AT91_TWI_CHDIV		(0xff <<  8)	/* Clock High Divisor */
+#define	AT91_TWI_CKDIV		(7    << 16)	/* Clock Divider */
+
+#define	AT91_TWI_SR		0x20		/* Status Register */
+#define	AT91_TWI_TXCOMP		(1 <<  0)	/* Transmission Complete */
+#define	AT91_TWI_RXRDY		(1 <<  1)	/* Receive Holding Register
+						 *  Ready */
+#define	AT91_TWI_TXRDY		(1 <<  2)	/* Transmit Holding Register
+						 *  Ready */
+#define	AT91_TWI_SVREAD		(1 <<  3)	/* Slave Read [SAM9260 only] */
+#define	AT91_TWI_SVACC		(1 <<  4)	/* Slave Access
+						 *  [SAM9260 only] */
+#define	AT91_TWI_GACC		(1 <<  5)	/* General Call Access
+						 *  [SAM9260 only] */
+#define	AT91_TWI_OVRE		(1 <<  6)	/* Overrun Error
+						 *  [AT91RM9200 only] */
+#define	AT91_TWI_UNRE		(1 <<  7)	/* Underrun Error
+						 *  [AT91RM9200 only] */
+#define	AT91_TWI_NACK		(1 <<  8)	/* Not Acknowledged */
+#define	AT91_TWI_ARBLST		(1 <<  9)	/* Arbitration Lost
+						 *  [SAM9260 only] */
+#define	AT91_TWI_SCLWS		(1 << 10)	/* Clock Wait State
+						 *  [SAM9260 only] */
+#define	AT91_TWI_EOSACC		(1 << 11)	/* End of Slave Address
+						 *  [SAM9260 only] */
+
+#define	AT91_TWI_IER		0x24		/* Interrupt Enable Register */
+#define	AT91_TWI_IDR		0x28		/* Interrupt Disable Register */
+#define	AT91_TWI_IMR		0x2c		/* Interrupt Mask Register */
+#define	AT91_TWI_RHR		0x30		/* Receive Holding Register */
+#define	AT91_TWI_THR		0x34		/* Transmit Holding Register */
+
+#endif
+
-- 
1.7.5.4

Re: [PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Felipe Balbi]

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

Hi,

On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote:
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index e8a1852..fba6da6 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
>  obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
>  
>  # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
>  obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
>  obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
>  obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> new file mode 100644
> index 0000000..5f4be34
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -0,0 +1,417 @@
> +/*
> +    i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
> +
> +    Copyright (C) 2011 Nikolaus Voss <n.voss-+umVssTZoCsb1SvskN2V4Q@public.gmane.org>
> +
> +    Evolved from original work by:
> +    Copyright (C) 2004 Rick Bronson
> +    Converted to 2.6 by Andrew Victor <andrew-eS41wJS13H5l57MIdRCFDg@public.gmane.org>
> +
> +    Borrowed heavily from original work by:
> +    Copyright (C) 2000 Philip Edelbrock <phil-LT64U7CwzWEZMC/lefXSXFaTQe2KTcn/@public.gmane.org>
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; either version 2 of the License, or
> +    (at your option) any later version.
> +*/

wrong multi-line comment style.

> +static void at91_set_twi_clock(struct at91_twi_dev *dev)

this should be __devinit as it's only used by at91_twi_hwinit()

> +{
> +	unsigned long cdiv, ckdiv;
> +
> +	/* Calcuate clock dividers and round up */

typo: calculate.

> +	cdiv = (clk_get_rate(dev->clk) / (2 * TWI_CLOCK_HZ)) - 3 + 1;

DIV_ROUND_UP() ??

> +	ckdiv = 0;
> +	while (cdiv > 255) {
> +		ckdiv++;
> +		cdiv = cdiv >> 1;
> +	}
> +
> +	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
> +		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");

is it really an error ? Or would it be enough as dev_dbg() ?

> +static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
> +{
> +	int ret;
> +
> +	INIT_COMPLETION(dev->cmd_complete);
> +	if (is_read) {
> +		if (!dev->buf_len)
> +			at91_twi_write(dev, AT91_TWI_CR,
> +				       AT91_TWI_START | AT91_TWI_STOP);
> +		else
> +			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
> +		at91_twi_write(dev, AT91_TWI_IER,
> +			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
> +	} else {
> +		at91_twi_write_next_byte(dev);
> +		at91_twi_write(dev, AT91_TWI_IER,
> +			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
> +	}
> +
> +	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
> +							dev->adapter.timeout);
> +	if (ret == 0) {
> +		dev_err(dev->dev, "controller timed out\n");
> +		at91_init_twi_bus(dev);
> +		return -ETIMEDOUT;
> +	}
> +	if (dev->transfer_status & AT91_TWI_NACK) {
> +		dev_dbg(dev->dev, "received nack\n");
> +		return -ENODEV;

not sure error code matches here. If the HW replies with NACK you tell
your users there's no I2C adapter ? Sounds a bit weird to me...

> +static int __devinit at91_twi_probe(struct platform_device *pdev)
> +{
> +	struct at91_twi_dev *dev;
> +	struct resource *mem, *irq, *ioarea;
> +	int rc;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem)
> +		return -ENODEV;
> +
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);

int irq = platform_get_irq(pdev, 0), would work better for you. In case
of error it will return -ENXIO

> +	if (!irq)
> +		return -ENODEV;
> +
> +	ioarea = request_mem_region(mem->start, resource_size(mem), pdev->name);
> +	if (!ioarea)
> +		return -EBUSY;
> +
> +	dev = kzalloc(sizeof(struct at91_twi_dev), GFP_KERNEL);

sizeof(*dev) would allow you to change the type of dev without having to
patch this line. Quite unlikely, I know, but still...

-- 
balbi

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

Re: [PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Felipe Balbi]

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

Hi,

On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote:
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index e8a1852..fba6da6 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
>  obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
>  
>  # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
>  obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
>  obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
>  obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> new file mode 100644
> index 0000000..5f4be34
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -0,0 +1,417 @@
> +/*
> +    i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
> +
> +    Copyright (C) 2011 Nikolaus Voss <n.voss@weinmann.de>
> +
> +    Evolved from original work by:
> +    Copyright (C) 2004 Rick Bronson
> +    Converted to 2.6 by Andrew Victor <andrew@sanpeople.com>
> +
> +    Borrowed heavily from original work by:
> +    Copyright (C) 2000 Philip Edelbrock <phil@stimpy.netroedge.com>
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; either version 2 of the License, or
> +    (at your option) any later version.
> +*/

wrong multi-line comment style.

> +static void at91_set_twi_clock(struct at91_twi_dev *dev)

this should be __devinit as it's only used by at91_twi_hwinit()

> +{
> +	unsigned long cdiv, ckdiv;
> +
> +	/* Calcuate clock dividers and round up */

typo: calculate.

> +	cdiv = (clk_get_rate(dev->clk) / (2 * TWI_CLOCK_HZ)) - 3 + 1;

DIV_ROUND_UP() ??

> +	ckdiv = 0;
> +	while (cdiv > 255) {
> +		ckdiv++;
> +		cdiv = cdiv >> 1;
> +	}
> +
> +	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
> +		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");

is it really an error ? Or would it be enough as dev_dbg() ?

> +static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
> +{
> +	int ret;
> +
> +	INIT_COMPLETION(dev->cmd_complete);
> +	if (is_read) {
> +		if (!dev->buf_len)
> +			at91_twi_write(dev, AT91_TWI_CR,
> +				       AT91_TWI_START | AT91_TWI_STOP);
> +		else
> +			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
> +		at91_twi_write(dev, AT91_TWI_IER,
> +			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
> +	} else {
> +		at91_twi_write_next_byte(dev);
> +		at91_twi_write(dev, AT91_TWI_IER,
> +			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
> +	}
> +
> +	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
> +							dev->adapter.timeout);
> +	if (ret == 0) {
> +		dev_err(dev->dev, "controller timed out\n");
> +		at91_init_twi_bus(dev);
> +		return -ETIMEDOUT;
> +	}
> +	if (dev->transfer_status & AT91_TWI_NACK) {
> +		dev_dbg(dev->dev, "received nack\n");
> +		return -ENODEV;

not sure error code matches here. If the HW replies with NACK you tell
your users there's no I2C adapter ? Sounds a bit weird to me...

> +static int __devinit at91_twi_probe(struct platform_device *pdev)
> +{
> +	struct at91_twi_dev *dev;
> +	struct resource *mem, *irq, *ioarea;
> +	int rc;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem)
> +		return -ENODEV;
> +
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);

int irq = platform_get_irq(pdev, 0), would work better for you. In case
of error it will return -ENXIO

> +	if (!irq)
> +		return -ENODEV;
> +
> +	ioarea = request_mem_region(mem->start, resource_size(mem), pdev->name);
> +	if (!ioarea)
> +		return -EBUSY;
> +
> +	dev = kzalloc(sizeof(struct at91_twi_dev), GFP_KERNEL);

sizeof(*dev) would allow you to change the type of dev without having to
patch this line. Quite unlikely, I know, but still...

-- 
balbi

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

[PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Felipe Balbi]

Hi,

On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote:
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index e8a1852..fba6da6 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_I2C_HYDRA)		+= i2c-hydra.o
>  obj-$(CONFIG_I2C_POWERMAC)	+= i2c-powermac.o
>  
>  # Embedded system I2C/SMBus host controller drivers
> +obj-$(CONFIG_I2C_AT91)		+= i2c-at91.o
>  obj-$(CONFIG_I2C_AU1550)	+= i2c-au1550.o
>  obj-$(CONFIG_I2C_BLACKFIN_TWI)	+= i2c-bfin-twi.o
>  obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> new file mode 100644
> index 0000000..5f4be34
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -0,0 +1,417 @@
> +/*
> +    i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
> +
> +    Copyright (C) 2011 Nikolaus Voss <n.voss@weinmann.de>
> +
> +    Evolved from original work by:
> +    Copyright (C) 2004 Rick Bronson
> +    Converted to 2.6 by Andrew Victor <andrew@sanpeople.com>
> +
> +    Borrowed heavily from original work by:
> +    Copyright (C) 2000 Philip Edelbrock <phil@stimpy.netroedge.com>
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; either version 2 of the License, or
> +    (at your option) any later version.
> +*/

wrong multi-line comment style.

> +static void at91_set_twi_clock(struct at91_twi_dev *dev)

this should be __devinit as it's only used by at91_twi_hwinit()

> +{
> +	unsigned long cdiv, ckdiv;
> +
> +	/* Calcuate clock dividers and round up */

typo: calculate.

> +	cdiv = (clk_get_rate(dev->clk) / (2 * TWI_CLOCK_HZ)) - 3 + 1;

DIV_ROUND_UP() ??

> +	ckdiv = 0;
> +	while (cdiv > 255) {
> +		ckdiv++;
> +		cdiv = cdiv >> 1;
> +	}
> +
> +	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
> +		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");

is it really an error ? Or would it be enough as dev_dbg() ?

> +static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
> +{
> +	int ret;
> +
> +	INIT_COMPLETION(dev->cmd_complete);
> +	if (is_read) {
> +		if (!dev->buf_len)
> +			at91_twi_write(dev, AT91_TWI_CR,
> +				       AT91_TWI_START | AT91_TWI_STOP);
> +		else
> +			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
> +		at91_twi_write(dev, AT91_TWI_IER,
> +			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
> +	} else {
> +		at91_twi_write_next_byte(dev);
> +		at91_twi_write(dev, AT91_TWI_IER,
> +			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
> +	}
> +
> +	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
> +							dev->adapter.timeout);
> +	if (ret == 0) {
> +		dev_err(dev->dev, "controller timed out\n");
> +		at91_init_twi_bus(dev);
> +		return -ETIMEDOUT;
> +	}
> +	if (dev->transfer_status & AT91_TWI_NACK) {
> +		dev_dbg(dev->dev, "received nack\n");
> +		return -ENODEV;

not sure error code matches here. If the HW replies with NACK you tell
your users there's no I2C adapter ? Sounds a bit weird to me...

> +static int __devinit at91_twi_probe(struct platform_device *pdev)
> +{
> +	struct at91_twi_dev *dev;
> +	struct resource *mem, *irq, *ioarea;
> +	int rc;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem)
> +		return -ENODEV;
> +
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);

int irq = platform_get_irq(pdev, 0), would work better for you. In case
of error it will return -ENXIO

> +	if (!irq)
> +		return -ENODEV;
> +
> +	ioarea = request_mem_region(mem->start, resource_size(mem), pdev->name);
> +	if (!ioarea)
> +		return -EBUSY;
> +
> +	dev = kzalloc(sizeof(struct at91_twi_dev), GFP_KERNEL);

sizeof(*dev) would allow you to change the type of dev without having to
patch this line. Quite unlikely, I know, but still...

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

Re: [PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Ryan Mallon]

On 10/11/11 06:59, Felipe Balbi wrote:

> Hi,
> 
> On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote:
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index e8a1852..fba6da6 100644

>> +	ckdiv = 0;
>> +	while (cdiv > 255) {
>> +		ckdiv++;
>> +		cdiv = cdiv >> 1;
>> +	}
>> +
>> +	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
>> +		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");
> 
> is it really an error ? Or would it be enough as dev_dbg() ?


dev_warn is probably appropriate.

> 
>> +static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
>> +{
>> +	int ret;
>> +
>> +	INIT_COMPLETION(dev->cmd_complete);
>> +	if (is_read) {
>> +		if (!dev->buf_len)
>> +			at91_twi_write(dev, AT91_TWI_CR,
>> +				       AT91_TWI_START | AT91_TWI_STOP);
>> +		else
>> +			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
>> +		at91_twi_write(dev, AT91_TWI_IER,
>> +			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
>> +	} else {
>> +		at91_twi_write_next_byte(dev);
>> +		at91_twi_write(dev, AT91_TWI_IER,
>> +			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
>> +	}
>> +
>> +	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
>> +							dev->adapter.timeout);
>> +	if (ret == 0) {
>> +		dev_err(dev->dev, "controller timed out\n");
>> +		at91_init_twi_bus(dev);
>> +		return -ETIMEDOUT;
>> +	}
>> +	if (dev->transfer_status & AT91_TWI_NACK) {
>> +		dev_dbg(dev->dev, "received nack\n");
>> +		return -ENODEV;
> 
> not sure error code matches here. If the HW replies with NACK you tell
> your users there's no I2C adapter ? Sounds a bit weird to me...


I think -ENODEV was used because a NACK can mean that there is no device
at the address you are trying to talk to. Other drivers appear to use
-EIO or -EREMOTEIO. The latter is possibly more correct.

~Ryan

Re: [PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Ryan Mallon]

On 10/11/11 06:59, Felipe Balbi wrote:

> Hi,
> 
> On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote:
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index e8a1852..fba6da6 100644

>> +	ckdiv = 0;
>> +	while (cdiv > 255) {
>> +		ckdiv++;
>> +		cdiv = cdiv >> 1;
>> +	}
>> +
>> +	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
>> +		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");
> 
> is it really an error ? Or would it be enough as dev_dbg() ?


dev_warn is probably appropriate.

> 
>> +static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
>> +{
>> +	int ret;
>> +
>> +	INIT_COMPLETION(dev->cmd_complete);
>> +	if (is_read) {
>> +		if (!dev->buf_len)
>> +			at91_twi_write(dev, AT91_TWI_CR,
>> +				       AT91_TWI_START | AT91_TWI_STOP);
>> +		else
>> +			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
>> +		at91_twi_write(dev, AT91_TWI_IER,
>> +			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
>> +	} else {
>> +		at91_twi_write_next_byte(dev);
>> +		at91_twi_write(dev, AT91_TWI_IER,
>> +			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
>> +	}
>> +
>> +	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
>> +							dev->adapter.timeout);
>> +	if (ret == 0) {
>> +		dev_err(dev->dev, "controller timed out\n");
>> +		at91_init_twi_bus(dev);
>> +		return -ETIMEDOUT;
>> +	}
>> +	if (dev->transfer_status & AT91_TWI_NACK) {
>> +		dev_dbg(dev->dev, "received nack\n");
>> +		return -ENODEV;
> 
> not sure error code matches here. If the HW replies with NACK you tell
> your users there's no I2C adapter ? Sounds a bit weird to me...


I think -ENODEV was used because a NACK can mean that there is no device
at the address you are trying to talk to. Other drivers appear to use
-EIO or -EREMOTEIO. The latter is possibly more correct.

~Ryan

[PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver

[Ryan Mallon]

On 10/11/11 06:59, Felipe Balbi wrote:

> Hi,
> 
> On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote:
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index e8a1852..fba6da6 100644

>> +	ckdiv = 0;
>> +	while (cdiv > 255) {
>> +		ckdiv++;
>> +		cdiv = cdiv >> 1;
>> +	}
>> +
>> +	if (cpu_is_at91rm9200() && (ckdiv > 5)) {
>> +		dev_err(dev->dev, "AT91RM9200 Erratum #22: using ckdiv = 5.\n");
> 
> is it really an error ? Or would it be enough as dev_dbg() ?


dev_warn is probably appropriate.

> 
>> +static int at91_do_twi_transfer(struct at91_twi_dev *dev, bool is_read)
>> +{
>> +	int ret;
>> +
>> +	INIT_COMPLETION(dev->cmd_complete);
>> +	if (is_read) {
>> +		if (!dev->buf_len)
>> +			at91_twi_write(dev, AT91_TWI_CR,
>> +				       AT91_TWI_START | AT91_TWI_STOP);
>> +		else
>> +			at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_START);
>> +		at91_twi_write(dev, AT91_TWI_IER,
>> +			       AT91_TWI_TXCOMP | AT91_TWI_RXRDY);
>> +	} else {
>> +		at91_twi_write_next_byte(dev);
>> +		at91_twi_write(dev, AT91_TWI_IER,
>> +			       AT91_TWI_TXCOMP | AT91_TWI_TXRDY);
>> +	}
>> +
>> +	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete,
>> +							dev->adapter.timeout);
>> +	if (ret == 0) {
>> +		dev_err(dev->dev, "controller timed out\n");
>> +		at91_init_twi_bus(dev);
>> +		return -ETIMEDOUT;
>> +	}
>> +	if (dev->transfer_status & AT91_TWI_NACK) {
>> +		dev_dbg(dev->dev, "received nack\n");
>> +		return -ENODEV;
> 
> not sure error code matches here. If the HW replies with NACK you tell
> your users there's no I2C adapter ? Sounds a bit weird to me...


I think -ENODEV was used because a NACK can mean that there is no device
at the address you are trying to talk to. Other drivers appear to use
-EIO or -EREMOTEIO. The latter is possibly more correct.

~Ryan

[PATCH v4 4/4] Add lookup entries for twi_clk for devices with more than one TWI port

[Nikolaus Voss]

Signed-off-by: Nikolaus Voss <n.voss@weinmann.de>
---
 arch/arm/mach-at91/at91sam9g45.c |    2 ++
 arch/arm/mach-at91/at91sam9rl.c  |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index 318b040..a374899 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -220,6 +220,8 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("spi_clk", "atmel_spi.1", &spi1_clk),
 	CLKDEV_CON_DEV_ID("t0_clk", "atmel_tcb.0", &tcb0_clk),
 	CLKDEV_CON_DEV_ID("t0_clk", "atmel_tcb.1", &tcb0_clk),
+	CLKDEV_CON_DEV_ID("twi_clk", "at91_i2c.0", &twi0_clk),
+	CLKDEV_CON_DEV_ID("twi_clk", "at91_i2c.1", &twi1_clk),
 	CLKDEV_CON_DEV_ID("pclk", "ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "ssc.1", &ssc1_clk),
 	CLKDEV_CON_DEV_ID(NULL, "atmel-trng", &trng_clk),
diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
index a238105..ab81223 100644
--- a/arch/arm/mach-at91/at91sam9rl.c
+++ b/arch/arm/mach-at91/at91sam9rl.c
@@ -184,6 +184,8 @@ static struct clk_lookup periph_clocks_lookups[] = {
 	CLKDEV_CON_DEV_ID("t2_clk", "atmel_tcb.0", &tc2_clk),
 	CLKDEV_CON_DEV_ID("pclk", "ssc.0", &ssc0_clk),
 	CLKDEV_CON_DEV_ID("pclk", "ssc.1", &ssc1_clk),
+	CLKDEV_CON_DEV_ID("twi_clk", "at91_i2c.0", &twi0_clk),
+	CLKDEV_CON_DEV_ID("twi_clk", "at91_i2c.1", &twi1_clk),
 };
 
 static struct clk_lookup usart_clocks_lookups[] = {
-- 
1.7.5.4

[PATCH v4 3/4] G45 TWI: remove open drain setting for twi function gpios

[Nikolaus Voss]

The G45 datasheets explicitly states that setting the open drain property
on peripheral function gpios is not allowed. (How about other A91 chips?)

Signed-off-by: Nikolaus Voss <n.voss@weinmann.de>
---
 arch/arm/mach-at91/at91sam9g45_devices.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 000b5e1..e0f67e7 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -684,18 +684,12 @@ void __init at91_add_device_i2c(short i2c_id, struct i2c_board_info *devices, in
 	/* pins used for TWI interface */
 	if (i2c_id == 0) {
 		at91_set_A_periph(AT91_PIN_PA20, 0);		/* TWD */
-		at91_set_multi_drive(AT91_PIN_PA20, 1);
-
 		at91_set_A_periph(AT91_PIN_PA21, 0);		/* TWCK */
-		at91_set_multi_drive(AT91_PIN_PA21, 1);
 
 		platform_device_register(&at91sam9g45_twi0_device);
 	} else {
 		at91_set_A_periph(AT91_PIN_PB10, 0);		/* TWD */
-		at91_set_multi_drive(AT91_PIN_PB10, 1);
-
 		at91_set_A_periph(AT91_PIN_PB11, 0);		/* TWCK */
-		at91_set_multi_drive(AT91_PIN_PB11, 1);
 
 		platform_device_register(&at91sam9g45_twi1_device);
 	}
-- 
1.7.5.4