All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues
@ 2015-03-17 12:04 Ben Dooks
       [not found] ` <1426593869-8724-1-git-send-email-ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Ben Dooks @ 2015-03-17 12:04 UTC (permalink / raw)
  To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-4yDnlxn2s6sWdaTGBSpHTA, Ben Dooks,
	Haavard Skinnemoen, Wolfram Sang

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues
       [not found] ` <1426593869-8724-1-git-send-email-ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
@ 2015-03-27 15:46   ` Wolfram Sang
  2015-04-22  8:40     ` Ben Dooks
  2015-04-22  9:05     ` Uwe Kleine-König
  0 siblings, 2 replies; 5+ messages in thread
From: Wolfram Sang @ 2015-03-27 15:46 UTC (permalink / raw)
  To: Ben Dooks
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-4yDnlxn2s6sWdaTGBSpHTA, Haavard Skinnemoen

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues
  2015-03-27 15:46   ` Wolfram Sang
@ 2015-04-22  8:40     ` Ben Dooks
       [not found]       ` <55375E6B.9050405-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
  2015-04-22  9:05     ` Uwe Kleine-König
  1 sibling, 1 reply; 5+ messages in thread
From: Ben Dooks @ 2015-04-22  8:40 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-4yDnlxn2s6sWdaTGBSpHTA, Haavard Skinnemoen

-----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-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Linux-kernel] [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues
       [not found]       ` <55375E6B.9050405-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
@ 2015-04-22  8:43         ` Ben Dooks
  0 siblings, 0 replies; 5+ messages in thread
From: Ben Dooks @ 2015-04-22  8:43 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel-4yDnlxn2s6sWdaTGBSpHTA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Haavard Skinnemoen

-----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-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues
  2015-03-27 15:46   ` Wolfram Sang
  2015-04-22  8:40     ` Ben Dooks
@ 2015-04-22  9:05     ` Uwe Kleine-König
  1 sibling, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2015-04-22  9:05 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Ben Dooks, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-4yDnlxn2s6sWdaTGBSpHTA, Haavard Skinnemoen

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/  |

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-22  9:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-17 12:04 [PATCH] i2c: i2c-gpio: fix some of gpio sleep issues Ben Dooks
     [not found] ` <1426593869-8724-1-git-send-email-ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2015-03-27 15:46   ` Wolfram Sang
2015-04-22  8:40     ` Ben Dooks
     [not found]       ` <55375E6B.9050405-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2015-04-22  8:43         ` [Linux-kernel] " Ben Dooks
2015-04-22  9:05     ` Uwe Kleine-König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.