RE: [PATCH V3 2/4] drivers/i2c/busses/i2c-at91.c: add new driver
Hi Ryan, +#include mach/at91_twi.h This include file looks like it only contains register definitions which are used in this file. Would be good to either move them directly into this driver or move the header file to this directory and make it a local include. Ok. + + dev-dev = get_device(pdev-dev); Is this (and the put_device(pdev-dev) in the exit code) necessary? I see that some other i2c drivers do this also, but I'm not familiar with using it in other device drivers. Isn't the reference count for pdev-dev already incremented by the fact we are probing the device? It is incremented by the platform_device_register() which does a device_add(). This seems to be enough, I removed the get-/put_device(). + dev-irq = irq-start; + platform_set_drvdata(pdev, dev); + + dev-clk = clk_get(pdev-dev, twi_clk); This didn't get answered. Can't you just do: dev-clk = clk_get(pdev-dev, NULL); and clkdev should figure out the correct clock based on the device pointer? No, this doesn't work on at91 since the clocks have no dev_id property but only con_id. I cannot see a reason for this, but that's the way it's done. Multiple hardware interfaces are handled via a lookup table. + rc = i2c_del_adapter(dev-adapter); Most of the other i2c drivers don't check the return code for i2c_del_adapter. If this fails then you will unload the module, but potentially leak resources that i2c_del_adapter failed to free up. Not sure what the correct answer is here. I don't really check the value but use it has exit code for the remove()- function to indicate something went wrong. Just ignoring it wouldn't heal the resource leakage. 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 v4 0/4] AT91: replace broken TWI driver i2c-at91.c
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 -- 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 v4 2/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 | 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 000..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.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_CLOCK_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);
+ at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_MSEN);
[PATCH v4 4/4] Add lookup entries for twi_clk for devices with more than one TWI port
Signed-off-by: Nikolaus Voss n.v...@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
--
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 v4 1/4] drivers/i2c/busses/i2c-at91.c: remove broken driver
Signed-off-by: Nikolaus Voss n.v...@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..000 --- 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 - -#defineAT91_TWI_CR 0x00/* Control Register */ -#defineAT91_TWI_START (1 0) /* Send a Start Condition */ -#defineAT91_TWI_STOP (1 1) /* Send a Stop Condition */ -#defineAT91_TWI_MSEN (1 2) /* Master Transfer Enable */ -#defineAT91_TWI_MSDIS (1 3) /* Master Transfer Disable */ -#defineAT91_TWI_SVEN (1 4) /* Slave Transfer Enable [SAM9260 only] */ -#defineAT91_TWI_SVDIS (1 5) /* Slave Transfer Disable [SAM9260 only] */ -#defineAT91_TWI_SWRST (1 7) /* Software Reset */ - -#defineAT91_TWI_MMR0x04/* Master Mode Register */ -#defineAT91_TWI_IADRSZ (3 8)/* Internal Device Address Size */ -#defineAT91_TWI_IADRSZ_NO (0 8) -#defineAT91_TWI_IADRSZ_1 (1 8) -#defineAT91_TWI_IADRSZ_2 (2 8) -#defineAT91_TWI_IADRSZ_3 (3 8) -#defineAT91_TWI_MREAD (1 12)/* Master Read Direction */ -#defineAT91_TWI_DADR (0x7f 16)/* Device Address */ - -#defineAT91_TWI_SMR0x08/* Slave Mode Register [SAM9260 only] */ -#defineAT91_TWI_SADR (0x7f 16)/* Slave Address */ - -#defineAT91_TWI_IADR 0x0c/* Internal Address Register */ - -#defineAT91_TWI_CWGR 0x10/* Clock Waveform Generator Register */ -#defineAT91_TWI_CLDIV (0xff 0)/* Clock Low Divisor */ -#defineAT91_TWI_CHDIV (0xff 8)/* Clock High Divisor */ -#defineAT91_TWI_CKDIV (7 16)/* Clock Divider */ - -#defineAT91_TWI_SR 0x20/* Status Register */ -#defineAT91_TWI_TXCOMP (1 0) /* Transmission Complete */ -#defineAT91_TWI_RXRDY (1 1) /* Receive Holding Register Ready */ -#defineAT91_TWI_TXRDY (1 2) /* Transmit Holding Register Ready */ -#defineAT91_TWI_SVREAD (1 3) /* Slave Read [SAM9260 only] */ -#defineAT91_TWI_SVACC (1 4) /* Slave Access [SAM9260 only] */ -#defineAT91_TWI_GACC (1 5) /* General Call Access [SAM9260 only] */ -#defineAT91_TWI_OVRE (1 6) /* Overrun Error [AT91RM9200 only] */ -#defineAT91_TWI_UNRE (1 7) /* Underrun Error [AT91RM9200 only] */ -#defineAT91_TWI_NACK (1 8) /* Not Acknowledged */ -#defineAT91_TWI_ARBLST (1 9) /* Arbitration Lost [SAM9260 only] */ -#defineAT91_TWI_SCLWS (1 10) /* Clock Wait State [SAM9260 only] */ -#defineAT91_TWI_EOSACC (1 11) /* End of Slave Address [SAM9260 only] */ - -#defineAT91_TWI_IER0x24/* Interrupt Enable Register */ -#defineAT91_TWI_IDR0x28/* Interrupt Disable Register */ -#defineAT91_TWI_IMR0x2c/* Interrupt Mask Register */ -#defineAT91_TWI_RHR0x30/* Receive Holding Register */ -#defineAT91_TWI_THR0x34/* Transmit Holding Register */ - -#endif - diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index fba6da6..e8a1852
[PATCH v4 3/4] G45 TWI: remove open drain setting for twi function gpios
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.v...@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
--
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 V3 2/4] drivers/i2c/busses/i2c-at91.c: add new driver
On Wed, Nov 09, 2011 at 05:01:07PM +0100, Voss, Nikolaus wrote: Hi Ryan, + dev-irq = irq-start; + platform_set_drvdata(pdev, dev); + + dev-clk = clk_get(pdev-dev, twi_clk); This didn't get answered. Can't you just do: dev-clk = clk_get(pdev-dev, NULL); and clkdev should figure out the correct clock based on the device pointer? No, this doesn't work on at91 since the clocks have no dev_id property but only con_id. I cannot see a reason for this, but that's the way it's done. Multiple hardware interfaces are handled via a lookup table. If you can, you could move over to doing this via the standard method and start switching some of this stuff over to using dev_id. We'll then have another platform starting to use this stuff correctly. -- 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 v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver
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 000..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.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.
+*/
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
signature.asc
Description: Digital signature
Re: [PATCH v4 2/4] drivers/i2c/busses/i2c-at91.c: add new driver
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
--
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 1/1] i2c: imx: convert clk_prepare/clk_unprepare
Signed-off-by: Richard Zhao richard.z...@linaro.org --- drivers/i2c/busses/i2c-imx.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 58832e5..aad623b 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -196,6 +196,7 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx) dev_dbg(i2c_imx-adapter.dev, %s\n, __func__); + clk_prepare(i2c_imx-clk); clk_enable(i2c_imx-clk); writeb(i2c_imx-ifdr, i2c_imx-base + IMX_I2C_IFDR); /* Enable I2C controller */ @@ -246,6 +247,7 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx) /* Disable I2C controller */ writeb(0, i2c_imx-base + IMX_I2C_I2CR); clk_disable(i2c_imx-clk); + clk_unprepare(i2c_imx-clk); } static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx, -- 1.7.5.4 -- 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
