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, &hd->pins[PIN_DATA4],
> > - &values[PIN_DATA4]);
> > + gpiod_set_array_value_cansleep(n, &hd->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 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
[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) != descs->ndescs)) {
+ __clear_bit(descs->ndescs, array_info->get_mask);
+ __cle
[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():: struct gpio_de
[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 the GPIOs (se
[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;
@@ -2820,7 +2849,12 @@ int gpiod_get_array_value_complex(bool raw, bool
can_sleep,
Re: [PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
On Sun, Sep 02, 2018 at 02:01:41PM +0200, Janusz Krzysztofik wrote: > @@ -461,7 +461,7 @@ static long linehandle_ioctl(struct file *filep, unsigned > int cmd, > > /* Clamp all values to [0,1] */ > for (i = 0; i < lh->numdescs; i++) > - vals[i] = !!ghd.values[i]; > + __assign_bit(i, vals, !!ghd.values[i]); The "!!" becomes unnecessary and can be removed, same for the code comment above. > /** > * gpiod_get_array_value() - read values from an array of GPIOs > - * @array_size: number of elements in the descriptor / value arrays > + * @array_size: number of elements in the descriptor array / value bitmap > * @desc_array: array of GPIO descriptors whose values will be read > - * @value_array: array to store the read values > + * @value_bitnap: bitmap to store the read values Typo, s/bitnap/bitmap/ Otherwise LGTM. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: SKylake Edac support in linux 2.6.32.696 kernel build
On Sun, Sep 02, 2018 at 03:45:01PM +0530, Nitin Gupta wrote: > Hi Greg > > i am working on some project which is having huge changes and working > on kernel-2.6.32-131.696 > it will be very difficult to back port them into 4.8 . Why 4.8? That too is a totally unsupported kernel version. Anyway, randomly pulling in a driver from a newer by many years, if not almost a decade old (2.6.32 was released in 2009), is a very difficult task, and usually almost impossible to go that far back. That is not how Linux is developed or meant to be worked with at all. You are _really_ on your own here, only do this if you _really_ know what you are doing. Even then, I would not recommend you do this, as you are guaranteeing to create a monstrosity that only you can support, for forever, on your own. good luck! greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c
On 2018/8/30 23:13, Pavel Zemlyanoy wrote: > > This patch does not change the logic, it only > corrects the formatting and checkpatch warnings by > adding "int" to the unsigned type. > > The patch fixes 11 warnings of the type: > "WARNING: Prefer 'unsigned int' to bare use of 'unsigned'" > > Signed-off-by: Pavel Zemlyanoy Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison
On 2018/8/30 23:13, Pavel Zemlyanoy wrote: > This patch does not change the logic, it only > corrects the formatting and checkpatch checks by > to NULL comparison. > > The patch fixes 5 checks of type: > "Comparison to NULL could be written". > > Signed-off-by: Pavel Zemlyanoy Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] staging: erofs: formatting spaces around '-'
On 2018/8/30 23:13, Pavel Zemlyanoy wrote: > This patch does not change the logic, it only > corrects the formatting and checkpatch checks by > adding spaces around '-'. > > The patch fixes 4 checks of type: > "Check: spaces preferred around that '-'". > > Signed-off-by: Pavel Zemlyanoy Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: erofs: formatting alignment parenthesis
On 2018/8/30 23:14, Pavel Zemlyanoy wrote: > This patch does not change the logic, it only > corrects the formatting and checkpatch check by > alignment should match open parenthesis. > > The patch fixes 2 check of type: > "Check: Alignment should match open parenthesis". > > Signed-off-by: Pavel Zemlyanoy Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: erofs: formatting add spaces arround '*'
On 2018/8/30 23:14, Pavel Zemlyanoy wrote: > This patch does not change the logic, it only > corrects the formatting and checkpatch check by > adding spaces around '*'. > > The patch fixes 1 check of type: > "Check: spaces preferred around that '*'". > > Signed-off-by: Pavel Zemlyanoy Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: erofs: fix 1 warning and 9 checks
On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch checks by
> braces {} should be used on all arms of this statement,
> unbalanced braces around else statement and warning by
> braces {} are not necessary for any arm of this statement.
>
> The patch fixes 9 checks of type:
> "Check: braces {} should be used on all arms of this statement";
> "Check: Unbalanced braces around else statement";
>
> and 1 warning of type:
> "WARNING: braces {} are not necessary for any arm of this statement".
>
> Signed-off-by: Pavel Zemlyanoy
Reviewed-by: Chao Yu
Thanks,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: erofs: use explicit unsigned int type
Hi,
It looks like there is another patch from Pavel Zemlyanoy changing the same
place, I think it needs to rebase this patch on that one.
[PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c
On 2018/8/31 4:56, Thomas Weißschuh wrote:
> Signed-off-by: Thomas Weißschuh
> ---
> drivers/staging/erofs/data.c | 4 +--
> drivers/staging/erofs/dir.c | 14
> drivers/staging/erofs/inode.c | 6 ++--
> drivers/staging/erofs/namei.c | 28
> drivers/staging/erofs/super.c | 2 +-
> drivers/staging/erofs/unzip_vle.c | 48 +--
> drivers/staging/erofs/unzip_vle_lz4.c | 22 ++--
> drivers/staging/erofs/utils.c | 2 +-
> drivers/staging/erofs/xattr.c | 40 +++---
> 9 files changed, 83 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index 3c0d9159514e..e1916101ad75 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -202,7 +202,7 @@ static inline struct bio *erofs_read_raw_page(
> struct address_space *mapping,
> struct page *page,
> erofs_off_t *last_block,
> - unsigned nblocks,
> + unsigned int nblocks,
> bool ra)
> {
> struct inode *inode = mapping->host;
> @@ -236,7 +236,7 @@ static inline struct bio *erofs_read_raw_page(
> .m_la = blknr_to_addr(current_block),
> };
> erofs_blk_t blknr;
> - unsigned blkoff;
> + unsigned int blkoff;
>
> err = erofs_map_blocks(inode, &map, EROFS_GET_BLOCKS_RAW);
> if (unlikely(err))
> diff --git a/drivers/staging/erofs/dir.c b/drivers/staging/erofs/dir.c
> index be6ae3b1bdbe..87f12b0f983f 100644
> --- a/drivers/staging/erofs/dir.c
> +++ b/drivers/staging/erofs/dir.c
> @@ -24,8 +24,8 @@ static const unsigned char
> erofs_filetype_table[EROFS_FT_MAX] = {
> };
>
> static int erofs_fill_dentries(struct dir_context *ctx,
> - void *dentry_blk, unsigned *ofs,
> - unsigned nameoff, unsigned maxsize)
> + void *dentry_blk, unsigned int *ofs,
> + unsigned int nameoff, unsigned int maxsize)
> {
> struct erofs_dirent *de = dentry_blk;
> const struct erofs_dirent *end = dentry_blk + nameoff;
> @@ -36,7 +36,7 @@ static int erofs_fill_dentries(struct dir_context *ctx,
> int de_namelen;
> unsigned char d_type;
> #ifdef CONFIG_EROFS_FS_DEBUG
> - unsigned dbg_namelen;
> + unsigned int dbg_namelen;
> unsigned char dbg_namebuf[EROFS_NAME_LEN];
> #endif
>
> @@ -81,15 +81,15 @@ static int erofs_readdir(struct file *f, struct
> dir_context *ctx)
> struct inode *dir = file_inode(f);
> struct address_space *mapping = dir->i_mapping;
> const size_t dirsize = i_size_read(dir);
> - unsigned i = ctx->pos / EROFS_BLKSIZ;
> - unsigned ofs = ctx->pos % EROFS_BLKSIZ;
> + unsigned int i = ctx->pos / EROFS_BLKSIZ;
> + unsigned int ofs = ctx->pos % EROFS_BLKSIZ;
> int err = 0;
> bool initial = true;
>
> while (ctx->pos < dirsize) {
> struct page *dentry_page;
> struct erofs_dirent *de;
> - unsigned nameoff, maxsize;
> + unsigned int nameoff, maxsize;
>
> dentry_page = read_mapping_page(mapping, i, NULL);
> if (IS_ERR(dentry_page))
> @@ -109,7 +109,7 @@ static int erofs_readdir(struct file *f, struct
> dir_context *ctx)
> goto skip_this;
> }
>
> - maxsize = min_t(unsigned, dirsize - ctx->pos + ofs, PAGE_SIZE);
> + maxsize = min_t(unsigned int, dirsize - ctx->pos + ofs,
> PAGE_SIZE);
>
> /* search dirents at the arbitrary position */
> if (unlikely(initial)) {
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index fbf6ff25cd1b..70d34b0a97fe 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -19,7 +19,7 @@ static int read_inode(struct inode *inode, void *data)
> {
> struct erofs_vnode *vi = EROFS_V(inode);
> struct erofs_inode_v1 *v1 = data;
> - const unsigned advise = le16_to_cpu(v1->i_advise);
> + const unsigned int advise = le16_to_cpu(v1->i_advise);
>
> vi->data_mapping_mode = __inode_data_mapping(advise);
>
> @@ -112,7 +112,7 @@ static int read_inode(struct inode *inode, void *data)
> * try_lock since it takes no much overhead and
> * will success immediately.
> */
> -static int fill_inline_data(struct inode *inode, void *data, unsigned m_pofs)
> +static int fill_inline_data(struct inode *inode, void *data, unsigned int
> m_pofs)
> {
> struct erofs_vnode *vi = EROFS_V(inode);
> struct erofs_sb_info *sbi = EROFS_I_SB(inode);
> @@ -152,7 +152,7 @@ static int fill_inode(struct inode *inode, int is
Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison
On 2018/8/31 17:41, Dan Carpenter wrote: > On Fri, Aug 31, 2018 at 11:29:03AM +0800, Chao Yu wrote: >> >> Hi Xiang, >> >> I'm not against this change which follows checkpatch's rule, since I think >> this >> can help to unify coding style in different modules of Linux. Maybe cleanup >> in >> other filesystem is needed as well. >> > > That code is old, and those filesystems are not in staging so we're not > going to change them. Yup, anyway, it can be decided by their maintainer if they want the cleanup. > > Let's just apply the patch and not spend any time thinking about it. > Part of the point of style guidelines is so that we don't have to > repeat all these discussions over and over... > > Btw, I have a rename_rev.pl patch for reviewing these. I've attached > it. rename_rev.pl -r NULL. I've seen some people screw up the > conversion so having an automated review is nice. Cool, thanks very much for providing this tools, it actually can help to save some time during review. :) > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison
Hi, On 2018/9/3 9:59, Chao Yu wrote: > On 2018/8/31 17:41, Dan Carpenter wrote: >> On Fri, Aug 31, 2018 at 11:29:03AM +0800, Chao Yu wrote: >>> >>> Hi Xiang, >>> >>> I'm not against this change which follows checkpatch's rule, since I think >>> this >>> can help to unify coding style in different modules of Linux. Maybe cleanup >>> in >>> other filesystem is needed as well. >>> >> >> That code is old, and those filesystems are not in staging so we're not >> going to change them. > > Yup, anyway, it can be decided by their maintainer if they want the cleanup. > >> >> Let's just apply the patch and not spend any time thinking about it. >> Part of the point of style guidelines is so that we don't have to >> repeat all these discussions over and over... >> >> Btw, I have a rename_rev.pl patch for reviewing these. I've attached >> it. rename_rev.pl -r NULL. I've seen some people screw up the >> conversion so having an automated review is nice. > > Cool, thanks very much for providing this tools, it actually can help to save > some time during review. :) > I am traveling in Tibet. Sorry for the late response. OK, that is fine. I will follow the new kernel coding style strictly for the upcoming code. Reviewed-by: Gao Xiang Thanks, Gao Xiang >> >> regards, >> dan carpenter >> > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
> +++ b/drivers/auxdisplay/hd44780.c
> @@ -62,17 +62,12 @@ 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;
> -
> - for (i = 0; i < 8; i++)
> - values[PIN_DATA0 + i] = !!(val & BIT(i));
> - values[PIN_CTRL_RS] = rs;
> - n = 9;
> - if (hd->pins[PIN_CTRL_RW]) {
> - values[PIN_CTRL_RW] = 0;
> - n++;
> - }
> + DECLARE_BITMAP(values, 10); /* for DATA[0-7], RS, RW */
> + unsigned int n;
> +
> + *values = val;
> + __assign_bit(8, values, rs);
> + n = hd->pins[PIN_CTRL_RW] ? 10 : 9;
Doesn't this assume little endian bitmaps? Has anyone tested this on
big-endian machines?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] staging: erofs: formatting spaces around '-'
On 2018/9/3 9:48, Chao Yu wrote: > On 2018/8/30 23:13, Pavel Zemlyanoy wrote: >> This patch does not change the logic, it only >> corrects the formatting and checkpatch checks by >> adding spaces around '-'. >> >> The patch fixes 4 checks of type: >> "Check: spaces preferred around that '-'". >> >> Signed-off-by: Pavel Zemlyanoy > > Reviewed-by: Chao Yu > Reviewed-by: Gao Xiang Thanks, Gao Xiang > Thanks, > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: erofs: formatting add spaces arround '*'
On 2018/9/3 9:48, Chao Yu wrote: > On 2018/8/30 23:14, Pavel Zemlyanoy wrote: >> This patch does not change the logic, it only >> corrects the formatting and checkpatch check by >> adding spaces around '*'. >> >> The patch fixes 1 check of type: >> "Check: spaces preferred around that '*'". >> >> Signed-off-by: Pavel Zemlyanoy > > Reviewed-by: Chao Yu > Reviewed-by: Gao Xiang Thanks, Gao Xiang > Thanks, > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c
On 2018/9/3 9:47, Chao Yu wrote: > On 2018/8/30 23:13, Pavel Zemlyanoy wrote: >> >> This patch does not change the logic, it only >> corrects the formatting and checkpatch warnings by >> adding "int" to the unsigned type. >> >> The patch fixes 11 warnings of the type: >> "WARNING: Prefer 'unsigned int' to bare use of 'unsigned'" >> >> Signed-off-by: Pavel Zemlyanoy > > Reviewed-by: Chao Yu > Reviewed-by: Gao Xiang Thanks, Gao Xiang > Thanks, > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: erofs: fix 1 warning and 9 checks
On 2018/9/3 9:49, Chao Yu wrote:
> On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch checks by
>> braces {} should be used on all arms of this statement,
>> unbalanced braces around else statement and warning by
>> braces {} are not necessary for any arm of this statement.
>>
>> The patch fixes 9 checks of type:
>> "Check: braces {} should be used on all arms of this statement";
>> "Check: Unbalanced braces around else statement";
>>
>> and 1 warning of type:
>> "WARNING: braces {} are not necessary for any arm of this statement".
>>
>> Signed-off-by: Pavel Zemlyanoy
>
> Reviewed-by: Chao Yu
>
Reviewed-by: Gao Xiang
Thanks,
Gao Xiang
> Thanks,
>
> ___
> devel mailing list
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: erofs: formatting alignment parenthesis
On 2018/9/3 9:49, Chao Yu wrote: > On 2018/8/30 23:14, Pavel Zemlyanoy wrote: >> This patch does not change the logic, it only >> corrects the formatting and checkpatch check by >> alignment should match open parenthesis. >> >> The patch fixes 2 check of type: >> "Check: Alignment should match open parenthesis". >> >> Signed-off-by: Pavel Zemlyanoy > > Reviewed-by: Chao Yu > Reviewed-by: Gao Xiang Thanks, Gao Xiang > Thanks, > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Urgent Attention Needed
Good Day, My name is Mr.Guang Heng, I am writing to inform you about a legitimate Investment business project. It is an investment project we can do together as partners. The funds belong to my client who wants to invest large amount of funds in any great business opportunity/investments. Do you have any interesting business plan/idea we can invest into? I am in Europe on an official assignment. Do your best to call me on this number 0031687540591. If you can’t call then send your reply to this email address: ghen...@outlook.com I will give you more information about the project after I get your reply. Thank You. Mr. Guang Heng. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Urgent Attention Needed
Good Day, My name is Mr.Guang Heng, I am writing to inform you about a legitimate Investment business project. It is an investment project we can do together as partners. The funds belong to my client who wants to invest large amount of funds in any great business opportunity/investments. Do you have any interesting business plan/idea we can invest into? I am in Europe on an official assignment. Do your best to call me on this number 0031687540591. If you can’t call then send your reply to this email address: ghen...@outlook.com I will give you more information about the project after I get your reply. Thank You. Mr. Guang Heng. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
