[PATCH linux dev-4.10] drivers: i2c: fsi: Add proper abort method

[Andrew Jeffery]

On Thu, 2017-10-12 at 15:42 -0500, Eddie James wrote:
> From: "Edward A. James" <eajames at us.ibm.com>
> 
> Driver wasn't cleaning up on timeout or in an error situation properly.
> Need to do a full reset if we fail in order to re-stablish a good state
> of the engine.
> 
> Signed-off-by: Edward A. James <eajames at us.ibm.com>
> ---
>  drivers/i2c/busses/i2c-fsi.c | 259 +++++++++++++++++++++++++++++++------------
>  1 file changed, 191 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index 1af9c01..6c582d8 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -133,6 +133,11 @@
>  #define I2C_ESTAT_SELF_BUSY	0x00000040
>  #define I2C_ESTAT_VERSION	0x0000001f
>  
> +#define I2C_PORT_BUSY_RESET	0x80000000
> +
> +#define I2C_LOCAL_WAIT_TIMEOUT	2		/* jiffies */
> +#define I2C_ABORT_TIMEOUT	msecs_to_jiffies(100)
> +
>  struct fsi_i2c_master {
>  	struct fsi_device	*fsi;
>  	u8			fifo_size;
> @@ -351,22 +356,185 @@ static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
>  	return rc;
>  }
>  
> +static int fsi_i2c_reset_bus(struct fsi_i2c_master *i2c)
> +{
> +	int i, rc;
> +	u32 mode, dummy = 0;
> +
> +	rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> +	if (rc)
> +		return rc;
> +
> +	mode |= I2C_MODE_DIAG;
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> +	if (rc)
> +		return rc;
> +
> +	for (i = 0; i < 9; ++i) {
> +		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SCL, &dummy);
> +		if (rc)
> +			return rc;
> +
> +		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SCL, &dummy);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SCL, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_SDA, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SCL, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_SET_SDA, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	mode &= ~I2C_MODE_DIAG;
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> +
> +	return rc;
> +}
> +
> +static int fsi_i2c_reset(struct fsi_i2c_master *i2c, u16 port)
> +{
> +	int rc;
> +	u32 mode, stat, dummy = 0;
> +
> +	/* reset engine */
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_I2C, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	/* re-init engine */
> +	rc = fsi_i2c_dev_init(i2c);
> +	if (rc)
> +		return rc;
> +
> +	rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> +	if (rc)
> +		return rc;
> +
> +	/* set port; default after reset is 0 */
> +	if (port) {
> +		mode = SETFIELD(I2C_MODE_PORT, mode, port);
> +		rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_MODE, &mode);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	/* reset busy register; hw workaround */
> +	dummy = I2C_PORT_BUSY_RESET;
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_PORT_BUSY, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	/* force bus reset */
> +	rc = fsi_i2c_reset_bus(i2c);
> +	if (rc)
> +		return rc;
> +
> +	/* reset errors */
> +	dummy = 0;
> +	rc = fsi_i2c_write_reg(i2c->fsi, I2C_FSI_RESET_ERR, &dummy);
> +	if (rc)
> +		return rc;
> +
> +	/* wait for command complete */
> +	set_current_state(TASK_INTERRUPTIBLE);
> +	schedule_timeout(I2C_LOCAL_WAIT_TIMEOUT);

Shouldn't we use the uninterruptible state to ensure we get at least
I2C_LOCAL_WAIT_TIMEOUT in the face of signals?

Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20171020/9269d94e/attachment.sig>