diff mbox series

[U-Boot] dm: i2c: implement gpio-based I2C deblock

Message ID 1519997824-7693-1-git-send-email-al.kochet@gmail.com
State Changes Requested
Delegated to: Heiko Schocher
Headers show
Series [U-Boot] dm: i2c: implement gpio-based I2C deblock | expand

Commit Message

Alexander Kochetkov March 2, 2018, 1:37 p.m. UTC
The commit extract gpio description from device tree,
setup pins and toggle them until I2C slave device
release SDA.

Any comments? Ideas?

Could someone review the patch and tell that should
I do with it in order to bring the patch to u-boot?

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/i2c/i2c-uclass.c |   95 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 93 insertions(+), 2 deletions(-)

Comments

Heiko Schocher March 23, 2018, 6:50 a.m. UTC | #1
Hello Alexander,

Am 02.03.2018 um 14:37 schrieb Alexander Kochetkov:
> The commit extract gpio description from device tree,
> setup pins and toggle them until I2C slave device
> release SDA.
> 
> Any comments? Ideas?
> 
> Could someone review the patch and tell that should
> I do with it in order to bring the patch to u-boot?
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> ---
>   drivers/i2c/i2c-uclass.c |   95 +++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 93 insertions(+), 2 deletions(-)

I am fine with this patch, but checkpatch drops some warnings:

WARNING: line over 80 characters
#61: FILE: drivers/i2c/i2c-uclass.c:465:
+static void i2c_deblock_gpio_run(struct gpio_desc *sda_pin, struct gpio_desc *scl_pin)

