On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote:
> Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c
> protocol Rev. 03 section 3.1.16 titled "Bus clear".
> 
> http://www.nxp.com/documents/user_manual/UM10204.pdf
> 
> Sometimes during operation i2c bus hangs and we need to give dummy clocks to
> slave device to start the transfer again. Now we may have capability in the 
> bus
> controller to generate these clocks or platform may have gpio pins which can 
> be
> toggled to generate dummy clocks. This patch supports both.
> 
> This patch also adds in generic bus recovery routines gpio or scl line based
> which can be used by bus controller. In addition controller driver may provide
> its own version of the bus recovery routine.
> 
> This doesn't support multi-master recovery for now.
> 
> Reviewed-by: Paul Carpenter <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> Most number of versions for any patchset i submitted :)
> 
> V8->V9:
> - removed skip_sda_polling variable
> - simplified/fixed i2c_generic_recovery() routine
> - added new routine i2c_recover_bus()
> - added gpio_is_valid() checks over scl and sda gpios
> - code is rearranged at places (based on Wolfram's comments)
> 
>  drivers/i2c/i2c-core.c | 155 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h    |  39 +++++++++++++
>  2 files changed, 194 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index e388590..8076e52 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -27,7 +27,9 @@
>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> +#include <linux/delay.h>
>  #include <linux/errno.h>
> +#include <linux/gpio.h>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
> @@ -109,6 +111,127 @@ static int i2c_device_uevent(struct device *dev, struct 
> kobj_uevent_env *env)
>  #define i2c_device_uevent    NULL
>  #endif       /* CONFIG_HOTPLUG */
>  
> +/* i2c bus recovery routines */
> +#define RECOVERY_CLK_CNT     9
> +#define DEFAULT_CLK_RATE     100     /* KHz */

RECOVERY_CLK_RATE

> +/* 10^6/KHz for delay in ns */
> +unsigned long clk_delay = DIV_ROUND_UP(1000000, DEFAULT_CLK_RATE * 2);

Since all inputs are constant values, the result will be a constant as
well. We might even get rid of this variable altogether.

> +
> +static int get_scl_gpio_value(struct i2c_adapter *adap)
> +{
> +     return gpio_get_value(adap->bus_recovery_info->scl_gpio);
> +}
> +
> +static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
> +{
> +     gpio_set_value(adap->bus_recovery_info->scl_gpio, val);
> +}
> +
> +static int get_sda_gpio_value(struct i2c_adapter *adap)
> +{
> +     return gpio_get_value(adap->bus_recovery_info->sda_gpio);
> +}
> +
> +static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
> +{
> +     struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +     struct device *dev = &adap->dev;
> +     int ret = 0;
> +
> +     ret = gpio_request_one(bri->scl_gpio, GPIOF_OPEN_DRAIN |
> +                     GPIOF_OUT_INIT_HIGH, "i2c-scl");
> +     if (ret) {
> +             dev_warn(dev, "gpio request fail: %d\n", bri->scl_gpio);
> +             return ret;
> +     }
> +
> +     if (bri->get_sda) {
> +             if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-sda")) {
> +                     /* work without sda polling */
> +                     dev_warn(dev, "can't get sda: %d. Skip sda polling\n",
> +                                     bri->sda_gpio);
> +                     bri->get_sda = NULL;
> +             }
> +     }
> +
> +     return ret;
> +}
> +
> +static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
> +{
> +     struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +
> +     if (bri->get_sda)
> +             gpio_free(bri->sda_gpio);
> +
> +     gpio_free(bri->scl_gpio);
> +}
> +
> +static int i2c_generic_recovery(struct i2c_adapter *adap)
> +{
> +     struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +     int i = 0, val = 1, ret = 0;
> +
> +     if (bri->prepare_recovery)
> +             bri->prepare_recovery(bri);
> +
> +     /*
> +      * By this time SCL is high, as we need to give 9 falling-rising edges
> +      */
> +     while (i++ < RECOVERY_CLK_CNT * 2) {
> +             /* SCL shouldn't be low here */
> +             if (val && !bri->get_scl(adap)) {
> +                     dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");

s/Low/low/

> +                     ret = -EBUSY;
> +                     goto unprepare;
> +             }
> +
> +             val = !val;
> +             bri->set_scl(adap, val);
> +             ndelay(clk_delay);
> +
> +             /* break if sda got high, check only when scl line is high */
> +             if (bri->get_sda && val)
> +                     if (bri->get_sda(adap))
> +                             break;

if (val && bri->get_sda && bri->get_sda(adap)
        break;

Didn't we agree to move the SDA check before the SCL setting?

> +     }
> +
> +unprepare:
> +     if (bri->unprepare_recovery)
> +             bri->unprepare_recovery(bri);
> +
> +     return ret;
> +}
> +
> +int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> +{
> +     adap->bus_recovery_info->set_scl(adap, 1);
> +     return i2c_generic_recovery(adap);
> +}
> +
> +int i2c_generic_gpio_recovery(struct i2c_adapter *adap)
> +{
> +     int ret;
> +
> +     ret = i2c_get_gpios_for_recovery(adap);
> +     if (ret)
> +             return ret;
> +
> +     ret = i2c_generic_recovery(adap);
> +     i2c_put_gpios_for_recovery(adap);
> +
> +     return ret;
> +}
> +
> +int i2c_recover_bus(struct i2c_adapter *adap)
> +{
> +     if (!adap->bus_recovery_info)
> +             return -EOPNOTSUPP;
> +
> +     dev_dbg(&adap->dev, "try i2c bus recovery\n");

s/try/trying/

> +     return adap->bus_recovery_info->recover_bus(adap);
> +}
> +
>  static int i2c_device_probe(struct device *dev)
>  {
>       struct i2c_client       *client = i2c_verify_client(dev);
> @@ -902,6 +1025,38 @@ static int i2c_register_adapter(struct i2c_adapter 
> *adap)
>                        "Failed to create compatibility class link\n");
>  #endif
>  
> +     /* bus recovery specific initialization */
> +     if (adap->bus_recovery_info) {
> +             struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +
> +             if (!bri->recover_bus) {
> +                     dev_err(&adap->dev, "Invalid recover_bus(), skip 
> recovery\n");

s/Invalid/No/
s/skip/not using/
The latter for all later occurences, too.

> +                     adap->bus_recovery_info = NULL;
> +                     goto exit_recovery;
> +             }
> +
> +             /* GPIO recovery */
> +             if (bri->recover_bus == i2c_generic_gpio_recovery) {
> +                     if (!gpio_is_valid(bri->scl_gpio)) {
> +                             dev_err(&adap->dev, "Invalid scl gpio, skip 
> recovery\n");
> +                             adap->bus_recovery_info = NULL;
> +                             goto exit_recovery;
> +                     }
> +
> +                     if (gpio_is_valid(bri->sda_gpio))
> +                             bri->get_sda = get_sda_gpio_value;
> +                     else
> +                             bri->get_sda = NULL;
> +
> +                     bri->get_scl = get_scl_gpio_value;
> +                     bri->set_scl = set_scl_gpio_value;
> +             } else if (!bri->set_scl) {
> +                     dev_warn(&adap->dev,
> +                                     "set_scl() is must for scl recovery\n");

What about get_scl. You don't check here, but you don't check when you
use it above.

> +             }
> +     }
> +
> +exit_recovery:
>       /* create pre-declared device nodes */
>       if (adap->nr < __i2c_first_dynamic_bus_num)
>               i2c_scan_static_board_info(adap);
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d0c4db7..da9358b 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -370,6 +370,43 @@ struct i2c_algorithm {
>       u32 (*functionality) (struct i2c_adapter *);
>  };
>  
> +/**
> + * struct i2c_bus_recovery_info - I2C bus recovery information
> + * @recover_bus: Recover routine. Either pass driver's recover_bus() 
> routine, or
> + *   i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
> + * @get_scl: This gets current value of scl line. Mandatory for generic SCL
> + *      recovery. Used internally for generic GPIO recovery.
> + * @set_scl: This sets/clears scl line. Mandatory for generic SCL recovery. 
> Used
> + *      internally for generic GPIO recovery.
> + * @get_sda: This gets current value of sda line. Optional for generic SCL
> + *      recovery. Used internally, if sda_gpio != -1, for generic GPIO 
> recovery.

s/!= -1/is a valid GPIO/

> + * @prepare_recovery: This will be called before starting recovery. Platform 
> may
> + *   configure padmux here for sda/scl line or something else they want.
> + * @unprepare_recovery: This will be called after completing recovery. 
> Platform
> + *   may configure padmux here for sda/scl line or something else they want.
> + * @scl_gpio: gpio number of the scl line. Only required for GPIO recovery.
> + * @sda_gpio: gpio number of the sda line. Only required for GPIO recovery.
> + */
> +struct i2c_bus_recovery_info {
> +     int (*recover_bus)(struct i2c_adapter *);
> +
> +     int (*get_scl)(struct i2c_adapter *);
> +     void (*set_scl)(struct i2c_adapter *, int val);
> +     int (*get_sda)(struct i2c_adapter *);
> +
> +     void (*prepare_recovery)(struct i2c_bus_recovery_info *bri);
> +     void (*unprepare_recovery)(struct i2c_bus_recovery_info *bri);
> +
> +     /* gpio recovery */
> +     unsigned scl_gpio;
> +     unsigned sda_gpio;
> +};
> +
> +/* Generic recovery routines */
> +int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
> +int i2c_generic_scl_recovery(struct i2c_adapter *adap);
> +int i2c_recover_bus(struct i2c_adapter *adap);
> +
>  /*
>   * i2c_adapter is the structure used to identify a physical i2c bus along
>   * with the access algorithms necessary to access it.
> @@ -393,6 +430,8 @@ struct i2c_adapter {
>  
>       struct mutex userspace_clients_lock;
>       struct list_head userspace_clients;
> +
> +     struct i2c_bus_recovery_info *bus_recovery_info;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)

Regards,

   Wolfram

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

Attachment: signature.asc
Description: Digital signature

Reply via email to