Skip to contentSkip to conversation

i2c: designware: add i2c gpio recovery option

PATCH New Phil Reid 6 participants 17 messages Nov 2, 2017 – Nov 27, 2017

Subsystems (7)
Arm i.MX and Layerscape I2C subsystem host drivers I2C Intel LPSS (Low Power SubSystem) drivers +3 more subsystems
lore
Share & tools
lore

Phil Reid

author
#0 lore raw

v5 cover i2c: designware: add i2c gpio recovery option

Changes from V3:
- fix typ in message s/reset/rest/
- move *_gpiod field comments to the first commit where the fields are actually added.
- Add Jarkko/Andy Ack/Reviewed.

Changes from V3:
- rebase on i2c/for-next
- Add patches to convert other drivers using gpio recover to gpiod
- Remove legacy gpio
- Address minor issues raised in review and add acks received so far

Changes from V2:
- Rebase on https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git/
i2c/for-next
No intentional changes, but needed to move i2c_dw_plat_prepare_clk to common for the master recovery functions to use. which is included as two additional patches.

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 (6):
i2c: Switch to using gpiod interface for gpio bus recovery
i2c: designware: move i2c_dw_plat_prepare_clk to common
i2c: designware: rename i2c_dw_plat_prepare_clk to i2c_dw_prepare_clk
i2c: imx: switch to using gpiod for bus recovery gpios
i2c: davinci: switch to using gpiod for bus recovery gpios
i2c: remove legacy integer scl/sda gpio for recovery

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

 drivers/i2c/busses/i2c-davinci.c            | 20 +++++--
 drivers/i2c/busses/i2c-designware-common.c  | 19 ++++++-
 drivers/i2c/busses/i2c-designware-core.h    |  2 +
 drivers/i2c/busses/i2c-designware-master.c  | 57 ++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-platdrv.c | 18 ++-----
 drivers/i2c/busses/i2c-imx.c                | 18 +++----
 drivers/i2c/i2c-core-base.c                 | 83 ++++-------------------------
 include/linux/i2c.h                         | 11 ++--
 8 files changed, 120 insertions(+), 108 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v5 1/7 i2c: Switch to using gpiod interface for gpio bus recovery

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.

Reviewed-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/i2c-core-base.c | 21 +++++++++++++++++----
 include/linux/i2c.h         |  4 ++++
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/i2c/i2c-core-base.c +17 -4
index 706164b..fdc6a9d 100644
@@ -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")) {

@@ -167,6 +168,7 @@ static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
 					bri->sda_gpio);
 			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;
 }
 
 /*

@@ -277,6 +282,14 @@ 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)) {
include/linux/i2c.h +4 -0
index 0f77440..25d5db5 100644
@@ -499,6 +499,8 @@ struct i2c_timings {
  *	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.
+ * @scl_gpiod: gpiod of the SCL line. Only required for GPIO recovery.
+ * @sda_gpiod: gpiod of the SDA line. Only required for GPIO recovery.
  */
 struct i2c_bus_recovery_info {
 	int (*recover_bus)(struct i2c_adapter *);

@@ -513,6 +515,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);
Signature · 1 line
1.8.3.1
author
#2 lore raw

PATCH v5 2/7 i2c: designware: move i2c_dw_plat_prepare_clk to common

In reply to Phil Reid

Move the i2c_dw_plat_prepare_clk funciton to common file in preparation for its use also by the master driver.

Acked-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/busses/i2c-designware-common.c  | 13 +++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  1 +
 drivers/i2c/busses/i2c-designware-platdrv.c | 12 ------------
 3 files changed, 14 insertions(+), 12 deletions(-)
drivers/i2c/busses/i2c-designware-common.c +13 -0
index d1a6937..b79f342 100644
@@ -21,6 +21,7 @@
  * ----------------------------------------------------------------------------
  *
  */
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/errno.h>

@@ -185,6 +186,18 @@ unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
 	return dev->get_clk_rate_khz(dev);
 }
 
+int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
+{
+	if (IS_ERR(i_dev->clk))
+		return PTR_ERR(i_dev->clk);
+
+	if (prepare)
+		return clk_prepare_enable(i_dev->clk);
+
+	clk_disable_unprepare(i_dev->clk);
+	return 0;
+}
+
 int i2c_dw_acquire_lock(struct dw_i2c_dev *dev)
 {
 	int ret;
drivers/i2c/busses/i2c-designware-core.h +1 -0
index 21bf619..e80a14c 100644
@@ -301,6 +301,7 @@ struct dw_i2c_dev {
 void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable);
 void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable);
 unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev);
+int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare);
 int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
 void i2c_dw_release_lock(struct dw_i2c_dev *dev);
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
drivers/i2c/busses/i2c-designware-platdrv.c +0 -12
index 58add69..a5fd0ee 100644
@@ -214,18 +214,6 @@ static void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
 	}
 }
 
-static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
-{
-	if (IS_ERR(i_dev->clk))
-		return PTR_ERR(i_dev->clk);
-
-	if (prepare)
-		return clk_prepare_enable(i_dev->clk);
-
-	clk_disable_unprepare(i_dev->clk);
-	return 0;
-}
-
 static void dw_i2c_set_fifo_size(struct dw_i2c_dev *dev, int id)
 {
 	u32 param, tx_fifo_depth, rx_fifo_depth;
Signature · 1 line
1.8.3.1
author
#3 lore raw

PATCH v5 3/7 i2c: designware: rename i2c_dw_plat_prepare_clk to i2c_dw_prepare_clk

In reply to Phil Reid

For consistency with the rest of the file rename function and parameter to be consistent with the reset of the common file.

Acked-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/busses/i2c-designware-common.c  | 10 +++++-----
 drivers/i2c/busses/i2c-designware-core.h    |  2 +-
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)
drivers/i2c/busses/i2c-designware-common.c +5 -5
index b79f342..3d684c6 100644
@@ -186,15 +186,15 @@ unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev)
 	return dev->get_clk_rate_khz(dev);
 }
 
-int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare)
+int i2c_dw_prepare_clk(struct dw_i2c_dev *dev, bool prepare)
 {
-	if (IS_ERR(i_dev->clk))
-		return PTR_ERR(i_dev->clk);
+	if (IS_ERR(dev->clk))
+		return PTR_ERR(dev->clk);
 
 	if (prepare)
-		return clk_prepare_enable(i_dev->clk);
+		return clk_prepare_enable(dev->clk);
 
-	clk_disable_unprepare(i_dev->clk);
+	clk_disable_unprepare(dev->clk);
 	return 0;
 }
drivers/i2c/busses/i2c-designware-core.h +1 -1
index e80a14c..33c6c8f 100644
@@ -301,7 +301,7 @@ struct dw_i2c_dev {
 void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable);
 void __i2c_dw_enable_and_wait(struct dw_i2c_dev *dev, bool enable);
 unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev);
-int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare);
+int i2c_dw_prepare_clk(struct dw_i2c_dev *dev, bool prepare);
 int i2c_dw_acquire_lock(struct dw_i2c_dev *dev);
 void i2c_dw_release_lock(struct dw_i2c_dev *dev);
 int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev);
drivers/i2c/busses/i2c-designware-platdrv.c +3 -3
index a5fd0ee..9c5f649 100644
@@ -343,7 +343,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		i2c_dw_configure_master(dev);
 
 	dev->clk = devm_clk_get(&pdev->dev, NULL);
-	if (!i2c_dw_plat_prepare_clk(dev, true)) {
+	if (!i2c_dw_prepare_clk(dev, true)) {
 		dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
 
 		if (!dev->sda_hold_time && ht)

@@ -447,7 +447,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
 	}
 
 	i_dev->disable(i_dev);
-	i2c_dw_plat_prepare_clk(i_dev, false);
+	i2c_dw_prepare_clk(i_dev, false);
 
 	i_dev->suspended = true;
 

@@ -466,7 +466,7 @@ static int dw_i2c_plat_resume(struct device *dev)
 		return 0;
 	}
 
-	i2c_dw_plat_prepare_clk(i_dev, true);
+	i2c_dw_prepare_clk(i_dev, true);
 	i_dev->init(i_dev);
 
 	i_dev->suspended = false;
Signature · 1 line
1.8.3.1
author
#4 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

In reply to Phil Reid

From: Tim Sander <[email protected]>

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.

Acked-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Tim Sander <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/busses/i2c-designware-common.c |  6 +++-
 drivers/i2c/busses/i2c-designware-core.h   |  1 +
 drivers/i2c/busses/i2c-designware-master.c | 57 ++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)
drivers/i2c/busses/i2c-designware-common.c +5 -1
index 3d684c6..6b82809 100644
@@ -230,7 +230,11 @@ 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);
drivers/i2c/busses/i2c-designware-core.h +1 -0
index 33c6c8f..d58a336 100644
@@ -286,6 +286,7 @@ struct dw_i2c_dev {
 	void			(*disable_int)(struct dw_i2c_dev *dev);
 	int			(*init)(struct dw_i2c_dev *dev);
 	int			mode;
+	struct i2c_bus_recovery_info rinfo;
 };
 
 #define ACCESS_SWAP		0x00000001
drivers/i2c/busses/i2c-designware-master.c +57 -0

86 lines not on this page · raw message

Signature · 1 line
1.8.3.1
reviewer
#5 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

Quoted 8 lines · Phil Reid

From: Tim Sander <[email protected]>

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.

Acked-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>

Shame on me, I totally forgot one important comment on all this.

How do we switch pinctrl back to the native function? Is it guaranteed by pinctrl framework and all underneath drivers?

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
author
#6 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

Quoted 7 lines · Andy Shevchenko
Quoted 6 lines · Phil Reid

From: Tim Sander <[email protected]>

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.

Quoted 9 lines
Quoted 2 lines
Acked-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>

Shame on me, I totally forgot one important comment on all this.

How do we switch pinctrl back to the native function? Is it guaranteed by pinctrl framework and all underneath drivers?

G'day Andy,

That is a good question. I don't have an understanding of how the pinctrl framework works with respect to requesting gpios.
My device (Intel / Altera Cyclone V SOC) doesn't have a pinctrl for the i2c / gpio mux as yet. It's all setup by the bootloader and they don't expect you to change it. I'm using two separate GPIO's "wired" to the i2c bus via the SOC FPGA for the recovery. Tim was doing the same.

Signature · 2 lines
Regards
Phil Reid
#7 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

Hi

Sorry for beeing so silent, i have been swept to completly different area so this thing was swapped out.

Thanks Phil for your work on this!
Am Dienstag, 7. November 2017, 09:02:50 CET schrieb Phil Reid:

Quoted 13 lines · Andy Shevchenko
Quoted 12 lines · Phil Reid
Quoted 8 lines

From: Tim Sander <[email protected]>

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.

Acked-by Jarkko Nikula <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>

Shame on me, I totally forgot one important comment on all this.

Quoted 7 lines
Quoted 2 lines

How do we switch pinctrl back to the native function? Is it guaranteed by pinctrl framework and all underneath drivers?

That is a good question. I don't have an understanding of how the pinctrl framework works with respect to requesting gpios.
My device (Intel / Altera Cyclone V SOC) doesn't have a pinctrl for the i2c / gpio mux as yet.

According to the documentation from Intel/Altera it is not allowed to change the pinmux while running. My guess is that they are using a shift chain, so the output values of all pins in the chain are not stable. I think they have been lazy and just used the io config for fpgas with an jtag controller and connected the shift chain for pinconfig to this controller. So unfortunatly it is not possible to switch single pins on the run without interfering with other pins.

Quoted 3 lines

It's all setup by the bootloader and they don't expect
you to change it. I'm using two separate GPIO's "wired" to the i2c bus via the SOC FPGA for the recovery. Tim was doing the same.

Yes, i think thats the only way. But it is annoying that the i2c controllers of 201x have no way to recover from such bus errors >:-(.

Best regards
Tim

reviewer
#8 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

Quoted 10 lines · Tim Sander
Quoted 9 lines
Quoted 3 lines

How do we switch pinctrl back to the native function? Is it guaranteed
by pinctrl framework and all underneath drivers?

That is a good question. I don't have an understanding of how the pinctrl
framework works with respect to requesting gpios.
My device (Intel / Altera Cyclone V SOC) doesn't have a pinctrl for the i2c
/ gpio mux as yet.

Quoted 3 lines

According to the documentation from Intel/Altera it is not allowed to change
the pinmux while running.

The driver is used on many platforms and requesting GPIO function while running _is_ a pinmux change.

Quoted 10 lines

My guess is that they are using a shift chain, so
the output values of all pins in the chain are not stable. I think they have
been lazy and just used the io config for fpgas with an jtag controller and
connected the shift chain for pinconfig to this controller. So unfortunatly it
is not possible to switch single pins on the run without interfering with
other pins.

...for this certain SoC.

Quoted 8 lines
Quoted 4 lines

It's all setup by the bootloader and they don't expect you to change it. I'm using two separate GPIO's "wired" to the i2c bus via
the SOC FPGA for the recovery. Tim was doing the same.

Yes, i think thats the only way. But it is annoying that the i2c controllers
of 201x have no way to recover from such bus errors >:-(.

Yep, it's pity, and we need to cope somehow with it.

So, my understanding of the current case is that either this change goes in only for certain SoC(s), or waiting until there is a guarantee from pinctrl subsystem to return function to native back when recovery finished.

My personal vote is for latter.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
author
#9 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

Quoted 40 lines · Andy Shevchenko
Quoted 10 lines · Tim Sander
Quoted 9 lines
Quoted 3 lines

How do we switch pinctrl back to the native function? Is it guaranteed
by pinctrl framework and all underneath drivers?

That is a good question. I don't have an understanding of how the pinctrl
framework works with respect to requesting gpios.
My device (Intel / Altera Cyclone V SOC) doesn't have a pinctrl for the i2c
/ gpio mux as yet.

Quoted 3 lines

According to the documentation from Intel/Altera it is not allowed to change
the pinmux while running.

The driver is used on many platforms and requesting GPIO function while running _is_ a pinmux change. >

Quoted 10 lines

My guess is that they are using a shift chain, so
the output values of all pins in the chain are not stable. I think they have
been lazy and just used the io config for fpgas with an jtag controller and
connected the shift chain for pinconfig to this controller. So unfortunatly it
is not possible to switch single pins on the run without interfering with
other pins.

...for this certain SoC.

Quoted 8 lines
Quoted 4 lines

It's all setup by the bootloader and they don't expect you to change it. I'm using two separate GPIO's "wired" to the i2c bus via
the SOC FPGA for the recovery. Tim was doing the same.

Yes, i think thats the only way. But it is annoying that the i2c controllers
of 201x have no way to recover from such bus errors >:-(.

Quoted 6 lines

Yep, it's pity, and we need to cope somehow with it.

So, my understanding of the current case is that either this change goes in only for certain SoC(s), or waiting until there is a guarantee from pinctrl subsystem to return function to native back when recovery finished.

My personal vote is for latter.

I'm not sure my change set will affect anything..
Current drivers using the gpio functions are requesting the gpio using the old interface. If releasing the gpio doesn't restore the original functionality than nothing changes with this series.

For the designware driver, the recovery code wont be called unless the gpio's are specified via the device tree. So the pinmux config shouldn't get changed.

It'd be nice to have the code in mainline even if most soc's can't use it as yet.

Signature · 2 lines
Regards
Phil Reid
reviewer
#10 lore raw

PATCH v5 4/7 i2c: designware: add i2c gpio recovery option

Quoted 9 lines · Phil Reid
Quoted 1 line · Andy Shevchenko

On Wed, 2017-11-08 at 09:29 +0100, Tim Sander wrote:

I'm not sure my change set will affect anything..
Current drivers using the gpio functions are requesting the gpio using the old interface.
If releasing the gpio doesn't restore the original functionality than nothing changes with this series.

Quoted 6 lines

For the designware driver, the recovery code wont be called unless the gpio's are specified via the device tree.
So the pinmux config shouldn't get changed.

It'd be nice to have the code in mainline even if most soc's can't use it as yet.

I'm fine as long as it doesn't break existing users of DW I2C.

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
author
#11 lore raw

PATCH v5 5/7 i2c: imx: switch to using gpiod for bus recovery gpios

In reply to Phil Reid

Change the driver to use the gpio descriptors for the bus recovery information instead of the deprecated integer interface.

Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/busses/i2c-imx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/i2c/busses/i2c-imx.c +9 -9
index f96830f..c4cf265 100644
@@ -1006,26 +1006,26 @@ static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
 			PINCTRL_STATE_DEFAULT);
 	i2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(i2c_imx->pinctrl,
 			"gpio");
-	rinfo->sda_gpio = of_get_named_gpio(pdev->dev.of_node, "sda-gpios", 0);
-	rinfo->scl_gpio = of_get_named_gpio(pdev->dev.of_node, "scl-gpios", 0);
+	rinfo->sda_gpiod = devm_gpiod_get(&pdev->dev, "sda", GPIOD_OUT_HIGH);
+	rinfo->scl_gpiod = devm_gpiod_get(&pdev->dev, "scl", GPIOD_IN);
 
-	if (rinfo->sda_gpio == -EPROBE_DEFER ||
-	    rinfo->scl_gpio == -EPROBE_DEFER) {
+	if (PTR_ERR(rinfo->sda_gpiod) == -EPROBE_DEFER ||
+	    PTR_ERR(rinfo->scl_gpiod) == -EPROBE_DEFER) {
 		return -EPROBE_DEFER;
-	} else if (!gpio_is_valid(rinfo->sda_gpio) ||
-		   !gpio_is_valid(rinfo->scl_gpio) ||
+	} else if (IS_ERR(rinfo->sda_gpiod) ||
+		   IS_ERR(rinfo->scl_gpiod) ||
 		   IS_ERR(i2c_imx->pinctrl_pins_default) ||
 		   IS_ERR(i2c_imx->pinctrl_pins_gpio)) {
 		dev_dbg(&pdev->dev, "recovery information incomplete\n");
 		return 0;
 	}
 
-	dev_dbg(&pdev->dev, "using scl-gpio %d and sda-gpio %d for recovery\n",
-			rinfo->scl_gpio, rinfo->sda_gpio);
+	dev_dbg(&pdev->dev, "using scl%s for recovery\n",
+		rinfo->sda_gpiod ? ",sda" : "");
 
 	rinfo->prepare_recovery = i2c_imx_prepare_recovery;
 	rinfo->unprepare_recovery = i2c_imx_unprepare_recovery;
-	rinfo->recover_bus = i2c_generic_gpio_recovery;
+	rinfo->recover_bus = i2c_generic_scl_recovery;
 	i2c_imx->adapter.bus_recovery_info = rinfo;
 
 	return 0;
Signature · 1 line
1.8.3.1
author
#12 lore raw

PATCH v5 6/7 i2c: davinci: switch to using gpiod for bus recovery gpios

In reply to Phil Reid

Change the driver to use the gpio descriptors for the bus recovery information instead of the deprecated integer interface.

Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/busses/i2c-davinci.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
drivers/i2c/busses/i2c-davinci.c +15 -5
index 2ead9b9..2afb12a 100644
@@ -294,7 +294,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 }
 
 /*
- * This routine does i2c bus recovery by using i2c_generic_gpio_recovery
+ * This routine does i2c bus recovery by using i2c_generic_scl_recovery
  * which is provided by I2C Bus recovery infrastructure.
  */
 static void davinci_i2c_prepare_recovery(struct i2c_adapter *adap)

@@ -316,7 +316,7 @@ static void davinci_i2c_unprepare_recovery(struct i2c_adapter *adap)
 }
 
 static struct i2c_bus_recovery_info davinci_i2c_gpio_recovery_info = {
-	.recover_bus = i2c_generic_gpio_recovery,
+	.recover_bus = i2c_generic_scl_recovery,
 	.prepare_recovery = davinci_i2c_prepare_recovery,
 	.unprepare_recovery = davinci_i2c_unprepare_recovery,
 };

@@ -769,6 +769,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	struct davinci_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem;
+	struct i2c_bus_recovery_info *rinfo;
 	int r, irq;
 
 	irq = platform_get_irq(pdev, 0);

@@ -869,9 +870,18 @@ static int davinci_i2c_probe(struct platform_device *pdev)
 	if (dev->pdata->has_pfunc)
 		adap->bus_recovery_info = &davinci_i2c_scl_recovery_info;
 	else if (dev->pdata->scl_pin) {
-		adap->bus_recovery_info = &davinci_i2c_gpio_recovery_info;
-		adap->bus_recovery_info->scl_gpio = dev->pdata->scl_pin;
-		adap->bus_recovery_info->sda_gpio = dev->pdata->sda_pin;
+		rinfo =  &davinci_i2c_gpio_recovery_info;
+		adap->bus_recovery_info = rinfo;
+		r = gpio_request_one(dev->pdata->scl_pin, GPIOF_OPEN_DRAIN |
+				     GPIOF_OUT_INIT_HIGH, "i2c-scl");
+		if (r)
+			goto err_unuse_clocks;
+		rinfo->scl_gpiod = gpio_to_desc(dev->pdata->scl_pin);
+
+		r = gpio_request_one(dev->pdata->sda_pin, GPIOF_IN, "i2c-sda");
+		if (r)
+			goto err_unuse_clocks;
+		rinfo->sda_gpiod = gpio_to_desc(dev->pdata->scl_pin);
 	}
 
 	adap->nr = pdev->id;
Signature · 1 line
1.8.3.1
author
#14 lore raw

PATCH v5 7/7 i2c: remove legacy integer scl/sda gpio for recovery

In reply to Phil Reid

Remove all reference to code related to using integer based ids for scl/sda gpio for bus recovery. All in tree drivers are now using the gpio descriptors to specific the required gpios.

Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
 drivers/i2c/i2c-core-base.c | 78 ++-------------------------------------------
 include/linux/i2c.h         |  7 +---
 2 files changed, 3 insertions(+), 82 deletions(-)
drivers/i2c/i2c-core-base.c +2 -76
index fdc6a9d..54ffc8d 100644
@@ -147,46 +147,6 @@ static int get_sda_gpio_value(struct i2c_adapter *adap)
 	return gpiod_get_value_cansleep(adap->bus_recovery_info->sda_gpiod);
 }
 
