Re: [PATCH 3/3] i2c: xiic: Support forcing single-master in DT

[Michal Simek <michal.simek@xilinx.com>]

On 27. 08. 20 16:48, ext-jaakko.laine@vaisala.com wrote:
> From: Jaakko Laine <ext-jaakko.laine@vaisala.com>
> 
> I2C master operating in multimaster mode can get stuck
> indefinitely if I2C start is detected on bus, but no master
> has a transaction going.
> 
> This is a weakness in I2C standard, which defines no way
> to recover, since all masters are indefinitely disallowed
> from interrupting the currently operating master. A start
> condition can be created for example by an electromagnetic
> discharge applied near physical I2C lines. Or a already
> operating master could get reset immediately after sending
> a start.
> 
> If it is known during device tree creation that only a single
> I2C master will be present on the bus, this deadlock of the
> I2C bus could be avoided in the driver by ignoring the
> bus_is_busy register of the xiic, since bus can never be
> reserved by any other master.
> 
> This patch adds this support for detecting single-master flag
> in device tree and when provided, improves I2C reliability by
> ignoring the therefore unnecessary xiic bus_is_busy register.
> 
> Error can be reproduced by pulling I2C SDA -line temporarily low
> by shorting it to ground, while linux I2C master is operating on
> it using the xiic driver. The application using the bus will
> start receiving linux error code 16: "Device or resource busy"
> indefinitely:
> 
> kernel: pca953x 0-0020: failed writing register
> app: Error writing file, error: 16
> 
> With multi-master disabled device will instead receive error
> code 5: "I/O error" while SDA is grounded, but recover normal
> operation once short is removed.
> 
> kernel: pca953x 0-0020: failed reading register
> app: Error reading file, error: 5
> 
> Signed-off-by: Jaakko Laine <ext-jaakko.laine@vaisala.com>
> ---
>  drivers/i2c/busses/i2c-xiic.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 1453d82bb664..087b2951942e 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -59,6 +59,7 @@ enum xiic_endian {
>   * @endianness: big/little-endian byte order
>   * @clk: Pointer to AXI4-lite input clock
>   * @state: See STATE_
> + * @singlemaster: Indicates bus is single master
>   */
>  struct xiic_i2c {
>  	struct device *dev;
> @@ -74,6 +75,7 @@ struct xiic_i2c {
>  	enum xiic_endian endianness;
>  	struct clk *clk;
>  	enum xilinx_i2c_state state;
> +	bool singlemaster;
>  };
>  
>  
> @@ -526,6 +528,15 @@ static int xiic_busy(struct xiic_i2c *i2c)
>  	if (i2c->tx_msg)
>  		return -EBUSY;
>  
> +	/* In single master mode bus can only be busy, when in use by this
> +	 * driver. If the register indicates bus being busy for some reason we
> +	 * should ignore it, since bus will never be released and i2c will be
> +	 * stuck forever.
> +	 */
> +	if (i2c->singlemaster) {
> +		return 0;
> +	}
> +
>  	/* for instance if previous transfer was terminated due to TX error
>  	 * it might be that the bus is on it's way to become available
>  	 * give it at most 3 ms to wake
> @@ -811,6 +822,9 @@ static int xiic_i2c_probe(struct platform_device *pdev)
>  		goto err_clk_dis;
>  	}
>  
> +	i2c->singlemaster =
> +		of_property_read_bool(pdev->dev.of_node, "single-master");
> +
>  	/*
>  	 * Detect endianness
>  	 * Try to reset the TX FIFO. Then check the EMPTY flag. If it is not
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal