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, Januszquoted
From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001From: 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, Januszquoted
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, boolcan_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, boolcan_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, boolcan_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