Netdev List
 help / color / mirror / Atom feed
* IMPORTANTE: La fiscalía general le hace el segundo llamado a interrogatorio
From: Fiscalía General de la Nación @ 2018-08-29 20:37 UTC (permalink / raw)


[-- Attachment #1: Type: text/plain, Size: 728 bytes --]


Cordial saludo,


De acuerdo a la situación que se presenta y transcurre con el proceso No 305351T  Agosto de 2018, la fiscalía general le hace el segundo llamado a interrogatorio para declaraciones y pruebas al respectivo proceso.

Este interrogatorio tiene como objetivo la aclaración de hechos contundentes.

Recuerde que esta es la primera citación y así procederemos a dos citaciones más con aviso respectivo, le agradecemos su colaboración.


A continuación nos permitimos adjuntar el proceso No 305351T  y de la misma forma la citación a declaración correspondiente a usted.




IMPORTANTE:


Clave del archivo adjunto :


procesofiscalia30535120180821e68dd993c4a8bb9e3d5e6c066946se



[-- Attachment #2: Fiscalia proceso 305351T.rar --]
[-- Type: *, Size: 144622 bytes --]

^ permalink raw reply

* [PATCH v5 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-29 20:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
	Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
	David S. Miller, Dominik Brodowski, Greg Kroah-Hartman,
	Kishon Vijay Abraham I, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
	Jiri Slaby, Willy Tarreau, Geert Uytterhoeven
In-Reply-To: <20180829204900.19390-1-jmkrzyszt@gmail.com>

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 <corbet@lwn.net>
Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
Cc: Peter Korsgaard <peter.korsgaard@barco.com>
Cc: Peter Rosin <peda@axentia.se>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Michael Hennerich <Michael.Hennerich@analog.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Hartmut Knaack <knaack.h@gmx.de>
Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 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 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->desc,
-			      my_gpio_values);
+			      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
diff --git a/drivers/auxdisplay/hd44780.c b/drivers/auxdisplay/hd44780.c
index f1a42f0f1ded..bbbd6a29bf01 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 */
+	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, &hd->pins[PIN_DATA0], values);
+	gpiod_set_array_value_cansleep(n, &hd->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[1];
+	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] >>= 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);
 
 	/* 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);
 }
@@ -155,23 +153,23 @@ static void hd44780_write_cmd_gpio4(struct charlcd *lcd, int cmd)
 /* Send 4-bits of a command to the LCD panel in raw 4 bit GPIO mode */
 static void hd44780_write_cmd_raw_gpio4(struct charlcd *lcd, int cmd)
 {
-	int values[10];	/* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+	/* for DATA[0-7], RS, RW, but DATA[0-3] is unused */
+	unsigned long value_bitmap[1];
 	struct hd44780 *hd = lcd->drvdata;
-	unsigned int i, n;
+	unsigned int n;
 
 	/* Command nibble + RS, RW */
-	for (i = 0; i < 4; i++)
-		values[PIN_DATA4 + i] = !!(cmd & BIT(i));
-	values[PIN_CTRL_RS] = 0;
+	value_bitmap[0] = cmd << PIN_DATA4;
+	__clear_bit(PIN_CTRL_RS, value_bitmap);
 	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;
 
 	/* 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);
 }
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 073fd9011154..ce6c1e89236d 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -110,13 +110,9 @@ static void ts_nbus_set_direction(struct ts_nbus *ts_nbus, int direction)
  */
 static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
 {
-	int i;
-	int values[8];
-
-	for (i = 0; i < 8; i++)
-		values[i] = 0;
+	unsigned long value_bitmap[1] = { 0, };
 
-	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, values);
+	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc, value_bitmap);
 	gpiod_set_value_cansleep(ts_nbus->csn, 0);
 	gpiod_set_value_cansleep(ts_nbus->strobe, 0);
 	gpiod_set_value_cansleep(ts_nbus->ale, 0);
@@ -157,16 +153,9 @@ static int ts_nbus_read_byte(struct ts_nbus *ts_nbus, u8 *val)
 static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
 {
 	struct gpio_descs *gpios = ts_nbus->data;
-	int i;
-	int values[8];
-
-	for (i = 0; i < 8; i++)
-		if (byte & BIT(i))
-			values[i] = 1;
-		else
-			values[i] = 0;
+	unsigned long value_bitmap[1] = { byte, };
 
-	gpiod_set_array_value_cansleep(8, gpios->desc, values);
+	gpiod_set_array_value_cansleep(8, gpios->desc, value_bitmap);
 }
 
 /*
diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index b5b9cb1fda50..c4ec1c82af27 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -315,17 +315,20 @@ static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
 						  struct gpio_desc **desc,
 						  int value)
 {
-	int i, *values;
+	unsigned long *value_bitmap;
 
-	values = kmalloc_array(ndescs, sizeof(*values), GFP_KERNEL);
-	if (!values)
+	value_bitmap = kmalloc_array(BITS_TO_LONGS(ndescs),
+				     sizeof(*value_bitmap), GFP_KERNEL);
+	if (!value_bitmap)
 		return;
 
-	for (i = 0; i < ndescs; i++)
-		values[i] = value;
+	if (value)
+		bitmap_fill(value_bitmap, ndescs);
+	else
+		bitmap_zero(value_bitmap, ndescs);
 
-	gpiod_set_array_value_cansleep(ndescs, desc, values);
-	kfree(values);
+	gpiod_set_array_value_cansleep(ndescs, desc, value_bitmap);
+	kfree(value_bitmap);
 }
 
 static struct gpio_descs *devm_gpiod_get_array_optional_count(
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e8f8a1999393..f0e9ffa8cab6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -427,7 +427,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 	struct linehandle_state *lh = filep->private_data;
 	void __user *ip = (void __user *)arg;
 	struct gpiohandle_data ghd;
-	int vals[GPIOHANDLES_MAX];
+	unsigned long value_bitmap[BITS_TO_LONGS(GPIOHANDLES_MAX)];
 	int i;
 
 	if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) {
@@ -436,13 +436,13 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 							true,
 							lh->numdescs,
 							lh->descs,
-							vals);
+							value_bitmap);
 		if (ret)
 			return ret;
 
 		memset(&ghd, 0, sizeof(ghd));
 		for (i = 0; i < lh->numdescs; i++)
-			ghd.values[i] = vals[i];
+			ghd.values[i] = test_bit(i, value_bitmap);
 
 		if (copy_to_user(ip, &ghd, sizeof(ghd)))
 			return -EFAULT;
@@ -461,14 +461,14 @@ 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, value_bitmap, !!ghd.values[i]);
 
 		/* Reuse the array setting function */
 		return gpiod_set_array_value_complex(false,
 					      true,
 					      lh->numdescs,
 					      lh->descs,
-					      vals);
+					      value_bitmap);
 	}
 	return -EINVAL;
 }
@@ -2784,7 +2784,7 @@ static int gpio_chip_get_multiple(struct gpio_chip *chip,
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
 				  struct gpio_desc **desc_array,
-				  int *value_array)
+				  unsigned long *value_bitmap)
 {
 	int i = 0;
 
@@ -2835,7 +2835,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 
 			if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 				value = !value;
-			value_array[j] = value;
+			__assign_bit(j, value_bitmap, value);
 			trace_gpio_value(desc_to_gpio(desc), 1, value);
 		}
 
@@ -2895,9 +2895,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
 
 /**
  * gpiod_get_raw_array_value() - read raw 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_bitmap: bitmap to store the read values
  *
  * Read the raw values of the GPIOs, i.e. the values of the physical lines
  * without regard for their ACTIVE_LOW status.  Return 0 in case of success,
@@ -2907,20 +2907,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value);
  * and it will complain if the GPIO chip functions potentially sleep.
  */
 int gpiod_get_raw_array_value(unsigned int array_size,
-			      struct gpio_desc **desc_array, int *value_array)
+			      struct gpio_desc **desc_array,
+			      unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(true, false, array_size,
-					     desc_array, value_array);
+					     desc_array, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
 
 /**
  * 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
  *
  * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
  * into account.  Return 0 in case of success, else an error code.
@@ -2929,12 +2930,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value);
  * and it will complain if the GPIO chip functions potentially sleep.
  */
 int gpiod_get_array_value(unsigned int array_size,
-			  struct gpio_desc **desc_array, int *value_array)
+			  struct gpio_desc **desc_array,
+			  unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(false, false, array_size,
-					     desc_array, value_array);
+					     desc_array, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array_value);
 
@@ -3027,7 +3029,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip,
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				   unsigned int array_size,
 				   struct gpio_desc **desc_array,
-				   int *value_array)
+				   unsigned long *value_bitmap)
 {
 	int i = 0;
 
@@ -3056,7 +3058,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 		do {
 			struct gpio_desc *desc = desc_array[i];
 			int hwgpio = gpio_chip_hwgpio(desc);
-			int value = value_array[i];
+			int value = test_bit(i, value_bitmap);
 
 			if (!raw && test_bit(FLAG_ACTIVE_LOW, &desc->flags))
 				value = !value;
@@ -3152,9 +3154,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
 
 /**
  * gpiod_set_raw_array_value() - assign values to 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 assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
  *
  * Set the raw values of the GPIOs, i.e. the values of the physical lines
  * without regard for their ACTIVE_LOW status.
@@ -3163,20 +3165,21 @@ EXPORT_SYMBOL_GPL(gpiod_set_value);
  * complain if the GPIO chip functions potentially sleep.
  */
 int gpiod_set_raw_array_value(unsigned int array_size,
-			 struct gpio_desc **desc_array, int *value_array)
+			 struct gpio_desc **desc_array,
+			 unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_set_array_value_complex(true, false, array_size,
-					desc_array, value_array);
+					desc_array, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
 
 /**
  * gpiod_set_array_value() - assign values to 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 assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
  *
  * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
  * into account.
@@ -3185,12 +3188,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
  * complain if the GPIO chip functions potentially sleep.
  */
 void gpiod_set_array_value(unsigned int array_size,
-			   struct gpio_desc **desc_array, int *value_array)
+			   struct gpio_desc **desc_array,
+			   unsigned long *value_bitmap)
 {
 	if (!desc_array)
 		return;
 	gpiod_set_array_value_complex(false, false, array_size, desc_array,
-				      value_array);
+				      value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value);
 
@@ -3410,9 +3414,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
 
 /**
  * gpiod_get_raw_array_value_cansleep() - read raw 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_bitmap: bitmap to store the read values
  *
  * Read the raw values of the GPIOs, i.e. the values of the physical lines
  * without regard for their ACTIVE_LOW status.  Return 0 in case of success,
@@ -3422,21 +3426,21 @@ EXPORT_SYMBOL_GPL(gpiod_get_value_cansleep);
  */
 int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
 				       struct gpio_desc **desc_array,
-				       int *value_array)
+				       unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(true, true, array_size,
-					     desc_array, value_array);
+					     desc_array, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
 
 /**
  * gpiod_get_array_value_cansleep() - 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_bitmap: bitmap to store the read values
  *
  * Read the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
  * into account.  Return 0 in case of success, else an error code.
@@ -3445,13 +3449,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_raw_array_value_cansleep);
  */
 int gpiod_get_array_value_cansleep(unsigned int array_size,
 				   struct gpio_desc **desc_array,
-				   int *value_array)
+				   unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_get_array_value_complex(false, true, array_size,
-					     desc_array, value_array);
+					     desc_array, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
 
@@ -3493,9 +3497,9 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
 
 /**
  * gpiod_set_raw_array_value_cansleep() - assign values to 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 assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
  *
  * Set the raw values of the GPIOs, i.e. the values of the physical lines
  * without regard for their ACTIVE_LOW status.
@@ -3504,13 +3508,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
  */
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 					struct gpio_desc **desc_array,
-					int *value_array)
+					unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return -EINVAL;
 	return gpiod_set_array_value_complex(true, true, array_size, desc_array,
-				      value_array);
+				      value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);
 
@@ -3533,9 +3537,9 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
 
 /**
  * gpiod_set_array_value_cansleep() - assign values to 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 assigned
- * @value_array: array of values to assign
+ * @value_bitmap: bitmap of values to assign
  *
  * Set the logical values of the GPIOs, i.e. taking their ACTIVE_LOW status
  * into account.
@@ -3544,13 +3548,13 @@ void gpiod_add_lookup_tables(struct gpiod_lookup_table **tables, size_t n)
  */
 void gpiod_set_array_value_cansleep(unsigned int array_size,
 				    struct gpio_desc **desc_array,
-				    int *value_array)
+				    unsigned long *value_bitmap)
 {
 	might_sleep_if(extra_checks);
 	if (!desc_array)
 		return;
 	gpiod_set_array_value_complex(false, true, array_size, desc_array,
-				      value_array);
+				      value_bitmap);
 }
 EXPORT_SYMBOL_GPL(gpiod_set_array_value_cansleep);
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index a7e49fef73d4..11e83d2eef89 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -187,11 +187,11 @@ struct gpio_desc *gpiochip_get_desc(struct gpio_chip *chip, u16 hwnum);
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
 				  struct gpio_desc **desc_array,
-				  int *value_array);
+				  unsigned long *value_bitmap);
 int gpiod_set_array_value_complex(bool raw, bool can_sleep,
 				   unsigned int array_size,
 				   struct gpio_desc **desc_array,
-				   int *value_array);
+				   unsigned long *value_bitmap);
 
 /* This is just passed between gpiolib and devres */
 struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 401308e3d036..4e36e0eac7a3 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -22,18 +22,16 @@ struct gpiomux {
 	struct i2c_mux_gpio_platform_data data;
 	unsigned gpio_base;
 	struct gpio_desc **gpios;
-	int *values;
+	int *values; /* FIXME: no longer needed */
 };
 
 static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
 {
+	unsigned long value_bitmap[1] = { val, };
 	int i;
 
-	for (i = 0; i < mux->data.n_gpios; i++)
-		mux->values[i] = (val >> i) & 1;
-
 	gpiod_set_array_value_cansleep(mux->data.n_gpios,
-				       mux->gpios, mux->values);
+				       mux->gpios, value_bitmap);
 }
 
 static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index a8b9fee4d62a..0d6e3a5be3ba 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -40,18 +40,13 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 	struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
 
 	if (!IS_ERR(reset_gpios)) {
-		int i, *values;
+		unsigned long value_bitmap[1];
 		int nvalues = reset_gpios->ndescs;
 
-		values = kmalloc_array(nvalues, sizeof(int), GFP_KERNEL);
-		if (!values)
-			return;
+		value_bitmap[0] = value;
 
-		for (i = 0; i < nvalues; i++)
-			values[i] = value;
-
-		gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc, values);
-		kfree(values);
+		gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
+					       value_bitmap);
 	}
 }
 
diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index 6fdd9316db8b..734e1b43aed6 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -17,20 +17,17 @@
 
 struct mux_gpio {
 	struct gpio_descs *gpios;
-	int *val;
+	int *val; /* FIXME: no longer needed */
 };
 
 static int mux_gpio_set(struct mux_control *mux, int state)
 {
 	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
+	unsigned long value_bitmap[1] = { state, };
 	int i;
 
-	for (i = 0; i < mux_gpio->gpios->ndescs; i++)
-		mux_gpio->val[i] = (state >> i) & 1;
-
 	gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
-				       mux_gpio->gpios->desc,
-				       mux_gpio->val);
+				       mux_gpio->gpios->desc, value_bitmap);
 
 	return 0;
 }
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index bc90764a8b8d..8e1ec750277e 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -27,6 +27,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
 				   void *data)
 {
 	struct mdio_mux_gpio_state *s = data;
+	unsigned long value_bitmap[1] = { desired_child, };
 	unsigned int n;
 
 	if (current_child == desired_child)
@@ -36,7 +37,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
 		s->values[n] = (desired_child >> n) & 1;
 
 	gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
-				       s->values);
+				       value_bitmap);
 
 	return 0;
 }
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index c5f2344c189b..e0f89155c474 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -351,19 +351,22 @@ static int soc_common_pcmcia_config_skt(
 
 	if (ret == 0) {
 		struct gpio_desc *descs[2];
-		int values[2], n = 0;
+		unsigned long value_bitmap[1];
+		int n = 0;
 
 		if (skt->gpio_reset) {
 			descs[n] = skt->gpio_reset;
-			values[n++] = !!(state->flags & SS_RESET);
+			__assign_bit(n++, value_bitmap,
+				     !!(state->flags & SS_RESET));
 		}
 		if (skt->gpio_bus_enable) {
 			descs[n] = skt->gpio_bus_enable;
-			values[n++] = !!(state->flags & SS_OUTPUT_ENA);
+			__assign_bit(n++, value_bitmap,
+				     !!(state->flags & SS_OUTPUT_ENA));
 		}
 
 		if (n)
-			gpiod_set_array_value_cansleep(n, descs, values);
+			gpiod_set_array_value_cansleep(n, descs, value_bitmap);
 
 		/*
 		 * This really needs a better solution.  The IRQ
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index 0075fb0bef8c..b6477c3599c4 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -157,15 +157,12 @@ static const struct phy_ops gpio_usb_ops = {
  */
 static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
 {
-	int values[PHY_MDM6600_NR_CMD_LINES];
-	int i;
+	unsigned long value_bitmap[1];
 
-	val &= (1 << PHY_MDM6600_NR_CMD_LINES) - 1;
-	for (i = 0; i < PHY_MDM6600_NR_CMD_LINES; i++)
-		values[i] = (val & BIT(i)) >> i;
+	value_bitmap[0] = val & ((1 << PHY_MDM6600_NR_CMD_LINES) - 1);
 
 	gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
-				       ddata->cmd_gpios->desc, values);
+				       ddata->cmd_gpios->desc, value_bitmap);
 }
 
 /**
@@ -176,7 +173,7 @@ static void phy_mdm6600_status(struct work_struct *work)
 {
 	struct phy_mdm6600 *ddata;
 	struct device *dev;
-	int values[PHY_MDM6600_NR_STATUS_LINES];
+	unsigned long value_bitmap[1] = { 0, };
 	int error, i, val = 0;
 
 	ddata = container_of(work, struct phy_mdm6600, status_work.work);
@@ -184,14 +181,14 @@ static void phy_mdm6600_status(struct work_struct *work)
 
 	error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
 					       ddata->status_gpios->desc,
-					       values);
+					       value_bitmap);
 	if (error)
 		return;
 
 	for (i = 0; i < PHY_MDM6600_NR_STATUS_LINES; i++) {
-		val |= values[i] << i;
+		val |= test_bit(i, value_bitmap) << i;
 		dev_dbg(ddata->dev, "XXX %s: i: %i values[i]: %i val: %i\n",
-			__func__, i, values[i], val);
+			__func__, i, test_bit(i, value_bitmap), val);
 	}
 	ddata->status = val;
 
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 25b9fcd5e3a4..0eca047bc1cc 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -202,7 +202,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 			    long mask)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
-	int values[3];
+	unsigned long value_bitmap[1];
 	int ret, i;
 
 	switch (mask) {
@@ -227,13 +227,10 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			return ret;
 
-		values[0] = (ret >> 0) & 1;
-		values[1] = (ret >> 1) & 1;
-		values[2] = (ret >> 2) & 1;
+		value_bitmap[0] = ret;
 
 		mutex_lock(&st->lock);
-		gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
-				      values);
+		gpiod_set_array_value(3, st->gpio_os->desc, value_bitmap);
 		st->oversampling = val;
 		mutex_unlock(&st->lock);
 
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index 1c06325beaca..bb8b4756d72d 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -40,7 +40,7 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
 {
 	enum mctrl_gpio_idx i;
 	struct gpio_desc *desc_array[UART_GPIO_MAX];
-	int value_array[UART_GPIO_MAX];
+	unsigned long value_bitmap[BITS_TO_LONGS(UART_GPIO_MAX)];
 	unsigned int count = 0;
 
 	if (gpios == NULL)
@@ -49,10 +49,11 @@ void mctrl_gpio_set(struct mctrl_gpios *gpios, unsigned int mctrl)
 	for (i = 0; i < UART_GPIO_MAX; i++)
 		if (gpios->gpio[i] && mctrl_gpios_desc[i].dir_out) {
 			desc_array[count] = gpios->gpio[i];
-			value_array[count] = !!(mctrl & mctrl_gpios_desc[i].mctrl);
+			__assign_bit(count, value_bitmap,
+				     !!(mctrl & mctrl_gpios_desc[i].mctrl));
 			count++;
 		}
-	gpiod_set_array_value(count, desc_array, value_array);
+	gpiod_set_array_value(count, desc_array, value_bitmap);
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_set);
 
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 21ddbe440030..1b21dc7b0fad 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -104,36 +104,38 @@ int gpiod_direction_output_raw(struct gpio_desc *desc, int value);
 /* Value get/set from non-sleeping context */
 int gpiod_get_value(const struct gpio_desc *desc);
 int gpiod_get_array_value(unsigned int array_size,
-			  struct gpio_desc **desc_array, int *value_array);
+			  struct gpio_desc **desc_array,
+			  unsigned long *value_bitmap);
 void gpiod_set_value(struct gpio_desc *desc, int value);
 void gpiod_set_array_value(unsigned int array_size,
-			   struct gpio_desc **desc_array, int *value_array);
+			   struct gpio_desc **desc_array,
+			   unsigned long *value_bitmap);
 int gpiod_get_raw_value(const struct gpio_desc *desc);
 int gpiod_get_raw_array_value(unsigned int array_size,
 			      struct gpio_desc **desc_array,
-			      int *value_array);
+			      unsigned long *value_bitmap);
 void gpiod_set_raw_value(struct gpio_desc *desc, int value);
 int gpiod_set_raw_array_value(unsigned int array_size,
 			       struct gpio_desc **desc_array,
-			       int *value_array);
+			       unsigned long *value_bitmap);
 
 /* Value get/set from sleeping context */
 int gpiod_get_value_cansleep(const struct gpio_desc *desc);
 int gpiod_get_array_value_cansleep(unsigned int array_size,
 				   struct gpio_desc **desc_array,
-				   int *value_array);
+				   unsigned long *value_bitmap);
 void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
 void gpiod_set_array_value_cansleep(unsigned int array_size,
 				    struct gpio_desc **desc_array,
-				    int *value_array);
+				    unsigned long *value_bitmap);
 int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc);
 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_raw_value_cansleep(struct gpio_desc *desc, int value);
 int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
 					struct gpio_desc **desc_array,
-					int *value_array);
+					unsigned long *value_bitmap);
 
 int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce);
 int gpiod_set_transitory(struct gpio_desc *desc, bool transitory);
-- 
2.16.4

^ permalink raw reply related

* [PATCH net-next 3/3] net: nixge: Use sysdev instead of ndev->dev.parent for DMA
From: Moritz Fischer @ 2018-08-30  0:40 UTC (permalink / raw)
  To: davem
  Cc: keescook, f.fainelli, linux-kernel, netdev, alex.williams,
	Moritz Fischer
In-Reply-To: <20180830004046.9417-1-mdf@kernel.org>

Use sysdev instead of ndev->dev.parent for DMA in order to
enable usecases where an IOMMU might get in the way otherwise.
In the PCIe wrapper case on x86 the IOMMU group is not inherited
by the platform device that is created as a subdevice of the
PCI wrapper driver.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Hi,

not this patch is still in the early stages,
and as the rest of this series goes on top of [1].

The actual parent device might still change since
the parent device driver is still under development.

If there are clever suggestions on how to generalize
the assignment of sysdev, I'm all ears.

Thanks for your time,

Moritz


[1] https://lkml.org/lkml/2018/8/28/1011

---
 drivers/net/ethernet/ni/nixge.c | 50 +++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index fd8e5b02c459..25fb1642d558 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -16,6 +16,7 @@
 #include <linux/of_irq.h>
 #include <linux/skbuff.h>
 #include <linux/phy.h>
+#include <linux/pci.h>
 #include <linux/mii.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/ethtool.h>
@@ -165,6 +166,7 @@ struct nixge_priv {
 	struct net_device *ndev;
 	struct napi_struct napi;
 	struct device *dev;
+	struct device *sysdev;
 
 	/* Connection to PHY device */
 	struct device_node *phy_node;
@@ -250,7 +252,7 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev)
 		phys_addr = nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i],
 						     phys);
 
-		dma_unmap_single(ndev->dev.parent, phys_addr,
+		dma_unmap_single(priv->sysdev, phys_addr,
 				 NIXGE_MAX_JUMBO_FRAME_SIZE,
 				 DMA_FROM_DEVICE);
 
@@ -261,16 +263,16 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev)
 	}
 
 	if (priv->rx_bd_v)
-		dma_free_coherent(ndev->dev.parent,
+		dma_free_coherent(priv->sysdev,
 				  sizeof(*priv->rx_bd_v) * RX_BD_NUM,
 				  priv->rx_bd_v,
 				  priv->rx_bd_p);
 
 	if (priv->tx_skb)
-		devm_kfree(ndev->dev.parent, priv->tx_skb);
+		devm_kfree(priv->sysdev, priv->tx_skb);
 
 	if (priv->tx_bd_v)
-		dma_free_coherent(ndev->dev.parent,
+		dma_free_coherent(priv->sysdev,
 				  sizeof(*priv->tx_bd_v) * TX_BD_NUM,
 				  priv->tx_bd_v,
 				  priv->tx_bd_p);
@@ -290,19 +292,19 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
 	priv->rx_bd_ci = 0;
 
 	/* Allocate the Tx and Rx buffer descriptors. */
-	priv->tx_bd_v = dma_zalloc_coherent(ndev->dev.parent,
+	priv->tx_bd_v = dma_zalloc_coherent(priv->sysdev,
 					    sizeof(*priv->tx_bd_v) * TX_BD_NUM,
 					    &priv->tx_bd_p, GFP_KERNEL);
 	if (!priv->tx_bd_v)
 		goto out;
 
-	priv->tx_skb = devm_kcalloc(ndev->dev.parent,
+	priv->tx_skb = devm_kcalloc(priv->sysdev,
 				    TX_BD_NUM, sizeof(*priv->tx_skb),
 				    GFP_KERNEL);
 	if (!priv->tx_skb)
 		goto out;
 
-	priv->rx_bd_v = dma_zalloc_coherent(ndev->dev.parent,
+	priv->rx_bd_v = dma_zalloc_coherent(priv->sysdev,
 					    sizeof(*priv->rx_bd_v) * RX_BD_NUM,
 					    &priv->rx_bd_p, GFP_KERNEL);
 	if (!priv->rx_bd_v)
@@ -327,7 +329,7 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
 			goto out;
 
 		nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], skb);
-		phys = dma_map_single(ndev->dev.parent, skb->data,
+		phys = dma_map_single(priv->sysdev, skb->data,
 				      NIXGE_MAX_JUMBO_FRAME_SIZE,
 				      DMA_FROM_DEVICE);
 
@@ -438,10 +440,10 @@ static void nixge_tx_skb_unmap(struct nixge_priv *priv,
 {
 	if (tx_skb->mapping) {
 		if (tx_skb->mapped_as_page)
-			dma_unmap_page(priv->ndev->dev.parent, tx_skb->mapping,
+			dma_unmap_page(priv->sysdev, tx_skb->mapping,
 				       tx_skb->size, DMA_TO_DEVICE);
 		else
-			dma_unmap_single(priv->ndev->dev.parent,
+			dma_unmap_single(priv->sysdev,
 					 tx_skb->mapping,
 					 tx_skb->size, DMA_TO_DEVICE);
 		tx_skb->mapping = 0;
@@ -519,9 +521,9 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		return NETDEV_TX_OK;
 	}
 
-	cur_phys = dma_map_single(ndev->dev.parent, skb->data,
+	cur_phys = dma_map_single(priv->sysdev, skb->data,
 				  skb_headlen(skb), DMA_TO_DEVICE);
-	if (dma_mapping_error(ndev->dev.parent, cur_phys))
+	if (dma_mapping_error(priv->sysdev, cur_phys))
 		goto drop;
 	nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
 
@@ -539,10 +541,10 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		tx_skb = &priv->tx_skb[priv->tx_bd_tail];
 		frag = &skb_shinfo(skb)->frags[ii];
 
-		cur_phys = skb_frag_dma_map(ndev->dev.parent, frag, 0,
+		cur_phys = skb_frag_dma_map(priv->sysdev, frag, 0,
 					    skb_frag_size(frag),
 					    DMA_TO_DEVICE);
-		if (dma_mapping_error(ndev->dev.parent, cur_phys))
+		if (dma_mapping_error(priv->sysdev, cur_phys))
 			goto frag_err;
 		nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
 
@@ -579,7 +581,7 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		cur_p = &priv->tx_bd_v[priv->tx_bd_tail];
 		cur_p->status = 0;
 	}
-	dma_unmap_single(priv->ndev->dev.parent,
+	dma_unmap_single(priv->sysdev,
 			 tx_skb->mapping,
 			 tx_skb->size, DMA_TO_DEVICE);
 drop:
@@ -611,7 +613,7 @@ static int nixge_recv(struct net_device *ndev, int budget)
 		if (length > NIXGE_MAX_JUMBO_FRAME_SIZE)
 			length = NIXGE_MAX_JUMBO_FRAME_SIZE;
 
-		dma_unmap_single(ndev->dev.parent,
+		dma_unmap_single(priv->sysdev,
 				 nixge_hw_dma_bd_get_addr(cur_p, phys),
 				 NIXGE_MAX_JUMBO_FRAME_SIZE,
 				 DMA_FROM_DEVICE);
@@ -636,10 +638,10 @@ static int nixge_recv(struct net_device *ndev, int budget)
 		if (!new_skb)
 			return packets;
 
-		cur_phys = dma_map_single(ndev->dev.parent, new_skb->data,
+		cur_phys = dma_map_single(priv->sysdev, new_skb->data,
 					  NIXGE_MAX_JUMBO_FRAME_SIZE,
 					  DMA_FROM_DEVICE);
-		if (dma_mapping_error(ndev->dev.parent, cur_phys)) {
+		if (dma_mapping_error(priv->sysdev, cur_phys)) {
 			/* FIXME: bail out and clean up */
 			netdev_err(ndev, "Failed to map ...\n");
 		}
@@ -1303,6 +1305,7 @@ static int nixge_probe(struct platform_device *pdev)
 	struct net_device *ndev;
 	struct resource *dmares;
 	struct device_node *np;
+	struct device *sysdev;
 	const u8 *mac_addr;
 	int err;
 
@@ -1331,9 +1334,20 @@ static int nixge_probe(struct platform_device *pdev)
 		eth_hw_addr_random(ndev);
 	}
 
+	sysdev = &pdev->dev;
+#ifdef CONFIG_PCI
+	/* if this is a subdevice of a PCI device we'll have to
+	 * make sure we'll use parent instead of our own platform
+	 * device to make sure  the DMA API if we use an IOMMU
+	 */
+	if (sysdev->parent && sysdev->parent->bus == &pci_bus_type)
+		sysdev = sysdev->parent;
+#endif
+
 	priv = netdev_priv(ndev);
 	priv->ndev = ndev;
 	priv->dev = &pdev->dev;
+	priv->sysdev = sysdev;
 
 	netif_napi_add(ndev, &priv->napi, nixge_poll, NAPI_POLL_WEIGHT);
 
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice
From: Moritz Fischer @ 2018-08-30  0:40 UTC (permalink / raw)
  To: davem
  Cc: keescook, f.fainelli, linux-kernel, netdev, alex.williams,
	Moritz Fischer
In-Reply-To: <20180830004046.9417-1-mdf@kernel.org>

Add support for instantiating nixge as subdevice using
fixed-link and platform data to configure it.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Hi,

not this patch is still in the early stages,
and as the rest of this series goes on top of [1].

The actual platform data might still change since
the parent device driver is still under development.

Thanks for your time,

Moritz


[1] https://lkml.org/lkml/2018/8/28/1011

---
 drivers/net/ethernet/ni/nixge.c     | 71 ++++++++++++++++++++++++++---
 include/linux/platform_data/nixge.h | 19 ++++++++
 2 files changed, 83 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/platform_data/nixge.h

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 670249313ff0..fd8e5b02c459 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -7,6 +7,8 @@
 #include <linux/etherdevice.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/phy_fixed.h>
+#include <linux/platform_data/nixge.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
@@ -167,6 +169,7 @@ struct nixge_priv {
 	/* Connection to PHY device */
 	struct device_node *phy_node;
 	phy_interface_t		phy_mode;
+	struct phy_device *phydev;
 
 	int link;
 	unsigned int speed;
@@ -859,15 +862,25 @@ static void nixge_dma_err_handler(unsigned long data)
 static int nixge_open(struct net_device *ndev)
 {
 	struct nixge_priv *priv = netdev_priv(ndev);
-	struct phy_device *phy;
+	struct phy_device *phy = NULL;
 	int ret;
 
 	nixge_device_reset(ndev);
 
-	phy = of_phy_connect(ndev, priv->phy_node,
-			     &nixge_handle_link_change, 0, priv->phy_mode);
-	if (!phy)
-		return -ENODEV;
+	if (priv->dev->of_node) {
+		phy = of_phy_connect(ndev, priv->phy_node,
+				     &nixge_handle_link_change, 0,
+				     priv->phy_mode);
+		if (!phy)
+			return -ENODEV;
+	} else if (priv->phydev) {
+		ret = phy_connect_direct(ndev, priv->phydev,
+					 &nixge_handle_link_change,
+					 priv->phy_mode);
+		if (ret)
+			return ret;
+		phy = priv->phydev;
+	}
 
 	phy_start(phy);
 
@@ -1215,10 +1228,41 @@ static int nixge_of_get_phy(struct nixge_priv *priv, struct device_node *np)
 	return 0;
 }
 
+static int nixge_pdata_get_phy(struct nixge_priv *priv,
+			       struct nixge_platform_data *pdata)
+{
+	struct phy_device *phy = NULL;
+
+	if (!pdata)
+		return -EINVAL;
+
+	if (pdata && pdata->phy_interface == PHY_INTERFACE_MODE_NA) {
+		struct fixed_phy_status fphy_status = {
+			.link = 1,
+			.duplex = pdata->phy_duplex,
+			.speed = pdata->phy_speed,
+			.pause = 0,
+			.asym_pause = 0,
+		};
+
+		/* TODO: Pull out GPIO from pdata */
+		phy = fixed_phy_register(PHY_POLL, &fphy_status, -1,
+					 NULL);
+		if (IS_ERR_OR_NULL(phy)) {
+			dev_err(priv->dev,
+				"failed to register fixed PHY device\n");
+			return -ENODEV;
+		}
+	}
+	priv->phy_mode = pdata->phy_interface;
+	priv->phydev = phy;
+
+	return 0;
+}
+
 static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
 {
 	struct mii_bus *bus;
-	int err;
 
 	bus = devm_mdiobus_alloc(priv->dev);
 	if (!bus)
@@ -1254,6 +1298,7 @@ static void *nixge_get_nvmem_address(struct device *dev)
 
 static int nixge_probe(struct platform_device *pdev)
 {
+	struct nixge_platform_data *pdata = NULL;
 	struct nixge_priv *priv;
 	struct net_device *ndev;
 	struct resource *dmares;
@@ -1320,10 +1365,16 @@ static int nixge_probe(struct platform_device *pdev)
 		err = nixge_of_get_phy(priv, np);
 		if (err)
 			goto free_netdev;
+	} else {
+		pdata = dev_get_platdata(&pdev->dev);
+		err = nixge_pdata_get_phy(priv, pdata);
+		if (err)
+			goto free_netdev;
 	}
 
 	/* only if it's not a fixed link, do we care about MDIO at all */
-	if (priv->phy_node && !of_phy_is_fixed_link(np)) {
+	if ((priv->phy_node && !of_phy_is_fixed_link(np)) ||
+	    (pdata && pdata->phy_interface != PHY_INTERFACE_MODE_NA)) {
 		err = nixge_mdio_setup(priv, np);
 		if (err) {
 			dev_err(&pdev->dev, "error registering mdio bus");
@@ -1347,6 +1398,9 @@ static int nixge_probe(struct platform_device *pdev)
 		of_phy_deregister_fixed_link(np);
 		of_node_put(np);
 	}
+
+	if (priv->phydev && phy_is_pseudo_fixed_link(priv->phydev))
+		fixed_phy_unregister(priv->phydev);
 free_netdev:
 	free_netdev(ndev);
 
@@ -1357,6 +1411,7 @@ static int nixge_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct nixge_priv *priv = netdev_priv(ndev);
+	struct device_node *np = pdev->dev.of_node;
 
 	unregister_netdev(ndev);
 
@@ -1365,6 +1420,8 @@ static int nixge_remove(struct platform_device *pdev)
 
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+	else if (priv->phydev && phy_is_pseudo_fixed_link(priv->phydev))
+		fixed_phy_unregister(priv->phydev);
 
 	free_netdev(ndev);
 
diff --git a/include/linux/platform_data/nixge.h b/include/linux/platform_data/nixge.h
new file mode 100644
index 000000000000..aa5dd5760074
--- /dev/null
+++ b/include/linux/platform_data/nixge.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018 National Instruments Corp.
+ *
+ * Author: Moritz Fischer <mdf@kernel.org>
+ */
+
+#ifndef __NIXGE_PDATA_H__
+#define __NIXGE_PDATA_H__
+
+#include <linux/phy.h>
+
+struct nixge_platform_data {
+	phy_interface_t phy_interface;
+	int phy_speed;
+	int phy_duplex;
+};
+
+#endif /* __NIXGE_PDATA_H__ */
+
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes
From: Moritz Fischer @ 2018-08-30  0:40 UTC (permalink / raw)
  To: davem
  Cc: keescook, f.fainelli, linux-kernel, netdev, alex.williams,
	Moritz Fischer
In-Reply-To: <20180830004046.9417-1-mdf@kernel.org>

Add support for fixed-link cases where no MDIO is
actually required to run the device.
In that case no MDIO bus is instantiated since the
actual registers are not available in hardware.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/net/ethernet/ni/nixge.c | 72 ++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 74cf52e3fb09..670249313ff0 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -1189,9 +1189,36 @@ static int nixge_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val)
 	return err;
 }
 
+static int nixge_of_get_phy(struct nixge_priv *priv, struct device_node *np)
+{
+	priv->phy_mode = of_get_phy_mode(np);
+	if (priv->phy_mode < 0) {
+		dev_err(priv->dev, "not find \"phy-mode\" property\n");
+		return -EINVAL;
+	}
+
+	if (of_phy_is_fixed_link(np)) {
+		if (of_phy_register_fixed_link(np) < 0) {
+			dev_err(priv->dev, "broken fixed link spec\n");
+			return -EINVAL;
+		}
+
+		priv->phy_node = of_node_get(np);
+	} else {
+		priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+		if (!priv->phy_node) {
+			dev_err(priv->dev, "not find \"phy-handle\" property\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
 {
 	struct mii_bus *bus;
+	int err;
 
 	bus = devm_mdiobus_alloc(priv->dev);
 	if (!bus)
@@ -1230,6 +1257,7 @@ static int nixge_probe(struct platform_device *pdev)
 	struct nixge_priv *priv;
 	struct net_device *ndev;
 	struct resource *dmares;
+	struct device_node *np;
 	const u8 *mac_addr;
 	int err;
 
@@ -1237,6 +1265,8 @@ static int nixge_probe(struct platform_device *pdev)
 	if (!ndev)
 		return -ENOMEM;
 
+	np = pdev->dev.of_node;
+
 	platform_set_drvdata(pdev, ndev);
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 
@@ -1286,24 +1316,19 @@ static int nixge_probe(struct platform_device *pdev)
 	priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD;
 	priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD;
 
-	err = nixge_mdio_setup(priv, pdev->dev.of_node);
-	if (err) {
-		netdev_err(ndev, "error registering mdio bus");
-		goto free_netdev;
-	}
-
-	priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
-	if (priv->phy_mode < 0) {
-		netdev_err(ndev, "not find \"phy-mode\" property\n");
-		err = -EINVAL;
-		goto unregister_mdio;
+	if (np) {
+		err = nixge_of_get_phy(priv, np);
+		if (err)
+			goto free_netdev;
 	}
 
-	priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
-	if (!priv->phy_node) {
-		netdev_err(ndev, "not find \"phy-handle\" property\n");
-		err = -EINVAL;
-		goto unregister_mdio;
+	/* only if it's not a fixed link, do we care about MDIO at all */
+	if (priv->phy_node && !of_phy_is_fixed_link(np)) {
+		err = nixge_mdio_setup(priv, np);
+		if (err) {
+			dev_err(&pdev->dev, "error registering mdio bus");
+			goto free_phy;
+		}
 	}
 
 	err = register_netdev(priv->ndev);
@@ -1315,8 +1340,13 @@ static int nixge_probe(struct platform_device *pdev)
 	return 0;
 
 unregister_mdio:
-	mdiobus_unregister(priv->mii_bus);
-
+	if (priv->mii_bus)
+		mdiobus_unregister(priv->mii_bus);
+free_phy:
+	if (np && of_phy_is_fixed_link(np)) {
+		of_phy_deregister_fixed_link(np);
+		of_node_put(np);
+	}
 free_netdev:
 	free_netdev(ndev);
 
@@ -1330,7 +1360,11 @@ static int nixge_remove(struct platform_device *pdev)
 
 	unregister_netdev(ndev);
 
-	mdiobus_unregister(priv->mii_bus);
+	if (priv->mii_bus)
+		mdiobus_unregister(priv->mii_bus);
+
+	if (np && of_phy_is_fixed_link(np))
+		of_phy_deregister_fixed_link(np);
 
 	free_netdev(ndev);
 
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 0/3] nixge: fixed-link support
From: Moritz Fischer @ 2018-08-30  0:40 UTC (permalink / raw)
  To: davem
  Cc: keescook, f.fainelli, linux-kernel, netdev, alex.williams,
	Moritz Fischer

Hi,

this series adds support for using nixge with fixed-link nodes,
as well as an early attempt to support nixge as a subdevice of
another device.

This series goes on top of: https://lkml.org/lkml/2018/8/28/1011

Patch 1: Adds of based fixed-link support, and hopefully isn't too
         controversial barring grave mistakes.

Patch 2: Is an attempt at making sure that nixge works as a subdevice
         of another (pci) device without a PHY. Note that same as Patch
         3 the actual platform data might still change since the parent
         device driver is still under development. So this is more of an
         RFC, too.

Patch 3: Is more of an RFC at this point since the PCI parent device is
         still under development, but required to run with IOMMU
         support. Feedback on this one would be appreciated.

Thanks for your input,

Moritz

Moritz Fischer (3):
  net: nixge: Add support for fixed-link subnodes
  net: nixge: Add support for having nixge as subdevice
  net: nixge: Use sysdev instead of ndev->dev.parent for DMA

 drivers/net/ethernet/ni/nixge.c     | 187 ++++++++++++++++++++++------
 include/linux/platform_data/nixge.h |  19 +++
 2 files changed, 165 insertions(+), 41 deletions(-)
 create mode 100644 include/linux/platform_data/nixge.h

-- 
2.18.0

^ permalink raw reply

* Wine Enthusiasts List
From: Julie Wider @ 2018-08-29 19:41 UTC (permalink / raw)
  To: netdev


Hi,

Greeting of the day!

Would you be interested in reaching out to "Wine Enthusiasts list" from USA?

Our Databases:-     1.Beer Enthusiasts List        2.Alcohol Enthusiasts List
                     3.Beverage Consumers           4.Liquor Enthusiasts List
                     5.Chocolate Enthusiasts List   6.Students List
                     7.Luxury Brand Buyers List     8.Food Lovers List
                     9.Spa and Resort Visitors      10.Sports Enthusiasts List and many more.

Each record in the list contains Contact Name (First and Last Name), Mailing Address, List type and Opt-in email address.

All the contacts are opt-in verified, complete permission based and can be used for unlimited multi-channel marketing.

Let me know if you'd be interested in hearing more about it.

Waiting for your valuable and sincere reply.

Best Regards,
Julie Wider
Research Analyst

^ permalink raw reply

* Re: GPL compliance issue with liquidio/lio_23xx_vsw.bin firmware
From: Ben Hutchings @ 2018-08-29 20:26 UTC (permalink / raw)
  To: Felix Manlunas, Florian Weimer
  Cc: linux-firmware, netdev@vger.kernel.org, Derek Chickles,
	Satanand Burla, Felix Manlunas, Raghu Vatsavayi, Manish Awasthi,
	Manojkumar.Panicker
In-Reply-To: <20180828000445.GA7039@felix-thinkpad.cavium.com>

[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]

On Mon, 2018-08-27 at 17:04 -0700, Felix Manlunas wrote:
> On Mon, Aug 27, 2018 at 05:01:10PM +0200, Florian Weimer wrote:
> > liquidio/lio_23xx_vsw.bin contains a compiled MIPS Linux kernel:
> > 
> > $ tail --bytes=+1313 liquidio/lio_23xx_vsw.bin > elf
> > $ readelf -aW elf
> > […]
> >   [ 6] __ksymtab         PROGBITS        ffffffff80e495f8 64a5f8 00d130
> > 00   A  0   0  8
> >   [ 7] __ksymtab_gpl     PROGBITS        ffffffff80e56728 657728 008400
> > 00   A  0   0  8
> >   [ 8] __ksymtab_strings PROGBITS        ffffffff80e5eb28 65fb28 018868
> > 00   A  0   0  1
> > […]
> > Symbol table '.symtab' contains 1349 entries:
> >    Num:    Value          Size Type    Bind   Vis      Ndx Name
> >      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
> >      1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS
> > arch/mips/kernel/head.o
> >      2: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS init/main.c
> >      3: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS
> > include/linux/types.h
> > […]
> > 
> > Yet there is no corresponding source provided, and LICENCE.cavium lacks
> > the required notices.
> > 
> > Thanks,
> > Florian
> 
> Cavium apologizes for the oversight.  Cavium has been advertising the
> appropriate license terms including the existence of GPL in the firmware
> in our outbox releases. We will update the license terms in LICENCE.cavium
> in our upstream contribution in collaboration with our legal team.

Everything added to linux-firmware.git needs to be safe for Linux
distributions to redistribute.  (There are some ancient firmware images
with unclear licensing, but we don't want to add any more.)

My understanding is that GPL v2 requires that commercial distributors
either provide the complete and corresponding source alongside the
binaries, or include a written offer to provide it later.  They
*cannot* simply point to your offer to do this (only non-commercial
distributors can do that).  So the source needs to be published too.

Adding the complete Linux kernel source code to linux-firmware.git
doesn't seem like a sensible step, so maybe this particular firmware
needs to live elsewhere.

Ben.

-- 
Ben Hutchings
For every complex problem
there is a solution that is simple, neat, and wrong.



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: Fw: [Bug 200943] New: Repeating tcp_mark_head_lost in dmesg
From: Yuchung Cheng @ 2018-08-29 19:57 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180829080257.2ef072c6@xeon-e3>

On Wed, Aug 29, 2018 at 8:02 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
>
>
> Begin forwarded message:
>
> Date: Sun, 26 Aug 2018 22:24:12 +0000
> From: bugzilla-daemon@bugzilla.kernel.org
> To: stephen@networkplumber.org
> Subject: [Bug 200943] New: Repeating tcp_mark_head_lost in dmesg
>
>
> https://bugzilla.kernel.org/show_bug.cgi?id=200943
>
>             Bug ID: 200943
>            Summary: Repeating tcp_mark_head_lost in dmesg
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 4.14.66
>           Hardware: All
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: IPV4
>           Assignee: stephen@networkplumber.org
>           Reporter: rm+bko@romanrm.net
>         Regression: No
>
> Getting a bunch of these now every hour during continuous ~100 Mbit of network
> traffic.
> What's up with that? Seems harmless, as in the kernel doesn't crash and the
> network connection is not interrupted. (Maybe the particular TCP session is?)
> If there are no ill-effects from this condition, is such spammy WARN_ON really
> necessary?
This warning is likely triggered by buggy remote SACK behaviors, and
is pretty harmless - in my opinion the warning tcp_verify_left_out()
is still worthy to detect other inflight states inconsistencies.

The good news the particular loss recovery code path is disabled by
default on 4.18+ kernels by this patch

commit b38a51fec1c1f693f03b1aa19d0622123634d4b7
Author: Yuchung Cheng <ycheng@google.com>
Date:   Wed May 16 16:40:11 2018 -0700

    tcp: disable RFC6675 loss detection


>
> [Mon Aug 27 02:16:11 2018] ------------[ cut here ]------------
> [Mon Aug 27 02:16:11 2018] WARNING: CPU: 5 PID: 0 at net/ipv4/tcp_input.c:2263
> tcp_mark_head_lost+0x247/0x260
> [Mon Aug 27 02:16:11 2018] Modules linked in: dm_snapshot loop vhost_net vhost
> tap tun ip6t_MASQUERADE nf_nat_masquerade_ipv6 ipt_MASQUERADE
> nf_nat_masquerade_ipv4 xt_DSCP xt_mark ip6t_REJECT nf_reject_ipv6 ipt_REJECT
> nf_reject_ipv4 xt_owner xt_tcpudp xt_set ip_set_hash_net ip_set nfnetlink
> xt_limit xt_length xt_multiport xt_conntrack ip6t_rpfilter ipt_rpfilter
> ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_raw
> wireguard ip6_udp_tunnel udp_tunnel ip6table_mangle iptable_nat
> nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_raw
> iptable_mangle ip6table_filter ip6_tables matroxfb_base matroxfb_g450
> matroxfb_Ti3026 matroxfb_accel matroxfb_DAC1064 g450_pll matroxfb_misc
> iptable_filter ip_tables x_tables cpufreq_powersave cpufreq_userspace
> cpufreq_conservative 8021q garp mrp
> [Mon Aug 27 02:16:11 2018]  bridge stp llc bonding tcp_bbr sch_fq tcp_illinois
> fuse radeon ttm drm_kms_helper drm i2c_algo_bit it87 hwmon_vid eeepc_wmi
> asus_wmi sparse_keymap rfkill video wmi_bmof mxm_wmi edac_mce_amd kvm_amd kvm
> snd_pcm snd_timer snd soundcore joydev evdev pcspkr k10temp fam15h_power
> sp5100_tco sg shpchp wmi pcc_cpufreq acpi_cpufreq button ext4 crc16 mbcache
> jbd2 fscrypto btrfs zstd_decompress zstd_compress xxhash algif_skcipher af_alg
> dm_crypt dm_thin_pool dm_persistent_data dm_bio_prison dm_bufio dm_mod
> hid_generic usbhid hid raid10 raid456 async_raid6_recov async_memcpy async_pq
> async_xor async_tx xor sd_mod raid6_pq libcrc32c crc32c_generic raid1 raid0
> multipath linear md_mod vfio_pci irqbypass vfio_virqfd vfio_iommu_type1 vfio
> ohci_pci crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel
> [Mon Aug 27 02:16:11 2018]  pcbc aesni_intel aes_x86_64 crypto_simd glue_helper
> cryptd r8169 ahci xhci_pci libahci ohci_hcd ehci_pci mii xhci_hcd ehci_hcd
> i2c_piix4 libata usbcore scsi_mod bnx2
> [Mon Aug 27 02:16:11 2018] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G        W
>    4.14.66-rm1+ #132
> [Mon Aug 27 02:16:11 2018] Hardware name: To be filled by O.E.M. To be filled
> by O.E.M./SABERTOOTH 990FX R2.0, BIOS 2901 05/04/2016
> [Mon Aug 27 02:16:11 2018] task: ffff8ba79c679dc0 task.stack: ffffb4d741928000
> [Mon Aug 27 02:16:11 2018] RIP: 0010:tcp_mark_head_lost+0x247/0x260
> [Mon Aug 27 02:16:11 2018] RSP: 0018:ffff8ba7aed437d8 EFLAGS: 00010202
> [Mon Aug 27 02:16:11 2018] RAX: 0000000000000018 RBX: ffff8ba3901a0800 RCX:
> 0000000000000000
> [Mon Aug 27 02:16:11 2018] RDX: 0000000000000017 RSI: 0000000000000001 RDI:
> ffff8ba4d47e9000
> [Mon Aug 27 02:16:11 2018] RBP: ffff8ba4d47e9000 R08: 000000000000000d R09:
> 0000000000000000
> [Mon Aug 27 02:16:11 2018] R10: 000000000000100c R11: 0000000000000000 R12:
> 0000000000000001
> [Mon Aug 27 02:16:11 2018] R13: ffff8ba4d47e9158 R14: 0000000000000001 R15:
> 000000009d0b6708
> [Mon Aug 27 02:16:11 2018] FS:  0000000000000000(0000)
> GS:ffff8ba7aed40000(0000) knlGS:0000000000000000
> [Mon Aug 27 02:16:11 2018] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [Mon Aug 27 02:16:11 2018] CR2: 0000000001fbdff0 CR3: 000000040c8d6000 CR4:
> 00000000000406e0
> [Mon Aug 27 02:16:11 2018] Call Trace:
> [Mon Aug 27 02:16:11 2018]  <IRQ>
> [Mon Aug 27 02:16:11 2018]  tcp_fastretrans_alert+0x5c3/0xa20
> [Mon Aug 27 02:16:11 2018]  tcp_ack+0x95a/0x1170
> [Mon Aug 27 02:16:11 2018]  ? __slab_free.isra.70+0x79/0x200
> [Mon Aug 27 02:16:11 2018]  tcp_rcv_established+0x16a/0x5a0
> [Mon Aug 27 02:16:11 2018]  ? tcp_v4_inbound_md5_hash+0x76/0x1e0
> [Mon Aug 27 02:16:11 2018]  tcp_v4_do_rcv+0x130/0x1f0
> [Mon Aug 27 02:16:11 2018]  tcp_v4_rcv+0x9ac/0xaa0
> [Mon Aug 27 02:16:11 2018]  ip_local_deliver_finish+0x9a/0x1c0
> [Mon Aug 27 02:16:11 2018]  ip_local_deliver+0x6b/0xe0
> [Mon Aug 27 02:16:11 2018]  ? ip_rcv_finish+0x440/0x440
> [Mon Aug 27 02:16:11 2018]  ip_rcv+0x2b0/0x3c0
> [Mon Aug 27 02:16:11 2018]  ? inet_del_offload+0x50/0x50
> [Mon Aug 27 02:16:11 2018]  __netif_receive_skb_core+0x85f/0xb50
> [Mon Aug 27 02:16:11 2018]  ? br_allowed_egress+0x2d/0x50 [bridge]
> [Mon Aug 27 02:16:11 2018]  ? br_forward+0x49/0xe0 [bridge]
> [Mon Aug 27 02:16:11 2018]  ? br_vlan_lookup+0xdd/0x150 [bridge]
> [Mon Aug 27 02:16:11 2018]  netif_receive_skb_internal+0x34/0xe0
> [Mon Aug 27 02:16:11 2018]  ? br_handle_vlan+0x4b/0xf0 [bridge]
> [Mon Aug 27 02:16:11 2018]  br_pass_frame_up+0xd4/0x180 [bridge]
> [Mon Aug 27 02:16:11 2018]  ? br_allowed_ingress+0x1ea/0x2e0 [bridge]
> [Mon Aug 27 02:16:11 2018]  br_handle_frame_finish+0x23f/0x530 [bridge]
> [Mon Aug 27 02:16:11 2018]  ? get_partial_node.isra.69+0x13c/0x1d0
> [Mon Aug 27 02:16:11 2018]  br_handle_frame+0x1b7/0x320 [bridge]
> [Mon Aug 27 02:16:11 2018]  __netif_receive_skb_core+0x367/0xb50
> [Mon Aug 27 02:16:11 2018]  ? inet_gro_receive+0x203/0x2b0
> [Mon Aug 27 02:16:11 2018]  netif_receive_skb_internal+0x34/0xe0
> [Mon Aug 27 02:16:11 2018]  napi_gro_receive+0xb8/0xe0
> [Mon Aug 27 02:16:11 2018]  bnx2_poll_work+0x71a/0x12e0 [bnx2]
> [Mon Aug 27 02:16:11 2018]  bnx2_poll_msix+0x41/0xf0 [bnx2]
> [Mon Aug 27 02:16:11 2018]  net_rx_action+0x28c/0x3f0
> [Mon Aug 27 02:16:11 2018]  __do_softirq+0x10a/0x2a2
> [Mon Aug 27 02:16:11 2018]  irq_exit+0xbe/0xd0
> [Mon Aug 27 02:16:11 2018]  do_IRQ+0x66/0x100
> [Mon Aug 27 02:16:11 2018]  common_interrupt+0x7d/0x7d
> [Mon Aug 27 02:16:11 2018]  </IRQ>
> [Mon Aug 27 02:16:11 2018] RIP: 0010:cpuidle_enter_state+0xa4/0x2d0
> [Mon Aug 27 02:16:11 2018] RSP: 0018:ffffb4d74192bea0 EFLAGS: 00000246
> ORIG_RAX: ffffffffffffff1a
> [Mon Aug 27 02:16:11 2018] RAX: ffff8ba7aed61800 RBX: 0000f8f2a31961cb RCX:
> 000000000000001f
> [Mon Aug 27 02:16:11 2018] RDX: 0000f8f2a31961cb RSI: fffffff1cd4e0887 RDI:
> 0000000000000000
> [Mon Aug 27 02:16:11 2018] RBP: 0000000000000002 R08: 000000000000000a R09:
> 000000000000000a
> [Mon Aug 27 02:16:11 2018] R10: 0000000000000364 R11: 00000000000002a6 R12:
> ffff8ba7961b3200
> [Mon Aug 27 02:16:11 2018] R13: ffffffff90cb2c58 R14: 0000f8f2a3139a63 R15:
> ffffffff90cb2b80
> [Mon Aug 27 02:16:11 2018]  do_idle+0x19d/0x200
> [Mon Aug 27 02:16:11 2018]  cpu_startup_entry+0x6f/0x80
> [Mon Aug 27 02:16:11 2018]  start_secondary+0x1ae/0x200
> [Mon Aug 27 02:16:11 2018]  secondary_startup_64+0xa5/0xb0
> [Mon Aug 27 02:16:11 2018] Code: e8 df aa 00 00 85 c0 78 0c 0f b6 43 39 44 89
> e6 e9 16 ff ff ff 8b 95 ec 05 00 00 8b 85 80 06 00 00 03 85 84 06 00 00 39 d0
> 76 a7 <0f> 0b eb a3 31 f6 e9 12 fe ff ff 66 66 2e 0f 1f 84 00 00 00 00
> [Mon Aug 27 02:16:11 2018] ---[ end trace 3d7c0b943ef03b6a ]---
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.

^ permalink raw reply

* [PATCH net] nfp: wait for posted reconfigs when disabling the device
From: Jakub Kicinski @ 2018-08-29 19:46 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski

To avoid leaking a running timer we need to wait for the
posted reconfigs after netdev is unregistered.  In common
case the process of deinitializing the device will perform
synchronous reconfigs which wait for posted requests, but
especially with VXLAN ports being actively added and removed
there can be a race condition leaving a timer running after
adapter structure is freed leading to a crash.

Add an explicit flush after deregistering and for a good
measure a warning to check if timer is running just before
structures are freed.

Fixes: 3d780b926a12 ("nfp: add async reconfiguration mechanism")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 .../ethernet/netronome/nfp/nfp_net_common.c   | 48 +++++++++++++------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index a8b9fbab5f73..253bdaef1505 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -229,29 +229,16 @@ static void nfp_net_reconfig_post(struct nfp_net *nn, u32 update)
 	spin_unlock_bh(&nn->reconfig_lock);
 }
 
-/**
- * nfp_net_reconfig() - Reconfigure the firmware
- * @nn:      NFP Net device to reconfigure
- * @update:  The value for the update field in the BAR config
- *
- * Write the update word to the BAR and ping the reconfig queue.  The
- * poll until the firmware has acknowledged the update by zeroing the
- * update word.
- *
- * Return: Negative errno on error, 0 on success
- */
-int nfp_net_reconfig(struct nfp_net *nn, u32 update)
+static void nfp_net_reconfig_sync_enter(struct nfp_net *nn)
 {
 	bool cancelled_timer = false;
 	u32 pre_posted_requests;
-	int ret;
 
 	spin_lock_bh(&nn->reconfig_lock);
 
 	nn->reconfig_sync_present = true;
 
 	if (nn->reconfig_timer_active) {
-		del_timer(&nn->reconfig_timer);
 		nn->reconfig_timer_active = false;
 		cancelled_timer = true;
 	}
@@ -260,14 +247,43 @@ int nfp_net_reconfig(struct nfp_net *nn, u32 update)
 
 	spin_unlock_bh(&nn->reconfig_lock);
 
-	if (cancelled_timer)
+	if (cancelled_timer) {
+		del_timer_sync(&nn->reconfig_timer);
 		nfp_net_reconfig_wait(nn, nn->reconfig_timer.expires);
+	}
 
 	/* Run the posted reconfigs which were issued before we started */
 	if (pre_posted_requests) {
 		nfp_net_reconfig_start(nn, pre_posted_requests);
 		nfp_net_reconfig_wait(nn, jiffies + HZ * NFP_NET_POLL_TIMEOUT);
 	}
+}
+
+static void nfp_net_reconfig_wait_posted(struct nfp_net *nn)
+{
+	nfp_net_reconfig_sync_enter(nn);
+
+	spin_lock_bh(&nn->reconfig_lock);
+	nn->reconfig_sync_present = false;
+	spin_unlock_bh(&nn->reconfig_lock);
+}
+
+/**
+ * nfp_net_reconfig() - Reconfigure the firmware
+ * @nn:      NFP Net device to reconfigure
+ * @update:  The value for the update field in the BAR config
+ *
+ * Write the update word to the BAR and ping the reconfig queue.  The
+ * poll until the firmware has acknowledged the update by zeroing the
+ * update word.
+ *
+ * Return: Negative errno on error, 0 on success
+ */
+int nfp_net_reconfig(struct nfp_net *nn, u32 update)
+{
+	int ret;
+
+	nfp_net_reconfig_sync_enter(nn);
 
 	nfp_net_reconfig_start(nn, update);
 	ret = nfp_net_reconfig_wait(nn, jiffies + HZ * NFP_NET_POLL_TIMEOUT);
@@ -3633,6 +3649,7 @@ struct nfp_net *nfp_net_alloc(struct pci_dev *pdev, bool needs_netdev,
  */
 void nfp_net_free(struct nfp_net *nn)
 {
+	WARN_ON(timer_pending(&nn->reconfig_timer) || nn->reconfig_posted);
 	if (nn->dp.netdev)
 		free_netdev(nn->dp.netdev);
 	else
@@ -3920,4 +3937,5 @@ void nfp_net_clean(struct nfp_net *nn)
 		return;
 
 	unregister_netdev(nn->dp.netdev);
+	nfp_net_reconfig_wait_posted(nn);
 }
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH bpf-next 00/11] AF_XDP zero-copy support for i40e
From: Alexei Starovoitov @ 2018-08-29 19:39 UTC (permalink / raw)
  To: Björn Töpel
  Cc: magnus.karlsson, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, ast, brouer, daniel, netdev, jesse.brandeburg,
	anjali.singhai, peter.waskiewicz.jr, Björn Töpel,
	michael.lundkvist, willemdebruijn.kernel, john.fastabend,
	jakub.kicinski, neerav.parikh, mykyta.iziumtsev, francois.ozog,
	ilias.apalodimas, brian.brooks, u9012063, pavel, qi.z.zhang
In-Reply-To: <20180828124435.30578-1-bjorn.topel@gmail.com>

On Tue, Aug 28, 2018 at 02:44:24PM +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> This patch set introduces zero-copy AF_XDP support for Intel's i40e
> driver. In the first preparatory patch we also add support for
> XDP_REDIRECT for zero-copy allocated frames so that XDP programs can
> redirect them. This was a ToDo from the first AF_XDP zero-copy patch
> set from early June. Special thanks to Alex Duyck and Jesper Dangaard
> Brouer for reviewing earlier versions of this patch set.
> 
> The i40e zero-copy code is located in its own file i40e_xsk.[ch]. Note
> that in the interest of time, to get an AF_XDP zero-copy implementation
> out there for people to try, some code paths have been copied from the
> XDP path to the zero-copy path. It is out goal to merge the two paths
> in later patch sets.
> 
> In contrast to the implementation from beginning of June, this patch
> set does not require any extra HW queues for AF_XDP zero-copy
> TX. Instead, the XDP TX HW queue is used for both XDP_REDIRECT and
> AF_XDP zero-copy TX.
> 
> Jeff, given that most of changes are in i40e, it is up to you how you
> would like to route these patches. The set is tagged bpf-next, but
> if taking it via the Intel driver tree is easier, let us know.
> 
> We have run some benchmarks on a dual socket system with two Broadwell
> E5 2660 @ 2.0 GHz with hyperthreading turned off. Each socket has 14
> cores which gives a total of 28, but only two cores are used in these
> experiments. One for TR/RX and one for the user space application. The
> memory is DDR4 @ 2133 MT/s (1067 MHz) and the size of each DIMM is
> 8192MB and with 8 of those DIMMs in the system we have 64 GB of total
> memory. The compiler used is gcc (Ubuntu 7.3.0-16ubuntu3) 7.3.0. The
> NIC is Intel I40E 40Gbit/s using the i40e driver.
> 
> Below are the results in Mpps of the I40E NIC benchmark runs for 64
> and 1500 byte packets, generated by a commercial packet generator HW
> outputing packets at full 40 Gbit/s line rate. The results are with
> retpoline and all other spectre and meltdown fixes, so these results
> are not comparable to the ones from the zero-copy patch set in June.
> 
> AF_XDP performance 64 byte packets.
> Benchmark   XDP_SKB    XDP_DRV    XDP_DRV with zerocopy
> rxdrop       2.6        8.2         15.0
> txpush       2.2        -           21.9
> l2fwd        1.7        2.3         11.3
> 
> AF_XDP performance 1500 byte packets:
> Benchmark   XDP_SKB   XDP_DRV     XDP_DRV with zerocopy
> rxdrop       2.0        3.3         3.3
> l2fwd        1.3        1.7         3.1
> 
> XDP performance on our system as a base line:
> 
> 64 byte packets:
> XDP stats       CPU     pps         issue-pps
> XDP-RX CPU      16      18.4M  0
> 
> 1500 byte packets:
> XDP stats       CPU     pps         issue-pps
> XDP-RX CPU      16      3.3M    0
> 
> The structure of the patch set is as follows:
> 
> Patch 1: Add support for XDP_REDIRECT of zero-copy allocated frames
> Patches 2-4: Preparatory patches to common xsk and net code
> Patches 5-7: Preparatory patches to i40e driver code for RX
> Patch 8: i40e zero-copy support for RX
> Patch 9: Preparatory patch to i40e driver code for TX
> Patch 10: i40e zero-copy support for TX
> Patch 11: Add flags to sample application to force zero-copy/copy mode
> 
> We based this patch set on bpf-next commit 050cdc6c9501 ("Merge
> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
> 
> 
> Magnus & Björn
> 
> Björn Töpel (8):
>   xdp: implement convert_to_xdp_frame for MEM_TYPE_ZERO_COPY
>   xdp: export xdp_rxq_info_unreg_mem_model
>   xsk: expose xdp_umem_get_{data,dma} to drivers
>   i40e: added queue pair disable/enable functions
>   i40e: refactor Rx path for re-use
>   i40e: move common Rx functions to i40e_txrx_common.h
>   i40e: add AF_XDP zero-copy Rx support
>   samples/bpf: add -c/--copy -z/--zero-copy flags to xdpsock
> 
> Magnus Karlsson (3):
>   net: add napi_if_scheduled_mark_missed
>   i40e: move common Tx functions to i40e_txrx_common.h
>   i40e: add AF_XDP zero-copy Tx support

Applied to bpf-next.
Thanks a lot to the authors, code reviewers and testers.

I hope all the issues brought up during code review can be quickly
addressed in the follow up patches and zerocopy feature in i40e
and other drivers will be mainline ready for the next merge window.

^ permalink raw reply

* Re: Kernel warnings from tcp.c
From: Stephen Hemminger @ 2018-08-29 19:35 UTC (permalink / raw)
  To: Adam Mitchell; +Cc: netdev
In-Reply-To: <CAEXp4ork9mZ9pY_o5LnG87tL5aRPLfpr1ZZ=p7qpZMJCKi-enA@mail.gmail.com>

On Wed, 29 Aug 2018 14:38:38 -0400
Adam Mitchell <adamhmitchell@gmail.com> wrote:

> Anyone with experience in tcp.c have an idea what's causing this on
> our busy database server?  Comes from this macro at line 2278:
>      WARN_ON(sock_owned_by_user(sk));
> 
> 
> [726780.788201] WARNING: CPU: 15 PID: 52245 at net/ipv4/tcp.c:2278
> tcp_close+0x40f/0x430
> 
> [726780.794947] Modules linked in: binfmt_misc nf_conntrack_netlink
> nfnetlink_queue tcp_diag inet_diag isofs ip6table_mangle ip6table_raw
> ip6table_nat nf_nat_ipv6 iptable_security xt_CT iptable_raw
> iptable_nat nf_nat_ipv4 nf_nat iptable_mangle xt_pkttype xt_NFLOG
> nfnetlink_log xt_u32 xt_multiport xt_set xt_conntrack
> ip_set_hash_netport ip_set_hash_ipport ip_set_hash_net ip_set_hash_ip
> ip_set nfnetlink nf_conntrack_proto_gre nf_conntrack_ipv6
> nf_defrag_ipv6 ip6table_filter ip6_tables xt_LOG nf_conntrack_tftp
> nf_conntrack_ftp nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack
> iptable_filter zfs(PO) zunicode(PO) zavl(PO) icp(PO) sb_edac
> intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
> pcbc aesni_intel crypto_simd glue_helper cryptd cirrus snd_seq
> zcommon(PO) ttm intel_rapl_perf snd_seq_device
> [726780.848696]  drm_kms_helper znvpair(PO) snd_pcm snd_timer spl(O)
> drm snd soundcore syscopyarea sysfillrect pcspkr sysimgblt input_leds
> fb_sys_fops i2c_piix4 ip_tables xfs libcrc32c ata_generic pata_acpi
> ata_piix xen_blkfront crc32c_intel libata ena(O) serio_raw floppy
> sunrpc
> [726780.863916] CPU: 15 PID: 52245 Comm: mysqld Tainted: P        W  O
>     4.16.13-1.el7.elrepo.x86_64 #1
>

Sorry, you have a tainted kernel with modules that are not upstream
therefore don't expect any support from kernel community

^ permalink raw reply

* Re: [PATCH bpf-next 08/11] i40e: add AF_XDP zero-copy Rx support
From: Alexei Starovoitov @ 2018-08-29 19:22 UTC (permalink / raw)
  To: Björn Töpel
  Cc: magnus.karlsson, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, ast, brouer, daniel, netdev, jesse.brandeburg,
	anjali.singhai, peter.waskiewicz.jr, Björn Töpel,
	michael.lundkvist, willemdebruijn.kernel, john.fastabend,
	jakub.kicinski, neerav.parikh, mykyta.iziumtsev, francois.ozog,
	ilias.apalodimas, brian.brooks, u9012063, pavel, qi.z.zhang
In-Reply-To: <20180828124435.30578-9-bjorn.topel@gmail.com>

On Tue, Aug 28, 2018 at 02:44:32PM +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> This patch adds zero-copy Rx support for AF_XDP sockets. Instead of
> allocating buffers of type MEM_TYPE_PAGE_SHARED, the Rx frames are
> allocated as MEM_TYPE_ZERO_COPY when AF_XDP is enabled for a certain
> queue.
> 
> All AF_XDP specific functions are added to a new file, i40e_xsk.c.
> 
> Note that when AF_XDP zero-copy is enabled, the XDP action XDP_PASS
> will allocate a new buffer and copy the zero-copy frame prior passing
> it to the kernel stack.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
...
> +/**
> + * i40e_construct_skb_zc - Create skbufff from zero-copy Rx buffer

typo sk_buff

overall the set looks great to me. The driver side changes look
particularly clean and independent from skb path of the driver.
I think the issues brought up by other reviews are not blockers
for the set, but certainly should be addressed in the follow up patches.

^ permalink raw reply

* [RFC] net: xsk: add a simple buffer reuse queue
From: Jakub Kicinski @ 2018-08-29 19:19 UTC (permalink / raw)
  To: Björn Töpel
  Cc: magnus.karlsson, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, ast, brouer, daniel, netdev, jesse.brandeburg,
	anjali.singhai, peter.waskiewicz.jr, Björn Töpel,
	michael.lundkvist, willemdebruijn.kernel, john.fastabend,
	neerav.parikh, mykyta.iziumtsev, francois.ozog, ilias.apalodimas,
	brian.brooks, u9012063, pavel, qi.z.zhang
In-Reply-To: <20180828124435.30578-1-bjorn.topel@gmail.com>

XSK UMEM is strongly single producer single consumer so reuse of
frames is challenging.  Add a simple "stash" of FILL packets to
reuse for drivers to optionally make use of.  This is useful
when driver has to free (ndo_stop) or resize a ring with an active
AF_XDP ZC socket.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/xdp_sock.h | 44 +++++++++++++++++++++++++++++++++
 net/xdp/xdp_umem.c     |  2 ++
 net/xdp/xsk_queue.c    | 56 ++++++++++++++++++++++++++++++++++++++++++
 net/xdp/xsk_queue.h    |  3 +++
 4 files changed, 105 insertions(+)

diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 6871e4755975..108c1c100de4 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -14,6 +14,7 @@
 #include <net/sock.h>
 
 struct net_device;
+struct xdp_umem_fq_reuse;
 struct xsk_queue;
 
 struct xdp_umem_props {
@@ -41,6 +42,7 @@ struct xdp_umem {
 	struct page **pgs;
 	u32 npgs;
 	struct net_device *dev;
+	struct xdp_umem_fq_reuse *fq_reuse;
 	u16 queue_id;
 	bool zc;
 	spinlock_t xsk_list_lock;
@@ -110,4 +112,46 @@ static inline dma_addr_t xdp_umem_get_dma(struct xdp_umem *umem, u64 addr)
 	return umem->pages[addr >> PAGE_SHIFT].dma + (addr & (PAGE_SIZE - 1));
 }
 
+struct xdp_umem_fq_reuse {
+	u32 nentries;
+	u32 length;
+	u64 handles[];
+};
+
+/* Following functions are not thread-safe in any way */
+struct xdp_umem_fq_reuse *xsk_reuseq_prepare(u32 nentries);
+struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
+					  struct xdp_umem_fq_reuse *newq);
+void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
+
+/* Reuse-queue aware version of FILL queue helpers */
+static inline u64 *xsk_umem_peek_addr_rq(struct xdp_umem *umem, u64 *addr)
+{
+	struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
+
+	if (!rq->length) {
+		return xsk_umem_peek_addr(umem, addr);
+	} else {
+		*addr = rq->handles[rq->length - 1];
+		return addr;
+	}
+}
+
+static inline void xsk_umem_discard_addr_rq(struct xdp_umem *umem)
+{
+	struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
+
+	if (!rq->length)
+		xsk_umem_discard_addr(umem);
+	else
+		rq->length--;
+}
+
+static inline void xsk_umem_fq_reuse(struct xdp_umem *umem, u64 addr)
+{
+	struct xdp_umem_fq_reuse *rq = umem->fq_reuse;
+
+	rq->handles[rq->length++] = addr;
+}
+
 #endif /* _LINUX_XDP_SOCK_H */
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index e762310c9bee..40303e24c954 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -170,6 +170,8 @@ static void xdp_umem_release(struct xdp_umem *umem)
 		umem->cq = NULL;
 	}
 
+	xsk_reuseq_destroy(umem);
+
 	xdp_umem_unpin_pages(umem);
 
 	task = get_pid_task(umem->pid, PIDTYPE_PID);
diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c
index 6c32e92e98fc..f9ee40a13a9a 100644
--- a/net/xdp/xsk_queue.c
+++ b/net/xdp/xsk_queue.c
@@ -3,7 +3,9 @@
  * Copyright(c) 2018 Intel Corporation.
  */
 
+#include <linux/log2.h>
 #include <linux/slab.h>
+#include <linux/overflow.h>
 
 #include "xsk_queue.h"
 
@@ -61,3 +63,57 @@ void xskq_destroy(struct xsk_queue *q)
 	page_frag_free(q->ring);
 	kfree(q);
 }
+
+struct xdp_umem_fq_reuse *xsk_reuseq_prepare(u32 nentries)
+{
+	struct xdp_umem_fq_reuse *newq;
+
+	/* Check for overflow */
+	if (nentries > (u32)roundup_pow_of_two(nentries))
+		return NULL;
+	nentries = roundup_pow_of_two(nentries);
+
+	newq = kvmalloc(struct_size(newq, handles, nentries), GFP_KERNEL);
+	if (!newq)
+		return NULL;
+	memset(newq, 0, offsetof(typeof(*newq), handles));
+
+	newq->nentries = nentries;
+	return newq;
+}
+EXPORT_SYMBOL_GPL(xsk_reuseq_prepare);
+
+struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
+					  struct xdp_umem_fq_reuse *newq)
+{
+	struct xdp_umem_fq_reuse *oldq = umem->fq_reuse;
+
+	if (!oldq) {
+		umem->fq_reuse = newq;
+		return NULL;
+	}
+
+	if (newq->nentries < oldq->length)
+		return newq;
+
+
+	memcpy(newq->handles, oldq->handles,
+	       array_size(oldq->length, sizeof(u64)));
+	newq->length = oldq->length;
+
+	umem->fq_reuse = newq;
+	return oldq;
+}
+EXPORT_SYMBOL_GPL(xsk_reuseq_swap);
+
+void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq)
+{
+	kvfree(rq);
+}
+EXPORT_SYMBOL_GPL(xsk_reuseq_free);
+
+void xsk_reuseq_destroy(struct xdp_umem *umem)
+{
+	xsk_reuseq_free(umem->fq_reuse);
+	umem->fq_reuse = NULL;
+}
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 8a64b150be54..7a480e3eb35d 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -257,4 +257,7 @@ void xskq_set_umem(struct xsk_queue *q, struct xdp_umem_props *umem_props);
 struct xsk_queue *xskq_create(u32 nentries, bool umem_queue);
 void xskq_destroy(struct xsk_queue *q_ops);
 
+/* Executed by the core when the entire UMEM gets freed */
+void xsk_reuseq_destroy(struct xdp_umem *umem);
+
 #endif /* _LINUX_XSK_QUEUE_H */
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH bpf-next 08/11] i40e: add AF_XDP zero-copy Rx support
From: Jakub Kicinski @ 2018-08-29 19:14 UTC (permalink / raw)
  To: Björn Töpel
  Cc: magnus.karlsson, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, ast, brouer, daniel, netdev, jesse.brandeburg,
	anjali.singhai, peter.waskiewicz.jr, Björn Töpel,
	michael.lundkvist, willemdebruijn.kernel, john.fastabend,
	neerav.parikh, mykyta.iziumtsev, francois.ozog, ilias.apalodimas,
	brian.brooks, u9012063, pavel, qi.z.zhang
In-Reply-To: <20180828124435.30578-9-bjorn.topel@gmail.com>

On Tue, 28 Aug 2018 14:44:32 +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> This patch adds zero-copy Rx support for AF_XDP sockets. Instead of
> allocating buffers of type MEM_TYPE_PAGE_SHARED, the Rx frames are
> allocated as MEM_TYPE_ZERO_COPY when AF_XDP is enabled for a certain
> queue.
> 
> All AF_XDP specific functions are added to a new file, i40e_xsk.c.
> 
> Note that when AF_XDP zero-copy is enabled, the XDP action XDP_PASS
> will allocate a new buffer and copy the zero-copy frame prior passing
> it to the kernel stack.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>

Mm.. I'm surprised you don't run into buffer reuse issues that I had
when playing with AF_XDP.  What happens in i40e if someone downs the
interface?  Will UMEMs get destroyed?  Will the RX buffers get freed?

I'll shortly send an RFC with my quick and dirty RX buffer reuse queue,
FWIW.

^ permalink raw reply

* pull-request: bpf 2018-08-29
From: Daniel Borkmann @ 2018-08-29 19:07 UTC (permalink / raw)
  To: davem; +Cc: daniel, ast, netdev

Hi David,

The following pull-request contains BPF updates for your *net* tree.

The main changes are:

1) Fix a build error in sk_reuseport_convert_ctx_access() when
   compiling with clang which cannot resolve hweight_long() at
   build time inside the BUILD_BUG_ON() assertion, from Stefan.

2) Several fixes for BPF sockmap, four of them in getting the
   bpf_msg_pull_data() helper to work, one use after free case
   in bpf_tcp_close() and one refcount leak in bpf_tcp_recvmsg(),
   from Daniel.

3) Another fix for BPF sockmap where we misaccount sk_mem_uncharge()
   in the socket redirect error case from unwinding scatterlist
   twice, from John.

Please consider pulling these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git

Thanks a lot!

----------------------------------------------------------------

The following changes since commit 53ae914d898e5dd5984d352d5fa0b23410f966a0:

  net/rds: Use rdma_read_gids to get connection SGID/DGID in IPv6 (2018-08-27 15:26:01 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git 

for you to fetch changes up to d65e6c80c6bb72ced46ce90dea4016d913a8ddd4:

  Merge branch 'bpf_msg_pull_data-fixes' (2018-08-29 10:47:18 -0700)

----------------------------------------------------------------
Alexei Starovoitov (1):
      Merge branch 'bpf_msg_pull_data-fixes'

Daniel Borkmann (6):
      bpf, sockmap: fix potential use after free in bpf_tcp_close
      bpf, sockmap: fix psock refcount leak in bpf_tcp_recvmsg
      bpf: fix several offset tests in bpf_msg_pull_data
      bpf: fix msg->data/data_end after sg shift repair in bpf_msg_pull_data
      bpf: fix shift upon scatterlist ring wrap-around in bpf_msg_pull_data
      bpf: fix sg shift repair start offset in bpf_msg_pull_data

John Fastabend (1):
      bpf: sockmap, decrement copied count correctly in redirect error case

Stefan Agner (1):
      bpf: fix build error with clang

 kernel/bpf/sockmap.c | 52 +++++++++++++++++++++++++---------------------------
 net/core/filter.c    | 52 +++++++++++++++++++++++++++++-----------------------
 2 files changed, 54 insertions(+), 50 deletions(-)

^ permalink raw reply

* Re: bpfilter causes a leftover kernel process
From: Olivier Brunel @ 2018-08-29 16:21 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: netdev, daniel
In-Reply-To: <20180829053536.en6crhdfs7r2d6vt@ast-mbp.dhcp.thefacebook.com>

On Tue, 28 Aug 2018 22:35:38 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> > Yeah, I have a similar thing happening on shutdown, except that
> > we're talking about a kernel thread here, so that process is
> > ignored by the mentionned killing spree as a result, thus leaving
> > that process running.  
> 
> it's not a kernel thread and sounds like there is a bug in your pid 1
> that is worth fixing.
 
Well, it sure does look like one. By which I mean that looking at
its /proc/$PID entry, one can see that it has an empty command line and
kthreadd as parent (ppid 2), which usually are only true for kernel
threads (unless I'm mistaken).

The tool I'm using to send the signals on shutdown does indeed not use
kill(-1,sig) but instead scans /proc and determines whether or not to
signal a process (thus allowing to ignore a few specific processes to
be killed later on).

Kernel threads are obviously to be skipped, and to identify them it
uses the classic method of checking whether or not it has an empty
command line. As I said, this bpfilter helper does and that's why it is
considered a kernel thread & thus not killed.

This is also what other tools do, like ps or top, which is indeed why
they also show it as a kernel thread ("[none]").

So is this way of doing this wrong? And if so, what would be a better
way to identify kernel threads?

Out of curiosity I also had a look at how systemd does things, and it
looks to me like it does the exact same thing[1], also identifying
kernel threads by their empty command line. So I would think that a
similar issue could be observed under systemd as well.

Thanks.


[1]
https://github.com/systemd/systemd/blob/master/src/core/killall.c#L44

^ permalink raw reply

* [PATCH net] Revert "packet: switch kvzalloc to allocate memory"
From: Eric Dumazet @ 2018-08-29 18:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Eric Dumazet, Zhang Yu, Li RongQing

This reverts commit 71e41286203c017d24f041a7cd71abea7ca7b1e0.

mmap()/munmap() can not be backed by kmalloced pages :

We fault in :

    VM_BUG_ON_PAGE(PageSlab(page), page);

    unmap_single_vma+0x8a/0x110
    unmap_vmas+0x4b/0x90
    unmap_region+0xc9/0x140
    do_munmap+0x274/0x360
    vm_munmap+0x81/0xc0
    SyS_munmap+0x2b/0x40
    do_syscall_64+0x13e/0x1c0
    entry_SYSCALL_64_after_hwframe+0x42/0xb7

Fixes: 71e41286203c ("packet: switch kvzalloc to allocate memory")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: John Sperbeck <jsperbeck@google.com>
Bisected-by: John Sperbeck <jsperbeck@google.com>
Cc: Zhang Yu <zhangyu31@baidu.com>
Cc: Li RongQing <lirongqing@baidu.com>
---
 net/packet/af_packet.c | 44 +++++++++++++++++++++++++++++-------------
 net/packet/internal.h  |  1 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 5610061e7f2e00b935ce44dd9cf82d10eb77a7bf..75c92a87e7b2481141161c8945f5e7eef8e0abf8 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -4137,36 +4137,52 @@ static const struct vm_operations_struct packet_mmap_ops = {
 	.close	=	packet_mm_close,
 };
 
-static void free_pg_vec(struct pgv *pg_vec, unsigned int len)
+static void free_pg_vec(struct pgv *pg_vec, unsigned int order,
+			unsigned int len)
 {
 	int i;
 
 	for (i = 0; i < len; i++) {
 		if (likely(pg_vec[i].buffer)) {
-			kvfree(pg_vec[i].buffer);
+			if (is_vmalloc_addr(pg_vec[i].buffer))
+				vfree(pg_vec[i].buffer);
+			else
+				free_pages((unsigned long)pg_vec[i].buffer,
+					   order);
 			pg_vec[i].buffer = NULL;
 		}
 	}
 	kfree(pg_vec);
 }
 
-static char *alloc_one_pg_vec_page(unsigned long size)
+static char *alloc_one_pg_vec_page(unsigned long order)
 {
 	char *buffer;
+	gfp_t gfp_flags = GFP_KERNEL | __GFP_COMP |
+			  __GFP_ZERO | __GFP_NOWARN | __GFP_NORETRY;
 
-	buffer = kvzalloc(size, GFP_KERNEL);
+	buffer = (char *) __get_free_pages(gfp_flags, order);
 	if (buffer)
 		return buffer;
 
-	buffer = kvzalloc(size, GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	/* __get_free_pages failed, fall back to vmalloc */
+	buffer = vzalloc(array_size((1 << order), PAGE_SIZE));
+	if (buffer)
+		return buffer;
+
+	/* vmalloc failed, lets dig into swap here */
+	gfp_flags &= ~__GFP_NORETRY;
+	buffer = (char *) __get_free_pages(gfp_flags, order);
+	if (buffer)
+		return buffer;
 
-	return buffer;
+	/* complete and utter failure */
+	return NULL;
 }
 
-static struct pgv *alloc_pg_vec(struct tpacket_req *req)
+static struct pgv *alloc_pg_vec(struct tpacket_req *req, int order)
 {
 	unsigned int block_nr = req->tp_block_nr;
-	unsigned long size = req->tp_block_size;
 	struct pgv *pg_vec;
 	int i;
 
@@ -4175,7 +4191,7 @@ static struct pgv *alloc_pg_vec(struct tpacket_req *req)
 		goto out;
 
 	for (i = 0; i < block_nr; i++) {
-		pg_vec[i].buffer = alloc_one_pg_vec_page(size);
+		pg_vec[i].buffer = alloc_one_pg_vec_page(order);
 		if (unlikely(!pg_vec[i].buffer))
 			goto out_free_pgvec;
 	}
@@ -4184,7 +4200,7 @@ static struct pgv *alloc_pg_vec(struct tpacket_req *req)
 	return pg_vec;
 
 out_free_pgvec:
-	free_pg_vec(pg_vec, block_nr);
+	free_pg_vec(pg_vec, order, block_nr);
 	pg_vec = NULL;
 	goto out;
 }
@@ -4194,9 +4210,9 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 {
 	struct pgv *pg_vec = NULL;
 	struct packet_sock *po = pkt_sk(sk);
+	int was_running, order = 0;
 	struct packet_ring_buffer *rb;
 	struct sk_buff_head *rb_queue;
-	int was_running;
 	__be16 num;
 	int err = -EINVAL;
 	/* Added to avoid minimal code churn */
@@ -4258,7 +4274,8 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 			goto out;
 
 		err = -ENOMEM;
-		pg_vec = alloc_pg_vec(req);
+		order = get_order(req->tp_block_size);
+		pg_vec = alloc_pg_vec(req, order);
 		if (unlikely(!pg_vec))
 			goto out;
 		switch (po->tp_version) {
@@ -4312,6 +4329,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 		rb->frame_size = req->tp_frame_size;
 		spin_unlock_bh(&rb_queue->lock);
 
+		swap(rb->pg_vec_order, order);
 		swap(rb->pg_vec_len, req->tp_block_nr);
 
 		rb->pg_vec_pages = req->tp_block_size/PAGE_SIZE;
@@ -4337,7 +4355,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
 	}
 
 	if (pg_vec)
-		free_pg_vec(pg_vec, req->tp_block_nr);
+		free_pg_vec(pg_vec, order, req->tp_block_nr);
 out:
 	return err;
 }
diff --git a/net/packet/internal.h b/net/packet/internal.h
index 8f50036f62f05c76632dea82491d6a60dba39f0f..3bb7c5fb3bff2fd5d91c3d973d006d0cdde29a0b 100644
--- a/net/packet/internal.h
+++ b/net/packet/internal.h
@@ -64,6 +64,7 @@ struct packet_ring_buffer {
 	unsigned int		frame_size;
 	unsigned int		frame_max;
 
+	unsigned int		pg_vec_order;
 	unsigned int		pg_vec_pages;
 	unsigned int		pg_vec_len;
 
-- 
2.19.0.rc0.228.g281dcd1b4d0-goog

^ permalink raw reply related

* Kernel warnings from tcp.c
From: Adam Mitchell @ 2018-08-29 18:38 UTC (permalink / raw)
  To: netdev

Anyone with experience in tcp.c have an idea what's causing this on
our busy database server?  Comes from this macro at line 2278:
     WARN_ON(sock_owned_by_user(sk));


[726780.788201] WARNING: CPU: 15 PID: 52245 at net/ipv4/tcp.c:2278
tcp_close+0x40f/0x430

[726780.794947] Modules linked in: binfmt_misc nf_conntrack_netlink
nfnetlink_queue tcp_diag inet_diag isofs ip6table_mangle ip6table_raw
ip6table_nat nf_nat_ipv6 iptable_security xt_CT iptable_raw
iptable_nat nf_nat_ipv4 nf_nat iptable_mangle xt_pkttype xt_NFLOG
nfnetlink_log xt_u32 xt_multiport xt_set xt_conntrack
ip_set_hash_netport ip_set_hash_ipport ip_set_hash_net ip_set_hash_ip
ip_set nfnetlink nf_conntrack_proto_gre nf_conntrack_ipv6
nf_defrag_ipv6 ip6table_filter ip6_tables xt_LOG nf_conntrack_tftp
nf_conntrack_ftp nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack
iptable_filter zfs(PO) zunicode(PO) zavl(PO) icp(PO) sb_edac
intel_powerclamp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
pcbc aesni_intel crypto_simd glue_helper cryptd cirrus snd_seq
zcommon(PO) ttm intel_rapl_perf snd_seq_device
[726780.848696]  drm_kms_helper znvpair(PO) snd_pcm snd_timer spl(O)
drm snd soundcore syscopyarea sysfillrect pcspkr sysimgblt input_leds
fb_sys_fops i2c_piix4 ip_tables xfs libcrc32c ata_generic pata_acpi
ata_piix xen_blkfront crc32c_intel libata ena(O) serio_raw floppy
sunrpc
[726780.863916] CPU: 15 PID: 52245 Comm: mysqld Tainted: P        W  O
    4.16.13-1.el7.elrepo.x86_64 #1
[726780.869486] Hardware name: Xen HVM domU, BIOS 4.2.amazon 08/24/2006
[726780.873756] RIP: 0010:tcp_close+0x40f/0x430
[726780.877049] RSP: 0018:ffffc90063a87dd0 EFLAGS: 00010202
[726780.880913] RAX: 0000000000000001 RBX: ffff8839cbb4b300 RCX:
0000000000000001
[726780.885708] RDX: 0000000000400001 RSI: 0000000000023540 RDI:
000000000000002b
[726780.890412] RBP: ffffc90063a87df0 R08: 0000000000000000 R09:
0000000000000101
[726780.895192] R10: 00000000000003ff R11: 0000000000002057 R12:
ffff8839cbb4b388
[726780.900029] R13: 0000000000000009 R14: ffff8839cbb4b3c8 R15:
ffff883c5ed14900
[726780.904777] FS:  00007f6acd467700(0000) GS:ffff883c8b1c0000(0000)
knlGS:0000000000000000
[726780.909964] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[726780.914078] CR2: 00007f6cd5d430c8 CR3: 0000003c7e6b6005 CR4:
00000000001606e0
[726780.918837] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[726780.923433] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[726780.927882] Call Trace:
[726780.930410]  inet_release+0x42/0x70
[726780.933423]  inet6_release+0x30/0x40
[726780.936439]  sock_release+0x25/0x80
[726780.939421]  sock_close+0x12/0x20
[726780.942342]  __fput+0xea/0x220
[726780.945170]  ____fput+0xe/0x10
[726780.947963]  task_work_run+0x8c/0xb0
[726780.951052]  exit_to_usermode_loop+0x6b/0x95
[726780.954480]  do_syscall_64+0x182/0x1b0
[726780.957586]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[726780.961288] RIP: 0033:0x7fd82b27085d
[726780.964349] RSP: 002b:00007f6acd466b50 EFLAGS: 00000293 ORIG_RAX:
0000000000000003
[726780.969280] RAX: 0000000000000000 RBX: 00007f65b2418220 RCX:
00007fd82b27085d
[726780.973987] RDX: 0000000000000003 RSI: 00007f6d5e9990c0 RDI:
00000000000002d8
[726780.978689] RBP: 00007f6acd466c00 R08: 0000000001622848 R09:
00000000000001f8
[726780.983373] R10: 0000000000000000 R11: 0000000000000293 R12:
0000000000000000
[726780.988049] R13: 00007f6d5e9990c0 R14: 0000000001d87cc0 R15:
00000000000002d8
[726780.992700] Code: ff 48 8b 43 28 31 f6 48 89 df 48 8b 40 10 e8 49
2d 4e 00 48 8b 43 30 48 8b 80 98 01 00 00 65 48 ff 80 90 01 00 00 e9
49 ff ff ff <0f> 0b e9 ab fc ff ff 48 8b 43 28 31 f6 48 89 df 48 8b 40
10 e8
[726781.004156] ---[ end trace 8525f27644ac4631 ]---

^ permalink raw reply

* [PATCH net v2] net: bcmgenet: use MAC link status for fixed phy
From: Doug Berger @ 2018-08-29 18:27 UTC (permalink / raw)
  To: David S. Miller; +Cc: Florian Fainelli, netdev, linux-kernel, Doug Berger

When using the fixed PHY with GENET (e.g. MOCA) the PHY link
status can be determined from the internal link status captured
by the MAC. This allows the PHY state machine to use the correct
link state with the fixed PHY even if MAC link event interrupts
are missed when the net device is opened.

Fixes: 8d88c6ebb34c ("net: bcmgenet: enable MoCA link state change detection")
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
v2: increased "Fixes" sha1 to 12 digits

 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  3 +++
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 10 ++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index b773bc07edf7..14b49612aa86 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -186,6 +186,9 @@ struct bcmgenet_mib_counters {
 #define UMAC_MAC1			0x010
 #define UMAC_MAX_FRAME_LEN		0x014
 
+#define UMAC_MODE			0x44
+#define  MODE_LINK_STATUS		(1 << 5)
+
 #define UMAC_EEE_CTRL			0x064
 #define  EN_LPI_RX_PAUSE		(1 << 0)
 #define  EN_LPI_TX_PFC			(1 << 1)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 5333274a283c..4241ae928d4a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -115,8 +115,14 @@ void bcmgenet_mii_setup(struct net_device *dev)
 static int bcmgenet_fixed_phy_link_update(struct net_device *dev,
 					  struct fixed_phy_status *status)
 {
-	if (dev && dev->phydev && status)
-		status->link = dev->phydev->link;
+	struct bcmgenet_priv *priv;
+	u32 reg;
+
+	if (dev && dev->phydev && status) {
+		priv = netdev_priv(dev);
+		reg = bcmgenet_umac_readl(priv, UMAC_MODE);
+		status->link = !!(reg & MODE_LINK_STATUS);
+	}
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC RFT PATCH 0/4] gpiolib: speed up GPIO array processing
From: Janusz Krzysztofik @ 2018-08-29 18:16 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Corbet, Miguel Ojeda Sandonis, Peter Korsgaard,
	Peter Rosin, Ulf Hansson, Andrew Lunn, Florian Fainelli,
	David S. Miller, Dominik Brodowski, kishon, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald, Greg KH, Jiri Slaby, open list:GPIO SUBSYSTEM,
	linux-doc, linux-i2c
In-Reply-To: <CACRpkdbgY6VNP-K-c5ErQtmO7E83D6c=49TN5siF8VTNh_66Jg@mail.gmail.com>

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 <jmkrzyszt@gmail.com> 
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
> 

^ permalink raw reply

* Re: [PATCH 0/2] net/sched: Add hardware specific counters to TC actions
From: Jakub Kicinski @ 2018-08-29 18:12 UTC (permalink / raw)
  To: Eelco Chaudron
  Cc: David Miller, netdev, jhs, xiyou.wangcong, jiri, simon.horman,
	Marcelo Ricardo Leitner, louis.peens
In-Reply-To: <E4A2D662-0C89-4B61-BB68-ED985D9EC4B3@redhat.com>

On Wed, 29 Aug 2018 11:43:47 +0200, Eelco Chaudron wrote:
> On 23 Aug 2018, at 20:14, Jakub Kicinski wrote:
> 
> > On Mon, 20 Aug 2018 16:03:40 +0200, Eelco Chaudron wrote:  
> >> On 17 Aug 2018, at 13:27, Jakub Kicinski wrote:  
> >>> On Thu, 16 Aug 2018 14:02:44 +0200, Eelco Chaudron wrote:  
> >>>> On 11 Aug 2018, at 21:06, David Miller wrote:
> >>>>  
> >>>>> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> >>>>> Date: Thu, 9 Aug 2018 20:26:08 -0700
> >>>>>  
> >>>>>> It is not immediately clear why this is needed.  The memory and
> >>>>>> updating two sets of counters won't come for free, so perhaps a
> >>>>>> stronger justification than troubleshooting is due? :S
> >>>>>>
> >>>>>> Netdev has counters for fallback vs forwarded traffic, so you'd
> >>>>>> know
> >>>>>> that traffic hits the SW datapath, plus the rules which are in_hw
> >>>>>> will
> >>>>>> most likely not match as of today for flower (assuming
> >>>>>> correctness).  
> >>>>
> >>>> I strongly believe that these counters are a requirement for a 
> >>>> mixed
> >>>> software/hardware (flow) based forwarding environment. The global
> >>>> counters will not help much here as you might have chosen to have
> >>>> certain traffic forwarded by software.
> >>>>
> >>>> These counters are probably the only option you have to figure out
> >>>> why
> >>>> forwarding is not as fast as expected, and you want to blame the TC
> >>>> offload NIC.  
> >>>
> >>> The suggested debugging flow would be:
> >>>  (1) check the global counter for fallback are incrementing;
> >>>  (2) find a flow with high stats but no in_hw flag set.
> >>>
> >>> The in_hw indication should be sufficient in most cases (unless 
> >>> there
> >>> are shared blocks between netdevs of different ASICs...).  
> >>
> >> I guess the aim is to find miss behaving hardware, i.e. having the 
> >> in_hw
> >> flag set, but flows still coming to the kernel.  
> >
> > For misbehaving hardware in_hw will not work indeed.  Whether we need
> > these extra always-on stats for such use case could be debated :)
> >  
> >>>>>> I'm slightly concerned about potential performance impact, would
> >>>>>> you
> >>>>>> be able to share some numbers for non-trivial number of flows 
> >>>>>> (100k
> >>>>>> active?)?  
> >>>>>
> >>>>> Agreed, features used for diagnostics cannot have a harmful 
> >>>>> penalty
> >>>>> for fast path performance.  
> >>>>
> >>>> Fast path performance is not affected as these counters are not
> >>>> incremented there. They are only incremented by the nic driver when
> >>>> they
> >>>> gather their statistics from hardware.  
> >>>
> >>> Not by much, you are adding state to performance-critical 
> >>> structures,
> >>> though, for what is effectively debugging purposes.
> >>>
> >>> I was mostly talking about the HW offload stat updates (sorry for 
> >>> not
> >>> being clear).
> >>>
> >>> We can have some hundreds of thousands active offloaded flows, each 
> >>> of
> >>> them can have multiple actions, and stats have to be updated 
> >>> multiple
> >>> times per second and dumped probably around once a second, too.  On 
> >>> a
> >>> busy system the stats will get evicted from cache between each 
> >>> round.
> >>>
> >>> But I'm speculating let's see if I can get some numbers on it (if 
> >>> you
> >>> could get some too, that would be great!).  
> >>
> >> I’ll try to measure some of this later this week/early next week.  
> >
> > I asked Louis to run some tests while I'm travelling, and he reports
> > that my worry about reporting the extra stats was unfounded.  Update
> > function does not show up in traces at all.  It seems under stress
> > (generated with stress-ng) the thread dumping the stats in userspace
> > (in OvS it would be the revalidator) actually consumes less CPU in
> > __gnet_stats_copy_basic (0.4% less for ~2.0% total).
> >
> > Would this match with your results?  I'm not sure why dumping would be
> > faster with your change..  
> 
> Tested with OVS and https://github.com/chaudron/ovs_perf using 300K TC 
> rules installed in HW.
> 
> For __gnet_stats_copy_basic() being faster I have (had) a theory. Now 
> this function is called twice, and I assumed the first call would cache 
> memory and the second call would be faster.
> 
> Sampling a lot of perf data, I get an average of 1115ns with the base 
> kernel and 954ns with the fix applied, so about ~14%.
> 
> Thought I would perf tcf_action_copy_stats() as it is the place updating 
> the additional counter. But even in this case, I see a better 
> performance with the patch applied.
> 
> In average 13581ns with the fix, vs base kernel at 1391ns, so about 
> 2.3%.
> 
> I guess the changes to the tc_action structure got better cache 
> alignment.

Interesting you could reproduce the speed up too!  +1 for the guess.
Seems like my caution about slowing down SW paths to support HW offload
landed on a very unfortunate patch :)

^ permalink raw reply

* Re: [bpf-next, 01/11] xdp: implement convert_to_xdp_frame for MEM_TYPE_ZERO_COPY
From: Maciek Fijalkowski @ 2018-08-29 18:06 UTC (permalink / raw)
  To: bjorn.topel, magnus.karlsson, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, ast, brouer, daniel, netdev, jesse.brandeburg,
	anjali.singhai, peter.waskiewicz.jr
  Cc: michael.lundkvist, willemdebruijn.kernel, john.fastabend,
	jakub.kicinski, neerav.parikh, mykyta.iziumtsev, francois.ozog,
	ilias.apalodimas, brian.brooks, u9012063, pavel, qi.z.zhang
In-Reply-To: <20180828124435.30578-2-bjorn.topel@gmail.com>

From: Maciej Fijalkowski <macfij7@wp.pl>

> From: Björn Töpel <bjorn.topel@intel.com>
> 
> This commit adds proper MEM_TYPE_ZERO_COPY support for
> convert_to_xdp_frame. Converting a MEM_TYPE_ZERO_COPY xdp_buff to an
> xdp_frame is done by transforming the MEM_TYPE_ZERO_COPY buffer into a
> MEM_TYPE_PAGE_ORDER0 frame. This is costly, and in the future it might
> make sense to implement a more sophisticated thread-safe alloc/free
> scheme for MEM_TYPE_ZERO_COPY, so that no allocation and copy is
> required in the fast-path.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
>  include/net/xdp.h |  5 +++--
>  net/core/xdp.c    | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 76b95256c266..0d5c6fb4b2e2 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -91,6 +91,8 @@ static inline void xdp_scrub_frame(struct xdp_frame *frame)
>  	frame->dev_rx = NULL;
>  }
>  
> +struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp);
> +
>  /* Convert xdp_buff to xdp_frame */
>  static inline
>  struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
> @@ -99,9 +101,8 @@ struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
>  	int metasize;
>  	int headroom;
>  
> -	/* TODO: implement clone, copy, use "native" MEM_TYPE */
>  	if (xdp->rxq->mem.type == MEM_TYPE_ZERO_COPY)
> -		return NULL;
> +		return xdp_convert_zc_to_xdp_frame(xdp);
>  
>  	/* Assure headroom is available for storing info */
>  	headroom = xdp->data - xdp->data_hard_start;
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 89b6785cef2a..be6cb2f0e722 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -398,3 +398,42 @@ void xdp_attachment_setup(struct xdp_attachment_info *info,
>  	info->flags = bpf->flags;
>  }
>  EXPORT_SYMBOL_GPL(xdp_attachment_setup);
> +
> +struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp)
> +{
> +	unsigned int metasize, headroom, totsize;
> +	void *addr, *data_to_copy;
> +	struct xdp_frame *xdpf;
> +	struct page *page;
> +
> +	/* Clone into a MEM_TYPE_PAGE_ORDER0 xdp_frame. */
> +	metasize = xdp_data_meta_unsupported(xdp) ? 0 :
> +		   xdp->data - xdp->data_meta;
> +	headroom = xdp->data - xdp->data_hard_start;

You actually don't use the headroom calculated here;
xdpf->headroom is assigned to 0 - was this your intention?
If so, the local variable can be removed.

> +	totsize = xdp->data_end - xdp->data + metasize;
> +
> +	if (sizeof(*xdpf) + totsize > PAGE_SIZE)
> +		return NULL;
> +
> +	page = dev_alloc_page();
> +	if (!page)
> +		return NULL;
> +
> +	addr = page_to_virt(page);
> +	xdpf = addr;
> +	memset(xdpf, 0, sizeof(*xdpf));
> +
> +	addr += sizeof(*xdpf);
> +	data_to_copy = metasize ? xdp->data_meta : xdp->data;
> +	memcpy(addr, data_to_copy, totsize);
> +
> +	xdpf->data = addr + metasize;
> +	xdpf->len = totsize - metasize;
> +	xdpf->headroom = 0;
> +	xdpf->metasize = metasize;
> +	xdpf->mem.type = MEM_TYPE_PAGE_ORDER0;
> +
> +	xdp_return_buff(xdp);
> +	return xdpf;
> +}
> +EXPORT_SYMBOL_GPL(xdp_convert_zc_to_xdp_frame);

^ permalink raw reply

* Re: [PATCH 0/2] net/sched: Add hardware specific counters to TC actions
From: Jakub Kicinski @ 2018-08-29 18:06 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Eelco Chaudron, David Miller, netdev, jhs, xiyou.wangcong, jiri,
	simon.horman, Marcelo Ricardo Leitner, louis.peens
In-Reply-To: <3d74699054d901e963bdac39898cac441f00658f.camel@redhat.com>

On Wed, 29 Aug 2018 12:23:15 +0200, Paolo Abeni wrote:
> On Thu, 2018-08-23 at 20:14 +0200, Jakub Kicinski wrote:
> > I asked Louis to run some tests while I'm travelling, and he reports
> > that my worry about reporting the extra stats was unfounded.  Update
> > function does not show up in traces at all.  It seems under stress
> > (generated with stress-ng) the thread dumping the stats in userspace
> > (in OvS it would be the revalidator) actually consumes less CPU in
> > __gnet_stats_copy_basic (0.4% less for ~2.0% total).
> > 
> > Would this match with your results?  I'm not sure why dumping would be
> > faster with your change..  
> 
> Wild guess on my side: the relevant patch changes a bit the binary
> layout of the 'tc_action' struct, possibly (I still need to check with
> pahole) moving the tcf_lock and the stats field on different
> cachelines, reducing false sharing that could affect badly such test.

I think in our tests we tried with and without pinning relevant
processing to one core, and both results shown improvement.  I don't
have the actual samples any more, just perf script dump without CPU
IDs to confirm things were pinned correctly.. :(

^ permalink raw reply

* Re: [RFC RFT PATCH v4 1/4] gpiolib: Pass bitmaps, not integer arrays, to get/set array
From: Janusz Krzysztofik @ 2018-08-29 18:01 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Linus Walleij, Jonathan Corbet, Peter Korsgaard, Peter Rosin,
	Ulf Hansson, Andrew Lunn, Florian Fainelli, David S. Miller,
	Dominik Brodowski, Kishon Vijay Abraham I, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Greg Kroah-Hartman, Jiri Slaby,
	linux-gpio, Linux Doc Mailing List
In-Reply-To: <CANiq72kM9bYJ1Q+dbLumjfQLZW223ZTrYEFqfQ2Jv2SAjrD1NA@mail.gmail.com>

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 <jmkrzyszt@gmail.com> 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 <jmkrzyszt@gmail.com>
> > ---
> >  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, &hd->pins[PIN_DATA0], values);
> > +       gpiod_set_array_value_cansleep(n, &hd->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., &values[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_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);
> 
> Are you sure this is correct? You are basically doing an or of
> value_bitmap and val and clearing the low-nibble.

having the rs and optional 0 already assigned to bits 4 and 5 of the bitmap, I just cleared bits 0-3 still containing the high nibble of val and assigned the low nibble of it to those bits, getting a result ready to be passed as an argument to gpiod_set_array_value_cansleep() below.

I hope I didn't miss anything.

Thanks,
Janusz

> >
> >         /* 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);
> >  }
> 
> Cheers,
> Miguel
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox