Thread (75 messages) 75 messages, 12 authors, 2018-10-01

Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array

From: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Date: 2018-09-21 11:26:30
Also in: linux-doc, linux-gpio, linux-i2c, linux-iio, linux-mmc, linux-samsung-soc, linux-serial, lkml

Hi Marek,

2018-09-21 12:51 GMT+02:00, Janusz Krzysztofik [off-list ref]:
Hi Marek,

2018-09-21 10:18 GMT+02:00, Marek Szyprowski [off-list ref]:
quoted
Hi Janusz,

On 2018-09-20 18:21, Janusz Krzysztofik wrote:
quoted
On Thursday, September 20, 2018 5:48:22 PM CEST Janusz Krzysztofik
wrote:
quoted
On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski
wrote:
quoted
On 2018-09-02 14:01, Janusz Krzysztofik wrote:
quoted
Certain GPIO descriptor arrays returned by gpio_get_array() may
contain
information on direct mapping of array members to pins of a single
GPIO
chip in hardware order.  In such cases, bitmaps of values can be
passed
directly from/to the chip's .get/set_multiple() callbacks without
wasting time on iterations.

Add respective code to gpiod_get/set_array_bitmap_complex()
functions.
Pins not applicable for fast path are processed as before, skipping
over the 'fast' ones.

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
I've just noticed that this patch landed in today's linux-next. Sadly
it
breaks booting of Exynos5250-based Samsung Snow Chromebook (ARM 32bit,
device-tree source arch/arm/boot/dts/exynos5250-snow.dts).

Booting hangs after detecting MMC cards. Reverting this patch fixes
the
boot. I will try later to add some debugs and investigate it further
what
really happens when booting hangs.
Hi Marek,

Thanks for reporting. Could you please try the following fix?
Hi again,

I realized the patch was not correct, j, not i, should be updated in
second
hunk. Please try the following one.

Thanks,
Janusz
quoted
From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001
From: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Date: Thu, 20 Sep 2018 17:37:21 +0200
Subject: [PATCH] gpiolib: Fix bitmap index not updated
While skipping fast path bits, bitmap index is not updated with next
found zero bit position. Fix it.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
This one also doesn't help. A quick compare of logs with this version and
a working system shows, that with your patch (and fix) there are no calls
to
gpx0-2 pin (which are a part of mmc pwrseq), what causes mmc failure. If
you need any more information (what kind of logs will help?), let me
know.
One more question. You said before that booting hanged after detecting MMC
cards.  Without the fix, I could imagine it keeps iterating with index not
updated and simply never returns from gpiod_get/set_array_bitmap_complex().
Is the behaviour you observe the same with the fix applied?

Thanks,
Janusz
There is a debug message on array_info content available at the end of
gpiod_get_array(), could you please activate it and post the message so
we can understand better what is going on?

On the other hand, I've had a look your device-tree configuration and
it looks like that specific setup won't benefit from the fast bitmap path.
You have pin 2 at position 0 and pin 1 at position 1 of the array.
Hence, the fast bitmap path covers only pin 1, and pin 2 is processed
by the old path with apparently buggy code for skipping over fast pins.

As a temporary workaround, you could try to revert the order of pins in
your dts file (pin 1 at position 0, pin 2 at 1) and the mmc pwrseq code
should work for you again by taking the original old path, not skipping
over fast pins.  Results of such check may also help us to better
understand and resolve the issue.

Thanks,
Janusz
quoted
quoted
---
  drivers/gpio/gpiolib.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a53d17745d21..369bdd358fcc 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2880,7 +2880,7 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
  			__set_bit(hwgpio, mask);

  			if (array_info)
-				find_next_zero_bit(array_info->get_mask,
+				i = find_next_zero_bit(array_info->get_mask,
  						   array_size, i);
  			else
  				i++;
@@ -2905,7 +2905,8 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
  			trace_gpio_value(desc_to_gpio(desc), 1, value);

  			if (array_info)
-				find_next_zero_bit(array_info->get_mask, i, j);
+				j = find_next_zero_bit(array_info->get_mask, i,
+						       j);
  			else
  				j++;
  		}
@@ -3192,7 +3193,7 @@ int gpiod_set_array_value_complex(bool raw, bool
can_sleep,
  			}

  			if (array_info)
-				find_next_zero_bit(array_info->set_mask,
+				i = find_next_zero_bit(array_info->set_mask,
  						   array_size, i);
  			else
  				i++;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help