[PATCH v2 0/2] i2c: designware: add i2c gpio recovery option

[PATCH v2 0/2] i2c: designware: add i2c gpio recovery option

[Phil Reid]

Changes from V1:
- In review Andy suggested change the i2c core to use the gpiod
  I've added a patch that allows the gradual switching of drivers 
  to using gpiod interface. The old interface is preserved so
  that changes can be made incrementally.
- I've update Tim's patch for the designware driver to use the new
  interface. Tweaked a couple of things to his patch and fixed
  up things Andy id in last review. 
  The core changes in p1 don't require the get/set scl/sda functions.
  Hopefully I've done the right thing with preserving authorship and
  signoff.

Phil Reid (1):
  i2c: Switch to using gpiod interface for gpio bus recovery

Tim Sander (1):
  i2c: designware: add i2c gpio recovery option

 drivers/i2c/busses/i2c-designware-core.c    | 12 ++++--
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-platdrv.c | 58 +++++++++++++++++++++++++++++
 drivers/i2c/i2c-core-base.c                 | 22 +++++++++--
 include/linux/i2c.h                         |  2 +
 5 files changed, 88 insertions(+), 7 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/2] i2c: Switch to using gpiod interface for gpio bus recovery

[Phil Reid]

Currently the i2c gpio recovery code uses gpio integer interface
instead of the gpiod. This change switch the core code to use
the gpiod while still retaining compatibility with the gpio integer
interface. This will allow individual driver to be updated and tested
individual to switch to using the gpiod interface.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/i2c-core-base.c | 22 ++++++++++++++++++----
 include/linux/i2c.h         |  2 ++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 426e2e0..47a1b75 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -134,17 +134,17 @@ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 /* i2c bus recovery routines */
 static int get_scl_gpio_value(struct i2c_adapter *adap)
 {
-	return gpio_get_value(adap->bus_recovery_info->scl_gpio);
+	return gpiod_get_value_cansleep(adap->bus_recovery_info->scl_gpiod);
 }
 
 static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
 {
-	gpio_set_value(adap->bus_recovery_info->scl_gpio, val);
+	gpiod_set_value_cansleep(adap->bus_recovery_info->scl_gpiod, val);
 }
 
 static int get_sda_gpio_value(struct i2c_adapter *adap)
 {
-	return gpio_get_value(adap->bus_recovery_info->sda_gpio);
+	return gpiod_get_value_cansleep(adap->bus_recovery_info->sda_gpiod);
 }
 
 static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
@@ -159,6 +159,7 @@ static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
 		dev_warn(dev, "Can't get SCL gpio: %d\n", bri->scl_gpio);
 		return ret;
 	}
+	bri->scl_gpiod = gpio_to_desc(bri->scl_gpio);
 
 	if (bri->get_sda) {
 		if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-sda")) {
@@ -168,6 +169,7 @@ static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
 			bri->get_sda = NULL;
 		}
 	}
+	bri->sda_gpiod = gpio_to_desc(bri->sda_gpio);
 
 	return ret;
 }
@@ -176,10 +178,13 @@ 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)
+	if (bri->get_sda) {
 		gpio_free(bri->sda_gpio);
+		bri->sda_gpiod = NULL;
+	}
 
 	gpio_free(bri->scl_gpio);
+	bri->scl_gpiod = NULL;
 }
 
 /*
@@ -273,6 +278,15 @@ static void i2c_init_recovery(struct i2c_adapter *adap)
 		goto err;
 	}
 
+	if ((bri->scl_gpiod) &&
+	    (bri->recover_bus == i2c_generic_scl_recovery)) {
+		bri->get_scl = get_scl_gpio_value;
+		bri->set_scl = set_scl_gpio_value;
+		if (bri->sda_gpiod)
+			bri->get_sda = get_sda_gpio_value;
+		return;
+	}
+
 	/* Generic GPIO recovery */
 	if (bri->recover_bus == i2c_generic_gpio_recovery) {
 		if (!gpio_is_valid(bri->scl_gpio)) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 72d0ece..84802a2 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -509,6 +509,8 @@ struct i2c_bus_recovery_info {
 	/* gpio recovery */
 	int scl_gpio;
 	int sda_gpio;
+	struct gpio_desc *scl_gpiod;
+	struct gpio_desc *sda_gpiod;
 };
 
 int i2c_recover_bus(struct i2c_adapter *adap);
-- 
1.8.3.1

[PATCH v2 2/2] i2c: designware: add i2c gpio recovery option

[Phil Reid]

From: Tim Sander <tim@krieglstein.org>

This patch contains much input from Phil Reid and has been tested
on Intel/Altera Cyclone V SOC Hardware with Altera GPIO's for the
SCL and SDA GPIO's. I am still a little unsure about the recover
in the timeout case (i2c-designware-core.c:770) as i could not
test this codepath.

Signed-off-by: Tim Sander <tim@krieglstein.org>
Tested-by: Phil Reid <preid@electromag.com.au>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/busses/i2c-designware-core.c    | 12 ++++--
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-platdrv.c | 58 +++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 3c41995..9696433 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -463,7 +463,11 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 	while (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
 		if (timeout <= 0) {
 			dev_warn(dev->dev, "timeout waiting for bus ready\n");
-			return -ETIMEDOUT;
+			i2c_recover_bus(&dev->adapter);
+
+			if (dw_readl(dev, DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY)
+				return -ETIMEDOUT;
+			return 0;
 		}
 		timeout--;
 		usleep_range(1000, 1100);
@@ -719,9 +723,10 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 	for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
 		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
 
-	if (abort_source & DW_IC_TX_ARB_LOST)
+	if (abort_source & DW_IC_TX_ARB_LOST) {
+		i2c_recover_bus(&dev->adapter);
 		return -EAGAIN;
-	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
+	} else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
 		return -EINVAL; /* wrong msgs[] data */
 	else
 		return -EIO;
@@ -766,6 +771,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 	if (!wait_for_completion_timeout(&dev->cmd_complete, adap->timeout)) {
 		dev_err(dev->dev, "controller timed out\n");
 		/* i2c_dw_init implicitly disables the adapter */
+		i2c_recover_bus(&dev->adapter);
 		i2c_dw_init(dev);
 		ret = -ETIMEDOUT;
 		goto done;
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index a7cf429..dc203e4 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -129,6 +129,7 @@ struct dw_i2c_dev {
 	int			(*acquire_lock)(struct dw_i2c_dev *dev);
 	void			(*release_lock)(struct dw_i2c_dev *dev);
 	bool			pm_disabled;
+	struct i2c_bus_recovery_info rinfo;
 };
 
 #define ACCESS_SWAP		0x00000001
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d1263b8..f88c061 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -31,6 +31,7 @@
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -206,6 +207,59 @@ static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev, int id)
 	}
 }
 
+static void i2c_dw_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct platform_device *pdev = to_platform_device(&adap->dev);
+	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
+
+	i2c_dw_disable(i_dev);
+	reset_control_assert(i_dev->rst);
+	i2c_dw_plat_prepare_clk(i_dev, false);
+}
+
+static void i2c_dw_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct platform_device *pdev = to_platform_device(&adap->dev);
+	struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
+
+	i2c_dw_plat_prepare_clk(i_dev, true);
+	reset_control_deassert(i_dev->rst);
+	i2c_dw_init(i_dev);
+}
+
+static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev,
+				     struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
+	struct gpio_desc *gpio;
+	int r;
+
+	gpio = devm_gpiod_get(dev->dev, "scl", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio)) {
+		r = PTR_ERR(gpio);
+		if ((r == -ENOENT) || (r == -ENOENT))
+			return 0;
+		return r;
+	}
+	rinfo->scl_gpiod = gpio;
+
+	gpio = devm_gpiod_get_optional(dev->dev, "sda", GPIOD_IN);
+	if (IS_ERR(gpio))
+		return PTR_ERR(gpio);
+	rinfo->sda_gpiod = gpio;
+
+	rinfo->recover_bus = i2c_generic_scl_recovery;
+	rinfo->prepare_recovery = i2c_dw_prepare_recovery;
+	rinfo->unprepare_recovery = i2c_dw_unprepare_recovery;
+	adap->bus_recovery_info = rinfo;
+
+	dev_info(dev->dev,
+		"adapter: %s running with gpio recovery mode! scl:%i sda:%i\n",
+		adap->name, !!rinfo->scl_gpiod, !!rinfo->sda_gpiod);
+
+	return 0;
+}
+
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -327,6 +381,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		pm_runtime_enable(&pdev->dev);
 	}
 
+	r = i2c_dw_init_recovery_info(dev, adap);
+	if (r == -EPROBE_DEFER)
+		goto exit_probe;
+
 	r = i2c_dw_probe(dev);
 	if (r)
 		goto exit_probe;
-- 
1.8.3.1

Re: [PATCH v2 0/2] i2c: designware: add i2c gpio recovery option