WARNING: line over 80 characters
#102: FILE: drivers/i2c/i2c-uclass.c:506:
+               debug("%s: I2C Node '%s' has no 'gpios' property %s\n", __func__,

WARNING: line over 80 characters
#113: FILE: drivers/i2c/i2c-uclass.c:517:
+               debug("%s: I2C Node '%s' has no 'gpio' pinctrl state. %s\n", __func__,

WARNING: line over 80 characters
#122: FILE: drivers/i2c/i2c-uclass.c:526:
+               debug("%s: I2C Node '%s' has no 'default' pinctrl state. %s\n", __func__,

WARNING: ENOSYS means 'invalid syscall nr' and nothing else
#147: FILE: drivers/i2c/i2c-uclass.c:551:
+               return ret ? -ENOSYS : 0;


Could you fix them, also remove from the commit message the question.

Thanks!

bye,
Heiko
Heiko Schocher March 23, 2018, 7:35 a.m. UTC | #2
Hello Alexander,

Am 23.03.2018 um 07:50 schrieb Heiko Schocher:
> Hello Alexander,
> 
> Am 02.03.2018 um 14:37 schrieb Alexander Kochetkov:
>> The commit extract gpio description from device tree,
>> setup pins and toggle them until I2C slave device
>> release SDA.
>>
>> Any comments? Ideas?
>>
>> Could someone review the patch and tell that should
>> I do with it in order to bring the patch to u-boot?
>>
>> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
>> ---
>>   drivers/i2c/i2c-uclass.c |   95 +++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 93 insertions(+), 2 deletions(-)
> 
> I am fine with this patch, but checkpatch drops some warnings:
> 
> WARNING: line over 80 characters
> #61: FILE: drivers/i2c/i2c-uclass.c:465:
> +static void i2c_deblock_gpio_run(struct gpio_desc *sda_pin, struct gpio_desc *scl_pin)
> 
> WARNING: line over 80 characters
> #102: FILE: drivers/i2c/i2c-uclass.c:506:
> +               debug("%s: I2C Node '%s' has no 'gpios' property %s\n", __func__,
> 
> WARNING: line over 80 characters
> #113: FILE: drivers/i2c/i2c-uclass.c:517:
> +               debug("%s: I2C Node '%s' has no 'gpio' pinctrl state. %s\n", __func__,
> 
> WARNING: line over 80 characters
> #122: FILE: drivers/i2c/i2c-uclass.c:526:
> +               debug("%s: I2C Node '%s' has no 'default' pinctrl state. %s\n", __func__,
> 
> WARNING: ENOSYS means 'invalid syscall nr' and nothing else
> #147: FILE: drivers/i2c/i2c-uclass.c:551:
> +               return ret ? -ENOSYS : 0;
> 
> 
> Could you fix them, also remove from the commit message the question.

Just triggered an travis build (not finsihed yet), and your patch drops
errors for arm926ejs boards:

https://travis-ci.org/hsdenx/u-boot-i2c/jobs/357258790

So, can you do a full travis build for your patch before sending a v2
and fix the errors it triggers?

Thanks!

bye,
Heiko
Alexander Kochetkov March 23, 2018, 7:50 a.m. UTC | #3
> 23 марта 2018 г., в 10:35, Heiko Schocher <hs@denx.de> написал(а):
> 
> Just triggered an travis build (not finsihed yet), and your patch drops
> errors for arm926ejs boards:
> 
> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/357258790
> 
> So, can you do a full travis build for your patch before sending a v2
> and fix the errors it triggers?

Hi Heiko!

Thanks for review.

I’ll fix warnings and travis build errors. But I don’t know how to run full travis build.
Can you provide a link to page describing how to run travis build?

Regards,
Alexander.
Heiko Schocher March 23, 2018, 8:04 a.m. UTC | #4
Hello Alexander,

Am 23.03.2018 um 08:50 schrieb Alexander Kochetkov:
> 
>> 23 марта 2018 г., в 10:35, Heiko Schocher <hs@denx.de> написал(а):
>>
>> Just triggered an travis build (not finsihed yet), and your patch drops
>> errors for arm926ejs boards:
>>
>> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/357258790
>>
>> So, can you do a full travis build for your patch before sending a v2
>> and fix the errors it triggers?
> 
> Hi Heiko!
> 
> Thanks for review.
> 
> I’ll fix warnings and travis build errors. But I don’t know how to run full travis build.

Thanks!

> Can you provide a link to page describing how to run travis build?

Hmm.. IIRC:

you need a github account, to which you push your (for example) u-boot
tree.

Than read the travis docs:

https://docs.travis-ci.com/user/getting-started/

(you do not need to create .travis.yml, as it is already in u-boot)

Hope this helps?

bye,
Heiko
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 920811a..a451e41 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -11,6 +11,8 @@ 
 #include <malloc.h>
 #include <dm/device-internal.h>
 #include <dm/lists.h>
+#include <dm/pinctrl.h>
+#include <asm/gpio.h>
 
 #define I2C_MAX_OFFSET_LEN	4
 
@@ -445,9 +447,96 @@  int i2c_get_chip_offset_len(struct udevice *dev)
 	return chip->offset_len;
 }
 
+static void i2c_gpio_set_pin(struct gpio_desc *pin, int bit)
+{
+	if (bit)
+		dm_gpio_set_dir_flags(pin, GPIOD_IS_IN);
+	else
+		dm_gpio_set_dir_flags(pin, GPIOD_IS_OUT |
+					   GPIOD_ACTIVE_LOW |
+					   GPIOD_IS_OUT_ACTIVE);
+}
+
+static int i2c_gpio_get_pin(struct gpio_desc *pin)
+{
+	return dm_gpio_get_value(pin);
+}
+
+static void i2c_deblock_gpio_run(struct gpio_desc *sda_pin, struct gpio_desc *scl_pin)
+{
+	int counter = 16;
+
+	i2c_gpio_set_pin(sda_pin, 1);
+	i2c_gpio_set_pin(scl_pin, 1);
+	udelay(5);
+
+	while (counter-- >= 0) {
+		i2c_gpio_set_pin(scl_pin, 1);
+		udelay(5);
+		i2c_gpio_set_pin(scl_pin, 0);
+		udelay(5);
+		if (i2c_gpio_get_pin(sda_pin))
+			break;
+	}
+
+	i2c_gpio_set_pin(sda_pin, 0);
+	udelay(5);
+
+	i2c_gpio_set_pin(scl_pin, 1);
+	udelay(5);
+
+	i2c_gpio_set_pin(sda_pin, 1);
+	udelay(5);
+}
+
+enum {
+	PIN_SDA = 0,
+	PIN_SCL,
+	PIN_COUNT,
+};
+
+static int i2c_deblock_gpio(struct udevice *bus)
+{
+	struct gpio_desc gpios[PIN_COUNT];
+	int ret;
+
+	ret = gpio_request_list_by_name(bus, "gpios", gpios,
+					ARRAY_SIZE(gpios), GPIOD_IS_IN);
+	if (ret != ARRAY_SIZE(gpios)) {
+		debug("%s: I2C Node '%s' has no 'gpios' property %s\n", __func__,
+		      dev_read_name(bus), bus->name);
+		if (ret >= 0) {
+			gpio_free_list(bus, gpios, ret);
+			ret = -ENOENT;
+		}
+		goto out;
+	}
+
+	ret = pinctrl_select_state(bus, "gpio");
+	if (ret) {
+		debug("%s: I2C Node '%s' has no 'gpio' pinctrl state. %s\n", __func__,
+		      dev_read_name(bus), bus->name);
+		goto out_no_pinctrl;
+	}
+
+	i2c_deblock_gpio_run(&gpios[PIN_SDA], &gpios[PIN_SCL]);
+
+	ret = pinctrl_select_state(bus, "default");
+	if (ret) {
+		debug("%s: I2C Node '%s' has no 'default' pinctrl state. %s\n", __func__,
+		      dev_read_name(bus), bus->name);
+	}
+
+out_no_pinctrl:
+	gpio_free_list(bus, gpios, ARRAY_SIZE(gpios));
+out:
+	return ret;
+}
+
 int i2c_deblock(struct udevice *bus)
 {
 	struct dm_i2c_ops *ops = i2c_get_ops(bus);
+	int ret;
 
 	/*
 	 * We could implement a software deblocking here if we could get
@@ -457,8 +546,10 @@  int i2c_deblock(struct udevice *bus)
 	 *
 	 * See https://patchwork.ozlabs.org/patch/399040/
 	 */
-	if (!ops->deblock)
-		return -ENOSYS;
+	if (!ops->deblock) {
+		ret = i2c_deblock_gpio(bus);
+		return ret ? -ENOSYS : 0;
+	}
 
 	return ops->deblock(bus);
 }