[PATCH] gpiolib: Fix incorrect use of find_next_zero_bit()
Commit b17566a6b08b ("gpiolib: Implement fast processing path in
get/set array"), already fixed to some extent with commit 5d581d7e8cdc
("gpiolib: Fix missing updates of bitmap index"), introduced a new mode
of processing bitmaps where bits applicable for fast bitmap processing
path are supposed to be skipped while iterating bits which don't apply.
Unfortunately, find_next_zero_bit() function supposed to skip over
those fast bits is always called with a 'start' argument equal to an
index of last zero bit found and returns that index value again an
again, causing an infinite loop.
Fix it by incrementing the index uncoditionally before
find_next_zero_bit() is optionally called.
Reported-by: Marek Szyprowski
Signed-off-by: Janusz Krzysztofik
---
Marek,
Could you please test it on top of next-20180920 with "gpiolib: Fix
missing updates of bitmap index" and optionally "mmc: pwrseq_simple:
Fix incorrect handling of GPIO bitmap" also applied?
Thanks,
Janusz
drivers/gpio/gpiolib.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6ae13e3e05f1..940b543e966d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2878,12 +2878,11 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
int hwgpio = gpio_chip_hwgpio(desc);
__set_bit(hwgpio, mask);
+ i++;
if (array_info)
i = find_next_zero_bit(array_info->get_mask,
array_size, i);
- else
- i++;
} while ((i < array_size) &&
(desc_array[i]->gdev->chip == chip));
@@ -2903,12 +2902,11 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
value = !value;
__assign_bit(j, value_bitmap, value);
trace_gpio_value(desc_to_gpio(desc), 1, value);
+ j++;
if (array_info)
j = find_next_zero_bit(array_info->get_mask, i,
j);
- else
- j++;
}
if (mask != fastpath)
@@ -3191,12 +3189,11 @@ int gpiod_set_array_value_complex(bool raw, bool
can_sleep,
__clear_bit(hwgpio, bits);
count++;
}
+ i++;
if (array_info)
i = find_next_zero_bit(array_info->set_mask,
array_size, i);
- else
- i++;
} while ((i < array_size) &&
(desc_array[i]->gdev->chip == chip));
/* push collected bits to outputs */
--
2.16.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] gpiolib: Fix issues introduced by fast bitmap processing path
Hi Marek,
2018-09-24 13:38 GMT+02:00, Marek Szyprowski :
> Hi Janusz,
>
> On 2018-09-24 13:08, Janusz Krzysztofik wrote:
>> 2018-09-24 11:43 GMT+02:00, Marek Szyprowski :
>>> On 2018-09-24 01:53, Janusz Krzysztofik wrote:
>>>> While investigating possible reasons of GPIO fast bitmap processing
>>>> related boot hang on Samsung Snow Chromebook, reported by Marek
>>>> Szyprowski (thanks!), I've discovered one coding bug, addressed by
>>>> PATCH 1/2 of this series, and one potential regression introduced at
>>>> design level of the solution, hopefully fixed by PATCH 2/2. See
>>>> commit messages for details.
>>>>
>>>> Janusz Krzysztofik (2):
>>>> gpiolib: Fix missing updates of bitmap index
>>>> gpiolib: Fix array members of same chip processed separately
>>>>
>>>> The fixes should resolve the boot hang observed by Marek, however the
>>>> second change excludes that particular case from fast bitmap processing
>>>> and restores the old behaviour.
>>> I confirm, that the above 2 patches fixes boot issue on Samsung Snow
>>> Chromebook with next-20180920.
>>>
>>> Tested-by: Marek Szyprowski
>>>
>>>> Hence, it is possible still another
>>>> issue which have had an influence on that boot hang exists in the code.
>>>> In order to fully verify the fix, it would have to be tested on a
>>>> platform where an array of GPIO descriptors is used which starts from
>>>> at least two consecutive pins of one GPIO chip in hardware order,
>>>> starting ftom 0, followed by one or more pins belonging to other
>>>> chip(s).
>>>>
>>>> In order to verify if separate calls to .set() chip callback for each
>>>> pin instead of one call to .set_multiple() is actually the reason of
>>>> boot hang on Samsung Snow Chromebook, the affected driver -
>>>> drivers/mmc/core/pwrseq_simple.c - would have to be temporarily
>>>> modified for testing purposes so it calls gpiod_set_value() for each
>>>> pin instead of gpiod_set_array_value() for all of them. If that would
>>>> also result in boot hang, we could be sure the issue was really the
>>>> one addressed by the second fix. Marek, could you please try to
>>>> perform such test?
>>> Yes, I've just tested next-20180920 only with the first patch from this
>>> patchset and the mentioned change to drivers/mmc/core/pwrseq_simple.c.
>>> It boots fine, so indeed the issue is in handling of arrays of gpios.
>>>
>>> Just to be sure I did it right, this is my change to the mentioned file:
>> Yeah, that's what I had on mind. However, I'd be more lucky if it didn't
>> work
>> for you. Setting the pins sequentially, not simultaneously as before,
>> was
>> exactly what I hoped was the reason of the hang.
>>
>>> diff --git a/drivers/mmc/core/pwrseq_simple.c
>>> b/drivers/mmc/core/pwrseq_simple.c
>>> index 7f882a2bb872..9397dc1f2e38 100644
>>> --- a/drivers/mmc/core/pwrseq_simple.c
>>> +++ b/drivers/mmc/core/pwrseq_simple.c
>>> @@ -38,16 +38,11 @@ static void mmc_pwrseq_simple_set_gpios_value(struct
>>> mmc_pwrseq_simple *pwrseq,
>>> int value)
>>>{
>>> struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
>>> + int i;
>>>
>>> - if (!IS_ERR(reset_gpios)) {
>>> - DECLARE_BITMAP(values, BITS_PER_TYPE(value));
>>> - int nvalues = reset_gpios->ndescs;
>>> -
>>> - values[0] = value;
>>> -
>>> - gpiod_set_array_value_cansleep(nvalues,
>>> reset_gpios->desc,
>>> - reset_gpios->info,
>>> values);
>>> - }
>>> + if (!IS_ERR(reset_gpios))
>>> + for (i = 0; i < reset_gpios->ndescs; i++)
>> The only difference from the behaviour when the hang was occurring is now
>> the order the pins are manipulated. Maybe that matters?
>> Could you please retry the same with the order of pins reversed, either
>> in
>> the .dts file or here inside this for loop?
>
> I've switched the order of pins in dts and next-20180920 + first patch +
> above
> change also boots fine.
Thanks for performing those tests.
Since we are not able to reproduce the issue by any means other than
using the original code introduced by fast bitmap processing changes,
regardless of the first fix being applied or not, and we are only able to
resolve the hangup by excluding affected use case from the fast path,
we have to assume one or more bugs which affect mixed arrays, i.e.,
those which apply for fast bitmap processing only in part, may still exist
in the code introduced by the fast bitmap processing series. I hope we
are able to resolve it soon, before the changes reach mainline.
Thanks,
Janusz
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R Institute Poland
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] gpiolib: Fix issues introduced by fast bitmap processing path
Hi Marek,
2018-09-24 11:43 GMT+02:00, Marek Szyprowski :
> Hi Janusz,
>
> On 2018-09-24 01:53, Janusz Krzysztofik wrote:
>> While investigating possible reasons of GPIO fast bitmap processing
>> related boot hang on Samsung Snow Chromebook, reported by Marek
>> Szyprowski (thanks!), I've discovered one coding bug, addressed by
>> PATCH 1/2 of this series, and one potential regression introduced at
>> design level of the solution, hopefully fixed by PATCH 2/2. See
>> commit messages for details.
>>
>> Janusz Krzysztofik (2):
>>gpiolib: Fix missing updates of bitmap index
>>gpiolib: Fix array members of same chip processed separately
>>
>> The fixes should resolve the boot hang observed by Marek, however the
>> second change excludes that particular case from fast bitmap processing
>> and restores the old behaviour.
>
> I confirm, that the above 2 patches fixes boot issue on Samsung Snow
> Chromebook with next-20180920.
>
> Tested-by: Marek Szyprowski
>
>> Hence, it is possible still another
>> issue which have had an influence on that boot hang exists in the code.
>> In order to fully verify the fix, it would have to be tested on a
>> platform where an array of GPIO descriptors is used which starts from
>> at least two consecutive pins of one GPIO chip in hardware order,
>> starting ftom 0, followed by one or more pins belonging to other
>> chip(s).
>>
>> In order to verify if separate calls to .set() chip callback for each
>> pin instead of one call to .set_multiple() is actually the reason of
>> boot hang on Samsung Snow Chromebook, the affected driver -
>> drivers/mmc/core/pwrseq_simple.c - would have to be temporarily
>> modified for testing purposes so it calls gpiod_set_value() for each
>> pin instead of gpiod_set_array_value() for all of them. If that would
>> also result in boot hang, we could be sure the issue was really the
>> one addressed by the second fix. Marek, could you please try to
>> perform such test?
>
> Yes, I've just tested next-20180920 only with the first patch from this
> patchset and the mentioned change to drivers/mmc/core/pwrseq_simple.c.
> It boots fine, so indeed the issue is in handling of arrays of gpios.
>
> Just to be sure I did it right, this is my change to the mentioned file:
Yeah, that's what I had on mind. However, I'd be more lucky if it didn't work
for you. Setting the pins sequentially, not simultaneously as before, was
exactly what I hoped was the reason of the hang.
> diff --git a/drivers/mmc/core/pwrseq_simple.c
> b/drivers/mmc/core/pwrseq_simple.c
> index 7f882a2bb872..9397dc1f2e38 100644
> --- a/drivers/mmc/core/pwrseq_simple.c
> +++ b/drivers/mmc/core/pwrseq_simple.c
> @@ -38,16 +38,11 @@ static void mmc_pwrseq_simple_set_gpios_value(struct
> mmc_pwrseq_simple *pwrseq,
>int value)
> {
> struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
> + int i;
>
> - if (!IS_ERR(reset_gpios)) {
> - DECLARE_BITMAP(values, BITS_PER_TYPE(value));
> - int nvalues = reset_gpios->ndescs;
> -
> - values[0] = value;
> -
> - gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
> - reset_gpios->info, values);
> - }
> + if (!IS_ERR(reset_gpios))
> + for (i = 0; i < reset_gpios->ndescs; i++)
The only difference from the behaviour when the hang was occurring is now
the order the pins are manipulated. Maybe that matters?
Could you please retry the same with the order of pins reversed, either in
the .dts file or here inside this for loop?
Thanks,
Janusz
> + gpiod_set_value_cansleep(reset_gpios->desc[i], value);
> }
>
> static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host)
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R Institute Poland
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] gpiolib: Fix missing updates of bitmap index
In new code introduced by commit b17566a6b08b ("gpiolib: Implement fast
processing path in get/set array"), bitmap index is not updated with
next found zero bit position as it should while skipping over pins
already processed via fast bitmap path, possibly resulting in an
infinite loop. Fix it.
Signed-off-by: Janusz Krzysztofik
---
drivers/gpio/gpiolib.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a53d17745d21..7d9536a79a66 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2880,8 +2880,8 @@ 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,
- array_size, i);
+ i = find_next_zero_bit(array_info->get_mask,
+ array_size, i);
else
i++;
} while ((i < array_size) &&
@@ -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,8 +3193,8 @@ int gpiod_set_array_value_complex(bool raw, bool
can_sleep,
}
if (array_info)
- find_next_zero_bit(array_info->set_mask,
- array_size, i);
+ i = find_next_zero_bit(array_info->set_mask,
+ array_size, i);
else
i++;
} while ((i < array_size) &&
--
2.16.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] gpiolib: Fix array members of same chip processed separately
New code introduced by commit bf9346f5d47b ("gpiolib: Identify arrays
matching GPIO hardware") forcibly tries to find an array member which
has its array index number equal to its hardware pin number and set
up an array info for possible fast bitmap processing of all arrray
pins belonging to that chip which also satisfy that numbering rule.
Depending on array content, it may happen that consecutive array
members which belong to the same chip but don't have array indexes
equal to their pin hardware numbers will be split into groups, some of
them processed together via the fast bitmap path, and rest of them
separetely. However, applications may expect all those pins being
processed together with a single call to .set_multiple() chip callback,
like that was done before the change.
Limit applicability of fast bitmap processing path to cases where all
pins of consecutive array members starting from 0 which belong to the
same chip have their hardware numbers equal to their corresponding
array indexes. That should still speed up processing of applications
using whole GPIO banks as I/O ports, while not breaking simultaneous
manipulation of consecutive pins of the same chip which don't follow
the equal numbering rule.
Cc: Jonathan Corbet
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/board.rst | 19 +-
drivers/gpio/gpiolib.c | 35 +++--
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index c66821e033c2..a0f294e2e250 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -202,9 +202,18 @@ mapped to the device determines if the array qualifies for
fast bitmap
processing. If yes, a bitmap is passed over get/set array functions directly
between a caller and a respective .get/set_multiple() callback of a GPIO chip.
-In order to qualify for fast bitmap processing, the pin mapping must meet the
+In order to qualify for fast bitmap processing, the array must meet the
following requirements:
-- it must belong to the same chip as other 'fast' pins of the function,
-- its index within the function must match its hardware number within the chip.
-
-Open drain and open source pins are excluded from fast bitmap output
processing.
+- pin hardware number of array member 0 must also be 0,
+- pin hardware numbers of consecutive array members which belong to the same
+ chip as member 0 does must also match their array indexes.
+
+Otherwise fast bitmap processing path is not used in order to avoid consecutive
+pins which belong to the same chip but are not in hardware order being
processed
+separately.
+
+If the array applies for fast bitmap processing path, pins which belong to
+different chips than member 0 does, as well as those with indexes different
from
+their hardware pin numbers, are excluded from the fast path, both input and
+output. Moreover, open drain and open source pins are excluded from fast
bitmap
+output processing.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7d9536a79a66..6ae13e3e05f1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4376,11 +4376,10 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
chip = gpiod_to_chip(desc);
/*
-* Select a chip of first array member
-* whose index matches its pin hardware number
-* as a candidate for fast bitmap processing.
+* If pin hardware number of array member 0 is also 0, select
+* its chip as a candidate for fast bitmap processing path.
*/
- if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ if (descs->ndescs == 0 && gpio_chip_hwgpio(desc) == 0) {
struct gpio_descs *array;
bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
@@ -4414,14 +4413,30 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
count - descs->ndescs);
descs->info = array_info;
}
- /*
-* Unmark members which don't qualify for fast bitmap
-* processing (different chip, not in hardware order)
-*/
- if (array_info && (chip != array_info->chip ||
- gpio_chip_hwgpio(desc) != descs->ndescs)) {
+ /* Unmark array members which don't belong to the 'fast' chip */
+ if (array_info && array_info->chip != chip) {
__clear_bit(descs->ndescs, array_info->get_mask);
__clear_bit(descs->ndescs, array_info->set_mask);
+ }
+
[PATCH 0/2] gpiolib: Fix issues introduced by fast bitmap processing path
While investigating possible reasons of GPIO fast bitmap processing related boot hang on Samsung Snow Chromebook, reported by Marek Szyprowski (thanks!), I've discovered one coding bug, addressed by PATCH 1/2 of this series, and one potential regression introduced at design level of the solution, hopefully fixed by PATCH 2/2. See commit messages for details. Janusz Krzysztofik (2): gpiolib: Fix missing updates of bitmap index gpiolib: Fix array members of same chip processed separately The fixes should resolve the boot hang observed by Marek, however the second change excludes that particular case from fast bitmap processing and restores the old behaviour. Hence, it is possible still another issue which have had an influence on that boot hang exists in the code. In order to fully verify the fix, it would have to be tested on a platform where an array of GPIO descriptors is used which starts from at least two consecutive pins of one GPIO chip in hardware order, starting ftom 0, followed by one or more pins belonging to other chip(s). In order to verify if separate calls to .set() chip callback for each pin instead of one call to .set_multiple() is actually the reason of boot hang on Samsung Snow Chromebook, the affected driver - drivers/mmc/core/pwrseq_simple.c - would have to be temporarily modified for testing purposes so it calls gpiod_set_value() for each pin instead of gpiod_set_array_value() for all of them. If that would also result in boot hang, we could be sure the issue was really the one addressed by the second fix. Marek, could you please try to perform such test? Thanks, Janusz diffstat: Documentation/driver-api/gpio/board.rst | 19 + drivers/gpio/gpiolib.c | 46 +--- 2 files changed, 45 insertions(+), 20 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
On Friday, September 21, 2018 4:14:06 PM CEST Marek Szyprowski wrote: > Hi Janusz, > > > On 2018-09-21 12:51, Janusz Krzysztofik wrote: > > 2018-09-21 10:18 GMT+02:00, Marek Szyprowski : > >> On 2018-09-20 18:21, Janusz Krzysztofik wrote: > >>> On Thursday, September 20, 2018 5:48:22 PM CEST Janusz Krzysztofik wrote: > >>>> On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski wrote: > >>>>> On 2018-09-02 14:01, Janusz Krzysztofik wrote: > >>>>>> 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 > >>>>>> Signed-off-by: Janusz Krzysztofik > >>>>> 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 > >>> > >>> >From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001 > >>> From: Janusz Krzysztofik > >>> 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 > >> 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. > > 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? > > With debug enabled on next-20180919: > [2.499153] pwrseq_simple mmc3_pwrseq: GPIO array info: chip=gpx0, > size=2, get_mask=2, set_mask=2, invert_mask=2 Looks good to me, i..e., in line with what one could expect. However, ... > On next-20180920 I get no this message and booting hangs. > > Same with next-20180920 + your second fix from this thread. > > I will try to debug this more on Monday. > > > 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. > > Changing the order of mmc pwrseq gpio pins fixes boot hang. Not being able to discover more coding bugs in the code modified by the series, I'm wondering if the reason for the issue you are observing comes from the fact both pins are no longer manipulated together within a single .set_multiple() chip callback. I'm working on a fix which prevents from that. Thanks, Janusz ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
Hi Marek, 2018-09-21 12:51 GMT+02:00, Janusz Krzysztofik : > Hi Marek, > > 2018-09-21 10:18 GMT+02:00, Marek Szyprowski : >> Hi Janusz, >> >> On 2018-09-20 18:21, Janusz Krzysztofik wrote: >>> On Thursday, September 20, 2018 5:48:22 PM CEST Janusz Krzysztofik >>> wrote: >>>> On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski >>>> wrote: >>>>> On 2018-09-02 14:01, Janusz Krzysztofik wrote: >>>>>> 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 >>>>>> Signed-off-by: Janusz Krzysztofik >>>>> 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 >>> >>> >From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001 >>> From: Janusz Krzysztofik >>> 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 >> >> 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 > >> >>> --- >>> 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); >&g
Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
Hi Marek, 2018-09-21 10:18 GMT+02:00, Marek Szyprowski : > Hi Janusz, > > On 2018-09-20 18:21, Janusz Krzysztofik wrote: >> On Thursday, September 20, 2018 5:48:22 PM CEST Janusz Krzysztofik wrote: >>> On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski wrote: >>>> On 2018-09-02 14:01, Janusz Krzysztofik wrote: >>>>> 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 >>>>> Signed-off-by: Janusz Krzysztofik >>>> 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 >> >> >From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001 >> From: Janusz Krzysztofik >> 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 > > 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. 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 > >> --- >> 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, >> +
Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
On Thursday, September 20, 2018 5:48:22 PM CEST Janusz Krzysztofik wrote: > On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski wrote: > > Hi All, > > > > On 2018-09-02 14:01, Janusz Krzysztofik wrote: > > > 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 > > > Signed-off-by: Janusz Krzysztofik > > > > 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 >From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001 From: Janusz Krzysztofik 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 --- 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++; -- 2.16.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski wrote: > Hi All, > > On 2018-09-02 14:01, Janusz Krzysztofik wrote: > > 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 > > Signed-off-by: Janusz Krzysztofik > > 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? Thanks, Janusz >From d7ecd435bfb4972766b63ac383a43875700c7452 Mon Sep 17 00:00:00 2001 From: Janusz Krzysztofik 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 --- 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..5bc3447949c9 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); + i = 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++; -- 2.16.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v8 3/4] gpiolib: Pass array info to get/set array functions
In order to make use of array info obtained from gpiod_get_array() and speed up processing of arrays matching single GPIO chip layout, that information must be passed to get/set array functions. Extend the functions' API with that additional parameter and update all users. Pass NULL if a user builds an array itself from single GPIOs. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Geert Uytterhoeven Cc: Sebastien Bourdelin Cc: Lukas Wunner Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Rojhalat Ibrahim Cc: Dominik Brodowski Cc: Russell King Cc: Kishon Vijay Abraham I Cc: Tony Lindgren Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Yegor Yefremov Cc: Uwe Kleine-König Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 14 -- drivers/auxdisplay/hd44780.c| 8 +++--- drivers/bus/ts-nbus.c | 5 ++-- drivers/gpio/gpio-max3191x.c| 6 +++-- drivers/gpio/gpiolib.c | 40 +++-- drivers/gpio/gpiolib.h | 2 ++ drivers/i2c/muxes/i2c-mux-gpio.c| 3 ++- drivers/mmc/core/pwrseq_simple.c| 2 +- drivers/mux/gpio.c | 3 ++- drivers/net/phy/mdio-mux-gpio.c | 2 +- drivers/pcmcia/soc_common.c | 2 +- drivers/phy/motorola/phy-mapphone-mdm6600.c | 4 ++- drivers/staging/iio/adc/ad7606.c| 3 ++- drivers/tty/serial/serial_mctrl_gpio.c | 2 +- include/linux/gpio/consumer.h | 16 15 files changed, 86 insertions(+), 26 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index 7e0298b9a7b9..0afd95a12b10 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access @@ -358,6 +366,7 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors + * array_info- optional information obtained from gpiod_array_get() * value_bitmap - a bitmap to store the GPIOs' values (get) or a bitmap of values to assign to the GPIOs (set) @@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array():: struct
[PATCH v8 2/4] gpiolib: Identify arrays matching GPIO hardware
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip. Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion. Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.
Cc: Jonathan Corbet
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/consumer.rst | 4 +-
drivers/gpio/gpiolib.c | 72 +-
drivers/gpio/gpiolib.h | 9
include/linux/gpio/consumer.h | 9
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be
obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b66b2191c5c5..141f39308a53 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
- int count;
+ struct gpio_array *array_info = NULL;
+ struct gpio_chip *chip;
+ int count, bitmap_size;
count = gpiod_count(dev, con_id);
if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
descs->desc[descs->ndescs] = desc;
+
+ chip = gpiod_to_chip(desc);
+ /*
+* Select a chip of first array member
+* whose index matches its pin hardware number
+* as a candidate for fast bitmap processing.
+*/
+ if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ struct gpio_descs *array;
+
+ bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+ chip->ngpio : count);
+
+ array = kzalloc(struct_size(descs, desc, count) +
+ struct_size(array_info, invert_mask,
+ 3 * bitmap_size), GFP_KERNEL);
+ if (!array) {
+ gpiod_put_array(descs);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ memcpy(array, descs,
+ struct_size(descs, desc, descs->ndescs + 1));
+ kfree(descs);
+
+ descs = array;
+ array_info = (void *)(descs->desc + count);
+ array_info->get_mask = array_info->invert_mask +
+ bitmap_size;
+ array_info->set_mask = array_info->get_mask +
+ bitmap_size;
+
+ array_info->desc = descs->desc;
+ array_info->size = count;
+ array_info->chip = chip;
+ bitmap_set(array_info->get_mask, descs->ndescs,
+ count - descs->ndescs);
+ bitmap_set(array_info->set_mask, descs->ndescs,
+ count - descs->ndescs);
+ descs->info = array_info;
+ }
+ /*
+* Unmark members which don't qualify for fast bitmap
+* processing (different chip, not in hardware order)
+*/
+ if (array_info && (chip != array_info->chip ||
+ gpio_chip_hwgpio(desc) !
[PATCH v8 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
Most users of get/set array functions iterate consecutive bits of data, usually a single integer, while processing array of results obtained from, or building an array of values to be passed to those functions. Save time wasted on those iterations by changing the functions' API to accept bitmaps. All current users are updated as well. More benefits from the change are expected as soon as planned support for accepting/passing those bitmaps directly from/to respective GPIO chip callbacks if applicable is implemented. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Sebastien Bourdelin Cc: Lukas Wunner Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Rojhalat Ibrahim Cc: Dominik Brodowski Cc: Russell King Cc: Kishon Vijay Abraham I Cc: Tony Lindgren Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Yegor Yefremov Cc: Uwe Kleine-König Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson Reviewed-by: Geert Uytterhoeven Tested-by: Geert Uytterhoeven --- Documentation/driver-api/gpio/consumer.rst | 22 drivers/auxdisplay/hd44780.c| 59 +++-- drivers/bus/ts-nbus.c | 15 ++ drivers/gpio/gpio-max3191x.c| 10 ++-- drivers/gpio/gpiolib.c | 82 +++-- drivers/gpio/gpiolib.h | 4 +- drivers/i2c/muxes/i2c-mux-gpio.c| 13 ++--- drivers/mmc/core/pwrseq_simple.c| 13 ++--- drivers/mux/gpio.c | 13 ++--- drivers/net/phy/mdio-mux-gpio.c | 11 ++-- drivers/pcmcia/soc_common.c | 7 +-- drivers/phy/motorola/phy-mapphone-mdm6600.c | 15 +++--- drivers/staging/iio/adc/ad7606.c| 9 ++-- drivers/tty/serial/serial_mctrl_gpio.c | 7 +-- include/linux/gpio/consumer.h | 34 ++-- 15 files changed, 137 insertions(+), 177 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index aa03f389d41d..ed68042ddccf 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access GPIOs belonging to the same bank or chip simultaneously if supported by the @@ -356,8 +356,8 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors - * value_array - an array to store the GPIOs' v
[PATCH v8 4/4] gpiolib: Implement fast processing path in get/set array
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
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/board.rst| 15 ++
Documentation/driver-api/gpio/consumer.rst | 8 +++
drivers/gpio/gpiolib.c | 87 --
3 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
The line will be hogged as soon as the gpiochip is created or - in case the
chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function. The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing. If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output
processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
Note that for optimal performance GPIOs belonging to the same chip should be
contiguous within the array of descriptors.
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip. If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip. That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
The return value of gpiod_get_array_value() and its variants is 0 on success
or negative on error. Note the difference to gpiod_get_value(), which returns
0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cd7c1deed132..d7532aa6c0bc 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2789,7 +2789,36 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap)
{
- int i = 0;
+ int err, i = 0;
+
+ /*
+* Validate array_info against desc_array and its size.
+* It should immediately follow desc_array if both
+* have been obtained from the same gpiod_get_array() call.
+*/
+ if (array_info && array_info->desc == desc_array &&
+ array_size <= array_info->size &&
+ (void *)array_info == desc_array + array_info->size) {
+ if (!can_sleep)
+ WARN_ON(array_info->chip->can_sleep);
+
+ err = gpio_chip_get_multiple(array_info->chip,
+array_info->get_mask,
+value_bitmap);
+ if (err)
+ return err;
+
+ if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+ bitmap_xor(value_bitmap, value_bitmap,
+ array_info->invert_mask, array_size);
+
+ if (bitmap_full(array_info->get_mask, array_size))
+ return 0;
+
+ i = find_first_zero_bit(array_info->get_mask, array_size);
+ } else {
+ array_info = NULL;
+ }
while (i < array_size) {
struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2
[PATCH v8 0/4] gpiolib: speed up GPIO array processing
The goal is to boost performance of get/set array functions while processing GPIO arrays which represent pins of a signle chip in hardware order. If resulting performance is close to PIO, GPIO API can be used for data I/O without much loss of speed. Created and tested on a low end Amstrad Delta board with NAND driver updated to use GPIO API for data I/O. Performance degrade compared to PIO is much better than before the optimization though not quite satisfactory on my test hardware. Janusz Krzysztofik (4): gpiolib: Pass bitmaps, not integer arrays, to get/set array gpiolib: Identify arrays matching GPIO hardware gpiolib: Pass array info to get/set array functions gpiolib: Implement fast processing path in get/set array Changelog: v8: [PATCH v8 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array: - replace *values with values[0] where applicable - suggessted by Geert Uytterhoeven, thanks, - drop !! from 3rd argument of __assign_bit() where applicable - suggested by Lukas Wunner, thanks, drivers/gpio/gpiolib.c: - s/bitnap/bitmap/ - catched by Lukas Wunner, thanks, drivers/phy/motorola/phy-mapphone-mdm6600.c: - assign ddata->status directly from bitmap, not an intermediate 'val' variable, now used only for debugging purposes, include/linux/gpio/consumer.h: - also update API of inline function replacements used if CONFIG_GPIOLIB is not defined - catched by kbuild test robot, [PATCH v8 3/4] gpiolib: Pass array info to get/set array functions: commit message: - s/bulids/builds/ - catched by Geert Uytterhoeven, thanks, drivers/gpio/gpiolib.c: - add information on array_info arguments to array function descriptions - catched by kbuild test robot, include/linux/gpio/consumer.h: - also update API of inline function replacements used if CONFIG_GPIOLIB is not defined - catched by kbuild test robot. v7: - add more people to Cc: - authors and/or those who contributed most to the drivers in scope of the change, [PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set: - avoid VLAs, use data source type bit number as bitmap size if not constant - great recommendation by Peter Rosin, thanks, - revert names of local variables declared with DECLARE_BITMAP() from 'value_bitmap' to original names of value arrays they replace (but not 'value_array') - inspired by Peter Rosin suggestion - thanks! drivers/gpio/gpio-max3191x.c: - use bitmap_alloc() to be more consistent with DECLARE_BITMAP() pattern used by other consumers, drivers/phy/motorola/phy-mapphone-mdm6600.c: - no need to mask unused bits of val before its assignment to bitmap, passing PHY_MDM6600_NR_CMD_LINES to gpiod_set_array_value() as array/ bitmap size provides sufficient protection. v6: [PATCH v6 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - use DECLARE_BITMAP() macro for declaring value_bitmap - great idea by David Laight, thanks! drivers/auxdisplay/hd44780.c: - simplify the code and adjust comments as recommended by Geert Uytterhoeven - thanks!, drivers/i2c/muxes/i2c-mux-gpio.c: - drop .values member of struct gpiomux - details provided by Peter Rosin, thanks!, drivers/mux/gpio.c: - drop .val member of struct mux_gpio - details provided by Peter Rosin, thanks!, drivers/net/phy/mdio-mux-gpio.c: - drop .values member of struct mdio_mux_gpio_state and its processsing. v5: [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - drivers/i2c/muxes/i2c-mux-gpio.c: - drop assigment of values to struct gpiomux.values, as recommended by Peter Rosin - thanks!, - mark the .values member of the structure as obsolete, - drivers/mux/gpio.c: - drop assigment of values to struct mux_gpio.val, also recommended by Peter Rosin - thanks!, - merk the .val member of the structure as obsolete, - drivers/auxdisplay/hd44780.c: - fix incorrect bitmap size, - use >>= operator to simplify notation, both catched by Miguel Ojeda - thanks!, - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware - add Cc: clause. [PATCH v5 3/4] gpiolib: Pass array info to get/set array functions - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 4/4] gpiolib: Implement fast processing path in get/set - add Cc: clause. v4: That series was a follow up of the former "mtd: rawnand: ams-delta: Use gpio-omap accessors for data I/O" which already contained some changes to gpiolib. Those previous attempts were commented by Borris Brezillon who suggested using GPIO API modified to accept bitmaps, and by Linus Walleij who suggested still more great ideas for further immprovement of the proposed API changes - thanks! diffstat: Documentation/driver-api/gpio/board.rst | 15 + Documentation/driver-api/gpio/consumer.rst | 48 +++- drivers/auxdisplay/hd44780.c| 67 ++ drivers/bus/ts-nbus.c | 20 - drivers/g
[PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
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
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/board.rst| 15 ++
Documentation/driver-api/gpio/consumer.rst | 8 +++
drivers/gpio/gpiolib.c | 87 --
3 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
The line will be hogged as soon as the gpiochip is created or - in case the
chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function. The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing. If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output
processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
Note that for optimal performance GPIOs belonging to the same chip should be
contiguous within the array of descriptors.
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip. If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip. That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
The return value of gpiod_get_array_value() and its variants is 0 on success
or negative on error. Note the difference to gpiod_get_value(), which returns
0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cef6ee31fe05..b9d083fb13ee 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2789,7 +2789,36 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap)
{
- int i = 0;
+ int err, i = 0;
+
+ /*
+* Validate array_info against desc_array and its size.
+* It should immediately follow desc_array if both
+* have been obtained from the same gpiod_get_array() call.
+*/
+ if (array_info && array_info->desc == desc_array &&
+ array_size <= array_info->size &&
+ (void *)array_info == desc_array + array_info->size) {
+ if (!can_sleep)
+ WARN_ON(array_info->chip->can_sleep);
+
+ err = gpio_chip_get_multiple(array_info->chip,
+array_info->get_mask,
+value_bitmap);
+ if (err)
+ return err;
+
+ if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+ bitmap_xor(value_bitmap, value_bitmap,
+ array_info->invert_mask, array_size);
+
+ if (bitmap_full(array_info->get_mask, array_size))
+ return 0;
+
+ i = find_first_zero_bit(array_info->get_mask, array_size);
+ } else {
+ array_info = NULL;
+ }
while (i < array_size) {
struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2
[PATCH v7 2/4] gpiolib: Identify arrays matching GPIO hardware
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip. Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion. Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.
Cc: Jonathan Corbet
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/consumer.rst | 4 +-
drivers/gpio/gpiolib.c | 72 +-
drivers/gpio/gpiolib.h | 9
include/linux/gpio/consumer.h | 9
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be
obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 434d09779a1f..141f2f290538 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
- int count;
+ struct gpio_array *array_info = NULL;
+ struct gpio_chip *chip;
+ int count, bitmap_size;
count = gpiod_count(dev, con_id);
if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
descs->desc[descs->ndescs] = desc;
+
+ chip = gpiod_to_chip(desc);
+ /*
+* Select a chip of first array member
+* whose index matches its pin hardware number
+* as a candidate for fast bitmap processing.
+*/
+ if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ struct gpio_descs *array;
+
+ bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+ chip->ngpio : count);
+
+ array = kzalloc(struct_size(descs, desc, count) +
+ struct_size(array_info, invert_mask,
+ 3 * bitmap_size), GFP_KERNEL);
+ if (!array) {
+ gpiod_put_array(descs);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ memcpy(array, descs,
+ struct_size(descs, desc, descs->ndescs + 1));
+ kfree(descs);
+
+ descs = array;
+ array_info = (void *)(descs->desc + count);
+ array_info->get_mask = array_info->invert_mask +
+ bitmap_size;
+ array_info->set_mask = array_info->get_mask +
+ bitmap_size;
+
+ array_info->desc = descs->desc;
+ array_info->size = count;
+ array_info->chip = chip;
+ bitmap_set(array_info->get_mask, descs->ndescs,
+ count - descs->ndescs);
+ bitmap_set(array_info->set_mask, descs->ndescs,
+ count - descs->ndescs);
+ descs->info = array_info;
+ }
+ /*
+* Unmark members which don't qualify for fast bitmap
+* processing (different chip, not in hardware order)
+*/
+ if (array_info && (chip != array_info->chip ||
+ gpio_chip_hwgpio(desc) !
[PATCH v7 3/4] gpiolib: Pass array info to get/set array functions
In order to make use of array info obtained from gpiod_get_array() and speed up processing of arrays matching single GPIO chip layout, that information must be passed to get/set array functions. Extend the functions' API with that additional parameter and update all users. Pass NULL if a user bulids an array itself from single GPIOs. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Geert Uytterhoeven Cc: Sebastien Bourdelin Cc: Lukas Wunner Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Rojhalat Ibrahim Cc: Dominik Brodowski Cc: Russell King Cc: Kishon Vijay Abraham I Cc: Tony Lindgren Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Yegor Yefremov Cc: Uwe Kleine-König Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 14 +++-- drivers/auxdisplay/hd44780.c| 8 drivers/bus/ts-nbus.c | 5 +++-- drivers/gpio/gpio-max3191x.c| 6 -- drivers/gpio/gpiolib.c | 32 + drivers/gpio/gpiolib.h | 2 ++ drivers/i2c/muxes/i2c-mux-gpio.c| 3 ++- drivers/mmc/core/pwrseq_simple.c| 2 +- drivers/mux/gpio.c | 3 ++- drivers/net/phy/mdio-mux-gpio.c | 2 +- drivers/pcmcia/soc_common.c | 2 +- drivers/phy/motorola/phy-mapphone-mdm6600.c | 4 +++- drivers/staging/iio/adc/ad7606.c| 3 ++- drivers/tty/serial/serial_mctrl_gpio.c | 2 +- include/linux/gpio/consumer.h | 8 15 files changed, 70 insertions(+), 26 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index 7e0298b9a7b9..0afd95a12b10 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access @@ -358,6 +366,7 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors + * array_info- optional information obtained from gpiod_array_get() * value_bitmap - a bitmap to store the GPIOs' values (get) or a bitmap of values to assign to the GPIOs (set) @@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array()::
[PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
Most users of get/set array functions iterate consecutive bits of data, usually a single integer, while processing array of results obtained from, or building an array of values to be passed to those functions. Save time wasted on those iterations by changing the functions' API to accept bitmaps. All current users are updated as well. More benefits from the change are expected as soon as planned support for accepting/passing those bitmaps directly from/to respective GPIO chip callbacks if applicable is implemented. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Geert Uytterhoeven Cc: Sebastien Bourdelin Cc: Lukas Wunner Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Rojhalat Ibrahim Cc: Dominik Brodowski Cc: Russell King Cc: Kishon Vijay Abraham I Cc: Tony Lindgren Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Yegor Yefremov Cc: Uwe Kleine-König Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 22 drivers/auxdisplay/hd44780.c| 59 +++-- drivers/bus/ts-nbus.c | 15 ++ drivers/gpio/gpio-max3191x.c| 10 ++-- drivers/gpio/gpiolib.c | 82 +++-- drivers/gpio/gpiolib.h | 4 +- drivers/i2c/muxes/i2c-mux-gpio.c| 13 ++--- drivers/mmc/core/pwrseq_simple.c| 13 ++--- drivers/mux/gpio.c | 13 ++--- drivers/net/phy/mdio-mux-gpio.c | 11 ++-- drivers/pcmcia/soc_common.c | 8 +-- drivers/phy/motorola/phy-mapphone-mdm6600.c | 13 ++--- drivers/staging/iio/adc/ad7606.c| 9 ++-- drivers/tty/serial/serial_mctrl_gpio.c | 7 +-- include/linux/gpio/consumer.h | 18 --- 15 files changed, 129 insertions(+), 168 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index aa03f389d41d..ed68042ddccf 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access GPIOs belonging to the same bank or chip simultaneously if supported by the @@ -356,8 +356,8 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors - * value_array - an array to store the GPIOs' values (get) or - an array of values to assign to
[PATCH v7 0/4] gpiolib: speed up GPIO array processing
The goal is to boost performance of get/set array functions while processing GPIO arrays which represent pins of a signle chip in hardware order. If resulting performance is close to PIO, GPIO API can be used for data I/O without much loss of speed. Created and tested on a low end Amstrad Delta board with NAND driver updated to use GPIO API for data I/O. Performance degrade compared to PIO is much better than before the optimization though not quite satisfactory on my test hardware. Janusz Krzysztofik (4): gpiolib: Pass bitmaps, not integer arrays, to get/set array gpiolib: Identify arrays matching GPIO hardware gpiolib: Pass array info to get/set array functions gpiolib: Implement fast processing path in get/set array Changelog: v7: - add more people to Cc: - authors and/or those who contributed most to the drivers in scope of the change, [PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set: - avoid VLAs, use data source type bit number as bitmap size if not constant - great recommendation by Peter Rosin, thanks, - revert names of local variables declared with DECLARE_BITMAP() from 'value_bitmap' to original names of value arrays they replace (but not 'value_array') - inspired by Peter Rosin suggestion - thanks! drivers/gpio/gpio-max3191x.c: - use bitmap_alloc() to be more consistent with DECLARE_BITMAP() pattern used by other consumers, drivers/phy/motorola/phy-mapphone-mdm6600.c: - no need to mask unused bits of val before its assignment to bitmap, passing PHY_MDM6600_NR_CMD_LINES to gpiod_set_array_value() as array/ bitmap size provides sufficient protection. v6: [PATCH v6 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - use DECLARE_BITMAP() macro for declaring value_bitmap - great idea by David Laight, thanks! drivers/auxdisplay/hd44780.c: - simplify the code and adjust comments as recommended by Geert Uytterhoeven - thanks!, drivers/i2c/muxes/i2c-mux-gpio.c: - drop .values member of struct gpiomux - details provided by Peter Rosin, thanks!, drivers/mux/gpio.c: - drop .val member of struct mux_gpio - details provided by Peter Rosin, thanks!, drivers/net/phy/mdio-mux-gpio.c: - drop .values member of struct mdio_mux_gpio_state and its processsing. v5: [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - drivers/i2c/muxes/i2c-mux-gpio.c: - drop assigment of values to struct gpiomux.values, as recommended by Peter Rosin - thanks!, - mark the .values member of the structure as obsolete, - drivers/mux/gpio.c: - drop assigment of values to struct mux_gpio.val, also recommended by Peter Rosin - thanks!, - merk the .val member of the structure as obsolete, - drivers/auxdisplay/hd44780.c: - fix incorrect bitmap size, - use >>= operator to simplify notation, both catched by Miguel Ojeda - thanks!, - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware - add Cc: clause. [PATCH v5 3/4] gpiolib: Pass array info to get/set array functions - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 4/4] gpiolib: Implement fast processing path in get/set - add Cc: clause. v4: That series was a follow up of the former "mtd: rawnand: ams-delta: Use gpio-omap accessors for data I/O" which already contained some changes to gpiolib. Those previous attempts were commented by Borris Brezillon who suggested using GPIO API modified to accept bitmaps, and by Linus Walleij who suggested still more great ideas for further immprovement of the proposed API changes - thanks! diffstat: Documentation/driver-api/gpio/board.rst | 15 + Documentation/driver-api/gpio/consumer.rst | 48 +++- drivers/auxdisplay/hd44780.c| 67 ++ drivers/bus/ts-nbus.c | 20 -- drivers/gpio/gpio-max3191x.c| 16 + drivers/gpio/gpiolib.c | 273 ++-- drivers/gpio/gpiolib.h | 15 + drivers/i2c/muxes/i2c-mux-gpio.c| 16 - drivers/mmc/core/pwrseq_simple.c| 15 - drivers/mux/gpio.c | 16 - drivers/net/phy/mdio-mux-gpio.c | 13 - drivers/pcmcia/soc_common.c | 10 - drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 - drivers/staging/iio/adc/ad7606.c| 12 - drivers/tty/serial/serial_mctrl_gpio.c |9 include/linux/gpio/consumer.h | 35 ++- 16 files changed, 396 insertions(+), 201 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
Hi Miguel,
On Thursday, August 30, 2018 1:10:59 PM CEST Miguel Ojeda wrote:
> Hi Janusz,
>
> On Wed, Aug 29, 2018 at 10:48 PM, Janusz Krzysztofik
> wrote:
> > ...
> > /* High nibble + RS, RW */
> > - for (i = 4; i < 8; i++)
> > - values[PIN_DATA0 + i] = !!(val & BIT(i));
> > - values[PIN_CTRL_RS] = rs;
> > + value_bitmap[0] = val;
> > + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> > n = 5;
> > if (hd->pins[PIN_CTRL_RW]) {
> > - values[PIN_CTRL_RW] = 0;
> > + __clear_bit(PIN_CTRL_RW, value_bitmap);
> > n++;
> > }
> > + value_bitmap[0] >>= PIN_DATA4;
> >
> > /* Present the data to the port */
> > - gpiod_set_array_value_cansleep(n, >pins[PIN_DATA4],
> > - [PIN_DATA4]);
> > + gpiod_set_array_value_cansleep(n, >pins[PIN_DATA4],
value_bitmap);
> >
> > hd44780_strobe_gpio(hd);
> >
> > /* Low nibble */
> > - for (i = 0; i < 4; i++)
> > - values[PIN_DATA4 + i] = !!(val & BIT(i));
> > + value_bitmap[0] &= ~((1 << PIN_DATA4) - 1);
> > + value_bitmap[0] |= val & ~((1 << PIN_DATA4) - 1);
>
> This is still wrong! What I originally meant in my v4 review is that
> there is an extra ~ in the second line.
Indeed, that's wrong, I missed your original point, sorry.
> Also, a couple of general comments:
>
> - Please review the list of CCs (I was not CC'd originally, so maybe
> there are other maintainers that aren't, either)
That's probably because early versions of the series, prior to v4, were not
touching existing GPIO API so there were no changes to users of gpiod_get/
set_array_value() and their variants. From v4 on, you are in the loop so don't
worry, you haven't missed anything.
But anyway, thanks for your suggestion to review the Cc; list, I've done that
for v7 and added still a few people who contributed most to the code being
changed.
> - In general, the new code seems harder to read than the original one
> (but that is my impression).
I hope we are slowly approaching acceptable readability in recent iterations.
Thanks,
Janusz
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v6 2/4] gpiolib: Identify arrays matching GPIO hardware
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip. Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion. Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.
Cc: Jonathan Corbet
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/consumer.rst | 4 +-
drivers/gpio/gpiolib.c | 72 +-
drivers/gpio/gpiolib.h | 9
include/linux/gpio/consumer.h | 9
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be
obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0e9ffa8cab6..c1ed1c759345 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
- int count;
+ struct gpio_array *array_info = NULL;
+ struct gpio_chip *chip;
+ int count, bitmap_size;
count = gpiod_count(dev, con_id);
if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
descs->desc[descs->ndescs] = desc;
+
+ chip = gpiod_to_chip(desc);
+ /*
+* Select a chip of first array member
+* whose index matches its pin hardware number
+* as a candidate for fast bitmap processing.
+*/
+ if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ struct gpio_descs *array;
+
+ bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+ chip->ngpio : count);
+
+ array = kzalloc(struct_size(descs, desc, count) +
+ struct_size(array_info, invert_mask,
+ 3 * bitmap_size), GFP_KERNEL);
+ if (!array) {
+ gpiod_put_array(descs);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ memcpy(array, descs,
+ struct_size(descs, desc, descs->ndescs + 1));
+ kfree(descs);
+
+ descs = array;
+ array_info = (void *)(descs->desc + count);
+ array_info->get_mask = array_info->invert_mask +
+ bitmap_size;
+ array_info->set_mask = array_info->get_mask +
+ bitmap_size;
+
+ array_info->desc = descs->desc;
+ array_info->size = count;
+ array_info->chip = chip;
+ bitmap_set(array_info->get_mask, descs->ndescs,
+ count - descs->ndescs);
+ bitmap_set(array_info->set_mask, descs->ndescs,
+ count - descs->ndescs);
+ descs->info = array_info;
+ }
+ /*
+* Unmark members which don't qualify for fast bitmap
+* processing (different chip, not in hardware order)
+*/
+ if (array_info && (chip != array_info->chip ||
+ gpio_chip_hwgpio(desc) !
[PATCH v6 3/4] gpiolib: Pass array info to get/set array functions
In order to make use of array info obtained from gpiod_get_array() and speed up processing of arrays matching single GPIO chip layout, that information must be passed to get/set array functions. Extend the functions' API with that additional parameter and update all users. Pass NULL if a user bulids an array itself from single GPIOs. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Dominik Brodowski Cc: Kishon Vijay Abraham I Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 14 ++-- drivers/auxdisplay/hd44780.c| 12 ++ drivers/bus/ts-nbus.c | 6 +++-- drivers/gpio/gpio-max3191x.c| 6 +++-- drivers/gpio/gpiolib.c | 34 - drivers/gpio/gpiolib.h | 2 ++ drivers/i2c/muxes/i2c-mux-gpio.c| 2 +- drivers/mmc/core/pwrseq_simple.c| 2 +- drivers/mux/gpio.c | 3 ++- drivers/net/phy/mdio-mux-gpio.c | 2 +- drivers/pcmcia/soc_common.c | 3 ++- drivers/phy/motorola/phy-mapphone-mdm6600.c | 4 +++- drivers/staging/iio/adc/ad7606.c| 3 ++- drivers/tty/serial/serial_mctrl_gpio.c | 2 +- include/linux/gpio/consumer.h | 8 +++ 15 files changed, 75 insertions(+), 28 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index 7e0298b9a7b9..0afd95a12b10 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access @@ -358,6 +366,7 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors + * array_info- optional information obtained from gpiod_array_get() * value_bitmap - a bitmap to store the GPIOs' values (get) or a bitmap of values to assign to the GPIOs (set) @@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array():: struct gpio_descs *my_gpio_descs = gpiod_get_array(...); gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc, - my_g
[PATCH v6 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
Most users of get/set array functions iterate consecutive bits of data, usually a single integer, while processing array of results obtained from, or building an array of values to be passed to those functions. Save time wasted on those iterations by changing the functions' API to accept bitmaps. All current users are updated as well. More benefits from the change are expected as soon as planned support for accepting/passing those bitmaps directly from/to respective GPIO chip callbacks if applicable is implemented. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Dominik Brodowski Cc: Kishon Vijay Abraham I Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 22 drivers/auxdisplay/hd44780.c| 62 - drivers/bus/ts-nbus.c | 21 ++- drivers/gpio/gpio-max3191x.c| 17 +++--- drivers/gpio/gpiolib.c | 86 +++-- drivers/gpio/gpiolib.h | 4 +- drivers/i2c/muxes/i2c-mux-gpio.c| 14 ++--- drivers/mmc/core/pwrseq_simple.c| 13 ++--- drivers/mux/gpio.c | 15 ++--- drivers/net/phy/mdio-mux-gpio.c | 11 ++-- drivers/pcmcia/soc_common.c | 11 ++-- drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++--- drivers/staging/iio/adc/ad7606.c| 9 +-- drivers/tty/serial/serial_mctrl_gpio.c | 7 ++- include/linux/gpio/consumer.h | 18 +++--- 15 files changed, 145 insertions(+), 182 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index aa03f389d41d..ed68042ddccf 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access GPIOs belonging to the same bank or chip simultaneously if supported by the @@ -356,8 +356,8 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors - * value_array - an array to store the GPIOs' values (get) or - an array of values to assign to the GPIOs (set) + * value_bitmap - a bitmap to store the GPIOs' values (get) or + a bitmap of values to assign to the
[PATCH v6 4/4] gpiolib: Implement fast processing path in get/set array
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
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/board.rst| 15 ++
Documentation/driver-api/gpio/consumer.rst | 8 +++
drivers/gpio/gpiolib.c | 87 --
3 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
The line will be hogged as soon as the gpiochip is created or - in case the
chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function. The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing. If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output
processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
Note that for optimal performance GPIOs belonging to the same chip should be
contiguous within the array of descriptors.
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip. If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip. That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
The return value of gpiod_get_array_value() and its variants is 0 on success
or negative on error. Note the difference to gpiod_get_value(), which returns
0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d26cdbdb7cf..b799a89c4c17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2787,7 +2787,36 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap)
{
- int i = 0;
+ int err, i = 0;
+
+ /*
+* Validate array_info against desc_array and its size.
+* It should immediately follow desc_array if both
+* have been obtained from the same gpiod_get_array() call.
+*/
+ if (array_info && array_info->desc == desc_array &&
+ array_size <= array_info->size &&
+ (void *)array_info == desc_array + array_info->size) {
+ if (!can_sleep)
+ WARN_ON(array_info->chip->can_sleep);
+
+ err = gpio_chip_get_multiple(array_info->chip,
+array_info->get_mask,
+value_bitmap);
+ if (err)
+ return err;
+
+ if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+ bitmap_xor(value_bitmap, value_bitmap,
+ array_info->invert_mask, array_size);
+
+ if (bitmap_full(array_info->get_mask, array_size))
+ return 0;
+
+ i = find_first_zero_bit(array_info->get_mask, array_size);
+ } else {
+ array_info = NULL;
+ }
while (i < array_size) {
struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2
[PATH v6 0/4] gpiolib: speed up GPIO array processing
The goal is to boost performance of get/set array functions while processing GPIO arrays which represent pins of a signle chip in hardware order. If resulting performance is close to PIO, GPIO API can be used for data I/O without much loss of speed. Created and tested on a low end Amstrad Delta board with NAND driver updated to use GPIO API for data I/O. Performance degrade compared to PIO is much better than before the optimization but still not quite satisfactory. Janusz Krzysztofik (4): gpiolib: Pass bitmaps, not integer arrays, to get/set array gpiolib: Identify arrays matching GPIO hardware gpiolib: Pass array info to get/set array functions gpiolib: Implement fast processing path in get/set array Changelog: v6: [PATCH v6 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - use DECLARE_BITMAP() macro for declaring value_bitmap - great idea by David Laight, thanks! drivers/auxdisplay/hd44780.c: - simplify the code and adjust comments as recommended by Geert Uytterhoeven - thanks!, drivers/i2c/muxes/i2c-mux-gpio.c: - drop .values member of struct gpiomux - details prvided by Peter Rosin, thanks!, drivers/mux/gpio.c: - drop .val member of struct mux_gpio - details prvided by Peter Rosin, thanks!, drivers/net/phy/mdio-mux-gpio.c: - drop .values member of struct mdio_mux_gpio_state and is processsing. v5: [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - drivers/i2c/muxes/i2c-mux-gpio.c: - drop assigment of values to struct gpiomux.values, as recommended by Peter Rosin - thanks!, - mark the .values member of the structure as obsolete, - drivers/mux/gpio.c: - drop assigment of values to struct mux_gpio.val, also recommended by Peter Rosin - thanks!, - merk the .val member of the structure as obsolete, - drivers/auxdisplay/hd44780.c: - fix incorrect bitmap size, - use >>= operator to simplify notation, both catched by Miguel Ojeda - thanks!, - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware - add Cc: clause. [PATCH v5 3/4] gpiolib: Pass array info to get/set array functions - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 4/4] gpiolib: Implement fast processing path in get/set - add Cc: clause. v4: That series was a follow up of the former "mtd: rawnand: ams-delta: Use gpio-omap accessors for data I/O" which already contained some changes to gpiolib. Those previous attempts were commented by Borris Brezillon who suggested using GPIO API modified to accept bitmaps, and by Linus Walleij who suggested still more great ideas for further immprovement of the proposed API changes - thanks! diffstat: Documentation/driver-api/gpio/board.rst | 15 + Documentation/driver-api/gpio/consumer.rst | 48 +++- drivers/auxdisplay/hd44780.c| 74 +++ drivers/bus/ts-nbus.c | 27 +- drivers/gpio/gpio-max3191x.c| 23 +- drivers/gpio/gpiolib.c | 279 ++-- drivers/gpio/gpiolib.h | 15 + drivers/i2c/muxes/i2c-mux-gpio.c| 16 - drivers/mmc/core/pwrseq_simple.c| 15 - drivers/mux/gpio.c | 18 - drivers/net/phy/mdio-mux-gpio.c | 13 - drivers/pcmcia/soc_common.c | 14 - drivers/phy/motorola/phy-mapphone-mdm6600.c | 21 +- drivers/staging/iio/adc/ad7606.c| 12 - drivers/tty/serial/serial_mctrl_gpio.c |9 include/linux/gpio/consumer.h | 35 ++- 16 files changed, 417 insertions(+), 217 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 4/4] gpiolib: Implement fast processing path in get/set array
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
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/board.rst| 15 ++
Documentation/driver-api/gpio/consumer.rst | 8 +++
drivers/gpio/gpiolib.c | 87 --
3 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
The line will be hogged as soon as the gpiochip is created or - in case the
chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function. The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing. If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output
processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
Note that for optimal performance GPIOs belonging to the same chip should be
contiguous within the array of descriptors.
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip. If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip. That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
The return value of gpiod_get_array_value() and its variants is 0 on success
or negative on error. Note the difference to gpiod_get_value(), which returns
0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d26cdbdb7cf..b799a89c4c17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2787,7 +2787,36 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap)
{
- int i = 0;
+ int err, i = 0;
+
+ /*
+* Validate array_info against desc_array and its size.
+* It should immediately follow desc_array if both
+* have been obtained from the same gpiod_get_array() call.
+*/
+ if (array_info && array_info->desc == desc_array &&
+ array_size <= array_info->size &&
+ (void *)array_info == desc_array + array_info->size) {
+ if (!can_sleep)
+ WARN_ON(array_info->chip->can_sleep);
+
+ err = gpio_chip_get_multiple(array_info->chip,
+array_info->get_mask,
+value_bitmap);
+ if (err)
+ return err;
+
+ if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+ bitmap_xor(value_bitmap, value_bitmap,
+ array_info->invert_mask, array_size);
+
+ if (bitmap_full(array_info->get_mask, array_size))
+ return 0;
+
+ i = find_first_zero_bit(array_info->get_mask, array_size);
+ } else {
+ array_info = NULL;
+ }
while (i < array_size) {
struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2
[PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
Most users of get/set array functions iterate consecutive bits of data, usually a single integer, while processing array of results obtained from, or building an array of values to be passed to those functions. Save time wasted on those iterations by changing the functions' API to accept bitmaps. All current users are updated as well. More benefits from the change are expected as soon as planned support for accepting/passing those bitmaps directly from/to respective GPIO chip callbacks if applicable is implemented. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Dominik Brodowski Cc: Kishon Vijay Abraham I Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 22 drivers/auxdisplay/hd44780.c| 52 + drivers/bus/ts-nbus.c | 19 ++- drivers/gpio/gpio-max3191x.c| 17 +++--- drivers/gpio/gpiolib.c | 86 +++-- drivers/gpio/gpiolib.h | 4 +- drivers/i2c/muxes/i2c-mux-gpio.c| 8 +-- drivers/mmc/core/pwrseq_simple.c| 13 ++--- drivers/mux/gpio.c | 9 +-- drivers/net/phy/mdio-mux-gpio.c | 3 +- drivers/pcmcia/soc_common.c | 11 ++-- drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++--- drivers/staging/iio/adc/ad7606.c| 9 +-- drivers/tty/serial/serial_mctrl_gpio.c | 7 ++- include/linux/gpio/consumer.h | 18 +++--- 15 files changed, 140 insertions(+), 155 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index aa03f389d41d..ed68042ddccf 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access GPIOs belonging to the same bank or chip simultaneously if supported by the @@ -356,8 +356,8 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors - * value_array - an array to store the GPIOs' values (get) or - an array of values to assign to the GPIOs (set) + * value_bitmap - a bitmap to store the GPIOs' values (get) or + a bitmap of values to assign to the GPIOs (set) The descri
[PATCH v5 3/4] gpiolib: Pass array info to get/set array functions
In order to make use of array info obtained from gpiod_get_array() and speed up processing of arrays matching single GPIO chip layout, that information must be passed to get/set array functions. Extend the functions' API with that additional parameter and update all users. Pass NULL if a user bulids an array itself from single GPIOs. Cc: Jonathan Corbet Cc: Miguel Ojeda Sandonis Cc: Peter Korsgaard Cc: Peter Rosin Cc: Andrew Lunn Cc: Florian Fainelli Cc: "David S. Miller" Cc: Dominik Brodowski Cc: Kishon Vijay Abraham I Cc: Lars-Peter Clausen Cc: Michael Hennerich Cc: Jonathan Cameron Cc: Hartmut Knaack Cc: Peter Meerwald-Stadler Cc: Greg Kroah-Hartman Cc: Jiri Slaby Signed-off-by: Janusz Krzysztofik Acked-by: Ulf Hansson --- Documentation/driver-api/gpio/consumer.rst | 14 ++-- drivers/auxdisplay/hd44780.c| 12 ++ drivers/bus/ts-nbus.c | 6 +++-- drivers/gpio/gpio-max3191x.c| 6 +++-- drivers/gpio/gpiolib.c | 34 - drivers/gpio/gpiolib.h | 2 ++ drivers/i2c/muxes/i2c-mux-gpio.c| 2 +- drivers/mmc/core/pwrseq_simple.c| 2 +- drivers/mux/gpio.c | 3 ++- drivers/net/phy/mdio-mux-gpio.c | 2 +- drivers/pcmcia/soc_common.c | 3 ++- drivers/phy/motorola/phy-mapphone-mdm6600.c | 4 +++- drivers/staging/iio/adc/ad7606.c| 3 ++- drivers/tty/serial/serial_mctrl_gpio.c | 2 +- include/linux/gpio/consumer.h | 8 +++ 15 files changed, 75 insertions(+), 28 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index 7e0298b9a7b9..0afd95a12b10 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access @@ -358,6 +366,7 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors + * array_info- optional information obtained from gpiod_array_get() * value_bitmap - a bitmap to store the GPIOs' values (get) or a bitmap of values to assign to the GPIOs (set) @@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array():: struct gpio_descs *my_gpio_descs = gpiod_get_array(...); gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc, - my_g
[PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip. Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion. Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.
Cc: Jonathan Corbet
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/consumer.rst | 4 +-
drivers/gpio/gpiolib.c | 72 +-
drivers/gpio/gpiolib.h | 9
include/linux/gpio/consumer.h | 9
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be
obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0e9ffa8cab6..c1ed1c759345 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
- int count;
+ struct gpio_array *array_info = NULL;
+ struct gpio_chip *chip;
+ int count, bitmap_size;
count = gpiod_count(dev, con_id);
if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
descs->desc[descs->ndescs] = desc;
+
+ chip = gpiod_to_chip(desc);
+ /*
+* Select a chip of first array member
+* whose index matches its pin hardware number
+* as a candidate for fast bitmap processing.
+*/
+ if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ struct gpio_descs *array;
+
+ bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+ chip->ngpio : count);
+
+ array = kzalloc(struct_size(descs, desc, count) +
+ struct_size(array_info, invert_mask,
+ 3 * bitmap_size), GFP_KERNEL);
+ if (!array) {
+ gpiod_put_array(descs);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ memcpy(array, descs,
+ struct_size(descs, desc, descs->ndescs + 1));
+ kfree(descs);
+
+ descs = array;
+ array_info = (void *)(descs->desc + count);
+ array_info->get_mask = array_info->invert_mask +
+ bitmap_size;
+ array_info->set_mask = array_info->get_mask +
+ bitmap_size;
+
+ array_info->desc = descs->desc;
+ array_info->size = count;
+ array_info->chip = chip;
+ bitmap_set(array_info->get_mask, descs->ndescs,
+ count - descs->ndescs);
+ bitmap_set(array_info->set_mask, descs->ndescs,
+ count - descs->ndescs);
+ descs->info = array_info;
+ }
+ /*
+* Unmark members which don't qualify for fast bitmap
+* processing (different chip, not in hardware order)
+*/
+ if (array_info && (chip != array_info->chip ||
+ gpio_chip_hwgpio(desc) !
[PATH v5 0/4] gpiolib: speed up GPIO array processing
The goal is to boost performance of get/set array functions while processing GPIO arrays which represent pins of a signle chip in hardware order. If resulting performance is close to PIO, GPIO API can be used for data I/O without much loss of speed. Created and tested on a low end Amstrad Delta board with NAND driver updated to use GPIO API for data I/O. Performance degrade compared to PIO is much better than before the optimization but still not quite satisfactory. Janusz Krzysztofik (4): gpiolib: Pass bitmaps, not integer arrays, to get/set array gpiolib: Identify arrays matching GPIO hardware gpiolib: Pass array info to get/set array functions gpiolib: Implement fast processing path in get/set array Changelog: v5: [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set - drivers/i2c/muxes/i2c-mux-gpio.c: - drop assigment of values to struct gpiomux.values, as recommended by Peter Rosin - thanks!, - mark the .values member of the structure as obsolete, - drivers/mux/gpio.c: - drop assigment of values to struct mux_gpio.val, also recommended by Peter Rosin - thanks!, - merk the .val member of the structure as obsolete, - drivers/auxdisplay/hd44780.c: - fix incorrect bitmap size, - use >>= operator to simplify notation, both catched by Miguel Ojeda - thanks!, - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 2/4] gpiolib: Identify arrays matching GPIO hardware - add Cc: clause. [PATCH v5 3/4] gpiolib: Pass array info to get/set array functions - add Cc: clauses as well as Acked-by: collected so far. [PATCH v5 4/4] gpiolib: Implement fast processing path in get/set - add Cc: clause. v4: That series was a follow up of the former "mtd: rawnand: ams-delta: Use gpio-omap accessors for data I/O" which already contained some changes to gpiolib. Those previous attempts were commented by Borris Brezillon who suggested using GPIO API modified to accept bitmaps, and by Linus Walleij who suggested still more great ideas for further immprovement of the proposed API changes - thanks! diffstat: Documentation/driver-api/gpio/board.rst | 15 + Documentation/driver-api/gpio/consumer.rst | 48 +++- drivers/auxdisplay/hd44780.c| 64 +++--- drivers/bus/ts-nbus.c | 25 -- drivers/gpio/gpio-max3191x.c| 23 +- drivers/gpio/gpiolib.c | 279 ++-- drivers/gpio/gpiolib.h | 15 + drivers/i2c/muxes/i2c-mux-gpio.c| 10 - drivers/mmc/core/pwrseq_simple.c| 15 - drivers/mux/gpio.c | 12 - drivers/net/phy/mdio-mux-gpio.c |5 drivers/pcmcia/soc_common.c | 14 - drivers/phy/motorola/phy-mapphone-mdm6600.c | 21 +- drivers/staging/iio/adc/ad7606.c| 12 - drivers/tty/serial/serial_mctrl_gpio.c |9 include/linux/gpio/consumer.h | 35 ++- 16 files changed, 412 insertions(+), 190 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC RFT PATCH 0/4] gpiolib: speed up GPIO array processing
Hi Linus, On Wednesday, August 29, 2018 11:06:21 AM CEST Linus Walleij wrote: > On Tue, Aug 21, 2018 at 1:42 AM Janusz Krzysztofik wrote: > > > This series is a follow up of the former "mtd: rawnand: ams-delta: Use > > gpio-omap accessors for data I/O" which already contained some changes > > to gpiolib. Those previous attempts were commented by Borris Brezillon > > who suggested using GPIO API modified to accept bitmaps, and by Linus > > Walleij who suggested still more great ideas for further immprovement > > of the proposed API changes - thanks! > > > > The goal is to boost performans of get/set array functions while > > processing GPIO arrays which represent pins of a signle chip in > > hardware order. If resulting performance is close to PIO, GPIO API > > can be used for data I/O without much loss of speed. > > Hands down, this is a very pretty patch set. I'm a big fan already. > > This is mainly because it fulfills the requirement for libraries > to be narrow and deep, which is what we want. > This refers to John Ousterhouts software design philosophy, > here is a great lecture if you haven't seen it already: > https://www.youtube.com/watch?v=bmSAYlu0NcY > > Let's get this into v1 and get some testing and merge it for v4.20 > ASAP Please hold on for a while, I'm going to resubmit soon, with the comment from Peter Rosin on i2c-mux-gpio addressed and the error discovered by Miguel Ojeda in hd44780 fixed. Thanks, Janusz > so we get some proper testing before the v4.20 merge > window. It would be excellent if some of the current users of > the array API could provide tested-by's or at least ACKs. > > For example ts-nbus.c must be a big benefactor. > > Yours, > Linus Walleij > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
On Wednesday, August 29, 2018 2:03:18 PM CEST Miguel Ojeda wrote:
> Hi Janusz,
>
> On Tue, Aug 21, 2018 at 1:43 AM, Janusz Krzysztofik
> wrote:
> > Most users of get/set array functions iterate consecutive bits of data,
> > usually a single integer, while or processing array of results obtained
> > from or building an array of values to be passed to those functions.
> > Save time wasted on those iterations by changing the functions' API to
> > accept bitmaps.
> >
> > All current users are updated as well.
> >
> > More benefits from the change are expected as soon as planned support
> > for accepting/passing those bitmaps directly from/to respective GPIO
> > chip callbacks if applicable is implemented.
> >
> > Signed-off-by: Janusz Krzysztofik
> > ---
> > Documentation/driver-api/gpio/consumer.rst | 22
> > drivers/auxdisplay/hd44780.c| 52 +
>
> [CC'ing Willy and Geert for hd44780]
>
> > diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
> > index f1a42f0f1ded..d340473aa142 100644
> > --- a/drivers/auxdisplay/hd44780.c
> > +++ b/drivers/auxdisplay/hd44780.c
> > @@ -62,20 +62,19 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
> > /* write to an LCD panel register in 8 bit GPIO mode */
> > static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int
> > rs)
> > {
> > - int values[10]; /* for DATA[0-7], RS, RW */
> > - unsigned int i, n;
> > + unsigned long value_bitmap[1]; /* for DATA[0-7], RS, RW */
>
> Why [1]? I understand it is because in other cases it may be more than
> one,
Yes, I tried to point out the fact the new API accepts a bitmap of an arbitrary
length, and I tried to use the same code pattern across changes to the API
users.
> but...
>
> > + unsigned int n;
> >
> > - for (i = 0; i < 8; i++)
> > - values[PIN_DATA0 + i] = !!(val & BIT(i));
> > - values[PIN_CTRL_RS] = rs;
> > + value_bitmap[0] = val;
> > + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> > n = 9;
> > if (hd->pins[PIN_CTRL_RW]) {
> > - values[PIN_CTRL_RW] = 0;
> > + __clear_bit(PIN_CTRL_RW, value_bitmap);
> > n++;
> > }
> >
> > /* Present the data to the port */
> > - gpiod_set_array_value_cansleep(n, >pins[PIN_DATA0], values);
> > + gpiod_set_array_value_cansleep(n, >pins[PIN_DATA0],
> > value_bitmap);
> >
> > hd44780_strobe_gpio(hd);
> > }
> > @@ -83,32 +82,31 @@ static void hd44780_write_gpio8(struct hd44780 *hd, u8
> > val, unsigned int rs)
> > /* write to an LCD panel register in 4 bit GPIO mode */
> > static void hd44780_write_gpio4(struct hd44780 *hd, u8 val, unsigned int
> > rs)
> > {
> > - int values[10]; /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> > - unsigned int i, n;
> > + /* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
> > + unsigned long value_bitmap[0];
>
> This one is even more strange... :-)
This one is an error, should be 1 of course :-), thanks.
> > + unsigned int n;
> >
> > /* High nibble + RS, RW */
> > - for (i = 4; i < 8; i++)
> > - values[PIN_DATA0 + i] = !!(val & BIT(i));
> > - values[PIN_CTRL_RS] = rs;
> > + value_bitmap[0] = val;
> > + __assign_bit(PIN_CTRL_RS, value_bitmap, rs);
> > n = 5;
> > if (hd->pins[PIN_CTRL_RW]) {
> > - values[PIN_CTRL_RW] = 0;
> > + __clear_bit(PIN_CTRL_RW, value_bitmap);
> > n++;
> > }
> > + value_bitmap[0] = value_bitmap[0] >> PIN_DATA4;
>
> Maybe >>=?
OK.
Answering you question below:
To make my changes as clear as I could imagine, I decided to use the same
indexing as in the original code, i.e., assign high nibble of val to bits 4-7
and two other values - rs and an optional 0 - to bits 8 and 9, respectively.
Unlike in case of array of integers, where for the high nibble part you could
just pass a pointer to a sub-array starting at the 5th value (i.e.,
[PIN_DATA4]), it was not possible to do the same for and arbitrary bit
of a bitmap, e.g., pass a pointer to the 5th bit of *value_bitmap as an
argument pointing to bit 0 of a bitmap to be processed. That's why I shifted
the bitmap right by 4 bits.
Then, ...
> >
> > /* Present the data to the port */
> > - gpiod_set_array_value_can
[RFC RFT PATCH v4 3/4] gpiolib: Pass array info to get/set array functions
In order to make use of array info obtained from gpiod_get_array() and speed up processing of arrays matching single GPIO chip layout, that information must be passed to get/set array functions. Extend the functions' API with that additional parameter and update all users. Pass NULL if a user bulids an array itself from single GPIOs. Signed-off-by: Janusz Krzysztofik --- Documentation/driver-api/gpio/consumer.rst | 14 ++-- drivers/auxdisplay/hd44780.c| 12 ++ drivers/bus/ts-nbus.c | 6 +++-- drivers/gpio/gpio-max3191x.c| 6 +++-- drivers/gpio/gpiolib.c | 34 - drivers/gpio/gpiolib.h | 2 ++ drivers/i2c/muxes/i2c-mux-gpio.c| 2 +- drivers/mmc/core/pwrseq_simple.c| 2 +- drivers/mux/gpio.c | 3 ++- drivers/net/phy/mdio-mux-gpio.c | 2 +- drivers/pcmcia/soc_common.c | 3 ++- drivers/phy/motorola/phy-mapphone-mdm6600.c | 4 +++- drivers/staging/iio/adc/ad7606.c| 3 ++- drivers/tty/serial/serial_mctrl_gpio.c | 2 +- include/linux/gpio/consumer.h | 8 +++ 15 files changed, 75 insertions(+), 28 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index 7e0298b9a7b9..0afd95a12b10 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -325,28 +325,36 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, + struct gpio_array *array_info, unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access @@ -358,6 +366,7 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors + * array_info- optional information obtained from gpiod_array_get() * value_bitmap - a bitmap to store the GPIOs' values (get) or a bitmap of values to assign to the GPIOs (set) @@ -368,12 +377,13 @@ the struct gpio_descs returned by gpiod_get_array():: struct gpio_descs *my_gpio_descs = gpiod_get_array(...); gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs->desc, - my_gpio_value_bitmap); + my_gpio_descs->info, my_gpio_value_bitmap); It is also possible to access a completely arbitrary array of descriptors. The descriptors may be obtained using any combination of gpiod_get() and gpiod_get_array(). Afterwards the array of descriptors has to be setup -manually before it can be passed to one of the above functions.
[RFC RFT PATCH v4 4/4] gpiolib: Implement fast processing path in get/set array
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.
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/board.rst| 15 ++
Documentation/driver-api/gpio/consumer.rst | 8 +++
drivers/gpio/gpiolib.c | 87 --
3 files changed, 105 insertions(+), 5 deletions(-)
diff --git a/Documentation/driver-api/gpio/board.rst
b/Documentation/driver-api/gpio/board.rst
index 2c112553df84..c66821e033c2 100644
--- a/Documentation/driver-api/gpio/board.rst
+++ b/Documentation/driver-api/gpio/board.rst
@@ -193,3 +193,18 @@ And the table can be added to the board code as follows::
The line will be hogged as soon as the gpiochip is created or - in case the
chip was created earlier - when the hog table is registered.
+
+Arrays of pins
+--
+In addition to requesting pins belonging to a function one by one, a device may
+also request an array of pins assigned to the function. The way those pins are
+mapped to the device determines if the array qualifies for fast bitmap
+processing. If yes, a bitmap is passed over get/set array functions directly
+between a caller and a respective .get/set_multiple() callback of a GPIO chip.
+
+In order to qualify for fast bitmap processing, the pin mapping must meet the
+following requirements:
+- it must belong to the same chip as other 'fast' pins of the function,
+- its index within the function must match its hardware number within the chip.
+
+Open drain and open source pins are excluded from fast bitmap output
processing.
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index 0afd95a12b10..cf992e5ab976 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -388,6 +388,14 @@ array_info should be set to NULL.
Note that for optimal performance GPIOs belonging to the same chip should be
contiguous within the array of descriptors.
+Still better performance may be achieved if array indexes of the descriptors
+match hardware pin numbers of a single chip. If an array passed to a get/set
+array function matches the one obtained from gpiod_get_array() and array_info
+associated with the array is also passed, the function may take a fast bitmap
+processing path, passing the value_bitmap argument directly to the respective
+.get/set_multiple() callback of the chip. That allows for utilization of GPIO
+banks as data I/O ports without much loss of performance.
+
The return value of gpiod_get_array_value() and its variants is 0 on success
or negative on error. Note the difference to gpiod_get_value(), which returns
0 or 1 on success to convey the GPIO value. With the array functions, the GPIO
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d26cdbdb7cf..b799a89c4c17 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2787,7 +2787,36 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap)
{
- int i = 0;
+ int err, i = 0;
+
+ /*
+* Validate array_info against desc_array and its size.
+* It should immediately follow desc_array if both
+* have been obtained from the same gpiod_get_array() call.
+*/
+ if (array_info && array_info->desc == desc_array &&
+ array_size <= array_info->size &&
+ (void *)array_info == desc_array + array_info->size) {
+ if (!can_sleep)
+ WARN_ON(array_info->chip->can_sleep);
+
+ err = gpio_chip_get_multiple(array_info->chip,
+array_info->get_mask,
+value_bitmap);
+ if (err)
+ return err;
+
+ if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
+ bitmap_xor(value_bitmap, value_bitmap,
+ array_info->invert_mask, array_size);
+
+ if (bitmap_full(array_info->get_mask, array_size))
+ return 0;
+
+ i = find_first_zero_bit(array_info->get_mask, array_size);
+ } else {
+ array_info = NULL;
+ }
while (i < array_size) {
struct gpio_chip *chip = desc_array[i]->gdev->chip;
@@ -2818,7 +2847,12 @@ int gpiod_g
[RFC RFT PATCH 0/4] gpiolib: speed up GPIO array processing
This series is a follow up of the former "mtd: rawnand: ams-delta: Use gpio-omap accessors for data I/O" which already contained some changes to gpiolib. Those previous attempts were commented by Borris Brezillon who suggested using GPIO API modified to accept bitmaps, and by Linus Walleij who suggested still more great ideas for further immprovement of the proposed API changes - thanks! The goal is to boost performans of get/set array functions while processing GPIO arrays which represent pins of a signle chip in hardware order. If resulting performance is close to PIO, GPIO API can be used for data I/O without much loss of speed. Created and tested on a low end Amstrad Delta board with NAND driver updated to use GPIO API for data I/O. Performance degrade compared to PIO is much better than before the optimization but still not quite satisfactory. Janusz Krzysztofik (4): gpiolib: Pass bitmaps, not integer arrays, to get/set array gpiolib: Identify arrays matching GPIO hardware gpiolib: Pass array info to get/set array functions gpiolib: Implement fast processing path in get/set array Documentation/driver-api/gpio/board.rst | 15 + Documentation/driver-api/gpio/consumer.rst | 48 +++- drivers/auxdisplay/hd44780.c| 64 +++--- drivers/bus/ts-nbus.c | 25 -- drivers/gpio/gpio-max3191x.c| 23 +- drivers/gpio/gpiolib.c | 279 ++-- drivers/gpio/gpiolib.h | 15 + drivers/i2c/muxes/i2c-mux-gpio.c|5 drivers/mmc/core/pwrseq_simple.c| 15 - drivers/mux/gpio.c |7 drivers/net/phy/mdio-mux-gpio.c |5 drivers/pcmcia/soc_common.c | 14 - drivers/phy/motorola/phy-mapphone-mdm6600.c | 21 +- drivers/staging/iio/adc/ad7606.c| 12 - drivers/tty/serial/serial_mctrl_gpio.c |9 include/linux/gpio/consumer.h | 35 ++- 16 files changed, 410 insertions(+), 182 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
Most users of get/set array functions iterate consecutive bits of data, usually a single integer, while or processing array of results obtained from or building an array of values to be passed to those functions. Save time wasted on those iterations by changing the functions' API to accept bitmaps. All current users are updated as well. More benefits from the change are expected as soon as planned support for accepting/passing those bitmaps directly from/to respective GPIO chip callbacks if applicable is implemented. Signed-off-by: Janusz Krzysztofik --- Documentation/driver-api/gpio/consumer.rst | 22 drivers/auxdisplay/hd44780.c| 52 + drivers/bus/ts-nbus.c | 19 ++- drivers/gpio/gpio-max3191x.c| 17 +++--- drivers/gpio/gpiolib.c | 86 +++-- drivers/gpio/gpiolib.h | 4 +- drivers/i2c/muxes/i2c-mux-gpio.c| 3 +- drivers/mmc/core/pwrseq_simple.c| 13 ++--- drivers/mux/gpio.c | 4 +- drivers/net/phy/mdio-mux-gpio.c | 3 +- drivers/pcmcia/soc_common.c | 11 ++-- drivers/phy/motorola/phy-mapphone-mdm6600.c | 17 +++--- drivers/staging/iio/adc/ad7606.c| 9 +-- drivers/tty/serial/serial_mctrl_gpio.c | 7 ++- include/linux/gpio/consumer.h | 18 +++--- 15 files changed, 138 insertions(+), 147 deletions(-) diff --git a/Documentation/driver-api/gpio/consumer.rst b/Documentation/driver-api/gpio/consumer.rst index aa03f389d41d..ed68042ddccf 100644 --- a/Documentation/driver-api/gpio/consumer.rst +++ b/Documentation/driver-api/gpio/consumer.rst @@ -323,29 +323,29 @@ The following functions get or set the values of an array of GPIOs:: int gpiod_get_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); int gpiod_get_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array); + unsigned long *value_bitmap); void gpiod_set_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) void gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_desc **desc_array, - int *value_array) + unsigned long *value_bitmap) The array can be an arbitrary set of GPIOs. The functions will try to access GPIOs belonging to the same bank or chip simultaneously if supported by the @@ -356,8 +356,8 @@ accessed sequentially. The functions take three arguments: * array_size- the number of array elements * desc_array- an array of GPIO descriptors - * value_array - an array to store the GPIOs' values (get) or - an array of values to assign to the GPIOs (set) + * value_bitmap - a bitmap to store the GPIOs' values (get) or + a bitmap of values to assign to the GPIOs (set) The descriptor array can be obtained using the gpiod_get_array() function or one of its variants. If the group of descriptors returned by that function @@ -366,7 +366,7 @@ the struct gpio_descs returned by gpiod_get_array():: struct gpio_descs *my_gpio_descs = gpiod_get_array(...); gpiod_set_array_value(my_gpio_descs->ndescs, my_gpio_descs-&g
[RFC RFT PATCH v4 2/4] gpiolib: Identify arrays matching GPIO hardware
Certain GPIO array lookup results may map directly to GPIO pins of a
single GPIO chip in hardware order. If that condition is recognized
and handled efficiently, significant performance gain of get/set array
functions may be possible.
While processing a request for an array of GPIO descriptors, identify
those which represent corresponding pins of a single GPIO chip. Skip
over pins which require open source or open drain special processing.
Moreover, identify pins which require inversion. Pass a pointer to
that information with the array to the caller so it can benefit from
enhanced performance as soon as get/set array functions can accept and
make efficient use of it.
Signed-off-by: Janusz Krzysztofik
---
Documentation/driver-api/gpio/consumer.rst | 4 +-
drivers/gpio/gpiolib.c | 72 +-
drivers/gpio/gpiolib.h | 9
include/linux/gpio/consumer.h | 9
4 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/Documentation/driver-api/gpio/consumer.rst
b/Documentation/driver-api/gpio/consumer.rst
index ed68042ddccf..7e0298b9a7b9 100644
--- a/Documentation/driver-api/gpio/consumer.rst
+++ b/Documentation/driver-api/gpio/consumer.rst
@@ -109,9 +109,11 @@ For a function using multiple GPIOs all of those can be
obtained with one call::
enum gpiod_flags flags)
This function returns a struct gpio_descs which contains an array of
-descriptors::
+descriptors. It also contains a pointer to a gpiolib private structure which,
+if passed back to get/set array functions, may speed up I/O proocessing::
struct gpio_descs {
+ struct gpio_array *info;
unsigned int ndescs;
struct gpio_desc *desc[];
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f0e9ffa8cab6..c1ed1c759345 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4174,7 +4174,9 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
{
struct gpio_desc *desc;
struct gpio_descs *descs;
- int count;
+ struct gpio_array *array_info = NULL;
+ struct gpio_chip *chip;
+ int count, bitmap_size;
count = gpiod_count(dev, con_id);
if (count < 0)
@@ -4190,9 +4192,77 @@ struct gpio_descs *__must_check gpiod_get_array(struct
device *dev,
gpiod_put_array(descs);
return ERR_CAST(desc);
}
+
descs->desc[descs->ndescs] = desc;
+
+ chip = gpiod_to_chip(desc);
+ /*
+* Select a chip of first array member
+* whose index matches its pin hardware number
+* as a candidate for fast bitmap processing.
+*/
+ if (!array_info && gpio_chip_hwgpio(desc) == descs->ndescs) {
+ struct gpio_descs *array;
+
+ bitmap_size = BITS_TO_LONGS(chip->ngpio > count ?
+ chip->ngpio : count);
+
+ array = kzalloc(struct_size(descs, desc, count) +
+ struct_size(array_info, invert_mask,
+ 3 * bitmap_size), GFP_KERNEL);
+ if (!array) {
+ gpiod_put_array(descs);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ memcpy(array, descs,
+ struct_size(descs, desc, descs->ndescs + 1));
+ kfree(descs);
+
+ descs = array;
+ array_info = (void *)(descs->desc + count);
+ array_info->get_mask = array_info->invert_mask +
+ bitmap_size;
+ array_info->set_mask = array_info->get_mask +
+ bitmap_size;
+
+ array_info->desc = descs->desc;
+ array_info->size = count;
+ array_info->chip = chip;
+ bitmap_set(array_info->get_mask, descs->ndescs,
+ count - descs->ndescs);
+ bitmap_set(array_info->set_mask, descs->ndescs,
+ count - descs->ndescs);
+ descs->info = array_info;
+ }
+ /*
+* Unmark members which don't qualify for fast bitmap
+* processing (different chip, not in hardware order)
+*/
+ if (array_info && (chip != array_info->chip ||
+ gpio_chip_hwgpio(desc) != descs->ndescs)) {
+ __cle
[RFC] [PATCH 3/3] staging: media: omap1: use dmaengine
Created and tested on Amstrad Delta on top of Linux-4.7-rc3 with
"staging: media: omap1: convert to videobuf2" applied.
Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com>
---
drivers/staging/media/omap1/Kconfig| 2 +-
drivers/staging/media/omap1/omap1_camera.c | 432 +
2 files changed, 135 insertions(+), 299 deletions(-)
diff --git a/drivers/staging/media/omap1/Kconfig
b/drivers/staging/media/omap1/Kconfig
index 12f1d7a..0b8456d 100644
--- a/drivers/staging/media/omap1/Kconfig
+++ b/drivers/staging/media/omap1/Kconfig
@@ -1,7 +1,7 @@
config VIDEO_OMAP1
tristate "OMAP1 Camera Interface driver"
depends on VIDEO_DEV && SOC_CAMERA
- depends on ARCH_OMAP1
+ depends on ARCH_OMAP1 && DMA_OMAP
depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
---help---
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 3761660..e22ba8a 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -33,11 +33,12 @@
#include
#include
-#include
+#include
+#include
#define DRIVER_NAME"omap1-camera"
-#define DRIVER_VERSION "0.0.4"
+#define DRIVER_VERSION "0.0.5"
#define OMAP_DMA_CAMERA_IF_RX 20
@@ -115,8 +116,8 @@
#define DMA_BURST_SHIFT(1 + OMAP_DMA_DATA_BURST_4)
#define DMA_BURST_SIZE BIT(DMA_BURST_SHIFT)
-#define DMA_ELEMENT_SHIFT OMAP_DMA_DATA_TYPE_S32
-#define DMA_ELEMENT_SIZE BIT(DMA_ELEMENT_SHIFT)
+#define DMA_ELEMENT_SIZE DMA_SLAVE_BUSWIDTH_4_BYTES
+#define DMA_ELEMENT_SHIFT __fls(DMA_ELEMENT_SIZE)
#define DMA_FRAME_SHIFT(FIFO_SHIFT - 1)
#define DMA_FRAME_SIZE BIT(DMA_FRAME_SHIFT)
@@ -124,7 +125,7 @@
#define THRESHOLD_LEVELDMA_FRAME_SIZE
#define OMAP1_CAMERA_MIN_BUF_COUNT \
- 3
+ 2
#define MAX_VIDEO_MEM 4 /* arbitrary video memory limit in MB */
@@ -145,7 +146,8 @@ struct omap1_cam_dev {
unsigned intirq;
void __iomem*base;
- int dma_ch;
+ struct dma_chan *dma_chan;
+ unsigned intdma_rq;
struct omap1_cam_platform_data *pdata;
unsigned long pflags;
@@ -156,10 +158,6 @@ struct omap1_cam_dev {
/* lock used to protect videobuf */
spinlock_t lock;
- /* Pointers to DMA buffers */
- struct omap1_cam_buf*active;
- struct omap1_cam_buf*ready;
-
struct vb2_alloc_ctx*alloc_ctx;
int sequence;
@@ -222,6 +220,16 @@ static int omap1_videobuf_setup(struct vb2_queue *vq,
unsigned int *count,
return 0;
}
+static int omap1_videobuf_init(struct vb2_buffer *vb)
+{
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+ struct omap1_cam_buf *buf = vb2_to_omap1_cam_buf(vbuf);
+
+ INIT_LIST_HEAD(>queue);
+
+ return 0;
+}
+
static int omap1_videobuf_prepare(struct vb2_buffer *vb)
{
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -236,96 +244,27 @@ static int omap1_videobuf_prepare(struct vb2_buffer *vb)
vb2_plane_size(vb, 0), size);
return -ENOBUFS;
}
-
vb2_set_plane_payload(vb, 0, size);
return 0;
}
-static void set_dma_dest_params(int dma_ch, struct omap1_cam_buf *buf)
-{
- dma_addr_t dma_addr =
- vb2_dma_contig_plane_dma_addr(>vb.vb2_buf, 0);
- unsigned int block_size = vb2_plane_size(>vb.vb2_buf, 0);
-
- omap_set_dma_dest_params(dma_ch,
- OMAP_DMA_PORT_EMIFF, OMAP_DMA_AMODE_POST_INC, dma_addr, 0, 0);
- omap_set_dma_transfer_params(dma_ch,
- OMAP_DMA_DATA_TYPE_S32, DMA_FRAME_SIZE,
- block_size >> (DMA_FRAME_SHIFT + DMA_ELEMENT_SHIFT),
- DMA_SYNC, 0, 0);
-}
-
-static struct omap1_cam_buf *prepare_next_vb(struct omap1_cam_dev *pcdev)
-{
- struct omap1_cam_buf *buf;
-
- /*
-* If there is already a buffer pointed out by the pcdev->ready,
-* (re)use it, otherwise try to fetch and configure a new one.
-*/
- buf = pcdev->ready;
- if (!buf) {
- if (list_empty(>capture))
- return buf;
- buf = list_entry(pcdev->capture.next,
- struct omap1_cam_buf, queue);
- pcdev->ready = buf;
- list_del_init(>queue);
- }
-
- /*
-* In CONTIG mode, we can safely enter next buffer parameters
-* into th
[RFC] [PATCH 2/3] staging: media: omap1: convert to videobuf2
Created and tested on Amstrad Delta on top of Linux-4.7-rc3 with
"staging: media: omap1: drop videobuf-dma-sg mode" applied.
Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com>
---
drivers/staging/media/omap1/Kconfig| 2 +-
drivers/staging/media/omap1/omap1_camera.c | 363 -
2 files changed, 151 insertions(+), 214 deletions(-)
diff --git a/drivers/staging/media/omap1/Kconfig
b/drivers/staging/media/omap1/Kconfig
index e2a39f5..12f1d7a 100644
--- a/drivers/staging/media/omap1/Kconfig
+++ b/drivers/staging/media/omap1/Kconfig
@@ -3,7 +3,7 @@ config VIDEO_OMAP1
depends on VIDEO_DEV && SOC_CAMERA
depends on ARCH_OMAP1
depends on HAS_DMA
- select VIDEOBUF_DMA_CONTIG
+ select VIDEOBUF2_DMA_CONTIG
---help---
This is a v4l2 driver for the TI OMAP1 camera interface
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 37ef4da..3761660 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -1,7 +1,7 @@
/*
* V4L2 SoC Camera driver for OMAP1 Camera Interface
*
- * Copyright (C) 2010, Janusz Krzysztofik <jkrzy...@tis.icnet.pl>
+ * Copyright (C) 2010, 2016 Janusz Krzysztofik <jmkrzy...@gmail.com>
*
* Based on V4L2 Driver for i.MXL/i.MXL camera (CSI) host
* Copyright (C) 2008, Paulius Zaleckas <paulius.zalec...@teltonika.lt>
@@ -31,13 +31,13 @@
#include
#include
#include
-#include
+#include
#include
#define DRIVER_NAME"omap1-camera"
-#define DRIVER_VERSION "0.0.3"
+#define DRIVER_VERSION "0.0.4"
#define OMAP_DMA_CAMERA_IF_RX 20
@@ -134,9 +134,8 @@
/* buffer for one video frame */
struct omap1_cam_buf {
- struct videobuf_buffer vb;
- u32 code;
- int inwork;
+ struct vb2_v4l2_buffer vb;
+ struct list_headqueue;
};
struct omap1_cam_dev {
@@ -161,10 +160,18 @@ struct omap1_cam_dev {
struct omap1_cam_buf*active;
struct omap1_cam_buf*ready;
+ struct vb2_alloc_ctx*alloc_ctx;
+ int sequence;
+
u32 reg_cache[0];
};
+static struct omap1_cam_buf *vb2_to_omap1_cam_buf(struct vb2_v4l2_buffer *vbuf)
+{
+ return container_of(vbuf, struct omap1_cam_buf, vb);
+}
+
static void cam_write(struct omap1_cam_dev *pcdev, u16 reg, u32 val)
{
pcdev->reg_cache[reg / sizeof(u32)] = val;
@@ -187,92 +194,59 @@ static u32 cam_read(struct omap1_cam_dev *pcdev, u16 reg,
bool from_cache)
/*
* Videobuf operations
*/
-static int omap1_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
- unsigned int *size)
+
+static int omap1_videobuf_setup(struct vb2_queue *vq, unsigned int *count,
+ unsigned int *num_planes, unsigned int sizes[],
+ void *alloc_ctxs[])
{
- struct soc_camera_device *icd = vq->priv_data;
+ struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+ struct omap1_cam_dev *pcdev = ici->priv;
+ unsigned int size = icd->sizeimage;
+
+ pcdev->sequence = 0;
- *size = icd->sizeimage;
+ *num_planes = 1;
+ sizes[0] = size;
+ alloc_ctxs[0] = pcdev->alloc_ctx;
if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT)
*count = OMAP1_CAMERA_MIN_BUF_COUNT;
- if (*size * *count > MAX_VIDEO_MEM * 1024 * 1024)
- *count = (MAX_VIDEO_MEM * 1024 * 1024) / *size;
+ if (size * *count > MAX_VIDEO_MEM * 1024 * 1024)
+ *count = (MAX_VIDEO_MEM * 1024 * 1024) / size;
dev_dbg(icd->parent,
- "%s: count=%d, size=%d\n", __func__, *count, *size);
+ "%s: count=%u, size=%u\n", __func__, *count, size);
return 0;
}
-static void free_buffer(struct videobuf_queue *vq, struct omap1_cam_buf *buf)
-{
- struct videobuf_buffer *vb = >vb;
-
- BUG_ON(in_interrupt());
-
- videobuf_waiton(vq, vb, 0, 0);
-
- videobuf_dma_contig_free(vq, vb);
-
- vb->state = VIDEOBUF_NEEDS_INIT;
-}
-
-static int omap1_videobuf_prepare(struct videobuf_queue *vq,
- struct videobuf_buffer *vb, enum v4l2_field field)
+static int omap1_videobuf_prepare(struct vb2_buffer *vb)
{
- struct soc_camera_device *icd = vq->priv_data;
- struct omap1_cam_buf *buf = container_of(vb, struct omap1_cam_buf, vb);
- int ret;
-
- WARN_ON(!list_empty(>queue));
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb)
[RFC] [PATCH 1/3] staging: media: omap1: drop videobuf-dma-sg mode
For over 20 last kernel versions the driver has been able to allocate
DMA buffers in videobuf-dma-contig mode without any issues. Drop the
no longer needed sg mode in preparation for conversion to videobuf2.
Created and tested on Amstrad Delta against Linux-4.7-rc3 with
omap1_camera and ov6650 fixes applied.
Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com>
---
drivers/staging/media/omap1/Kconfig | 1 -
drivers/staging/media/omap1/omap1_camera.c | 445 ---
include/linux/platform_data/media/omap1_camera.h | 9 -
3 files changed, 77 insertions(+), 378 deletions(-)
diff --git a/drivers/staging/media/omap1/Kconfig
b/drivers/staging/media/omap1/Kconfig
index 6cfab3a..e2a39f5 100644
--- a/drivers/staging/media/omap1/Kconfig
+++ b/drivers/staging/media/omap1/Kconfig
@@ -4,7 +4,6 @@ config VIDEO_OMAP1
depends on ARCH_OMAP1
depends on HAS_DMA
select VIDEOBUF_DMA_CONTIG
- select VIDEOBUF_DMA_SG
---help---
This is a v4l2 driver for the TI OMAP1 camera interface
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 9b6140a..37ef4da 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -32,13 +32,12 @@
#include
#include
#include
-#include
#include
#define DRIVER_NAME"omap1-camera"
-#define DRIVER_VERSION "0.0.2"
+#define DRIVER_VERSION "0.0.3"
#define OMAP_DMA_CAMERA_IF_RX 20
@@ -114,22 +113,18 @@
#define FIFO_SHIFT __fls(FIFO_SIZE)
#define DMA_BURST_SHIFT(1 + OMAP_DMA_DATA_BURST_4)
-#define DMA_BURST_SIZE (1 << DMA_BURST_SHIFT)
+#define DMA_BURST_SIZE BIT(DMA_BURST_SHIFT)
#define DMA_ELEMENT_SHIFT OMAP_DMA_DATA_TYPE_S32
-#define DMA_ELEMENT_SIZE (1 << DMA_ELEMENT_SHIFT)
+#define DMA_ELEMENT_SIZE BIT(DMA_ELEMENT_SHIFT)
-#define DMA_FRAME_SHIFT_CONTIG (FIFO_SHIFT - 1)
-#define DMA_FRAME_SHIFT_SG DMA_BURST_SHIFT
-
-#define DMA_FRAME_SHIFT(x) ((x) == OMAP1_CAM_DMA_CONTIG ? \
- DMA_FRAME_SHIFT_CONTIG : \
- DMA_FRAME_SHIFT_SG)
-#define DMA_FRAME_SIZE(x) (1 << DMA_FRAME_SHIFT(x))
+#define DMA_FRAME_SHIFT(FIFO_SHIFT - 1)
+#define DMA_FRAME_SIZE BIT(DMA_FRAME_SHIFT)
#define DMA_SYNC OMAP_DMA_SYNC_FRAME
#define THRESHOLD_LEVELDMA_FRAME_SIZE
-
+#define OMAP1_CAMERA_MIN_BUF_COUNT \
+ 3
#define MAX_VIDEO_MEM 4 /* arbitrary video memory limit in MB */
@@ -140,12 +135,8 @@
/* buffer for one video frame */
struct omap1_cam_buf {
struct videobuf_buffer vb;
- u32 code;
+ u32 code;
int inwork;
- struct scatterlist *sgbuf;
- int sgcount;
- int bytes_left;
- enum videobuf_state result;
};
struct omap1_cam_dev {
@@ -170,11 +161,6 @@ struct omap1_cam_dev {
struct omap1_cam_buf*active;
struct omap1_cam_buf*ready;
- enum omap1_cam_vb_mode vb_mode;
- int (*mmap_mapper)(struct videobuf_queue *q,
- struct videobuf_buffer *buf,
- struct vm_area_struct *vma);
-
u32 reg_cache[0];
};
@@ -205,13 +191,11 @@ static int omap1_videobuf_setup(struct videobuf_queue
*vq, unsigned int *count,
unsigned int *size)
{
struct soc_camera_device *icd = vq->priv_data;
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
- struct omap1_cam_dev *pcdev = ici->priv;
*size = icd->sizeimage;
- if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT(pcdev->vb_mode))
- *count = OMAP1_CAMERA_MIN_BUF_COUNT(pcdev->vb_mode);
+ if (!*count || *count < OMAP1_CAMERA_MIN_BUF_COUNT)
+ *count = OMAP1_CAMERA_MIN_BUF_COUNT;
if (*size * *count > MAX_VIDEO_MEM * 1024 * 1024)
*count = (MAX_VIDEO_MEM * 1024 * 1024) / *size;
@@ -222,8 +206,7 @@ static int omap1_videobuf_setup(struct videobuf_queue *vq,
unsigned int *count,
return 0;
}
-static void free_buffer(struct videobuf_queue *vq, struct omap1_cam_buf *buf,
- enum omap1_cam_vb_mode vb_mode)
+static void free_buffer(struct videobuf_queue *vq, struct omap1_cam_buf *buf)
{
struct videobuf_buffer *vb = >vb;
@@ -231,16 +214,7 @@ static void free_buffer(struct videobuf_queue *vq, struct
omap1_cam_buf *buf,
[RFC] [PATCH 0/3] media: an attempt to refresh omap1_camera driver
As requested by media subsystem maintainers, here is an attempt to convert the omap1_camera driver to the vb2 framework. Also, conversion to the dmaengine framework, long awaited by ARM/OMAP maintainers, is done. Next, I'm going to approach removal of soc-camera dependency. Please let me know how much time I have for that, i.e., when the soc-camera framework is going to be depreciated. Thanks, Janusz Janusz Krzysztofik (3): staging: media: omap1: drop videobuf-dma-sg mode staging: media: omap1: convert to videobuf2 staging: media: omap1: use dmaengine drivers/staging/media/omap1/Kconfig | 5 +- drivers/staging/media/omap1/omap1_camera.c | 948 +-- include/linux/platform_data/media/omap1_camera.h | 9 - 3 files changed, 217 insertions(+), 745 deletions(-) -- 2.7.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] media: i2c/soc_camera: fix ov6650 sensor getting wrong clock
After changes to v4l2_clk API introduced in v4.1 by commits a37462b919
'[media] V4L: remove clock name from v4l2_clk API' and 4f528afcfb
'[media] V4L: add CCF support to the v4l2_clk API', ov6650 sensor
stopped responding because v4l2_clk_get(), still called with
depreciated V4L2 clock name "mclk", started to return respective CCF
clock instead of the V4l2 one registered by soc_camera. Fix it by
calling v4l2_clk_get() with NULL clock name.
Created and tested on Amstrad Delta against Linux-4.7-rc3 with
omap1_camera fixes.
Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com>
---
drivers/media/i2c/soc_camera/ov6650.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/i2c/soc_camera/ov6650.c
b/drivers/media/i2c/soc_camera/ov6650.c
index 1f8af1e..1e4783b 100644
--- a/drivers/media/i2c/soc_camera/ov6650.c
+++ b/drivers/media/i2c/soc_camera/ov6650.c
@@ -1033,7 +1033,7 @@ static int ov6650_probe(struct i2c_client *client,
priv->code= MEDIA_BUS_FMT_YUYV8_2X8;
priv->colorspace = V4L2_COLORSPACE_JPEG;
- priv->clk = v4l2_clk_get(>dev, "mclk");
+ priv->clk = v4l2_clk_get(>dev, NULL);
if (IS_ERR(priv->clk)) {
ret = PTR_ERR(priv->clk);
goto eclkget;
--
2.7.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: media: omap1: fix sensor probe not working anymore
After clock_start() removal from from soc_camera_probe() (commit
9aea470b39 '[media] soc-camera: switch I2C subdevice drivers to use
v4l2-clk', introduced in v3.11), it occurred omap1_camera's sensor
can't be probed successfully without its clock being turned on in
advance. Fix that by surrounding soc_camera_host_register() invocation
with clock_start() / clock_stop().
Created and tested on Amstrad Delta against Linux-4.7-rc3 with
'staging: media: omap1: fix null pointer dereference in
omap1_cam_probe()' applied.
Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com>
---
drivers/staging/media/omap1/omap1_camera.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index dc35d30..9b6140a 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -1650,7 +1650,11 @@ static int omap1_cam_probe(struct platform_device *pdev)
pcdev->soc_host.v4l2_dev.dev= >dev;
pcdev->soc_host.nr = pdev->id;
- err = soc_camera_host_register(>soc_host);
+ err = omap1_cam_clock_start(>soc_host);
+ if (!err) {
+ err = soc_camera_host_register(>soc_host);
+ omap1_cam_clock_stop(>soc_host);
+ }
if (err)
return err;
--
2.7.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: media: omap1: fix null pointer dereference in omap1_cam_probe()
Commit 76e543382bd4 ("staging: media: omap1: Switch to
devm_ioremap_resource") moved assignment of struct resource *res =
platform_get_resource() several lines down. That resulted in the
following error:
[3.793237] Unable to handle kernel NULL pointer dereference at virtual
address 0004
[3.802198] pgd = c0004000
[3.805202] [0004] *pgd=
[3.809373] Internal error: Oops: c5 [#1] ARM
[3.814070] CPU: 0 PID: 1 Comm: swapper Not tainted 4.6.0-rc1+ #70
[3.820570] Hardware name: Amstrad E3 (Delta)
[3.825232] task: c1819440 ti: c181e000 task.ti: c181e000
[3.830973] PC is at omap1_cam_probe+0x48/0x2d4
[3.835873] LR is at devres_add+0x20/0x28
Move the assignment back up where it was before - it is used to build
an argument for a subsequent devm_kzalloc(). Also, restore the check
for null value of res - it shouldn't hurt.
While being at it:
- follow the recently introduced convention of direct return
instead of jump to return with err value assigned,
- drop no longer needed res member from the definition of struct
omap1_cam_dev.
Created and tested on Amstrad Delta aginst Linux-4.7-rc3
Signed-off-by: Janusz Krzysztofik <jmkrzy...@gmail.com>
---
drivers/staging/media/omap1/omap1_camera.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/omap1/omap1_camera.c
b/drivers/staging/media/omap1/omap1_camera.c
index 54b8dd2..dc35d30 100644
--- a/drivers/staging/media/omap1/omap1_camera.c
+++ b/drivers/staging/media/omap1/omap1_camera.c
@@ -158,7 +158,6 @@ struct omap1_cam_dev {
int dma_ch;
struct omap1_cam_platform_data *pdata;
- struct resource *res;
unsigned long pflags;
unsigned long camexclk;
@@ -1569,11 +1568,10 @@ static int omap1_cam_probe(struct platform_device *pdev)
unsigned int irq;
int err = 0;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(pdev, 0);
- if ((int)irq <= 0) {
- err = -ENODEV;
- goto exit;
- }
+ if (!res || (int)irq <= 0)
+ return -ENODEV;
clk = devm_clk_get(>dev, "armper_ck");
if (IS_ERR(clk))
@@ -1614,7 +1612,6 @@ static int omap1_cam_probe(struct platform_device *pdev)
INIT_LIST_HEAD(>capture);
spin_lock_init(>lock);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(>dev, res);
if (IS_ERR(base))
return PTR_ERR(base);
@@ -1663,7 +1660,6 @@ static int omap1_cam_probe(struct platform_device *pdev)
exit_free_dma:
omap_free_dma(pcdev->dma_ch);
-exit:
return err;
}
--
2.7.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] media: fixes for Amstrad Delta camera
Janusz Krzysztofik (3): staging: media: omap1: fix null pointer dereference in omap1_cam_probe() staging: media: omap1: fix sensor probe not working anymore media: i2c/soc_camera: fix ov6650 sensor getting wrong clock drivers/media/i2c/soc_camera/ov6650.c | 2 +- drivers/staging/media/omap1/omap1_camera.c | 16 2 files changed, 9 insertions(+), 9 deletions(-) -- 2.7.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
