Re: [Patch V8] i2c: imx: implement bus recovery

[Sascha Hauer <s.hauer@pengutronix.de>]

On Mon, Oct 19, 2015 at 10:17:02AM +0800, Gao Pan wrote:
> Implement bus recovery methods for i2c-imx so we can recover from
> situations where SCL/SDA are stuck low.
> 
> Once i2c bus SCL/SDA are stuck low during transfer, config the i2c
> pinctrl to gpio mode by calling pinctrl sleep set function, and then
> use GPIO to emulate the i2c protocol to send nine dummy clock to recover
> i2c device. After recovery, set i2c pinctrl to default group setting.
> 
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Gao Pan <b54642@freescale.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
> V2:
> As Uwe Kleine-König's suggestion, the version do below changes:
>  -replace of_get_named_gpio() with devm_gpiod_get_optional()
>  -move gpio_direction_input() and gpio_direction_output() call to the
>   prepare callback
>  -use 0 and 1 as return value for the get_scl and get_sda callbacks
> 
> V3:
>  -replace "...-gpio" with "...-gpios" in i2c binding doc
>  -document the requirement of using sleep state to configure
>   the pins as gpio in i2c binding doc
>  -remove i2c_recover_bus() in i2c_imx_trx_complete()
>  -use GPIOD_OUT_HIGH as the parameter of devm_gpiod_get_optional to
>   config the gpios as output high
>  -add error disposal as devm_gpiod_get_optional meets error
> 
> V4:
>  -remove include <linux/of_gpio.h>
>  -call i2c_recover_bus under the condition of the existing of
>   both sda and scl. Drop the sda and scl check in i2c_imx_get_scl,
>   i2c_imx_get_sda, i2c_imx_set_scl, i2c_imx_prepare_recovery and
>   i2c_imx_prepare_recovery
>  -use GPIOD_OUT_IN as the parameter of devm_gpiod_get_optional
>  -remove documenting the requirement of using sleep state to configure
>   the pins as gpio in i2c binding doc
> 
> V5:
>  -introduce a dedicated gpio state for bus recovery.
>  -assign adapter.bus_recovery_info after the two gpios were aquired successfully
> 
> V6:
>  -assign adapter.bus_recovery_info under the condition that gpios are acquired successfully
>  -not try the recovery when pinctrl_lookup_state returns an error
> 
> V7:
>  -execute the bus recovery once in a transfer, when the controller fails to start
>  -use i2c_generic_gpio_recovery() to handle the bus recovery with gpios
> 
> V8:
>  -remove unused symbol and unrelated whitespace changes
> 
>  Documentation/devicetree/bindings/i2c/i2c-imx.txt |  9 +++
>  drivers/i2c/busses/i2c-imx.c                      | 70 +++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-imx.txt b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> index ce4311d..eab5836 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-imx.txt
> @@ -14,6 +14,10 @@ Optional properties:
>    The absence of the propoerty indicates the default frequency 100 kHz.
>  - dmas: A list of two dma specifiers, one for each entry in dma-names.
>  - dma-names: should contain "tx" and "rx".
> +- scl-gpios: specify the gpio related to SCL pin
> +- sda-gpios: specify the gpio related to SDA pin
> +- pinctrl: add extra pinctrl to configure i2c pins to gpio function for i2c
> +  bus recovery, call it "gpio" state
>  
>  Examples:
>  
> @@ -37,4 +41,9 @@ i2c0: i2c@40066000 { /* i2c0 on vf610 */
>  	dmas = <&edma0 0 50>,
>  		<&edma0 0 51>;
>  	dma-names = "rx","tx";
> +	pinctrl-names = "default", "gpio";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	pinctrl-1 = <&pinctrl_i2c1_gpio>;
> +	scl-gpios = <&gpio5 26 GPIO_ACTIVE_HIGH>;
> +	sda-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>;
>  };
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 91bdf28..9afe107 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -49,6 +49,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_dma.h>
> +#include <linux/of_gpio.h>
>  #include <linux/platform_data/i2c-imx.h>
>  #include <linux/platform_device.h>
>  #include <linux/sched.h>
> @@ -210,6 +211,11 @@ struct imx_i2c_struct {
>  	unsigned int		cur_clk;
>  	unsigned int		bitrate;
>  	const struct imx_i2c_hwdata	*hwdata;
> +	struct i2c_bus_recovery_info rinfo;
> +
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *pinctrl_pins_default;
> +	struct pinctrl_state *pinctrl_pins_gpio;
>  
>  	struct imx_i2c_dma	*dma;
>  };
> @@ -899,6 +905,13 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>  
>  	/* Start I2C transfer */
>  	result = i2c_imx_start(i2c_imx);
> +	if (result) {
> +		if (i2c_imx->adapter.bus_recovery_info) {
> +			i2c_recover_bus(&i2c_imx->adapter);
> +			result = i2c_imx_start(i2c_imx);
> +		}
> +	}
> +
>  	if (result)
>  		goto fail0;
>  
> @@ -963,6 +976,55 @@ out:
>  	return (result < 0) ? result : num;
>  }
>  
> +static void i2c_imx_prepare_recovery(struct i2c_adapter *adap)
> +{
> +	struct imx_i2c_struct *i2c_imx;
> +
> +	i2c_imx = container_of(adap, struct imx_i2c_struct, adapter);
> +
> +	pinctrl_select_state(i2c_imx->pinctrl, i2c_imx->pinctrl_pins_gpio);
> +}
> +
> +static void i2c_imx_unprepare_recovery(struct i2c_adapter *adap)
> +{
> +	struct imx_i2c_struct *i2c_imx;
> +
> +	i2c_imx = container_of(adap, struct imx_i2c_struct, adapter);
> +
> +	pinctrl_select_state(i2c_imx->pinctrl, i2c_imx->pinctrl_pins_default);
> +}
> +
> +static void i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
> +		struct platform_device *pdev)
> +{
> +	struct i2c_bus_recovery_info *rinfo = &i2c_imx->rinfo;
> +
> +	i2c_imx->pinctrl_pins_default = pinctrl_lookup_state(i2c_imx->pinctrl,
> +			PINCTRL_STATE_DEFAULT);
> +	i2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(i2c_imx->pinctrl,
> +			"gpio");
> +	rinfo->sda_gpio = of_get_named_gpio_flags(pdev->dev.of_node,
> +			"sda-gpios", 0, NULL);
> +	rinfo->scl_gpio = of_get_named_gpio_flags(pdev->dev.of_node,
> +			"scl-gpios", 0, NULL);
> +
> +	if (!gpio_is_valid(rinfo->sda_gpio) ||
> +	    !gpio_is_valid(rinfo->scl_gpio) ||
> +	    IS_ERR(i2c_imx->pinctrl_pins_default) ||
> +	    IS_ERR(i2c_imx->pinctrl_pins_gpio)) {
> +		dev_dbg(&pdev->dev, "recovery information incomplete\n");
> +		return;
> +	}
> +
> +	dev_dbg(&pdev->dev, "using scl-gpio %d and sda-gpio %d for recovery\n",
> +			rinfo->sda_gpio, rinfo->scl_gpio);
> +
> +	rinfo->prepare_recovery = i2c_imx_prepare_recovery;
> +	rinfo->unprepare_recovery = i2c_imx_unprepare_recovery;
> +	rinfo->recover_bus = i2c_generic_gpio_recovery;
> +	i2c_imx->adapter.bus_recovery_info = rinfo;
> +}
> +
>  static u32 i2c_imx_func(struct i2c_adapter *adapter)
>  {
>  	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
> @@ -1031,6 +1093,12 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
> +	if (IS_ERR(i2c_imx->pinctrl)) {
> +		ret = PTR_ERR(i2c_imx->pinctrl);
> +		goto clk_disable;
> +	}
> +
>  	/* Request IRQ */
>  	ret = devm_request_irq(&pdev->dev, irq, i2c_imx_isr, 0,
>  				pdev->name, i2c_imx);
> @@ -1045,6 +1113,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  	/* Set up adapter data */
>  	i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
>  
> +	i2c_imx_init_recovery_info(i2c_imx, pdev);
> +
>  	/* Set up platform driver data */
>  	platform_set_drvdata(pdev, i2c_imx);
>  
> -- 
> 1.9.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |