Re: [PATCH V6 2/2] i2c/designware: Provide i2c bus recovery support

[Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 6995 bytes --]

On Thu, Oct 04, 2012 at 04:34:54PM +0530, Viresh Kumar wrote:
> Add bus recovery support for designware_i2c controller. It uses generic gpio
> based i2c_gpio_recover_bus() routine.
> 
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Shiraz Hashim <shiraz.hashim-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> V5->V6:
> - removed sda_gpio_flags
> 
>  drivers/i2c/busses/i2c-designware-core.c    |  7 ++++-
>  drivers/i2c/busses/i2c-designware-platdrv.c | 38 ++++++++++++++++++++++--
>  include/linux/i2c/i2c-designware.h          | 46 +++++++++++++++++++++++++++++
>  3 files changed, 88 insertions(+), 3 deletions(-)
>  create mode 100644 include/linux/i2c/i2c-designware.h
> 
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index cbba7db..9b7c9bf 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -538,7 +538,12 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete, HZ);
>  	if (ret == 0) {
>  		dev_err(dev->dev, "controller timed out\n");
> -		i2c_dw_init(dev);
> +		if (adap->bus_recovery_info &&
> +				adap->bus_recovery_info->recover_bus) {
> +			dev_dbg(dev->dev, "try i2c bus recovery\n");
> +			adap->bus_recovery_info->recover_bus(adap);
> +		}
> +

This should be in the core?

>  		ret = -ETIMEDOUT;
>  		goto done;
>  	} else if (ret < 0)
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 0506fef..afa0df9 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -29,6 +29,7 @@
>  #include <linux/module.h>
>  #include <linux/delay.h>
>  #include <linux/i2c.h>
> +#include <linux/i2c/i2c-designware.h>
>  #include <linux/clk.h>
>  #include <linux/errno.h>
>  #include <linux/sched.h>
> @@ -45,6 +46,7 @@ static struct i2c_algorithm i2c_dw_algo = {
>  	.master_xfer	= i2c_dw_xfer,
>  	.functionality	= i2c_dw_func,
>  };
> +
>  static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
>  {
>  	return clk_get_rate(dev->clk)/1000;
> @@ -55,6 +57,8 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
>  	struct dw_i2c_dev *dev;
>  	struct i2c_adapter *adap;
>  	struct resource *mem, *ioarea;
> +	struct i2c_dw_pdata *pdata;
> +	struct i2c_bus_recovery_info *recovery_info = NULL;
>  	int irq, r;
>  
>  	/* NOTE: driver uses the static register mapping */
> @@ -141,17 +145,45 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
>  	adap->dev.parent = &pdev->dev;
>  	adap->dev.of_node = pdev->dev.of_node;
>  
> +	/* Bus recovery support */
> +	pdata = dev_get_platdata(&pdev->dev);
> +	if (pdata) {
> +		recovery_info = kzalloc(sizeof(*recovery_info), GFP_KERNEL);
> +		if (!recovery_info) {
> +			adap->bus_recovery_info = NULL;
> +			dev_err(&pdev->dev,
> +				"failure to allocate memory for bus recovery\n");
> +			goto skip_recovery;
> +		}
> +
> +		recovery_info->is_gpio_recovery = true;
> +		recovery_info->get_gpio = pdata->get_gpio;
> +		recovery_info->put_gpio = pdata->put_gpio;
> +		recovery_info->scl_gpio = pdata->scl_gpio;
> +		recovery_info->scl_gpio_flags = pdata->scl_gpio_flags;
> +		recovery_info->clock_rate_khz = clk_get_rate(dev->clk) / 1000;

It is probably easier to define the whole bri structure in the platform
and simply pass it on here? Hmm, now devicetree also comes to my mind.
Will think about that a little, too, but main implementation should be
left for someone needing that.

> +
> +		if (!pdata->skip_sda_polling)
> +			recovery_info->sda_gpio = pdata->sda_gpio;
> +
> +		adap->bus_recovery_info = recovery_info;
> +	} else {
> +		adap->bus_recovery_info = NULL;
> +	}
> +
> +skip_recovery:
>  	adap->nr = pdev->id;
>  	r = i2c_add_numbered_adapter(adap);
>  	if (r) {
>  		dev_err(&pdev->dev, "failure adding adapter\n");
> -		goto err_free_irq;
> +		goto err_free_recovery_info;
>  	}
>  	of_i2c_register_devices(adap);
>  
>  	return 0;
>  
> -err_free_irq:
> +err_free_recovery_info:
> +	kfree(recovery_info);
>  	free_irq(dev->irq, dev);
>  err_iounmap:
>  	iounmap(dev->base);
> @@ -174,6 +206,8 @@ static int __devexit dw_i2c_remove(struct platform_device *pdev)
>  	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
>  	struct resource *mem;
>  
> +	kfree(dev->adapter.bus_recovery_info);
> +
>  	platform_set_drvdata(pdev, NULL);
>  	i2c_del_adapter(&dev->adapter);
>  	put_device(&pdev->dev);
> diff --git a/include/linux/i2c/i2c-designware.h b/include/linux/i2c/i2c-designware.h
> new file mode 100644
> index 0000000..522dec0
> --- /dev/null
> +++ b/include/linux/i2c/i2c-designware.h
> @@ -0,0 +1,46 @@
> +/*
> + * Synopsys DesignWare I2C adapter driver's platform data
> + *
> + * Copyright (C) 2012 ST Microelectronics.
> + * Author: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +#ifndef I2C_DESIGNWARE_H
> +#define I2C_DESIGNWARE_H
> +
> +#include <linux/platform_device.h>
> +
> +/*
> + * struct i2c_dw_pdata - Designware I2c platform data
> + * @scl_gpio: gpio number of the scl line.
> + * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies
> + *	GPIOF_OUT_INIT_LOW.
> + * @get_gpio: called before recover_bus() to get padmux configured for scl line
> + *	as gpio. Only required if is_gpio_recovery == true
> + * @put_gpio: called after recover_bus() to get padmux configured for scl line
> + *	as scl. Only required if is_gpio_recovery == true
> + * @skip_sda_polling: if true, bus recovery will not poll sda line to check if
> + *	it became high or not. Below are required only if this is false.
> + * @sda_gpio: gpio number of the sda line.
> + */
> +struct i2c_dw_pdata {
> +	int scl_gpio;
> +	int scl_gpio_flags;
> +	int (*get_gpio)(unsigned gpio);
> +	void (*put_gpio)(unsigned gpio);
> +
> +	/* sda polling specific */
> +	bool skip_sda_polling;
> +	int sda_gpio;
> +};
> +
> +#endif /* I2C_DESIGNWARE_H */
> -- 
> 1.7.12.rc2.18.g61b472e
> 
> 

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]