[PATCH] i2c: i2c-gpio: fix some of gpio sleep issues

[PATCH] i2c: i2c-gpio: fix some of gpio sleep issues

[Ben Dooks]

The i2c-gpio driver can be used in systems where the GPIO itself is
provided by a GPIO driver that may be sleeping (for example, SPI or
USB). In this case, it is preferable to use the _cansleep() variants
of the GPIO code.

We can only fix this up for the case where the i2c-gpio implementation
provides open-drain outputs as there are no gpio_direction_xxx_cansleep()
provided.

This removes the issues on the customer system with the console constantly
showing the following warning:

WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:2389 gpiod_set_raw_value

Signed-off-by: Ben Dooks <ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
--
CC: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
CC: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 drivers/i2c/busses/i2c-gpio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index d1050bd..9d4d59c 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -44,7 +44,7 @@ static void i2c_gpio_setsda_val(void *data, int state)
 {
 	struct i2c_gpio_platform_data *pdata = data;
 
-	gpio_set_value(pdata->sda_pin, state);
+	gpio_set_value_cansleep(pdata->sda_pin, state);
 }
 
 /* Toggle SCL by changing the direction of the pin. */
@@ -68,21 +68,21 @@ static void i2c_gpio_setscl_val(void *data, int state)
 {
 	struct i2c_gpio_platform_data *pdata = data;
 
-	gpio_set_value(pdata->scl_pin, state);
+	gpio_set_value_cansleep(pdata->scl_pin, state);
 }
 
 static int i2c_gpio_getsda(void *data)
 {
 	struct i2c_gpio_platform_data *pdata = data;
 
-	return gpio_get_value(pdata->sda_pin);
+	return gpio_get_value_cansleep(pdata->sda_pin);
 }
 
 static int i2c_gpio_getscl(void *data)
 {
 	struct i2c_gpio_platform_data *pdata = data;
 
-	return gpio_get_value(pdata->scl_pin);
+	return gpio_get_value_cansleep(pdata->scl_pin);
 }
 
 static int of_i2c_gpio_get_pins(struct device_node *np,
-- 
2.1.4

Re: [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues

[Wolfram Sang]

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

On Tue, Mar 17, 2015 at 12:04:29PM +0000, Ben Dooks wrote:
> The i2c-gpio driver can be used in systems where the GPIO itself is
> provided by a GPIO driver that may be sleeping (for example, SPI or
> USB). In this case, it is preferable to use the _cansleep() variants
> of the GPIO code.
> 
> We can only fix this up for the case where the i2c-gpio implementation
> provides open-drain outputs as there are no gpio_direction_xxx_cansleep()
> provided.
> 
> This removes the issues on the customer system with the console constantly
> showing the following warning:
> 
> WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:2389 gpiod_set_raw_value

??? It will silence a warning but will totally break the timing in case
sleeping is needed? I'd think the warning is very suitable.


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

Re: [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues

[Ben Dooks]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 27/03/15 17:46, Wolfram Sang wrote:
> On Tue, Mar 17, 2015 at 12:04:29PM +0000, Ben Dooks wrote:
>> The i2c-gpio driver can be used in systems where the GPIO itself
>> is provided by a GPIO driver that may be sleeping (for example,
>> SPI or USB). In this case, it is preferable to use the
>> _cansleep() variants of the GPIO code.
>> 
>> We can only fix this up for the case where the i2c-gpio
>> implementation provides open-drain outputs as there are no
>> gpio_direction_xxx_cansleep() provided.
>> 
>> This removes the issues on the customer system with the console
>> constantly showing the following warning:
>> 
>> WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:2389
>> gpiod_set_raw_value
> 
> ??? It will silence a warning but will totally break the timing in
> case sleeping is needed? I'd think the warning is very suitable.

This basically causes our system to fail to boot in any suitable time.

The problem is that the i2c bus is on the end of an SPI bus which means
that the identification EEPROM tests and other items read from it will
run through a gpio chip that is marked as 'can sleep'.

I don't have the exact times, but I got bored after the first thirty
seconds of these log messages and immediately patched them out of the
kernel.

- -- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVN15rAAoJEMuhVOkVU3uzkpkIALMkbTRYitWe9fgeG1hLTj0h
cTXO9bomIz1t9nZayRXnQKwgbP6Nlgi+QeNHO63BaKtNVTJBue5+f0oLw5ciJW4v
tD2lzcLWPioaZWlhjlgi1AVwC3yD9X7C7EKPuDgwqoblqW5o1UkwUGHKGk85Bueu
IrnC9ce0X56wRuyuRt/ixZwBivsW4APJsNePVebZMKnd1ISrSCDrjJEXQNdhbMjy
5hknHP7x64lMET+bqVvsPwkY1I485Xo8jPSwdzrsQrF4/qSHjou5U8EwIwRWnAf1
fprqYMVY6eN8e8z2T+LdqMOrh4Eb5R0sbmil58LmehttjR+9/KT0Iz28alULp/M=
=mUTb
-----END PGP SIGNATURE-----

Re: [Linux-kernel] [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues

[Ben Dooks]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 22/04/15 11:40, Ben Dooks wrote:
> On 27/03/15 17:46, Wolfram Sang wrote:
>> On Tue, Mar 17, 2015 at 12:04:29PM +0000, Ben Dooks wrote:
>>> The i2c-gpio driver can be used in systems where the GPIO
>>> itself is provided by a GPIO driver that may be sleeping (for
>>> example, SPI or USB). In this case, it is preferable to use
>>> the _cansleep() variants of the GPIO code.
>>> 
>>> We can only fix this up for the case where the i2c-gpio 
>>> implementation provides open-drain outputs as there are no 
>>> gpio_direction_xxx_cansleep() provided.
>>> 
>>> This removes the issues on the customer system with the
>>> console constantly showing the following warning:
>>> 
>>> WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:2389 
>>> gpiod_set_raw_value
> 
>> ??? It will silence a warning but will totally break the timing
>> in case sleeping is needed? I'd think the warning is very
>> suitable.
> 
> This basically causes our system to fail to boot in any suitable
> time.
> 
> The problem is that the i2c bus is on the end of an SPI bus which
> means that the identification EEPROM tests and other items read
> from it will run through a gpio chip that is marked as 'can
> sleep'.
> 
> I don't have the exact times, but I got bored after the first
> thirty seconds of these log messages and immediately patched them
> out of the kernel.

Would a probe time warn and then doing the same change be ok?

- -- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJVN180AAoJEMuhVOkVU3uzxSEH/jwcYKeGH8oz9k3aFfg6qHVf
u57lz2q2v2ajAhL6vA5zFOqBvJmzJnoy45uvhbZeZNV9C+F104a+Yk2Z+QIqJh5t
uQ5kZ5WCQ8QX/tS5cWEhq76zHvfooLa6Qw0WbuAKn36fA8n9Xedhuu4ozW9ofhZf
+dpTJQ3eK5RhaNyhSxFtXrfts4TgAHFI4UluUA9lyZ6acAWRDB3WWNt2sbol9os2
xrD5F2818etjsqWslJEw3MgTLHZFfswKAhYRiRjbasGpoLL0YGvfLsC+cnJrrVVy
N4OBsFs8KiXMk3MsuN1IxMa0fWvKhzbmMHmM74flp2tmStJ2iJbP6ta9U2Su3oU=
=A7M6
-----END PGP SIGNATURE-----

Re: [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues

[Uwe Kleine-König]

On Fri, Mar 27, 2015 at 04:46:09PM +0100, Wolfram Sang wrote:
> On Tue, Mar 17, 2015 at 12:04:29PM +0000, Ben Dooks wrote:
> > The i2c-gpio driver can be used in systems where the GPIO itself is
> > provided by a GPIO driver that may be sleeping (for example, SPI or
> > USB). In this case, it is preferable to use the _cansleep() variants
> > of the GPIO code.
> > 
> > We can only fix this up for the case where the i2c-gpio implementation
> > provides open-drain outputs as there are no gpio_direction_xxx_cansleep()
> > provided.
> > 
> > This removes the issues on the customer system with the console constantly
> > showing the following warning:
> > 
> > WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:2389 gpiod_set_raw_value
> 
> ??? It will silence a warning but will totally break the timing in case
> sleeping is needed? I'd think the warning is very suitable.
I don't understand this objection. What do you mean by "timing"? A
sleeping gpio is probably slower, but this shouldn't affect correctness
of the protocol implementation, should it?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |