Re: [PATCH v5 4/6] i2c: at91: add support for new alternative command mode

2015-06-09 Thread Nicolas Ferre
Le 03/06/2015 18:25, Cyrille Pitchen a écrit :
 The alternative command mode was introduced to simplify the transmission
 of STOP conditions and to solve timing and latency issues around them.
 
 This mode relies on a new register, the Alternative Command Register,
 which must be set at the same time as the Master Mode Register. This new
 register was designed to allow simple setup of basic combined transactions
 built from up to two unitary transactions.
 
 Indeed, the ACR is split into two areas, which describe one unitary
 transaction each. Each area is filled with Data Length 8bit counter, a
 Direction and a PEC Request bit. The PEC bit is only used in SMBus mode
 and is not supported by this driver yet. Also when using alternative
 command mode, the MREAD bit from the Master Mode Register is ignored.
 Instead the Direction bits from ACR are used to setup the direction, read
 or write, of each unitary transaction. Finally the 8bit counters must
 filled with the data length of their respective transaction. Then if only
 one transaction is to be used, the data length of the second one must be
 set to zero. At the moment, this driver uses only the first transaction.
 
 In addition to MMR and ACR, the Control Register also need to be written
 to enable the alternative command mode. That's the purpose of its ACMEN
 bit, which stands for Alternative Command Mode Enable.
 
 Note that the alternative command mode is compatible with the use of the
 Internal Address Register. So combined transactions for eeprom read are
 actually implemented with the Internal Address Register. This register is
 written with up to 3 bytes, which are the internal address sent to the
 slave through the first write transaction. Then the first area of the ACR
 describe the write transaction to follow, which carries the data to be
 read from the eeprom. The second area of the ACR is not used so its Data
 Length 8bit counter is cleared.
 
 For each byte sent or received by the device, the Data Length 8bit counter
 is decremented. When it reaches 0, a STOP condition is automatically sent.
 
 Signed-off-by: Cyrille Pitchen cyrille.pitc...@atmel.com
 ---
  drivers/i2c/busses/i2c-at91.c | 121 
 +++---
  1 file changed, 101 insertions(+), 20 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index 0e88b68..67b4f15 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -49,6 +49,11 @@
  #define  AT91_TWI_SVDIS  BIT(5)  /* Slave Transfer Disable */
  #define  AT91_TWI_QUICK  BIT(6)  /* SMBus quick command */
  #define  AT91_TWI_SWRST  BIT(7)  /* Software Reset */
 +#define  AT91_TWI_ACMEN  BIT(16) /* Alternative Command Mode 
 Enable */
 +#define  AT91_TWI_ACMDIS BIT(17) /* Alternative Command Mode 
 Disable */
 +#define  AT91_TWI_THRCLR BIT(24) /* Transmit Holding Register 
 Clear */
 +#define  AT91_TWI_RHRCLR BIT(25) /* Receive Holding Register 
 Clear */
 +#define  AT91_TWI_LOCKCLRBIT(26) /* Lock Clear */
  
  #define  AT91_TWI_MMR0x0004  /* Master Mode Register */
  #define  AT91_TWI_IADRSZ_1   0x0100  /* Internal Device Address Size 
 */
 @@ -65,6 +70,7 @@
  #define  AT91_TWI_OVRE   BIT(6)  /* Overrun Error */
  #define  AT91_TWI_UNRE   BIT(7)  /* Underrun Error */
  #define  AT91_TWI_NACK   BIT(8)  /* Not Acknowledged */
 +#define  AT91_TWI_LOCK   BIT(23) /* TWI Lock due to Frame Errors 
 */
  
  #define  AT91_TWI_INT_MASK \
   (AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY | AT91_TWI_NACK)
 @@ -75,10 +81,15 @@
  #define  AT91_TWI_RHR0x0030  /* Receive Holding Register */
  #define  AT91_TWI_THR0x0034  /* Transmit Holding Register */
  
 +#define  AT91_TWI_ACR0x0040  /* Alternative Command Register 
 */
 +#define  AT91_TWI_ACR_DATAL(len) ((len)  0xff)
 +#define  AT91_TWI_ACR_DIRBIT(8)
 +
  struct at91_twi_pdata {
   unsigned clk_max_div;
   unsigned clk_offset;
   bool has_unre_flag;
 + bool has_alt_cmd;
   struct at_dma_slave dma_slave;
  };
  
 @@ -204,7 +215,8 @@ static void at91_twi_write_next_byte(struct at91_twi_dev 
 *dev)
  
   /* send stop when last byte has been written */
   if (--dev-buf_len == 0)
 - at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
 + if (!dev-pdata-has_alt_cmd)
 + at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
  
   dev_dbg(dev-dev, wrote 0x%x, to go %d\n, *dev-buf, dev-buf_len);
  
 @@ -226,7 +238,8 @@ static void at91_twi_write_data_dma_callback(void *data)
* we just have to enable TXCOMP one.
*/
   at91_twi_write(dev, AT91_TWI_IER, AT91_TWI_TXCOMP);
 - at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
 + if (!dev-pdata-has_alt_cmd)
 + 

Re: [PATCH v5 4/6] i2c: at91: add support for new alternative command mode

2015-06-09 Thread Wolfram Sang
Nicolas,

  +static struct at91_twi_pdata at91sama5d2_config = {
 
 No, please name it:
 
 sama5d2_config

Thanks for your input, but please don't quote the whole mail for those
tiny comments. Only the relevant parts, please. Otherwise it is too easy
to miss things.

Thanks,

   Wolfram



signature.asc
Description: Digital signature