[Phil Reid]

On 16/06/2017 15:42, Phil Reid wrote:
> Changes from V1:
> - In review Andy suggested change the i2c core to use the gpiod
>    I've added a patch that allows the gradual switching of drivers
>    to using gpiod interface. The old interface is preserved so
>    that changes can be made incrementally.
> - I've update Tim's patch for the designware driver to use the new
>    interface. Tweaked a couple of things to his patch and fixed
>    up things Andy id in last review.
>    The core changes in p1 don't require the get/set scl/sda functions.
>    Hopefully I've done the right thing with preserving authorship and
>    signoff.
> 
> Phil Reid (1):
>    i2c: Switch to using gpiod interface for gpio bus recovery
> 
> Tim Sander (1):
>    i2c: designware: add i2c gpio recovery option
> 
>   drivers/i2c/busses/i2c-designware-core.c    | 12 ++++--
>   drivers/i2c/busses/i2c-designware-core.h    |  1 +
>   drivers/i2c/busses/i2c-designware-platdrv.c | 58 +++++++++++++++++++++++++++++
>   drivers/i2c/i2c-core-base.c                 | 22 +++++++++--
>   include/linux/i2c.h                         |  2 +
>   5 files changed, 88 insertions(+), 7 deletions(-)
> 

Ping:

Any comments on these 2 patches?



-- 
Regards
Phil Reid

Re: [PATCH v2 0/2] i2c: designware: add i2c gpio recovery option

[Andy Shevchenko]

On Thu, 2017-08-24 at 17:21 +0800, Phil Reid wrote:
> On 16/06/2017 15:42, Phil Reid wrote:
> > Changes from V1:
> > - In review Andy suggested change the i2c core to use the gpiod
> >    I've added a patch that allows the gradual switching of drivers
> >    to using gpiod interface. The old interface is preserved so
> >    that changes can be made incrementally.
> > - I've update Tim's patch for the designware driver to use the new
> >    interface. Tweaked a couple of things to his patch and fixed
> >    up things Andy id in last review.
> >    The core changes in p1 don't require the get/set scl/sda
> > functions.
> >    Hopefully I've done the right thing with preserving authorship
> > and
> >    signoff.
> > 
> > Phil Reid (1):
> >    i2c: Switch to using gpiod interface for gpio bus recovery
> > 
> > Tim Sander (1):
> >    i2c: designware: add i2c gpio recovery option
> > 
> >   drivers/i2c/busses/i2c-designware-core.c    | 12 ++++--
> >   drivers/i2c/busses/i2c-designware-core.h    |  1 +
> >   drivers/i2c/busses/i2c-designware-platdrv.c | 58
> > +++++++++++++++++++++++++++++
> >   drivers/i2c/i2c-core-base.c                 | 22 +++++++++--
> >   include/linux/i2c.h                         |  2 +
> >   5 files changed, 88 insertions(+), 7 deletions(-)
> > 
> 
> Ping:
> 
> Any comments on these 2 patches?

Oops, overloaded. I will do latest next week. I would like to massage a
bit this and see the result.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 0/2] i2c: designware: add i2c gpio recovery option

[Andy Shevchenko]

On Thu, 2017-08-24 at 14:59 +0300, Andy Shevchenko wrote:
> On Thu, 2017-08-24 at 17:21 +0800, Phil Reid wrote:
> > On 16/06/2017 15:42, Phil Reid wrote:

> > Any comments on these 2 patches?
> 
> Oops, overloaded. I will do latest next week. I would like to massage
> a
> bit this and see the result.

I have applied these to my tree (based on linux-next). It required some
work and still requires more.

Care to rebase to latest i2c-next tree and send as v3?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2 0/2] i2c: designware: add i2c gpio recovery option

[Phil Reid]

On 30/08/2017 00:28, Andy Shevchenko wrote:
> On Thu, 2017-08-24 at 14:59 +0300, Andy Shevchenko wrote:
>> On Thu, 2017-08-24 at 17:21 +0800, Phil Reid wrote:
>>> On 16/06/2017 15:42, Phil Reid wrote:
> 
>>> Any comments on these 2 patches?
>>
>> Oops, overloaded. I will do latest next week. I would like to massage
>> a
>> bit this and see the result.
> 
> I have applied these to my tree (based on linux-next). It required some
> work and still requires more.
> 
> Care to rebase to latest i2c-next tree and send as v3?
> 

No problem, thanks for looking at it.


-- 
Regards
Phil Reid