Re: [PATCH] i2c: i2c_mxs: Set ACK_MODE bit

["Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

On Tue, Jul 02, 2013 at 01:01:00PM -0300, Fabio Estevam wrote:
> According to mx23 erratum 2727:
> 
> "2727 : I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set.
> 
> Description:
> 
> When RETAIN_CLOCK is set, the ninth clock pulse (ACK) is not generated. 
> However, the SDA line is read at the proper timing interval. If 
> RETAIN_CLOCK is cleared, the ninth clock pulse is generated.
> Also, the HW_I2C_VERSION register incorrectly states the version is 1.2. 
> It should be 1.3.
> 
> Workaround:
> HW_I2C_CTRL1[ACK_MODE] has default value of 0. It should be set to 1 to 
> enable the fix for this issue."
> 
> It has also been noticed that mx28 needs to implement this fix in order to have
> SMBus to work properly.
> 
> Reported-by: Christoph Baumann <cb-/RsSufbtIHM@public.gmane.org>
> Signed-off-by: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Did you see this making the driver handle some situations that caused
failure before? 

> ---
>  drivers/i2c/busses/i2c-mxs.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 6d8094d..ce7ac86 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -56,6 +56,7 @@
>  #define MXS_I2C_CTRL1_CLR	(0x48)
>  
>  #define MXS_I2C_CTRL1_CLR_GOT_A_NAK		0x10000000
> +#define MXS_I2C_CTRL1_ACK_MODE			0x08000000
>  #define MXS_I2C_CTRL1_BUS_FREE_IRQ		0x80
>  #define MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ	0x40
>  #define MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ		0x20
> @@ -140,6 +141,14 @@ static void mxs_i2c_reset(struct mxs_i2c_dev *i2c)
>  	writel(0x00300030, i2c->regs + MXS_I2C_TIMING2);
>  
>  	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
> +
> +	/*
> +	 * According to mx23 erratum 2727:
> +	 * "I2C 9th Clock Pulse (ACK) not generated when RETAIN_CLOCK set"
> +	 *
> +	 * HW_I2C_CTRL1[ACK_MODE] needs to be set when RETAIN_CLOCK is set.
> +	 */
> +	writel(MXS_I2C_CTRL1_ACK_MODE, i2c->regs + MXS_I2C_CTRL1_SET);
So you set this bis in the reset routine. The first thing in
mxs_i2c_pio_setup_xfer is however:

	writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_CLR);

which unsets the ACK_MODE bit. Also when reading the docs I couldn't
find out the motivation to set RETAIN_CLOCK at all in the select
command.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |