RE: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi,
Carsten Behling wrote on 2011-11-23:
this case is already catched in at91_do_twi_transfer():
Sorry, I did not found this code in your patch.
(http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg06556.html):
+ if (is_read) {
+ if (!dev-buf_len)
yes, this won't work for buf_len == 1. It is corrected in
https://lkml.org/lkml/2011/11/18/223 which I held back for some time
as it should have been just a feature extension. I was not aware it
also fixed the buf_len = 1 bug. Sorry for that...
(Explanation: in the first implementation I immediately decremented
buf_len, so buf_len == 1 could not occur. Later I removed that but
did not fully fold it into the base patch.)
Thanks,
Niko
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
AW: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
this case is already catched in at91_do_twi_transfer():
Sorry, I did not found this code in your patch.
(http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg06556.html):
+ 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);
+ }
Is there a more recent version ?
Mit freundlichen Grüßen / Best regards
Carsten Behling
Development Engineer
Garz Fricke GmbH
Tempowerkring 2, 21079 Hamburg - Germany
Amtsgericht Hamburg HRB 60514
Geschäftsführer: Manfred Garz, Matthias Fricke
Phone: +49 (0) 40 791 899 - 56
Fax:+49 40 / 791 899 - 39
www.garz-fricke.com
-Ursprüngliche Nachricht-
Von: Voss, Nikolaus [mailto:n.v...@weinmann.de]
Gesendet: Dienstag, 22. November 2011 17:26
An: Carsten Behling
Cc: 'linux-i2c@vger.kernel.org'; 'linux-arm-ker...@lists.infradead.org';
'linux-ker...@vger.kernel.org'
Betreff: RE: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi,
Carsten Behling wrote on 2011-11-22:
+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);
If dev-buf_len =1 at the beginning of a read transfer, a stop condition will
never be send.
this case is already catched in at91_do_twi_transfer():
+ if (dev-msg-flags I2C_M_RD) {
+ unsigned start_flags = AT91_TWI_START;
+
+ /* if only one byte is to be read, immediately stop transfer */
+ if (dev-buf_len = 1 !(dev-msg-flags I2C_M_RECV_LEN))
+ start_flags |= AT91_TWI_STOP;
+ at91_twi_write(dev, AT91_TWI_CR, start_flags);
Thanks for reviewing,
Niko
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
AW: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi,
I try to use the at24 eeprom driver on top of this driver.
This EEPROM (24c32) works with two address bytes.
Writing results in a call to at91_twi_xfer() with num=1.
In this case the internal address register is not used and the address is sent
out within the buffer.
Reading results in a call to at91_twi_xfer() with num=2.
In this case the internal address register is used.
However the MSB of the internal address resides in msg-buf[0] and the LSB
resides in msg-buf[1] of the first message.
As a result the code:
+ 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);
constructs an internal address in a wrong byte order.
Example: Try to read from address 0x100:
msg[0]-buf[0] = 0x1;
msg[0]-buf[1] = 0x0;
results in
internal_address = 0x1;
I think it must be:
+ for (i = 0; i msg-len; ++i) {
+ internal_address |= ((unsigned)msg-buf[msg-len-1-i])
(8 * i);
+ int_addr_flag += AT91_TWI_IADRSZ_1;
+ }
+ at91_twi_write(dev, AT91_TWI_IADR, internal_address);
... or the at24 eeprom driver constructs the wrong internal address ...
Mit freundlichen Grüßen / Best regards
Carsten Behling
Development Engineer
Garz Fricke GmbH
Tempowerkring 2, 21079 Hamburg - Germany
Amtsgericht Hamburg HRB 60514
Geschäftsführer: Manfred Garz, Matthias Fricke
Phone: +49 (0) 40 791 899 - 56
Fax:+49 40 / 791 899 - 39
www.garz-fricke.com
-Ursprüngliche Nachricht-
Von: Voss, Nikolaus [mailto:n.v...@weinmann.de]
Gesendet: Mittwoch, 23. November 2011 11:29
An: Carsten Behling
Cc: 'linux-i2c@vger.kernel.org'; 'linux-arm-ker...@lists.infradead.org';
'linux-ker...@vger.kernel.org'
Betreff: RE: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi,
Carsten Behling wrote on 2011-11-23:
this case is already catched in at91_do_twi_transfer():
Sorry, I did not found this code in your patch.
(http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg06556.html):
+ if (is_read) {
+ if (!dev-buf_len)
yes, this won't work for buf_len == 1. It is corrected in
https://lkml.org/lkml/2011/11/18/223 which I held back for some time
as it should have been just a feature extension. I was not aware it
also fixed the buf_len = 1 bug. Sorry for that...
(Explanation: in the first implementation I immediately decremented
buf_len, so buf_len == 1 could not occur. Later I removed that but
did not fully fold it into the base patch.)
Thanks,
Niko
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi Carsten,
Carsten Behling wrote on 2011-11-23:
I think it must be:
+ for (i = 0; i msg-len; ++i) {
+ internal_address |= ((unsigned)msg-buf[msg-len-1-i])
(8 * i);
+ int_addr_flag += AT91_TWI_IADRSZ_1;
+ }
+ at91_twi_write(dev, AT91_TWI_IADR, internal_address);
yes, I think you're right. I tested only with 8 bit addresses
(msg-len = 1), so I haven't seen this error.
I will modify the patch and post it later today.
Thanks for this feedback,
Niko
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi,
Carsten Behling wrote on 2011-11-22:
+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);
If dev-buf_len =1 at the beginning of a read transfer, a stop condition will
never be send.
this case is already catched in at91_do_twi_transfer():
+ if (dev-msg-flags I2C_M_RD) {
+ unsigned start_flags = AT91_TWI_START;
+
+ /* if only one byte is to be read, immediately stop transfer */
+ if (dev-buf_len = 1 !(dev-msg-flags I2C_M_RECV_LEN))
+ start_flags |= AT91_TWI_STOP;
+ at91_twi_write(dev, AT91_TWI_CR, start_flags);
Thanks for reviewing,
Niko
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
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.v...@weinmann.de
---
drivers/i2c/busses/Kconfig| 11 +-
drivers/i2c/busses/Makefile |1 +
drivers/i2c/busses/i2c-at91.c | 410 +
drivers/i2c/busses/i2c-at91.h | 80
4 files changed, 495 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 000..9f4e0d0
--- /dev/null
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -0,0 +1,410 @@
+/*
+ * i2c Support for Atmel's AT91 Two-Wire Interface (TWI)
+ *
+ * Copyright (C) 2011 Weinmann Medical GmbH
+ * Author: Nikolaus Voss n.v...@weinmann.de
+ *
+ * Evolved from original work by:
+ * Copyright (C) 2004 Rick Bronson
+ * Converted to 2.6 by Andrew Victor and...@sanpeople.com
+ *
+ * Borrowed heavily from original work by:
+ * Copyright (C) 2000 Philip Edelbrock p...@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_CLK_HZ 10 /* 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;
+ unsignedtransfer_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);
+
