AW: [PATCH v7 0/5] AT91: replace broken TWI driver i2c-at91.c
Hi, I've tested this driver with the pca953x GPIO expander driver with a PCA9554. In the case of 8 GPIO pins (my case) i2c_smbus_read_byte_data(...) is called to read the registers of the GPIO expander. This results in a at91_twi_xfer with two messages. The first message is to put the register address into the IADR and the second is to read the one byte content of the register. At the end we have a one byte read with a repeated start condition. I observed that reading out the GPIO status is one read delayed. The first read to a register from the pca953x driver does not result in a RXRDY interrupt and at91_twi_read_next_byte(...) is not called. Only the completion routine is triggered with a TXCOMP interrupt. Additionally, I took a look at the status flags just after the control flags are set (in at91_do_twi_transfer(...), AT91_TWI_START|AT91_TWI_STOP for the one byte read). Surprisingly, the AT91_TWI_RXRDY flag is zero before the first register read and one for the following reads. According to the manual this flag should be zero after a read on AT91_TWI_RHR. Reading the AT91_TWI_RHR before the control flags are set to be sure that the AT91_TWI_RXRDY is cleared leads to a never occurring RXRDY interrupt. This looks very strange to me. Can anyone help? 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: Hubert Feurstein [mailto:h.feurst...@gmail.com] Gesendet: Freitag, 25. November 2011 16:42 An: Nikolaus Voss Cc: linux-i2c@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org; ben-li...@fluff.org; Carsten Behling Betreff: Re: [PATCH v7 0/5] AT91: replace broken TWI driver i2c-at91.c Hi, I've tested this driver on a 2.6.38 kernel with several i2c clients (temp-sensor, audio-codec, touchscreen-controller, w1-bridge, io-expanders) and works without problems. SoC: at91sam9g45 Because of the 2.6.38 kernel, I had to skip [PATCH v7 2/5] Replace clk_lookup.con_id with clk_lookup.dev_id entries for twi clk and used instead: at91_clock_associate(twi0_clk, at91sam9g45_twi0_device.dev, twi_clk); Best Regards Hubert On 2011-11-23 16:35, Nikolaus Voss wrote: 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. Tested-by: Hubert Feurstein h.feurst...@gmail.com -- 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
