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-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 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
Re: [PATCH v5 3/4] drivers/i2c/busses/i2c-at91.c: add new driver
On Tue, Nov 08, 2011 at 11:49:46AM +0100, Nikolaus Voss wrote: > 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 FWIW: Reviewed-by: Felipe Balbi -- balbi signature.asc Description: Digital signature