From 8d81f8799e431e66457ff164172bf98a47bdbbf2 Mon Sep 17 00:00:00 2001 From: Mulin Chao Date: Mon, 16 May 2016 14:47:06 -0700 Subject: [PATCH] npcx: i2c: Fix spurious NACK after i2cscan The NPCX_SMBCTL1_ACK bit (which tells the I2C master to produce a NACK rather than ACK) can be set but not cleared by SW -- it can only be cleared automatically after actually generating a NACK. During i2cscan, we want to NACK the first byte returned, but we won't actually produce a NACK unless the slave ACKs our scanned address. Therefore, we must wait until the slave ACKs its address before setting NPCX_SMBCTL1_ACK -- use the NPCX_SMBCTL1_STASTRE / stall interrupt to do so. BUG=chrome-os-partner:53323 BRANCH=None TEST=Manual on kevin. Verify faulty temperature sensor reads are not seen after `i2cscan` and verify i2c otherwise functions normally. Change-Id: I080d8804adb246129aaebbfbf5ad862e9513da3b Signed-off-by: Shawn Nematbakhsh Reviewed-on: https://chromium-review.googlesource.com/344818 Commit-Ready: Shawn N Tested-by: Shawn N Reviewed-by: Mulin Chao Reviewed-by: Shawn N --- chip/npcx/i2c.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/chip/npcx/i2c.c b/chip/npcx/i2c.c index 8a8b7815f7..1f71afffff 100644 --- a/chip/npcx/i2c.c +++ b/chip/npcx/i2c.c @@ -38,6 +38,7 @@ #define I2C_START(ctrl) SET_BIT(NPCX_SMBCTL1(ctrl), NPCX_SMBCTL1_START) #define I2C_STOP(ctrl) SET_BIT(NPCX_SMBCTL1(ctrl), NPCX_SMBCTL1_STOP) #define I2C_NACK(ctrl) SET_BIT(NPCX_SMBCTL1(ctrl), NPCX_SMBCTL1_ACK) +#define I2C_STALL(ctrl) SET_BIT(NPCX_SMBCTL1(ctrl), NPCX_SMBCTL1_STASTRE) #define I2C_WRITE_BYTE(ctrl, data) (NPCX_SMBSDA(ctrl) = data) #define I2C_READ_BYTE(ctrl, data) (data = NPCX_SMBSDA(ctrl)) @@ -301,6 +302,13 @@ enum smb_error i2c_master_transaction(int controller) /* Disable event and error interrupts */ task_disable_irq(i2c_irqs[controller]); + /* + * If Stall-After-Start mode is still enabled since NACK or BUS error + * occurs, disable it. + */ + if (IS_BIT_SET(NPCX_SMBCTL1(controller), NPCX_SMBCTL1_STASTRE)) + CLEAR_BIT(NPCX_SMBCTL1(controller), NPCX_SMBCTL1_STASTRE); + /* Handle bus timeout */ if ((events & TASK_EVENT_I2C_IDLE) == 0) { p_status->err_code = SMB_TIMEOUT_ERROR; @@ -333,11 +341,12 @@ inline void i2c_handle_sda_irq(int controller) if (p_status->sz_txbuf == 0) {/* Receive mode */ p_status->oper_state = SMB_READ_OPER; /* - * Receiving one byte only - set nack just - * before writing address byte + * Receiving one byte only - stall bus after START + * condition. If there's no slave devices on bus, FW + * needn't to set ACK bit. */ if (p_status->sz_rxbuf == 1) - I2C_NACK(controller); + I2C_STALL(controller); /* Write the address to the bus R bit*/ I2C_WRITE_BYTE(controller, (addr | 0x1)); @@ -511,7 +520,22 @@ void i2c_master_int_handler (int controller) CPUTS("-NA"); } - /* Condition 3: SDA status is set - transmit or receive */ + /* Condition 3: A Stall after START has occurred for READ-BYTE */ + if (IS_BIT_SET(NPCX_SMBST(controller), NPCX_SMBST_STASTR)) { + CPUTS("-STL"); + /* Clear STASTR Bit */ + SET_BIT(NPCX_SMBST(controller), NPCX_SMBST_STASTR); + /* Disable Stall-After-Start mode */ + CLEAR_BIT(NPCX_SMBCTL1(controller), NPCX_SMBCTL1_STASTRE); + /* + * Continue to handle protocol - release SCL bus & set ACK bit + * if necessary + */ + if (p_status->flags & I2C_XFER_STOP) + I2C_NACK(controller); + } + + /* Condition 4: SDA status is set - transmit or receive */ if (IS_BIT_SET(NPCX_SMBST(controller), NPCX_SMBST_SDAST)) { i2c_handle_sda_irq(controller); #if DEBUG_I2C