-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, "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")) {
-			/* work without SDA polling */
-			dev_warn(dev, "Can't get SDA gpio: %d. Not using SDA polling\n",
-					bri->sda_gpio);
-			bri->get_sda = NULL;
-		}
-		bri->sda_gpiod = gpio_to_desc(bri->sda_gpio);
-	}
-
-	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);
-		bri->sda_gpiod = NULL;
-	}
-
-	gpio_free(bri->scl_gpio);
-	bri->scl_gpiod = NULL;
-}
-
 /*
  * We are generating clock pulses. ndelay() determines durating of clk pulses.
  * We will generate clock with rate 100 KHz and so duration of both clock levels

@@ -195,7 +155,7 @@ static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
 #define RECOVERY_NDELAY		5000
 #define RECOVERY_CLK_CNT	9
 
-static int i2c_generic_recovery(struct i2c_adapter *adap)
+int i2c_generic_scl_recovery(struct i2c_adapter *adap)
 {
 	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
 	int i = 0, val = 1, ret = 0;

@@ -237,28 +197,8 @@ static int i2c_generic_recovery(struct i2c_adapter *adap)
 
 	return ret;
 }
-
-int i2c_generic_scl_recovery(struct i2c_adapter *adap)
-{
-	return i2c_generic_recovery(adap);
-}
 EXPORT_SYMBOL_GPL(i2c_generic_scl_recovery);
 
-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;
-}
-EXPORT_SYMBOL_GPL(i2c_generic_gpio_recovery);
-
 int i2c_recover_bus(struct i2c_adapter *adap)
 {
 	if (!adap->bus_recovery_info)

@@ -290,21 +230,7 @@ static void i2c_init_recovery(struct i2c_adapter *adap)
 		return;
 	}
 
-	/* Generic GPIO recovery */
-	if (bri->recover_bus == i2c_generic_gpio_recovery) {
-		if (!gpio_is_valid(bri->scl_gpio)) {
-			err_str = "invalid SCL gpio";
-			goto err;
-		}
-
-		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->recover_bus == i2c_generic_scl_recovery) {
+	if (bri->recover_bus == i2c_generic_scl_recovery) {
 		/* Generic SCL recovery */
 		if (!bri->set_scl || !bri->get_scl) {
 			err_str = "no {get|set}_scl() found";
include/linux/i2c.h +1 -6
index 25d5db5..8597eac 100644
@@ -485,7 +485,7 @@ struct i2c_timings {
 /**
  * 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().
+ *	i2c_generic_scl_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

@@ -497,8 +497,6 @@ struct i2c_timings {
  *	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.
  * @scl_gpiod: gpiod of the SCL line. Only required for GPIO recovery.
  * @sda_gpiod: gpiod of the SDA line. Only required for GPIO recovery.
  */

@@ -513,8 +511,6 @@ struct i2c_bus_recovery_info {
 	void (*unprepare_recovery)(struct i2c_adapter *);
 
 	/* gpio recovery */
-	int scl_gpio;
-	int sda_gpio;
 	struct gpio_desc *scl_gpiod;
 	struct gpio_desc *sda_gpiod;
 };

@@ -522,7 +518,6 @@ struct i2c_bus_recovery_info {
 int i2c_recover_bus(struct i2c_adapter *adap);
 
 /* Generic recovery routines */
-int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
 int i2c_generic_scl_recovery(struct i2c_adapter *adap);
 
 /**
Signature · 1 line
1.8.3.1
#15 lore raw

PATCH v5 7/7 i2c: remove legacy integer scl/sda gpio for recovery

Quoted 11 lines · Phil Reid

Remove all reference to code related to using integer based ids for scl/sda gpio for bus recovery. All in tree drivers are now using the gpio descriptors to specific the required gpios.

Reviewed-by Andy Shevchenko <[email protected]>
Signed-off-by Phil Reid <[email protected]>
---
  drivers/i2c/i2c-core-base.c | 78 ++-------------------------------------------
  include/linux/i2c.h         |  7 +---
  2 files changed, 3 insertions(+), 82 deletions(-)
Reviewed-by Jarkko Nikula <[email protected]>