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

[Eddie James]

On 10/20/2017 12:15 AM, Andrew Jeffery wrote:
> 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?

Good point, I'll switch that.

Thanks!
Eddie

>
> Andrew