RE: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver

2011-11-23 Thread Voss, Nikolaus 
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

2011-11-23 Thread Voss, Nikolaus 
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

2011-11-22 Thread Voss, Nikolaus 
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